diff --git a/api/bases/mariadb.openstack.org_mariadbaccounts.yaml b/api/bases/mariadb.openstack.org_mariadbaccounts.yaml index 9a92f28f..51dd7196 100644 --- a/api/bases/mariadb.openstack.org_mariadbaccounts.yaml +++ b/api/bases/mariadb.openstack.org_mariadbaccounts.yaml @@ -48,6 +48,12 @@ spec: spec: description: MariaDBAccountSpec defines the desired state of MariaDBAccount properties: + accountType: + default: User + enum: + - User + - System + type: string requireTLS: default: false description: Account must use TLS to connect to the database @@ -108,6 +114,14 @@ spec: - type type: object type: array + currentSecret: + description: |- + the Secret that's currently in use for the account. + keeping a handle to this secret allows us to remove its finalizer + when it's replaced with a new one. It also is useful for storing + the current "root" secret separate from a newly proposed one which is + needed when changing the database root password. + type: string hash: additionalProperties: type: string diff --git a/api/v1beta1/mariadbaccount_types.go b/api/v1beta1/mariadbaccount_types.go index 32819281..dd5885e2 100644 --- a/api/v1beta1/mariadbaccount_types.go +++ b/api/v1beta1/mariadbaccount_types.go @@ -48,10 +48,28 @@ type MariaDBAccountSpec struct { // Account must use TLS to connect to the database // +kubebuilder:default=false RequireTLS bool `json:"requireTLS"` + + // +kubebuilder:validation:Enum=User;System + // +kubebuilder:default=User + AccountType AccountType `json:"accountType,omitempty"` } +type AccountType string + +const ( + User AccountType = "User" + System AccountType = "System" +) + // MariaDBAccountStatus defines the observed state of MariaDBAccount type MariaDBAccountStatus struct { + // the Secret that's currently in use for the account. + // keeping a handle to this secret allows us to remove its finalizer + // when it's replaced with a new one. It also is useful for storing + // the current "root" secret separate from a newly proposed one which is + // needed when changing the database root password. + CurrentSecret string `json:"currentSecret,omitempty"` + // Deployment Conditions Conditions condition.Conditions `json:"conditions,omitempty" optional:"true"` @@ -85,3 +103,11 @@ type MariaDBAccountList struct { func init() { SchemeBuilder.Register(&MariaDBAccount{}, &MariaDBAccountList{}) } + +func (mariadbAccount MariaDBAccount) IsSystemAccount() bool { + return mariadbAccount.Spec.AccountType == System +} + +func (mariadbAccount MariaDBAccount) IsUserAccount() bool { + return mariadbAccount.Spec.AccountType == "" || mariadbAccount.Spec.AccountType == User +} diff --git a/config/crd/bases/mariadb.openstack.org_mariadbaccounts.yaml b/config/crd/bases/mariadb.openstack.org_mariadbaccounts.yaml index 9a92f28f..51dd7196 100644 --- a/config/crd/bases/mariadb.openstack.org_mariadbaccounts.yaml +++ b/config/crd/bases/mariadb.openstack.org_mariadbaccounts.yaml @@ -48,6 +48,12 @@ spec: spec: description: MariaDBAccountSpec defines the desired state of MariaDBAccount properties: + accountType: + default: User + enum: + - User + - System + type: string requireTLS: default: false description: Account must use TLS to connect to the database @@ -108,6 +114,14 @@ spec: - type type: object type: array + currentSecret: + description: |- + the Secret that's currently in use for the account. + keeping a handle to this secret allows us to remove its finalizer + when it's replaced with a new one. It also is useful for storing + the current "root" secret separate from a newly proposed one which is + needed when changing the database root password. + type: string hash: additionalProperties: type: string diff --git a/controllers/galera_controller.go b/controllers/galera_controller.go index 7b81606c..f1b478cf 100644 --- a/controllers/galera_controller.go +++ b/controllers/galera_controller.go @@ -1058,9 +1058,8 @@ func (r *GaleraReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// GetDatabaseObject - returns either a Galera or MariaDB object (and an associated client.Object interface). +// GetDatabaseObject - returns a Galera object. // used by both MariaDBDatabaseReconciler and MariaDBAccountReconciler -// this will later return only Galera objects, so as a lookup it's part of the galera controller func GetDatabaseObject(ctx context.Context, clientObj client.Client, name string, namespace string) (*mariadbv1.Galera, error) { dbGalera := &mariadbv1.Galera{ ObjectMeta: metav1.ObjectMeta{ diff --git a/controllers/mariadbaccount_controller.go b/controllers/mariadbaccount_controller.go index 498cffe3..ab2d330f 100644 --- a/controllers/mariadbaccount_controller.go +++ b/controllers/mariadbaccount_controller.go @@ -30,6 +30,7 @@ import ( util "github.com/openstack-k8s-operators/lib-common/modules/common/util" databasev1beta1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" mariadb "github.com/openstack-k8s-operators/mariadb-operator/pkg/mariadb" + batchv1 "k8s.io/api/batch/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -113,32 +114,43 @@ func (r *MariaDBAccountReconciler) Reconcile(ctx context.Context, req ctrl.Reque }() // initialize conditions used later as Status=Unknown - cl := condition.CreateList( - condition.UnknownCondition(condition.ReadyCondition, condition.InitReason, condition.ReadyInitMessage), - condition.UnknownCondition(databasev1beta1.MariaDBServerReadyCondition, condition.InitReason, databasev1beta1.MariaDBServerReadyInitMessage), - condition.UnknownCondition(databasev1beta1.MariaDBDatabaseReadyCondition, condition.InitReason, databasev1beta1.MariaDBDatabaseReadyInitMessage), - condition.UnknownCondition(databasev1beta1.MariaDBAccountReadyCondition, condition.InitReason, databasev1beta1.MariaDBAccountReadyInitMessage), - ) + cl := condition.Conditions{ + *condition.UnknownCondition(condition.ReadyCondition, condition.InitReason, condition.ReadyInitMessage), + *condition.UnknownCondition(databasev1beta1.MariaDBServerReadyCondition, condition.InitReason, databasev1beta1.MariaDBServerReadyInitMessage), + } + if instance.IsUserAccount() { + // user accounts also need the database ready condition + // the element is being inserted into the list in a specific location + // to preseve expectations of tests suites that are hardcoded to + // expect a specific ordering of Conditions in YAML displays + cl = append(cl, *condition.UnknownCondition(databasev1beta1.MariaDBDatabaseReadyCondition, condition.InitReason, databasev1beta1.MariaDBDatabaseReadyInitMessage)) + } + cl = append(cl, *condition.UnknownCondition(databasev1beta1.MariaDBAccountReadyCondition, condition.InitReason, databasev1beta1.MariaDBAccountReadyInitMessage)) instance.Status.Conditions.Init(&cl) if instance.DeletionTimestamp.IsZero() || isNewInstance { //revive:disable:indent-error-flow - return r.reconcileCreate(ctx, log, helper, instance) + return r.reconcileCreateOrUpdate(ctx, log, helper, instance) } else { return r.reconcileDelete(ctx, log, helper, instance) } } -// reconcileDelete - run reconcile for case where delete timestamp is zero -func (r *MariaDBAccountReconciler) reconcileCreate( +// reconcileCreateOrUpdate - run reconcile for case where delete timestamp is zero +func (r *MariaDBAccountReconciler) reconcileCreateOrUpdate( ctx context.Context, log logr.Logger, helper *helper.Helper, instance *databasev1beta1.MariaDBAccount) (result ctrl.Result, _err error) { - // get a handle to the current, active MariaDBDatabase. - // if not ready yet, requeue. - mariadbDatabase, result, err := r.getMariaDBDatabaseForCreate(ctx, log, instance) - if mariadbDatabase == nil { - return result, err + var mariadbDatabase *databasev1beta1.MariaDBDatabase + var err error + + if instance.IsUserAccount() { + // for User account, get a handle to the current, active MariaDBDatabase. + // if not ready yet, requeue. + mariadbDatabase, result, err = r.getMariaDBDatabaseForCreate(ctx, log, instance) + if mariadbDatabase == nil { + return result, err + } } if controllerutil.AddFinalizer(instance, helper.GetFinalizer()) { @@ -146,11 +158,13 @@ func (r *MariaDBAccountReconciler) reconcileCreate( return ctrl.Result{}, nil } - // MariaDBdatabase exists and we are a create case. ensure finalizers set up - if controllerutil.AddFinalizer(mariadbDatabase, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) { - err = r.Update(ctx, mariadbDatabase) - if err != nil { - return ctrl.Result{}, err + if instance.IsUserAccount() { + // MariaDBdatabase exists and we are a create case. ensure finalizers set up + if controllerutil.AddFinalizer(mariadbDatabase, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) { + err = r.Update(ctx, mariadbDatabase) + if err != nil { + return ctrl.Result{}, err + } } } @@ -174,13 +188,34 @@ func (r *MariaDBAccountReconciler) reconcileCreate( return result, err } - log.Info(fmt.Sprintf("Running account create '%s' MariaDBDatabase '%s'", - instance.Name, mariadbDatabase.Spec.Name)) - jobDef, err := mariadb.CreateDbAccountJob( - dbGalera, instance, mariadbDatabase.Spec.Name, dbHostname, - dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector) - if err != nil { - return ctrl.Result{}, err + var jobDef *batchv1.Job + + var createOrUpdate string + if instance.Status.CurrentSecret != "" { + createOrUpdate = "update" + } else { + createOrUpdate = "create" + } + + if instance.IsUserAccount() { + log.Info(fmt.Sprintf("Checking %s account job '%s' MariaDBDatabase '%s'", + createOrUpdate, + instance.Name, mariadbDatabase.Spec.Name)) + jobDef, err = mariadb.CreateOrUpdateDbAccountJob( + dbGalera, instance, mariadbDatabase.Spec.Name, dbHostname, + dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector) + if err != nil { + return ctrl.Result{}, err + } + } else { + log.Info(fmt.Sprintf("Checking %s system account job '%s'", createOrUpdate, + instance.Name)) + jobDef, err = mariadb.CreateOrUpdateDbAccountJob( + dbGalera, instance, "", dbHostname, + dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector) + if err != nil { + return ctrl.Result{}, err + } } accountCreateHash := instance.Status.Hash[databasev1beta1.AccountCreateHash] @@ -200,11 +235,24 @@ func (r *MariaDBAccountReconciler) reconcileCreate( } if accountCreateJob.HasChanged() { + if instance.Status.Hash == nil { instance.Status.Hash = make(map[string]string) } instance.Status.Hash[databasev1beta1.AccountCreateHash] = accountCreateJob.GetHash() - log.Info(fmt.Sprintf("Job %s hash added - %s", jobDef.Name, instance.Status.Hash[databasev1beta1.AccountCreateHash])) + log.Info(fmt.Sprintf("Job %s hash created or updated - %s", jobDef.Name, instance.Status.Hash[databasev1beta1.AccountCreateHash])) + + // set up new Secret and remove finalizer from old secret + if instance.Status.CurrentSecret != instance.Spec.Secret { + currentSecret := instance.Status.CurrentSecret + err = r.removeSecretFinalizer(ctx, helper, currentSecret, instance.Namespace) + if err == nil { + instance.Status.CurrentSecret = instance.Spec.Secret + } else { + return ctrl.Result{}, err + } + } + } // database creation finished @@ -228,9 +276,14 @@ func (r *MariaDBAccountReconciler) reconcileDelete( ctx context.Context, log logr.Logger, helper *helper.Helper, instance *databasev1beta1.MariaDBAccount) (result ctrl.Result, _err error) { - mariadbDatabase, result, err := r.getMariaDBDatabaseForDelete(ctx, log, helper, instance) - if mariadbDatabase == nil { - return result, err + var mariadbDatabase *databasev1beta1.MariaDBDatabase + var err error + + if instance.IsUserAccount() { + mariadbDatabase, result, err = r.getMariaDBDatabaseForDelete(ctx, log, helper, instance) + if mariadbDatabase == nil { + return result, err + } } // dont do actual DROP USER until finalizers from downstream controllers @@ -259,10 +312,12 @@ func (r *MariaDBAccountReconciler) reconcileDelete( databasev1beta1.MariaDBAccountReadyForDeleteMessage, ) - instance.Status.Conditions.MarkTrue( - databasev1beta1.MariaDBDatabaseReadyCondition, - databasev1beta1.MariaDBDatabaseReadyMessage, - ) + if instance.IsUserAccount() { + instance.Status.Conditions.MarkTrue( + databasev1beta1.MariaDBDatabaseReadyCondition, + databasev1beta1.MariaDBDatabaseReadyMessage, + ) + } // now proceed to do actual work. acquire the Galera instance // which will lead us to the hostname and container image to target @@ -278,20 +333,21 @@ func (r *MariaDBAccountReconciler) reconcileDelete( // scheme allows multiple MariaDBAccounts to claim the same MariaDBDatabase // as a dependency) and allows a delete of the MariaDBDatabase to proceed // assuming no other finalizers - if controllerutil.RemoveFinalizer(mariadbDatabase, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) { - err = r.Update(ctx, mariadbDatabase) + if instance.IsUserAccount() { + if controllerutil.RemoveFinalizer(mariadbDatabase, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) { + err = r.Update(ctx, mariadbDatabase) - if err != nil && !k8s_errors.IsNotFound(err) { - return ctrl.Result{}, err + if err != nil && !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, err + } } - } // remove finalizer "openstack.org/mariadbaccount" from both the // MariaDBAccount as well as the Secret which is referenced from the // MariaDBAccount, allowing both to be deleted assuming no other // finalizers - err = r.removeAccountAndSecretFinalizer(ctx, helper, instance) + err := r.removeAccountAndSecretFinalizer(ctx, helper, instance) return ctrl.Result{}, err } else if dbGalera == nil { return result, err @@ -300,13 +356,24 @@ func (r *MariaDBAccountReconciler) reconcileDelete( dbContainerImage := dbGalera.Spec.ContainerImage serviceAccountName := dbGalera.RbacResourceName() - log.Info(fmt.Sprintf("Running account delete '%s' MariaDBDatabase '%s'", instance.Name, mariadbDatabase.Spec.Name)) + var jobDef *batchv1.Job - jobDef, err := mariadb.DeleteDbAccountJob(dbGalera, instance, mariadbDatabase.Spec.Name, dbHostname, dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector) - if err != nil { - return ctrl.Result{}, err - } + if instance.IsUserAccount() { + + log.Info(fmt.Sprintf("Running account delete '%s' MariaDBDatabase '%s'", instance.Name, mariadbDatabase.Spec.Name)) + jobDef, err = mariadb.DeleteDbAccountJob(dbGalera, instance, mariadbDatabase.Spec.Name, dbHostname, dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector) + if err != nil { + return ctrl.Result{}, err + } + } else { + log.Info(fmt.Sprintf("Running system account delete '%s'", instance.Name)) + + jobDef, err = mariadb.DeleteDbAccountJob(dbGalera, instance, "", dbHostname, dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector) + if err != nil { + return ctrl.Result{}, err + } + } accountDeleteHash := instance.Status.Hash[databasev1beta1.AccountDeleteHash] accountDeleteJob := job.NewJob( jobDef, @@ -335,10 +402,12 @@ func (r *MariaDBAccountReconciler) reconcileDelete( // scheme allows multiple MariaDBAccounts to claim the same MariaDBDatabase // as a dependency) and allows a delete of the MariaDBDatabase to proceed // assuming no other finalizers - if controllerutil.RemoveFinalizer(mariadbDatabase, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) { - err = r.Update(ctx, mariadbDatabase) - if err != nil { - return ctrl.Result{}, err + if instance.IsUserAccount() { + if controllerutil.RemoveFinalizer(mariadbDatabase, fmt.Sprintf("%s-%s", helper.GetFinalizer(), instance.Name)) { + err = r.Update(ctx, mariadbDatabase) + if err != nil { + return ctrl.Result{}, err + } } } @@ -346,7 +415,6 @@ func (r *MariaDBAccountReconciler) reconcileDelete( // both the MariaDBAccount as well as the Secret which is referenced // from the MariaDBAccount, allowing both to be deleted err = r.removeAccountAndSecretFinalizer(ctx, helper, instance) - return ctrl.Result{}, err } @@ -526,9 +594,17 @@ func (r *MariaDBAccountReconciler) getGaleraForCreateOrDelete( helper *helper.Helper, instance *databasev1beta1.MariaDBAccount, mariadbDatabase *databasev1beta1.MariaDBDatabase) (*databasev1beta1.Galera, string, ctrl.Result, error) { - dbName := mariadbDatabase.Labels["dbName"] + var dbGalera *databasev1beta1.Galera + var err error + var dbName string - dbGalera, err := GetDatabaseObject(ctx, r.Client, dbName, instance.Namespace) + if instance.IsUserAccount() { + dbName = mariadbDatabase.Labels["dbName"] + } else { + // note mariadbDatabase is passed as nil in this case + dbName = instance.Labels["dbName"] + } + dbGalera, err = GetDatabaseObject(ctx, r.Client, dbName, instance.Namespace) if err != nil { log.Error(err, "Error retrieving Galera instance") @@ -657,7 +733,22 @@ func (r *MariaDBAccountReconciler) ensureAccountSecret( func (r *MariaDBAccountReconciler) removeAccountAndSecretFinalizer(ctx context.Context, helper *helper.Helper, instance *databasev1beta1.MariaDBAccount) error { - accountSecret, _, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace) + err := r.removeSecretFinalizer( + ctx, helper, instance.Spec.Secret, instance.Namespace, + ) + if err != nil && !k8s_errors.IsNotFound(err) { + return err + } + + // remove mariadbaccount finalizer which will update at end of reconcile + controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) + + return nil +} + +func (r *MariaDBAccountReconciler) removeSecretFinalizer(ctx context.Context, + helper *helper.Helper, secretName string, namespace string) error { + accountSecret, _, err := secret.GetSecret(ctx, helper, secretName, namespace) if err == nil { if controllerutil.RemoveFinalizer(accountSecret, helper.GetFinalizer()) { @@ -670,9 +761,6 @@ func (r *MariaDBAccountReconciler) removeAccountAndSecretFinalizer(ctx context.C return err } - // will take effect when reconcile ends - controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) - return nil } diff --git a/pkg/mariadb/account.go b/pkg/mariadb/account.go index 0dbf897f..15e9e87a 100644 --- a/pkg/mariadb/account.go +++ b/pkg/mariadb/account.go @@ -19,8 +19,8 @@ type accountCreateOrDeleteOptions struct { RequireTLS string } -// CreateDbAccountJob creates a Kubernetes job for creating a MariaDB database account -func CreateDbAccountJob(galera *mariadbv1.Galera, account *mariadbv1.MariaDBAccount, databaseName string, databaseHostName string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) { +// CreateOrUpdateDbAccountJob creates or updates a Kubernetes job for creating a MariaDB database account +func CreateOrUpdateDbAccountJob(galera *mariadbv1.Galera, account *mariadbv1.MariaDBAccount, databaseName string, databaseHostName string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) { var tlsStatement string if account.Spec.RequireTLS { tlsStatement = " REQUIRE SSL" @@ -47,7 +47,7 @@ func CreateDbAccountJob(galera *mariadbv1.Galera, account *mariadbv1.MariaDBAcco // provided db name is used as metadata name where underscore is a not allowed // character. Lets replace all underscores with hypen. Underscores in the db name are // possible. - Name: strings.ReplaceAll(account.Spec.UserName, "_", "-") + "-account-create", + Name: strings.ReplaceAll(account.Spec.UserName, "_", "-") + "-account-create-update", Namespace: account.Namespace, Labels: labels, }, @@ -58,7 +58,7 @@ func CreateDbAccountJob(galera *mariadbv1.Galera, account *mariadbv1.MariaDBAcco ServiceAccountName: serviceAccountName, Containers: []corev1.Container{ { - Name: "mariadb-account-create", + Name: "mariadb-account-create-update", Image: containerImage, Command: []string{"/bin/sh", "-c", dbCmd}, Env: []corev1.EnvVar{ diff --git a/templates/account.sh b/templates/account.sh index ba076cb3..891ede76 100755 --- a/templates/account.sh +++ b/templates/account.sh @@ -4,12 +4,35 @@ MYSQL_REMOTE_HOST={{.DatabaseHostname}} source /var/lib/operator-scripts/mysql_r export DatabasePassword=${DatabasePassword:?"Please specify a DatabasePassword variable."} -mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.UserName}}'@'localhost' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.UserName}}'@'%' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};" +MYSQL_CMD="mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306" + +if [ -n "{{.DatabaseName}}" ]; then + GRANT_DATABASE="{{.DatabaseName}}" +else + GRANT_DATABASE="*" +fi + +# going for maximum compatibility here: +# 1. MySQL 8 no longer allows implicit create user when GRANT is used +# 2. MariaDB has "CREATE OR REPLACE", but MySQL does not +# 3. create user with CREATE but then do all password and TLS with ALTER to +# support updates + +$MYSQL_CMD <