Skip to content

[PLAT-22764] Charts: Validate yugaware.defaultUser against YBA's register rules - #238

Merged
kv83821-yb merged 2 commits into
yugabyte:masterfrom
kv83821-yb:default-user-pwd-validation
Oct 1, 2026
Merged

kv83821-yb merged 2 commits into
yugabyte:masterfrom
kv83821-yb:default-user-pwd-validation

Conversation

@kv83821-yb

@kv83821-yb kv83821-yb commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes PLAT-22764.

With yugaware.defaultUser.enabled, the post-install Job registers the first customer and user through /api/register. It had several bugs, and the chart didn't check the values against YBA's rules, so a value YBA rejects left the release installed with no user.

Bugs fixed

  • Special characters in the password broke the install. The password was pasted into a JSON literal inside a single-quoted YAML scalar, and YBA's policy requires a special character. A ' made the manifest invalid YAML, so helm install failed. A " or \ produced a body YBA couldn't parse. Email and username had the same problem. The body is now built with dict | toJson.
  • Failures were reported as success. curl ran without --fail, so a 4xx/5xx response still exited 0. The hook was recorded as succeeded and then deleted. It now runs with --fail, and retries (--retry 30 --retry-delay 10 --retry-connrefused) while the platform is still starting. --fail-with-body isn't usable because the AlmaLinux 8 based YBA image ships curl 7.61.
  • yugaware.defaultUser.code was ignored. The customer code was hardcoded to "operator".
  • A null password failed with incompatible types for comparison.

Validation

The new yugaware.validateDefaultUser helper fails the render when YBA would reject the registration, and lists every problem at once. It runs only on install (.Release.IsInstall), because Helm also renders hooks on upgrade and the user already exists by then. Lengths are counted in UTF-16 code units, as Java counts them.

  • Fields (CustomerRegisterFormData):

    • email, username, code and password are all required.
    • code can be at most 15 characters.
    • username, which is sent as name, needs at least 3 characters.
    • email must fully match Play's @Email pattern. The pattern comes from play-java-forms 3.0.10, the version YBA builds against.
  • Password (PasswordPolicyService.checkPasswordPolicy): /api/register creates the customer and checks the password in the same call, so no customer-level policy exists yet and the yb.pwdpolicy defaults apply:

    • at least 8 characters, or 14 with yugaware.fips.enabled;
    • at least one upper case letter, lower case letter, digit and special character;
    • no backtick.

    The error messages are YBA's own.

Example:

yugaware.defaultUser cannot be registered with YugabyteDB Anywhere: yugaware.defaultUser.username is 2 characters, at least 3 are required; yugaware.defaultUser.password does not meet the YugabyteDB Anywhere password policy: Password should contain at least 1 special characters

Test plan

  • New tests/test_default_user.yaml covers each check, the FIPS length, the escaping, the customer code and the curl flags.
  • helm unittest -f "tests/test_*.yaml" stable/yugaware: 98 passed, 98 total.
  • Rendered passwords containing ', ", \, <&> and non-ASCII letters. Each one comes through the YAML and the JSON body unchanged.
  • Ran the email pattern through Go's regexp and Java's java.util.regex on 11 addresses; the results are identical.

Follow-up, not in this PR: the password is still visible in the Job's pod spec.

🤖 Generated with Claude Code

…ster rules

Summary:
With yugaware.defaultUser.enabled, the post-install Job registers the first customer and user
through /api/register. It had several bugs:

- The password was spliced into a JSON literal inside a single-quoted YAML scalar, and YBA's
  password policy requires a special character. A ' made the manifest invalid YAML, so
  helm install failed; a " or \ produced a body YBA could not parse. Email and username had
  the same problem. The body is now built with dict | toJson.
- curl ran without --fail, so a 4xx/5xx still exited 0. The hook was recorded as succeeded
  and deleted, and the release came up with no user. It now runs with --fail, and retries
  (--retry 30 --retry-delay 10 --retry-connrefused) while the platform is still starting.
  --fail-with-body is not usable: the AlmaLinux 8 based YBA image ships curl 7.61.
- The customer code was hardcoded to "operator"; yugaware.defaultUser.code is now used.
- A null password failed with "incompatible types for comparison".

The new yugaware.validateDefaultUser helper fails the render, listing every problem, when YBA
would reject the registration:

- CustomerRegisterFormData: every field is required, code is at most 15 characters, username
  (sent as name) at least 3, and email must fully match Play's @Email pattern (taken from
  play-java-forms 3.0.10, which YBA builds against).
- PasswordPolicyService.checkPasswordPolicy: /api/register creates the customer and checks the
  password in the same call, so no customer-level policy exists yet and the yb.pwdpolicy
  defaults apply - 8 characters (14 with yugaware.fips.enabled), at least one upper case
  letter, lower case letter, digit and special character, and no backtick. The messages are
  YBA's own.

Test Plan:
New tests/test_default_user.yaml covers each check, the FIPS length, the escaping, the
customer code and the curl flags.
helm unittest -f "tests/test_*.yaml" stable/yugaware: 95 passed, 95 total.
Rendered passwords containing ', ", \, <&> and non-ASCII letters; each round-trips exactly
through the YAML and the JSON body.
Checked the email pattern in Go against Java's java.util.regex on 11 addresses; the results
are identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yb-agentk-dev

yb-agentk-dev Bot commented Sep 30, 2026

Copy link
Copy Markdown

AgentK has started reviewing this pull request — findings will be posted here shortly.

Pushes are not reviewed automatically. Comment /agentk review to ask for another pass — but please only after substantial changes, not on every small push, and not for a push that just addresses the review comments.

@yb-agentk-dev yb-agentk-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 inline comments — 3 blocker, 1 major, 3 minor.

The password validation uses Unicode character class patterns (\p{Lu}, \p{Ll}, \p{Nd}) that Go's regexp package does not support, causing the validation to produce false positives and incorrectly reject valid passwords.

No REVIEW.md applies to the files this PR touches, so this review had only general judgement to go on. Please add one at the repo root, or beside the code it governs, to get better results.


React 👍 or 👎 on each comment above — right or wrong, both train the reviewer, and it is the only signal that says which.

Reviewed by Golum 1.0

{{- /* Java checks each char with Character.isUpperCase / isLowerCase / isDigit. */ -}}
{{- $checks := list
(list $minLength "(?s)." "characters")
(list 1 "\\p{Lu}" "upper case letters")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker — Regex pattern \p{Lu} uses Unicode character classes not supported by Go's regexp (which Helm uses). When the pattern fails to compile, regexFindAll returns empty, causing false-positive validation errors. Replace with [[:upper:]] (POSIX character class supported by Go).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this. Go's regexp (RE2) supports Unicode classes, including \p{Lu}, \p{Ll} and \p{Nd} (https://pkg.go.dev/regexp/syntax). A pattern that doesn't compile makes Sprig's regexFindAll panic, so it would never quietly return an empty list. The tests show the classes working: Passw0rd! passes, and abc reports every rule except the lower case one.

Switching to [[:upper:]] / [[:lower:]] / [[:digit:]] would make the check ASCII-only, and YBA uses Character.isUpperCase / isLowerCase / isDigit, which are Unicode-aware. The chart would then reject passwords like Pässwörd1! that YBA accepts.

{{- $checks := list
(list $minLength "(?s)." "characters")
(list 1 "\\p{Lu}" "upper case letters")
(list 1 "\\p{Ll}" "lower case letters")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker — Regex pattern \p{Ll} uses Unicode character classes not supported by Go's regexp. Replace with [[:lower:]].

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the \p{Lu} thread: RE2 supports \p{Ll}, and [[:lower:]] would be ASCII-only, unlike Java's Character.isLowerCase.

(list $minLength "(?s)." "characters")
(list 1 "\\p{Lu}" "upper case letters")
(list 1 "\\p{Ll}" "lower case letters")
(list 1 "\\p{Nd}" "digits")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker — Regex pattern \p{Nd} uses Unicode character classes not supported by Go's regexp. Replace with [[:digit:]].

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the \p{Lu} thread: RE2 supports \p{Nd}, which is the set Java's Character.isDigit tests (DECIMAL_DIGIT_NUMBER). [[:digit:]] would be ASCII-only.

@@ -1,4 +1,5 @@
{{- if .Values.yugaware.defaultUser.enabled -}}
{{- include "yugaware.validateDefaultUser" . -}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

major — This validation now breaks helm upgrade for releases that were already installed, not just new installs. The only gate is defaultUser.enabled at line 1. Helm renders hook templates on every upgrade, even though this Job only runs on post-install (line 18). So fail runs on every upgrade, and the rules it checks are stricter than anything an existing release was held to:

  • code was ignored until now. The body hardcoded "operator". A release installed with an empty code or one longer than 15 characters worked before, and now can't be upgraded at all.
  • Earlier installs could fail silently. Before --fail, an install whose registration YBA rejected (bad email, weak password, short username) still succeeded. --reuse-values carries those values forward, and every later upgrade now fails.
  • Turning on FIPS breaks upgrades. Suppose a release registered its user with a 10-character password and someone later runs helm upgrade --set yugaware.fips.enabled=true. The render fails on the 14-character rule, although the user already exists and this Job won't run again. The only way through is to edit defaultUser.password, which changes nothing in YBA. fips-validation.yaml exists to catch real FIPS problems (the DB password); this check blocks on a value that's irrelevant after install.

Wrap the include in {{- if .Release.IsInstall }} so the check only fires when the registration will actually be sent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 2105700. The include is now wrapped in {{- if .Release.IsInstall }}, which matches when the Job actually runs. New test: an upgrade with FIPS on and an empty code still renders.

Comment thread stable/yugaware/templates/_helpers.tpl Outdated
(list 1 "\\p{Ll}" "lower case letters")
(list 1 "\\p{Nd}" "digits")
(list 1 "[[:punct:]]" "special characters") -}}
{{- $policy := list -}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor — The test name says a backtick does not count as a special character, and values.yaml lists the special characters without it. But [[:punct:]] in Go's regexp is !-/:-@[-{-, and that range includes the backtick. For Passw0rd``, the check at this line finds one special character. The expected error in the test confirms it: the only message is "Password should not contain ", with no "at least 1 special characters". The render still fails because of the separate backtick check, but the reported problems don't match the documented rule. To exclude the backtick, spell the class out, e.g. [!-/:-@[-_{-]`, and add the missing special-characters message to the test's expected error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chart already matches YBA here. YBA's SPECIAL_CHARACTERS is !"#$%&'()*+,-./:;<=>?@[\]^_`{|}~, which includes the backtick and is exactly [[:punct:]]. So YBA also counts a backtick as special and then rejects it as disallowed: for Passw0rd` it returns only "Password should not contain `", which is what the test expects. The test name was wrong, though. It is renamed and commented in 2105700.

Comment thread stable/yugaware/templates/_helpers.tpl Outdated
{{- end -}}
{{- end -}}

{{- $code := len (regexFindAll "(?s)." $values.code -1) -}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor — The comment at line 386 says lengths are counted "as Java's String.length() does". regexFindAll "(?s)." counts code points, but String.length() counts UTF-16 code units, and a character outside the BMP (emoji, CJK extensions) counts as 2 in Java. The two disagree in both directions. A 15-code-point code ending in an emoji passes here and fails YBA's @maxlength(15). A username like a😀 is rejected here as 2 characters, but Java counts it as 3. Either count UTF-16 units (add one per code point above U+FFFF) or change the comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, fixed in 2105700. A new yugaware.utf16Length helper counts one unit per code point, plus one more for each code point above U+FFFF. It is used for code, username and the password's minimum length. Java also tests each UTF-16 char with isUpperCase etc., so half a surrogate pair never counts. The class checks therefore drop non-BMP characters first; for example, 𝐀 (U+1D400) doesn't count as upper case. Java agrees on each new test case (a😀 is 3, abcdefghijklmn😀 is 16, w0rd!😀𝐀 is 9 with no upper case letters).

{{- if and $values.username (lt $name 3) -}}
{{- $errors = append $errors (printf "yugaware.defaultUser.username is %d characters, at least 3 are required" $name) -}}
{{- end -}}
{{- $emailPattern := "^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$" -}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor — The comment says this is Play's @Email pattern, anchored to a full match. As far as I know, play-java-forms' EmailValidator pattern is \b[a-zA-Z0-9.!#$%&'*+/=?^_{|}~-]+@...\bused withmatches(). The leading \bthen requires the first character to be a word character. Play rejects addresses like-admin@example.comor.x@host, but ^...$accepts them, so the render passes and the Job still gets a 400. I couldn't open the play-java-forms 3.0.10 source from this review environment, so please check it. If the pattern starts with\b`, add the same first-character restriction here. Note that the Go-vs-Java comparison in the test plan ran this chart's pattern on both engines, not Play's.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this against the jar YBA builds with (sbt-plugin 3.0.10 in managed/project/plugins.sbt). javap -c -constants on play/data/validation/Constraints$EmailValidator.class in play-java-forms_2.13-3.0.10 shows the pattern is anchored with ^...$, not \b:

^[a-zA-Z0-9\.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$

The \b form comes from older Play releases. The Go-vs-Java comparison ran this exact pattern from the jar, so it is Play's pattern. Not changing this.

- Helm renders hooks on upgrade too, so the check blocked upgrades of releases whose user
  already exists (for example after turning on FIPS mode). It now runs only when
  .Release.IsInstall, as the Job itself does.
- Lengths are counted in UTF-16 code units, as Java's String.length() does, so a character
  outside the BMP counts as 2. The character-class checks skip such characters, since Java
  tests each UTF-16 char and half a surrogate pair never matches.
- Renamed the backtick test: YBA's SPECIAL_CHARACTERS includes the backtick, so it counts as
  special and is then rejected as disallowed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kv83821-yb
kv83821-yb merged commit d39a51b into yugabyte:master Oct 1, 2026
3 checks passed
kv83821-yb added a commit that referenced this pull request Oct 1, 2026
…against YBA's register rules (#239)

Summary:
Backport of #238 (d39a51b). The only conflict was in _helpers.tpl: 2026.1 lacks the
yugaware.hookDeletePolicy helper (PLAT-22692 is not backported), which appeared only as context.
The new helpers are added after yugaware.getPlatformUrl; on 2026.1 the customer-create Job keeps
its hardcoded hook-delete-policy.

With yugaware.defaultUser.enabled, the post-install Job registers the first customer and user
through /api/register. It had several bugs:

- The password was spliced into a JSON literal inside a single-quoted YAML scalar, and YBA's
  password policy requires a special character. A ' made the manifest invalid YAML, so
  helm install failed; a " or \ produced a body YBA could not parse. Email and username had
  the same problem. The body is now built with dict | toJson.
- curl ran without --fail, so a 4xx/5xx still exited 0. The hook was recorded as succeeded
  and deleted, and the release came up with no user. It now runs with --fail, and retries
  (--retry 30 --retry-delay 10 --retry-connrefused) while the platform is still starting.
  --fail-with-body is not usable: the AlmaLinux 8 based YBA image ships curl 7.61.
- The customer code was hardcoded to "operator"; yugaware.defaultUser.code is now used.
- A null password failed with "incompatible types for comparison".

The new yugaware.validateDefaultUser helper fails the render, listing every problem, when YBA
would reject the registration. It runs only on install, since Helm renders hooks on upgrade
too and the user already exists by then.

- CustomerRegisterFormData: every field is required, code is at most 15 characters, username
  (sent as name) at least 3, and email must fully match Play's @Email pattern (taken from
  play-java-forms 3.0.10, which YBA builds against). Lengths are counted in UTF-16 code units,
  as Java's String.length() does.
- PasswordPolicyService.checkPasswordPolicy: /api/register creates the customer and checks the
  password in the same call, so no customer-level policy exists yet and the yb.pwdpolicy
  defaults apply - 8 characters (14 with yugaware.fips.enabled), at least one upper case
  letter, lower case letter, digit and special character, and no backtick. The messages are
  YBA's own.

Test Plan:
helm unittest -f "tests/test_*.yaml" stable/yugaware: 93 passed, 2 failed. The 2 failures are
in tests/test_fips.yaml and already fail on 2026.1 without this change (74 passed, 2 failed);
all 19 new tests in tests/test_default_user.yaml pass.
helm lint stable/yugaware: 0 charts failed.
The rendered changes to customer-create-post-install-job.yaml, values.yaml and the new test file
are identical to master's.

(cherry picked from commit d39a51b)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants