diff --git a/api/bases/mariadb.openstack.org_mariadbdatabases.yaml b/api/bases/mariadb.openstack.org_mariadbdatabases.yaml index 9cd06a31..927dcc38 100644 --- a/api/bases/mariadb.openstack.org_mariadbdatabases.yaml +++ b/api/bases/mariadb.openstack.org_mariadbdatabases.yaml @@ -47,11 +47,8 @@ spec: description: Name of the database in MariaDB type: string secret: - description: Name of secret which contains DatabasePassword + description: Name of secret which contains DatabasePassword (deprecated) type: string - required: - - defaultCharacterSet - - defaultCollation type: object status: description: MariaDBDatabaseStatus defines the observed state of MariaDBDatabase diff --git a/api/v1beta1/mariadbdatabase_funcs.go b/api/v1beta1/mariadbdatabase_funcs.go index 8f5e818d..7e0d2255 100644 --- a/api/v1beta1/mariadbdatabase_funcs.go +++ b/api/v1beta1/mariadbdatabase_funcs.go @@ -121,6 +121,11 @@ func (d *Database) GetDatabase() *MariaDBDatabase { return d.database } +// GetAccount - returns the account +func (d *Database) GetAccount() *MariaDBAccount { + return d.account +} + // CreateOrPatchDB - create or patch the service DB instance // Deprecated. Use CreateOrPatchDBByName instead. If you want to use the // default the DB service instance of the deployment then pass "openstack" as @@ -162,6 +167,22 @@ func (d *Database) CreateOrPatchDBByName( } } + account := d.account + if account == nil { + account = &MariaDBAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: d.databaseUser, + Namespace: d.namespace, + Labels: map[string]string{ + "mariaDBDatabaseName": d.name, + }, + }, + Spec: MariaDBAccountSpec{ + UserName: d.databaseUser, + Secret: d.secret, + }, + } + } // set the database hostname on the db instance err := d.setDatabaseHostname(ctx, h, name) if err != nil { @@ -174,8 +195,6 @@ func (d *Database) CreateOrPatchDBByName( d.labels, ) - db.Spec.Secret = d.secret - err := controllerutil.SetControllerReference(h.GetBeforeObject(), db, h.GetScheme()) if err != nil { return err @@ -200,6 +219,36 @@ func (d *Database) CreateOrPatchDBByName( return ctrl.Result{RequeueAfter: time.Second * 5}, nil } + op_acc, err_acc := controllerutil.CreateOrPatch(ctx, h.GetClient(), account, func() error { + account.Labels = util.MergeStringMaps( + account.GetLabels(), + d.labels, + ) + + err := controllerutil.SetControllerReference(h.GetBeforeObject(), account, h.GetScheme()) + if err != nil { + return err + } + + // If the service object doesn't have our finalizer, add it. + controllerutil.AddFinalizer(account, h.GetFinalizer()) + + return nil + }) + + if err_acc != nil && !k8s_errors.IsNotFound(err_acc) { + return ctrl.Result{}, util.WrapErrorForObject( + fmt.Sprintf("Error create or update account object %s", account.Name), + account, + err_acc, + ) + } + + if op_acc != controllerutil.OperationResultNone { + util.LogForObject(h, fmt.Sprintf("Account object %s created or patched", account.Name), account) + return ctrl.Result{RequeueAfter: time.Second * 5}, nil + } + err = d.getDBWithName( ctx, h, @@ -211,7 +260,9 @@ func (d *Database) CreateOrPatchDBByName( return ctrl.Result{}, nil } -// WaitForDBCreatedWithTimeout - wait until the MariaDBDatabase is initialized and reports Status.Completed == true +// WaitForDBCreatedWithTimeout - wait until the MariaDBDatabase and MariaDBAccounts are +// initialized and reports Status.Conditions.IsTrue(MariaDBDatabaseReadyCondition) +// and Status.Conditions.IsTrue(MariaDBAccountReadyCondition) func (d *Database) WaitForDBCreatedWithTimeout( ctx context.Context, h *helper.Helper, @@ -226,7 +277,7 @@ func (d *Database) WaitForDBCreatedWithTimeout( return ctrl.Result{}, err } - if !d.database.Status.Completed || k8s_errors.IsNotFound(err) { + if !d.database.Status.Conditions.IsTrue(MariaDBDatabaseReadyCondition) { util.LogForObject( h, fmt.Sprintf("Waiting for service DB %s to be created", d.database.Name), @@ -236,6 +287,26 @@ func (d *Database) WaitForDBCreatedWithTimeout( return ctrl.Result{RequeueAfter: requeueAfter}, nil } + if !d.account.Status.Conditions.IsTrue(MariaDBAccountReadyCondition) { + util.LogForObject( + h, + fmt.Sprintf("Waiting for service account %s to be created", d.account.Name), + d.account, + ) + + return ctrl.Result{RequeueAfter: requeueAfter}, nil + } + + if k8s_errors.IsNotFound(err) { + util.LogForObject( + h, + fmt.Sprintf("DB or account objects not yet found %s", d.database.Name), + d.database, + ) + + return ctrl.Result{RequeueAfter: requeueAfter}, nil + } + return ctrl.Result{}, nil } @@ -262,6 +333,7 @@ func (d *Database) getDBWithName( if namespace == "" { namespace = h.GetBeforeObject().GetNamespace() } + err := h.GetClient().Get( ctx, types.NamespacedName{ @@ -269,6 +341,7 @@ func (d *Database) getDBWithName( Namespace: namespace, }, db) + if err != nil { if k8s_errors.IsNotFound(err) { return util.WrapErrorForObject( @@ -287,6 +360,35 @@ func (d *Database) getDBWithName( d.database = db + account := &MariaDBAccount{} + username := d.databaseUser + + err = h.GetClient().Get( + ctx, + types.NamespacedName{ + Name: username, + Namespace: namespace, + }, + account) + + if err != nil { + if k8s_errors.IsNotFound(err) { + return util.WrapErrorForObject( + fmt.Sprintf("Failed to get %s account %s ", username, namespace), + h.GetBeforeObject(), + err, + ) + } + + return util.WrapErrorForObject( + fmt.Sprintf("account error %s %s ", username, namespace), + h.GetBeforeObject(), + err, + ) + } + + d.account = account + return nil } diff --git a/api/v1beta1/mariadbdatabase_types.go b/api/v1beta1/mariadbdatabase_types.go index 51f3c857..6afcdf3f 100644 --- a/api/v1beta1/mariadbdatabase_types.go +++ b/api/v1beta1/mariadbdatabase_types.go @@ -31,16 +31,16 @@ const ( // MariaDBDatabaseSpec defines the desired state of MariaDBDatabase type MariaDBDatabaseSpec struct { - // Name of secret which contains DatabasePassword - Secret string `json:"secret,omitempty"` + // Name of secret which contains DatabasePassword (deprecated) + Secret *string `json:"secret,omitempty"` // Name of the database in MariaDB Name string `json:"name,omitempty"` // +kubebuilder:default=utf8 // Default character set for this database - DefaultCharacterSet string `json:"defaultCharacterSet"` + DefaultCharacterSet string `json:"defaultCharacterSet,omitempty"` // +kubebuilder:default=utf8_general_ci // Default collation for this database - DefaultCollation string `json:"defaultCollation"` + DefaultCollation string `json:"defaultCollation,omitempty"` } // MariaDBDatabaseStatus defines the observed state of MariaDBDatabase @@ -88,6 +88,7 @@ const ( // Database - type Database struct { database *MariaDBDatabase + account *MariaDBAccount databaseHostname string databaseName string databaseUser string diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 7e13b08a..b6e71507 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -49,6 +49,11 @@ func (in *Database) DeepCopyInto(out *Database) { *out = new(MariaDBDatabase) (*in).DeepCopyInto(*out) } + if in.account != nil { + in, out := &in.account, &out.account + *out = new(MariaDBAccount) + (*in).DeepCopyInto(*out) + } if in.labels != nil { in, out := &in.labels, &out.labels *out = make(map[string]string, len(*in)) @@ -322,7 +327,7 @@ func (in *MariaDBDatabase) DeepCopyInto(out *MariaDBDatabase) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -379,6 +384,11 @@ func (in *MariaDBDatabaseList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MariaDBDatabaseSpec) DeepCopyInto(out *MariaDBDatabaseSpec) { *out = *in + if in.Secret != nil { + in, out := &in.Secret, &out.Secret + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MariaDBDatabaseSpec. diff --git a/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml b/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml index 9cd06a31..927dcc38 100644 --- a/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml +++ b/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml @@ -47,11 +47,8 @@ spec: description: Name of the database in MariaDB type: string secret: - description: Name of secret which contains DatabasePassword + description: Name of secret which contains DatabasePassword (deprecated) type: string - required: - - defaultCharacterSet - - defaultCollation type: object status: description: MariaDBDatabaseStatus defines the observed state of MariaDBDatabase diff --git a/controllers/mariadbaccount_controller.go b/controllers/mariadbaccount_controller.go index 9da6d603..83e1e827 100644 --- a/controllers/mariadbaccount_controller.go +++ b/controllers/mariadbaccount_controller.go @@ -411,16 +411,19 @@ func (r *MariaDBAccountReconciler) reconcileDelete( // remove local finalizer controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) - } - instance.Status.Conditions.Set(condition.FalseCondition( - databasev1beta1.MariaDBServerReadyCondition, - condition.ErrorReason, - condition.SeverityError, - "Error retrieving MariaDB/Galera instance %s", - err)) + // galera DB does not exist, so return + return ctrl.Result{}, nil + } else { + instance.Status.Conditions.Set(condition.FalseCondition( + databasev1beta1.MariaDBServerReadyCondition, + condition.ErrorReason, + condition.SeverityError, + "Error retrieving MariaDB/Galera instance %s", + err)) - return ctrl.Result{}, err + return ctrl.Result{}, err + } } var dbInstance, dbAdminSecret, dbContainerImage, serviceAccountName string diff --git a/pkg/mariadb/database.go b/pkg/mariadb/database.go index 59b45719..6ec80719 100644 --- a/pkg/mariadb/database.go +++ b/pkg/mariadb/database.go @@ -35,6 +35,51 @@ func DbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName s labels := map[string]string{ "owner": "mariadb-operator", "cr": database.Spec.Name, "app": "mariadbschema", } + + var scriptEnv []corev1.EnvVar + + if database.Spec.Secret != nil { + scriptEnv = []corev1.EnvVar{ + { + Name: "MYSQL_PWD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: databaseSecret, + }, + Key: "DbRootPassword", + }, + }, + }, + // send deprecated Secret field but only if non-nil + { + Name: "DatabasePassword", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: *database.Spec.Secret, + }, + Key: "DatabasePassword", + }, + }, + }, + } + } else { + scriptEnv = []corev1.EnvVar{ + { + Name: "MYSQL_PWD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: databaseSecret, + }, + Key: "DbRootPassword", + }, + }, + }, + } + } + job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ // provided db name is used as metadata name where underscore is a not allowed @@ -54,30 +99,7 @@ func DbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName s Name: "mariadb-database-create", Image: containerImage, Command: []string{"/bin/sh", "-c", dbCmd}, - Env: []corev1.EnvVar{ - { - Name: "MYSQL_PWD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: databaseSecret, - }, - Key: "DbRootPassword", - }, - }, - }, - { - Name: "DatabasePassword", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: database.Spec.Secret, - }, - Key: "DatabasePassword", - }, - }, - }, - }, + Env: scriptEnv, }, }, }, @@ -105,6 +127,52 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost labels := map[string]string{ "owner": "mariadb-operator", "cr": database.Spec.Name, "app": "mariadbschema", } + + var scriptEnv []corev1.EnvVar + + if database.Spec.Secret != nil { + scriptEnv = []corev1.EnvVar{ + { + Name: "MYSQL_PWD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: databaseSecret, + }, + Key: "DbRootPassword", + }, + }, + }, + // send deprecated Secret field but only if non-nil. otherwise + // the script should not try to drop usernames from mysql.user + { + Name: "DatabasePassword", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: *database.Spec.Secret, + }, + Key: "DatabasePassword", + }, + }, + }, + } + } else { + scriptEnv = []corev1.EnvVar{ + { + Name: "MYSQL_PWD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: databaseSecret, + }, + Key: "DbRootPassword", + }, + }, + }, + } + } + job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: strings.Replace(database.Spec.Name, "_", "", -1) + "-database-delete", @@ -121,19 +189,7 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost Name: "mariadb-database-create", Image: containerImage, Command: []string{"/bin/sh", "-c", delCmd}, - Env: []corev1.EnvVar{ - { - Name: "MYSQL_PWD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: databaseSecret, - }, - Key: "DbRootPassword", - }, - }, - }, - }, + Env: scriptEnv, }, }, }, diff --git a/templates/database.sh b/templates/database.sh index 96484af9..1b0cfada 100755 --- a/templates/database.sh +++ b/templates/database.sh @@ -1,4 +1,8 @@ #!/bin/bash -export DatabasePassword=${DatabasePassword:?"Please specify a DatabasePassword variable."} -mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "CREATE DATABASE IF NOT EXISTS {{.DatabaseName}}; ALTER DATABASE {{.DatabaseName}} CHARACTER SET '{{.DefaultCharacterSet}}' COLLATE '{{.DefaultCollation}}'; GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.DatabaseName}}'@'localhost' IDENTIFIED BY '$DatabasePassword';GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.DatabaseName}}'@'%' IDENTIFIED BY '$DatabasePassword';" +mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "CREATE DATABASE IF NOT EXISTS {{.DatabaseName}}; ALTER DATABASE {{.DatabaseName}} CHARACTER SET '{{.DefaultCharacterSet}}' COLLATE '{{.DefaultCollation}}';" + +if [[ "${DatabasePassword}" != "" ]]; then + # legacy; create database with username + mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.DatabaseName}}'@'localhost' IDENTIFIED BY '$DatabasePassword';GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.DatabaseName}}'@'%' IDENTIFIED BY '$DatabasePassword';" +fi diff --git a/templates/delete_database.sh b/templates/delete_database.sh index 5ef9806f..f7016277 100755 --- a/templates/delete_database.sh +++ b/templates/delete_database.sh @@ -1,3 +1,11 @@ #!/bin/bash -mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "DROP DATABASE IF EXISTS {{.DatabaseName}}; DROP USER IF EXISTS '{{.DatabaseName}}'@'localhost'; DROP USER IF EXISTS '{{.DatabaseName}}'@'%';" +mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "DROP DATABASE IF EXISTS {{.DatabaseName}};" + +if [[ "${DatabasePassword}" != "" ]]; then + # legacy; drop the username also + # we can't do this unconditionally here because we only want to delete the + # mysql account if the MariaDBDatabase was using the legacy "secret" attribute; + # otherwise this could be from a valid MariaDBAccount + mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "DROP USER IF EXISTS '{{.DatabaseName}}'@'localhost'; DROP USER IF EXISTS '{{.DatabaseName}}'@'%';" +fi diff --git a/tests/kuttl/tests/account_create/02-create-database.yaml b/tests/kuttl/tests/account_create/02-create-database.yaml index 45a80ff2..95a029bd 100644 --- a/tests/kuttl/tests/account_create/02-create-database.yaml +++ b/tests/kuttl/tests/account_create/02-create-database.yaml @@ -5,5 +5,4 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_accounttest diff --git a/tests/kuttl/tests/database_create/02-assert.yaml b/tests/kuttl/tests/database_create/02-assert.yaml index 6fe01bb6..2ad1e430 100644 --- a/tests/kuttl/tests/database_create/02-assert.yaml +++ b/tests/kuttl/tests/database_create/02-assert.yaml @@ -1,18 +1,3 @@ ---- -apiVersion: batch/v1 -kind: Job -metadata: - name: kuttldb-utf8-db-create -status: - succeeded: 1 ---- -apiVersion: batch/v1 -kind: Job -metadata: - name: kuttldb-latin1-db-create -status: - succeeded: 1 ---- apiVersion: mariadb.openstack.org/v1beta1 kind: MariaDBDatabase metadata: @@ -20,7 +5,6 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_utf8 status: conditions: @@ -45,7 +29,6 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_latin1 status: conditions: diff --git a/tests/kuttl/tests/database_create/02-create-database.yaml b/tests/kuttl/tests/database_create/02-create-database.yaml index adaf5cec..04da7469 100644 --- a/tests/kuttl/tests/database_create/02-create-database.yaml +++ b/tests/kuttl/tests/database_create/02-create-database.yaml @@ -5,7 +5,6 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_utf8 --- apiVersion: mariadb.openstack.org/v1beta1 @@ -15,7 +14,25 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_latin1 defaultCharacterSet: latin1 defaultCollation: latin1_general_ci +--- +# test optional, deprecated secret field +apiVersion: v1 +data: + DatabasePassword: ZGJzZWNyZXQx +kind: Secret +metadata: + name: some-db-secret +type: Opaque +--- +apiVersion: mariadb.openstack.org/v1beta1 +kind: MariaDBDatabase +metadata: + name: kuttldb-legacy-secret + labels: + dbName: openstack +spec: + name: kuttldb_legacy_secret + secret: some-db-secret diff --git a/tests/kuttl/tests/database_create/03-assert.yaml b/tests/kuttl/tests/database_create/03-assert.yaml index 8be4ac8d..eafeddbb 100644 --- a/tests/kuttl/tests/database_create/03-assert.yaml +++ b/tests/kuttl/tests/database_create/03-assert.yaml @@ -13,3 +13,17 @@ commands: - script: | oc rsh -n ${NAMESPACE} -c galera openstack-galera-0 /bin/sh -c 'mysql -uroot -p${DB_ROOT_PASSWORD} -Nse "select @@character_set_database;" kuttldb_utf8' | grep -o -w utf8 oc rsh -n ${NAMESPACE} -c galera openstack-galera-0 /bin/sh -c 'mysql -uroot -p${DB_ROOT_PASSWORD} -Nse "select @@collation_database;" kuttldb_utf8' | grep -o -w utf8_general_ci +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +commands: + # for legacy secret non-present, test that a mariadb username was *not* made + - script: | + ${MARIADB_KUTTL_DIR:-tests/kuttl/tests}/../common/scripts/check_db_account.sh openstack-galera-0 kuttldb_utf8 kuttldb_utf_8 12345678 --reverse +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +commands: + # for legacy secret present, test that a mariadb username was made + - script: | + ${MARIADB_KUTTL_DIR:-tests/kuttl/tests}/../common/scripts/check_db_account.sh openstack-galera-0 kuttldb_legacy_secret kuttldb_legacy_secret dbsecret1 diff --git a/tests/kuttl/tests/database_create/04-teardown.yaml b/tests/kuttl/tests/database_create/04-teardown.yaml index 1b19d23a..c1ee147b 100644 --- a/tests/kuttl/tests/database_create/04-teardown.yaml +++ b/tests/kuttl/tests/database_create/04-teardown.yaml @@ -10,6 +10,12 @@ delete: - apiVersion: mariadb.openstack.org/v1beta1 kind: MariaDBDatabase name: kuttldb-utf8 +- apiVersion: mariadb.openstack.org/v1beta1 + kind: MariaDBDatabase + name: kuttldb-legacy-secret +- apiVersion: v1 + kind: Secret + name: some-db-secret commands: - script: | oc delete -n $NAMESPACE pvc mysql-db-openstack-galera-0