-
Notifications
You must be signed in to change notification settings - Fork 34
mariadbaccount system accounts (PR 4 of 6) #345
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
074a092 to
ab01f3f
Compare
7eb7a65 to
6fdbd46
Compare
8697ffc to
32c7c7b
Compare
|
/retest |
1 similar comment
|
/retest |
32c7c7b to
553cbb5
Compare
|
/retest |
553cbb5 to
6f20d70
Compare
dciabrin
left a comment
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 need to understand how the deletion of a MariaDBAccount of type System would work if we plan to create one for the root database user. Right now I have the feeling we should have a safeguard mechanism to prevent deleting root.
Also, could you please rewrite the new KUTTL test into a Chainsaw test? I'm trying to get rid of all remaining KUTTL tests before disabling that CI test in prow to free up some CI resource.
|
re: deletion of the root user, the root MariaDBAccount in PR 6 the GaleraSpec gets a field that refers directly to the name of the MariaDBAccount used for root, and like all services that use MariaDBAccount, a finalizer gets associated with it which in this case is from the Galera instance. Galera creates its MariaDBAccount using the same mechanism / function that all the other services use right now to create their MariaDBAccounts. |
|
if we are nervous about mistakes (because it's near impossible to recover root from mysql) we can add an additional explicit check in the mariadb controller that tests "this is not the account that Galera has". would have to be careful that doesnt mess up deletion scenarios |
6f20d70 to
c020dd5
Compare
|
All the changes are made here and seems to be passing |
c020dd5 to
7ea3f2c
Compare
introduce a new class of MariaDBAccount called a "system" MariaDBAccount, indicated by a new enumerated field AccountType on the CR. Such accounts link directly to a Galera instance and have no dependency on a MariaDBDatabase CR. The expected targets for "system" accounts will include the Galera/mysql root username and password, as well as a system account used by mariadbbackup for SST. Refactor mariadbaccount_controller to isolate logic used for acquiring MariaDBDatabase and Galera CRs into separate functions, and ensure all MariaDBDatabase logic takes place only for "user" accounts (which would be all current MariaDBAccount CRs). Also correct an oversight where MariaDBAccount would not unconditionally apply a finalizer to its Secret object. This logic now takes place in addition to an unconditional removal of the finalizer when the MariaDBAccount object is deleted. A subsequent change will allow system-level passwords to be changed in place by applying the secret name to two separate fields MariaDBAccount/Spec/Secret and MariaDBAccount/Status/Secret. When these two names differ it will indicate an in-place password change should take place.
7ea3f2c to
e72f0dc
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lmiccini, zzzeek The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test mariadb-operator-build-deploy-kuttl |
|
@zzzeek: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/recheck |
introduce a new class of MariaDBAccount called a "system"
MariaDBAccount, indicated by a new enumerated field AccountType
on the CR. Such accounts link directly to a Galera instance
and have no dependency on a MariaDBDatabase CR.
The expected targets for "system" accounts will include the
Galera/mysql root username and password, as well as a system
account used by mariadbbackup for SST.
Refactor mariadbaccount_controller to isolate logic used for
acquiring MariaDBDatabase and Galera CRs into separate functions,
and ensure all MariaDBDatabase logic takes place only for "user"
accounts (which would be all current MariaDBAccount CRs).
Also correct an oversight where MariaDBAccount would not unconditionally
apply a finalizer to its Secret object. This logic now takes place
in addition to an unconditional removal of the finalizer when the
MariaDBAccount object is deleted.
A subsequent change will allow system-level passwords to be changed
in place by applying the secret name to two separate fields
MariaDBAccount/Spec/Secret and MariaDBAccount/Status/Secret. When
these two names differ it will indicate an in-place password change
should take place.