DNM: mariadbaccount system accounts #320
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zzzeek The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/ok-to-test |
|
/retest-required |
zzzeek
force-pushed
the
OSPRH-14916
branch
2 times, most recently
from
517676e to
8744b52
Compare
|
/ok-to-test |
In order to facilitate an in-place change to the name of the Secret that is referenced by a Galera instance for the mysql root password, rework the approach used by pods and shell scripts to no longer require the root secret name and/or password be passed by environment variable, instead using a pod-level cluster query to retrieve the current root password. The logic to retrieve this password is encapsulated into a single shell script that is present as a volume mount on running containers. This allows Job objects to be created with hashes that do not link to a specific Secret name, as well as to create StatefulSet objects that don't refer to this name. When the Secret name changes on a Galera instance for an in-place root password change, the hashes / CRs for these objects will remain unchanged. A subsequent change to the mariadb operator will add the ability to change the mysql root password of a Galera cluster using a dual-reference architecture where the "current" root secret will be part of <CR>/Status, while the secret referenced in <CR>/Spec will be the "new" root secret. When these two names differ, that will indicate an in-place password change should take place, as well as allowing the pre-existing root password to be available at the same time as the new one in order to do a root password change. The same architecture will be applied to a new class of "system" MariaDBAccount objects that are for use only by the Galera instance itself and do not have a link to any MariaDBDatabase CR. The Galera CR itself will no longer use osp-secret for the mysql root password nor will the secret be directly referenced from the Galera CR, instead referenced by a "system" MariaDBAccount CR which the Galera operator itself will create.
zzzeek
changed the title
|
/retest |
|
/retest-required |
zzzeek
force-pushed
the
OSPRH-14916
branch
2 times, most recently
from
a73c8d4 to
94f22a7
Compare
|
/retest |
|
/retest |
this change reorganizes mariadbaccount_controller to move the majority of MariaDBDatabase fetch/wait logic as well as Galera fetch/wait logic to separate methods getMariaDBDatabaseForCreate, getMariaDBDatabaseForDelete, getGaleraForCreateOrDelete. This is to allow more flexibility for upcoming changes. All current functionality should remain identical within this commit.
|
/retest |
1 similar comment
|
/retest |
in mariadbdatabase_funcs, the EnsureMariaDBAccount function called by external controllers adds a finalizer for that calling controller to the Secret referenced by the MariaDBAccount. This seems a little off, since the Secret is most immediately needed by the MariaDBAccount CR itself, and the controller refers to that MariaDBAccount CR also. It seems more appropriate that MariaDBAccount itself should maintain its own finalizer on that Secret, so this logic is added there. The change here causes the API function EnsureMariaDBAccount to add a finalizer to the secret that is local to the mariadbaccount, rather than the helper passed for the calling controller. Existing "remove finalizer" calls which look for the calling controller's finalizer tag in the secret are maintained however to assist with backwards compatibility. This comes up now because we are seeking to add a new class of system-level MariaDBAccount that is used only by the Galera controller itself, but also that these accounts (really all accounts, but mainly the system ones) will support in-place password changes by updating the name of the Secret to be used, implying the old one is no longer needed once the change takes place; it therefore is most appropriate that MariaDBAccount maintain its own finalizers on these secrets.
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.
The existing mariadb operator in fact already "supports" in-place change of secret, since if you change Spec.Secret to a new secret name, that would imply a new job hash, and the account.sh script running using only GRANT statements would update the password per mariadb. This already works for flipping the TLS flag on and off too. So in this patch, we clean this up and add a test to include: * a new field Status.CurrentSecret, which is used to indicate the previous secret from which the finalizer should be removed. This will also be used when we migrate the root password to use MariaDBAccount by providing the "current" root password when changing to a new root password * improved messaging in log messages, name of job. This changes the job hash for mariadbaccount which will incur a run on existing environments, however the job hashes are already changing on existing environments due to the change in how the mysql root password is sent, i.e. via volume mounted script rather than env var secret * update account.sh to use modern idiomatic patterns for user create /alter, while mariadb is fine with the legacy style of using only GRANT statements, MySQL 8 no longer allows this statement to proceed without a CREATE USER, so formalize the commands used here to use distinct CREATE USER, ALTER USER, GRANT statements and clarify the script is good for all create/update user operations.
zzzeek
force-pushed
the
OSPRH-14916
branch
3 times, most recently
from
9c6111e to
e6c102c
Compare
|
/retest |
|
not related to the immediate failures but im noticing that deep in the prow scripts and probably elsewhere, osp-secret seems to be referenced for mariadb root password outside of the galera operator (that 12345678 password is no longer used as long as this change is kept generating a new root PW each time):
|
|
Well, what we can do is merge 9fff6b0 ahead of everything else, then change all scripts everywhere to use the root_auth.sh script so becomes that change can be made just with 9fff6b0 merged alone as the controller that creates new galera pods, then all the scripts everywhere else can be abstracted away from where the galera root password comes from |
|
seeing as mariadb-operator-build-deploy-kuttl is now what's failing, I probably need to restore copying osp-secret as the initial root password just to get the code out the door as there are likely other parts of build / CI / deployment that are relying on this structure |
zzzeek
force-pushed
the
OSPRH-14916
branch
5 times, most recently
from
7fcb95e to
b7710bc
Compare
|
|
||
| // ******** TEMPORARY ************ | ||
| // Pull the DbRootPassword from osp-secret to allow CI / external | ||
| // scripts to work, until they can adapt to root_auth.sh |
There was a problem hiding this comment.
some view into what scripts need modification can be seen using this query:
https://github.com/search?q=org%3Aopenstack-k8s-operators+DbRootPassword&type=code&p=1
This commit ties together the previous ones to create a new MariaDBAccount when a Galera instance is created, and then to use the password from that account/secret in the mariadb bootstrap/maintenance scripts. Galera gets bootstrapped with this secret, then the mariadbaccount controller, who is waiting for galera to be available to set up this new "root" account, wakes up when galera is running, and changes the root password to itself, establishing the initial job hash for the mariadbaccount. As we now have a mariadbaccount linked to the outermost lifecycle of a galera instance, some hardening of the deletion process has been added to clarify that mariadbaccount will run deletion jobs only if Galera is not marked for deletion. If galera is marked for deletion, then we have to assume the service/pods are gone and no more drops can take place, even if the Galera CR is still present (chainsaw conveniently adds its own finalizer to Galera when running, preventing it from being fully deleted, which exposed this issue).
|
I needed to push an update to the galera CRD not so long ago, so this needs to be rebased... I wonder if we could take that opportunity to maybe split those commits into separate PR (as suggested by @zzzeek to help move this forward), so that we can do a simpler review before merging? I had a chat with @zzzeek already to discuss the details behind the "system" MariaDBAccount approach, and I think it's solid and that's where we want to go. We now need to carefully review those changes before merging. |
|
PR needs rebase. 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. |
|
OK do you want all six ? if that makes it easier |
That'd be helpful for me at least :) |
|
six it is |
zzzeek
changed the title
|
@zzzeek: The following tests 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. |
3 participants
This PR is broken into individual PRs:
#342
#343
#344
#345
#346
#347
notes overall below:
this is for OSPRH-14916