Fixes #31: Minor discrepencies between MySQL and MariaDB

Putting the onus on the user to generate right combination
of ClearText and changes/setting AuthPlugin.

Also, stop reflecting back GRANT USAGE entry from
SHOW GRANTS. Unclear if hashed password returned by
MariaDB in this case is concerning or not, but no value in
it.

Password was always being monitored and updated in the
database. Removed TODO.

Author: cuppett

api/v1alpha1/databaseuser_types.go

@@ -52,7 +52,6 @@ type Identification struct {
 	// +kubebuilder:validation:Optional
 	AuthPlugin string `json:"authPlugin"`
 	// Relates to auth_string, See: MySQL CREATE USER
-	// TODO: We should watch this object, if it changes we can flush through a new password/token.
 	// +kubebuilder:validation:Optional
 	// +nullable
 	AuthString *SecretKeySource `json:"authString,omitEmpty"`
@@ -129,7 +128,7 @@ func init() {
 
 func (r *DatabaseUser) PermissionListEqual() bool {
 	// Always has GRANT USAGE as the first one. Only when we have something more complicated than
-	if len(r.Status.Grants)-1 != len(r.Spec.DatabaseList) {
+	if len(r.Status.Grants) != len(r.Spec.DatabaseList) {
 		return false
 	}
 	return reflect.DeepEqual(r.Spec.DatabaseList, r.Status.DatabaseList)

config/crd/bases/mysql.apps.cuppett.dev_databaseusers.yaml

@@ -69,9 +69,7 @@ spec:
                     description: 'Relates to auth_plugin, See: MySQL CREATE USER'
                     type: string
                   authString:
-                    description: 'Relates to auth_string, See: MySQL CREATE USER TODO:
-                      We should watch this object, if it changes we can flush through
-                      a new password/token.'
+                    description: 'Relates to auth_string, See: MySQL CREATE USER'
                     nullable: true
                     properties:
                       secretKeyRef:
@@ -158,9 +156,7 @@ spec:
                     description: 'Relates to auth_plugin, See: MySQL CREATE USER'
                     type: string
                   authString:
-                    description: 'Relates to auth_string, See: MySQL CREATE USER TODO:
-                      We should watch this object, if it changes we can flush through
-                      a new password/token.'
+                    description: 'Relates to auth_string, See: MySQL CREATE USER'
                     nullable: true
                     properties:
                       secretKeyRef:

controllers/databaseuser_controller.go

@@ -183,20 +183,20 @@ func (r *DatabaseUserReconciler) Reconcile(ctx context.Context, req ctrl.Request
 
 		if !exists {
 			err = r.userCreate(ctx, &loop)
-			if err == nil {
-				loop.instance.Status.CreationTime = metav1.NewTime(time.Now())
-				loop.instance.Status.Message = "Created user"
-			}
+			loop.instance.Status.CreationTime = metav1.NewTime(time.Now())
+			loop.instance.Status.Message = "Created user"
 		} else if loop.adminConnection.UserMine(loop.db, loop.instance) {
-			updated, err := r.userUpdate(ctx, &loop)
-			if err == nil && updated {
+			var updated bool
+			updated, err = r.userUpdate(ctx, &loop)
+			if updated {
 				loop.instance.Status.SyncTime = metav1.NewTime(time.Now())
 			}
 		}
 
-		err = r.Status().Update(ctx, loop.instance)
 		if err != nil {
 			r.Log.Error(err, "Failure to reconcile user.")
+		} else {
+			err = r.Status().Update(ctx, loop.instance)
 		}
 	}
 	return ctrl.Result{}, err
@@ -335,9 +335,7 @@ func (r *DatabaseUserReconciler) userUpdate(ctx context.Context, loop *UserLoopC
 		permsDiff = true
 		r.Log.Info("Permissions difference.", "Host", loop.adminConnection.Spec.Host,
 			"Name", loop.instance.Status.Username)
-		// Always has GRANT USAGE as the first one. Only when we have something more complicated than
-		// that do we need to revoke it.
-		if len(loop.instance.Status.Grants) > 1 {
+		if len(loop.instance.Status.Grants) > 0 {
 			_, err = r.revoke(loop)
 		}
 		if err == nil {
@@ -373,31 +371,30 @@ func (r *DatabaseUserReconciler) userDetailString(ctx context.Context, loop *Use
 		}
 		authString = mysqlv1alpha1.Escape(authString)
 
-		// Looking for currently known auth_plugin in both places. Where Status has it, grab to hang onto it.
 		authPlugin = loop.instance.Spec.Identification.AuthPlugin
-		if authPlugin == "" && loop.instance.Status.Identification != nil {
-			authPlugin = loop.instance.Status.Identification.AuthPlugin
-		} else if loop.instance.Status.Identification != nil &&
-			loop.instance.Spec.Identification.AuthPlugin != loop.instance.Status.Identification.AuthPlugin {
+		if loop.instance.Status.Identification != nil &&
+			loop.instance.Spec.Identification.AuthPlugin != loop.instance.Status.Identification.AuthPlugin &&
+			loop.instance.Spec.Identification.AuthPlugin != "" {
 			pluginsDiff = true
 		}
-		if authPlugin != "" {
-			authPlugin = mysqlv1alpha1.Escape(authPlugin)
-		}
+		authPlugin = mysqlv1alpha1.Escape(authPlugin)
 
 		if passwordUpdated || pluginsDiff {
 			if authPlugin != "" {
 				queryFragment += " IDENTIFIED WITH '" + authPlugin + "'"
 				if authString != "" {
+					// MySQL can do both here
+					// MariaDB can only do AS (otherwise requires deprecated 5.7 PASSWORD() function to use BY
 					if loop.instance.Spec.Identification.ClearText {
 						queryFragment += " BY '" + authString + "'"
 					} else {
 						queryFragment += " AS '" + authString + "'"
 					}
 				}
 			} else {
-				if authString != "" && !update {
+				if authString != "" {
 					queryFragment += " IDENTIFIED BY"
+					// Unique to MARIADB (and maybe MySQL databases <= 5.7?)
 					if !loop.instance.Spec.Identification.ClearText {
 						queryFragment += " PASSWORD"
 					}
@@ -417,7 +414,6 @@ func (r *DatabaseUserReconciler) userDetailString(ctx context.Context, loop *Use
 		// If this update pass is successful, our identification details will match.
 		loop.instance.Status.IdentificationResourceVersion = loop.secret.ResourceVersion
 		loop.instance.Status.Identification = loop.instance.Spec.Identification
-		loop.instance.Status.Identification.AuthPlugin = authPlugin // For the case where Spec=""
 		loop.instance.Status.TlsOptions = loop.instance.Spec.TlsOptions
 	}
 
@@ -513,14 +509,19 @@ func (r *DatabaseUserReconciler) grantStatusUpdate(loop *UserLoopContext, empty
 		return false, tx.Error
 	}
 
-	for _, row := range results {
-		for key := range row {
-			grant = fmt.Sprintf("%v", row[key])
-			if !contains(loop.instance.Status.Grants, grant) {
-				r.Log.Info("Existing grants do not contain this one.", "Grant", grant, "Host",
-					loop.adminConnection.Spec.Host, "Name", loop.instance.Status.Username)
-				update = true
-				loop.instance.Status.Grants = append(loop.instance.Status.Grants, grant)
+	for i, row := range results {
+		// Drop the first one in the results.
+		// It's a useless GRANT USAGE statement.
+		// On MariaDB it includes the user's password hash.
+		if i > 0 {
+			for key := range row {
+				grant = fmt.Sprintf("%v", row[key])
+				if !contains(loop.instance.Status.Grants, grant) {
+					r.Log.Info("Existing grants do not contain this one.", "Grant", grant, "Host",
+						loop.adminConnection.Spec.Host, "Name", loop.instance.Status.Username)
+					update = true
+					loop.instance.Status.Grants = append(loop.instance.Status.Grants, grant)
+				}
 			}
 		}
 	}