-
Notifications
You must be signed in to change notification settings - Fork 203
K8SPXC-1733 Ability to customize secrets generation #2227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Customize secrets generation: * keep current secret generation behavior as default * Ability to customize the special symbols to include in secret generation * Ability to customize min/max secret length Linked to percona#2222
commit: 2da0e2d |
| - name: DISABLE_TELEMETRY | ||
| value: "false" | ||
| image: perconalab/percona-xtradb-cluster-operator:main | ||
| image: perconalab/percona-xtradb-cluster-operator:feat-add-generated-secrets-options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes to the image should be reverted to main across all the similar occurrences.
| }) | ||
| for password := range userSecret.Data { | ||
| It("Should generate user secrets without symbols", func() { | ||
| Expect(strings.ContainsAny(string(password), "!#$%&()*+,-.<=>?@[]^_{}~")).To(BeFalse()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string(...) is not needed
| CRVersion string `json:"crVersion,omitempty"` | ||
| Pause bool `json:"pause,omitempty"` | ||
| SecretsName string `json:"secretsName,omitempty"` | ||
| GeneratedSecretsOptions *GeneratedSecretsOptions `json:"generatedSecretsOptions,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the configuration we introduce is related to the passwords that are provisioned through secrets and not for the secrets themselves, maybe we could adjust the naming of the new options accordingly. Some options:
- PasswordGenerationOptions - Most accurate and clear
- GeneratedPasswordOptions - Also clear and descriptive
- PasswordOptions - Simpler but still clear
PasswordGenerationOptions is the most suitable one IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think PasswordGenerationOptions is good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @hors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to reuse the naming from the “Password Validation Options” feature in MySQL. What do you think about:
spec:
secretGenerationOptions:
lengthMin: 16
lengthMax: 32
allowedSymbols: "!@#$%" # Overrides or supplements the default set
or
spec:
passwordGenerationOptions:
lengthMin: 16
lengthMax: 32
allowedSymbols: "!@#$%" # Overrides or supplements the default set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spec:
passwordGenerationOptions:
lengthMin: 16
lengthMax: 32
allowedSymbols: "!@#$%" # Overrides or supplements the default set
I would go with this ☝🏽
| }) | ||
| } | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe in this test should we should focus on the following 2 scenarios:
- Default configuration of passwords with nil the new option
- The new option configured and asserting the specified symbols and lengths
|
the unit test failures for proxysql password are unrelated to this pr and will be fixed under this pr: #2124 |
Customize secrets generation:
Linked to #2222
CHANGE DESCRIPTION
Problem:
Short explanation of the problem.
By default, the operator generates his secrets for system users (root, xtrabackup, proxyadmin, replication,...etc...).
The current secrets generation is based on constants, prohibiting any customization:
Cause:
Short explanation of the root cause of the issue if applicable.
Solution:
Short explanation of the solution we are providing with this PR.
Proposal would be to have a minimal customization, e.g.:
Suggested implementation would be a dedicated configuration structure for generated secrets:
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability