mariadbaccount system accounts

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.

Author: zzzeek

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

api/v1beta1/mariadbaccount_types.go

@@ -48,8 +48,19 @@ 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 {
 	// Deployment Conditions
@@ -85,3 +96,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
+}

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

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{

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,12 +114,18 @@ 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
@@ -134,23 +141,30 @@ func (r *MariaDBAccountReconciler) reconcileCreate(
 	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()) {
 		// we need to persist this right away
 		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,25 @@ 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
+
+	if instance.IsUserAccount() {
+		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
+		}
+	} else {
+		log.Info(fmt.Sprintf("Running system account create '%s'", instance.Name))
+		jobDef, err = mariadb.CreateDbAccountJob(
+			dbGalera, instance, "", dbHostname,
+			dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
 	}
 
 	accountCreateHash := instance.Status.Hash[databasev1beta1.AccountCreateHash]
@@ -228,9 +254,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 +290,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 +311,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 +334,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,18 +380,19 @@ 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
+			}
 		}
 	}
 
 	// 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
 	err = r.removeAccountAndSecretFinalizer(ctx, helper, instance)
-
 	return ctrl.Result{}, err
 }
 
@@ -526,9 +572,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")

templates/account.sh

@@ -4,8 +4,11 @@ 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}};"
-
+if [ -n "{{.DatabaseName}}" ]; then
+    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}};"
+else
+    mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "GRANT ALL PRIVILEGES ON *.* TO '{{.UserName}}'@'localhost' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};GRANT ALL PRIVILEGES ON *.* TO '{{.UserName}}'@'%' IDENTIFIED BY '$DatabasePassword'{{.RequireTLS}};"
+fi
 
 # search for the account.  not using SHOW CREATE USER to avoid displaying
 # password hash

tests/chainsaw/scripts/check_db_account.sh

@@ -0,0 +1,36 @@
+#!/bin/sh
+
+set -x
+
+galera="$1"
+dbname="$2"
+username="$3"
+password="$4"
+
+
+found=0
+not_found=1
+
+if [ "$5" = "--reverse" ];then
+    # sometimes we want to check that a user does not exist
+    found=1
+    not_found=0
+fi
+
+found_username=$(oc exec -n ${NAMESPACE} -c galera ${galera} -- /bin/sh -c 'source /var/lib/operator-scripts/mysql_root_auth.sh; mysql -uroot -p${DB_ROOT_PASSWORD} -Nse "select user from mysql.user"' | grep -o -w ${username})
+
+# username was not found, exit
+if [ -z "$found_username" ]; then
+    exit $not_found
+fi
+
+# username was found.  if we wanted it to be found, then check the login also.
+if [ "$found" = "0" ]; then
+    if [ -n "$dbname" ]; then
+        oc exec -n ${NAMESPACE} -c galera ${galera} -- /bin/sh -c "mysql -u${username} -p${password} -Nse 'select database();' ${dbname}" || exit -1
+    else
+        oc exec -n ${NAMESPACE} -c galera ${galera} -- /bin/sh -c "mysql -u${username} -p${password} -Nse 'select 1'" || exit -1
+    fi
+fi
+
+exit $found