migrate from databaseUsername to databaseAccount

[zzzeek]

This moves placement to fully use MariaDBAccount based on the dev work being done for mariadb-operator:

https://github.com/openstack-k8s-operators/mariadb-operator/pull/184/files

Lead Jira: OSPRH-4095

1. controller calls EnsureMariaDBAccount up front to make sure MariaDBAccount is present
2. error code from EnsureMariaDBAccount is handled, Conditions are amended when error is returned
3. controller calls NewDatabaseForAccount instead of NewDatabase
4. GetAccountAndSecret is used to retrieve account /secret to populate template
5. GetDatabaseByName() , normally used for delete finalizers, replaced with GetDatabaseByNameAndAccount
6. CreateOrPatchAll() used to patch the Database, replacing CreateOrPatchDB / CreateOrPatchDBByName
7. controller calls DeleteUnusedMariaDBAccountFinalizers when launched pods are definitely running on a new MariaDBAccount, returns error code if present
8. PasswordSelectors that refer to database are removed
9. all databaseUser replaced with databaseAccount inside of all XYZ_types.go
10. all databaseUser replaced with databaseAccount inside of all kuttl/*.yaml
11. all default databaseUser names become databaseAccount, replacing underscores with dashes inside of all XYZ_types.go
12. all default databaseUser names become databaseAccount, replacing underscores with dashes inside of all kuttl/*.yaml
13. MariaDBAccountSuiteTests are used in controller ginkgo tests if it has them
14. Use configsecrets for database URLs; remove from job hash - already was like that
15. 184 is merged and replaces from go.mod are removed

[gibizer]

do we need to set the requireTLS flag based on some input?

[stuggi]

tls is used as soon the mariadbdatabase reflects it is hosted on a galera supporting tls and the service has a CA to validate it. we could add setting requiredTLS as a follow up. maybe after we switched our default to enable podlevel/internal tls per default?

[gibizer]

I'm OK about a follow up, just seemed strange that we merged #165 yesterday in placement, and here we explicitly disable it. So I wanted to make sure it is intentional.

[zzzeek]

the ensuremariadbaccount is ultimately going to be a receiver of a MariaDBAccount that may be coming from openstack-operator, and the account is not associated with any database as of yet. at this stage, it's not appropriate to set TLS, because nothing is yet known about what kind of database this account will be applied to. if we are flipping a TLS flag on when we do the ensure MariaDBDatabase phase, then we would want to flip on "require TLS" on the MariaDBAccount as well, and that would be within the mariadbdatabase_funcs. if that's the use we want? that is, if DB supports TLS, then all accounts should require TLS. is that a req?

[stuggi]

I'm OK about a follow up, just seemed strange that we merged #165 yesterday in placement, and here we explicitly disable it. So I wanted to make sure it is intentional.

right, setting requireTLS == false is not that then no tls is used for db connection. it is just that this db user could use tls and non tls to connect. if tls is used is being controlled by the my.cnf content.

that is, if DB supports TLS, then all accounts should require TLS. is that a req?

I am not sure right now. I can discuss it in our tomorrows tls meeting and get back

[gibizer]

thanks for the explanation. I defer to @stuggi if this is OK as is.

[stuggi]

we did not came up with a real preference if "require TLS" should be used as soon the DB supports TLS. since the services will use tls as soon the DB supports it and the service has the CA we could set it, but atm it won't hurt keep it available. I'd personally do it in a follow up, after we tested all the different scenarios, also with adoption in mind. adoption of tlse env is still in early phase.

[zzzeek]

yah i think this should be all followup stuff. we're just trying to get all the levers available right now (like, every controller gets a distinct DB password, stuff like that :) )

[gibizer]

I feel that ensuring the DBAccount logically belongs to ensureDB, so I would move this new logic into that func.

[zzzeek]

Yes I noticed that the recent merge has consolidated DB functions to be ahead of configmaps in any case, which was one of the reasons ensureaccount was up there. since ensureaccount isn't actually going away, for those controllers where ensuredb is before configmaps (Which I guess will be all of them now), might as well put ensure account into it. it starts to look like the whole of the various ensureDB functions could just be one big function in mariadb-operator, but i dont know that we have time to do all that just yet.

[gibizer]

yes, @stuggi fixed the ordering of the steps to have db created before we generate config. I would do the ensureDB consolidation here locally, and maybe looking at a common mariadb-operator/api function later separatly

[gibizer]

looks good to me. We just need to land the mariadb dependency first

[zzzeek]

looks good to me. We just need to land the mariadb dependency first

yah once mariadb lands I can re-crank through everyone and add mistral / telemetry / barbican also

[gibizer]

I tested this and I was able to rotate the db password properly. Nice work!

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gibizer, 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:

• OWNERS [gibizer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment