Allow pre-definition of user passwords (#44)

## Summary Of Changes
This PR adds several new fields to allow import of passwords from a created Secret:
```yaml
apiVersion: v1
kind: Secret
metadata:
  name: credentials-secret
  namespace: testing
type: Opaque
data:
  username: bXktc2VjcmV0LXVzZXI=
  password: d2h5YXJleW91bG9va2luZ2hlcmU=
---
apiVersion: rabbitmq.com/v1alpha1
kind: User
metadata:
  name: import-user
  namespace: testing
spec:
  rabbitmqClusterReference:
    name: rmq
    namespace: rabbitmq-system
  importCredentialsSecret:
    name: credentials-secret
```

api/v1alpha1/user_types.go

@@ -16,9 +16,6 @@ import (
 
 // UserSpec defines the desired state of User.
 type UserSpec struct {
-	// Username of the user to create on a RabbitmqCluster.
-	// +kubebuilder:validation:Required
-	Name string `json:"name"`
 	// List of permissions tags to associate with the user. This determines the level of
 	// access to the RabbitMQ management UI granted to the user. Omitting this field will
 	// lead to a user than can still connect to the cluster through messaging protocols,
@@ -29,7 +26,13 @@ type UserSpec struct {
 	// exist for the User object to be created.
 	// +kubebuilder:validation:Required
 	RabbitmqClusterReference RabbitmqClusterReference `json:"rabbitmqClusterReference"`
-	// TODO: Allow the provision of the user with a pre-defined password through a Secret here
+	// Defines a Secret used to pre-define the username and password set for this User. User objects created
+	// with this field set will not have randomly-generated credentials, and will instead import
+	// the username/password values from this Secret.
+	// The Secret must contain the keys `username` and `password` in its Data field, or the import will fail.
+	// Note that this import only occurs at creation time, and is ignored once a password has been set
+	// on a User.
+	ImportCredentialsSecret *corev1.LocalObjectReference `json:"importCredentialsSecret,omitempty"`
 }
 
 // UserStatus defines the observed state of User.

api/v1alpha1/user_types_test.go

@@ -5,6 +5,7 @@ import (
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -22,7 +23,6 @@ var _ = Describe("user spec", func() {
 				Namespace: namespace,
 			},
 			Spec: UserSpec{
-				Name: "test-user",
 				RabbitmqClusterReference: RabbitmqClusterReference{
 					Name:      "some-cluster",
 					Namespace: namespace,
@@ -39,7 +39,6 @@ var _ = Describe("user spec", func() {
 			Name:      "some-cluster",
 			Namespace: namespace,
 		}))
-		Expect(fetcheduser.Spec.Name).To(Equal("test-user"))
 		Expect(len(fetcheduser.Spec.Tags)).To(Equal(0))
 	})
 
@@ -54,8 +53,10 @@ var _ = Describe("user spec", func() {
 					Namespace: namespace,
 				},
 				Spec: UserSpec{
-					Name: username,
 					Tags: tags,
+					ImportCredentialsSecret: &corev1.LocalObjectReference{
+						Name: "secret-name",
+					},
 					RabbitmqClusterReference: RabbitmqClusterReference{
 						Name:      "some-cluster",
 						Namespace: namespace,
@@ -79,8 +80,8 @@ var _ = Describe("user spec", func() {
 					Name:      "some-cluster",
 					Namespace: namespace,
 				}))
-				Expect(fetchedUser.Spec.Name).NotTo(BeEmpty())
-				Expect(fetchedUser.Spec.Name).To(Equal(username))
+				Expect(fetchedUser.Spec.ImportCredentialsSecret.Name).To(Equal("secret-name"))
+				Expect(fetchedUser.Spec.Tags).To(Equal([]UserTag{"policymaker", "monitoring"}))
 			})
 		})
 

config/crd/bases/rabbitmq.com_users.yaml

@@ -36,9 +36,20 @@ spec:
           spec:
             description: Spec configures the desired state of the User object.
             properties:
-              name:
-                description: Username of the user to create on a RabbitmqCluster.
-                type: string
+              importCredentialsSecret:
+                description: Defines a Secret used to pre-define the username and
+                  password set for this User. User objects created with this field
+                  set will not have randomly-generated credentials, and will instead
+                  import the username/password values from this Secret. The Secret
+                  must contain the keys `username` and `password` in its Data field,
+                  or the import will fail. Note that this import only occurs at creation
+                  time, and is ignored once a password has been set on a User.
+                properties:
+                  name:
+                    description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                      TODO: Add other useful fields. apiVersion, kind, uid?'
+                    type: string
+                type: object
               rabbitmqClusterReference:
                 description: Reference to the RabbitmqCluster that the user will be
                   created for. This cluster must exist for the User object to be created.
@@ -69,7 +80,6 @@ spec:
                   type: string
                 type: array
             required:
-            - name
             - rabbitmqClusterReference
             type: object
           status:

controllers/user_controller.go

@@ -83,12 +83,15 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 	logger.Info("Start reconciling",
 		"spec", string(spec))
 
-	if err := r.declareCredentials(ctx, user); err != nil {
-		return ctrl.Result{}, err
-	}
+	if user.Status.Credentials == nil {
+		logger.Info("User does not yet have a Credentials Secret; generating", "user", user.Name)
+		if err := r.declareCredentials(ctx, user); err != nil {
+			return ctrl.Result{}, err
+		}
 
-	if err := r.setUserStatus(ctx, user); err != nil {
-		return ctrl.Result{}, err
+		if err := r.setUserStatus(ctx, user); err != nil {
+			return ctrl.Result{}, err
+		}
 	}
 
 	if err := r.declareUser(ctx, rabbitClient, user); err != nil {
@@ -103,14 +106,14 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 func (r *UserReconciler) declareCredentials(ctx context.Context, user *topologyv1alpha1.User) error {
 	logger := ctrl.LoggerFrom(ctx)
 
-	// TODO: If a user has provided a Secret containing the desired password, use it instead here.
-	password, err := internal.RandomEncodedString(24)
+	username, password, err := r.generateCredentials(ctx, user)
 	if err != nil {
-		msg := "failed to generate random password"
-		r.Recorder.Event(user, corev1.EventTypeWarning, "FailedDeclare", msg)
+		msg := "failed to generate credentials"
+		r.Recorder.Event(user, corev1.EventTypeWarning, "CredentialGenerateFailure", msg)
 		logger.Error(err, msg)
 		return err
 	}
+	logger.Info("Credentials generated for User", "user", user.Name, "generatedUsername", username)
 
 	credentialSecret := corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -121,7 +124,7 @@ func (r *UserReconciler) declareCredentials(ctx context.Context, user *topologyv
 		// The format of the generated Secret conforms to the Provisioned Service
 		// type Spec. For more information, see https://k8s-service-bindings.github.io/spec/#provisioned-service.
 		Data: map[string][]byte{
-			"username": []byte(user.Spec.Name),
+			"username": []byte(username),
 			"password": []byte(password),
 		},
 	}
@@ -144,19 +147,72 @@ func (r *UserReconciler) declareCredentials(ctx context.Context, user *topologyv
 		return err
 	}
 
-	logger.Info("Successfully declared credentials secret", "user", credentialSecret.ObjectMeta.Name)
+	logger.Info("Successfully declared credentials secret", "secret", credentialSecret.Name, "namespace", credentialSecret.Namespace)
 	r.Recorder.Event(&credentialSecret, corev1.EventTypeNormal, "SuccessfulDeclare", "Successfully declared user")
 	return nil
 }
 
+func (r *UserReconciler) generateCredentials(ctx context.Context, user *topologyv1alpha1.User) (string, string, error) {
+	logger := ctrl.LoggerFrom(ctx)
+
+	var err error
+	msg := fmt.Sprintf("generating/importing credentials for User %s: %#v", user.Name, user)
+	logger.Info(msg)
+
+	if user.Spec.ImportCredentialsSecret != nil {
+		logger.Info("An import secret was provided in the user spec", "user", user.Name, "secretName", user.Spec.ImportCredentialsSecret.Name)
+		return r.importCredentials(ctx, user.Spec.ImportCredentialsSecret.Name, user.Namespace)
+	}
+
+	username, err := internal.RandomEncodedString(24)
+	if err != nil {
+		msg := "failed to generate random username"
+		r.Recorder.Event(user, corev1.EventTypeWarning, "FailedDeclare", msg)
+		logger.Error(err, msg)
+		return "", "", err
+	}
+	password, err := internal.RandomEncodedString(24)
+	if err != nil {
+		msg := "failed to generate random password"
+		r.Recorder.Event(user, corev1.EventTypeWarning, "FailedDeclare", msg)
+		logger.Error(err, msg)
+		return "", "", err
+	}
+	return username, password, nil
+
+}
+
+func (r *UserReconciler) importCredentials(ctx context.Context, secretName, secretNamespace string) (string, string, error) {
+	logger := ctrl.LoggerFrom(ctx)
+	logger.Info("Importing user credentials from provided Secret", "secretName", secretName, "secretNamespace", secretNamespace)
+
+	var credentialsSecret corev1.Secret
+	err := r.Client.Get(ctx, types.NamespacedName{Name: secretName, Namespace: secretNamespace}, &credentialsSecret)
+	if err != nil {
+		return "", "", fmt.Errorf("Could not find password secret %s in namespace %s; Err: %w", secretName, secretNamespace, err)
+	}
+	username, ok := credentialsSecret.Data["username"]
+	if !ok {
+		return "", "", fmt.Errorf("Could not find username key in credentials secret: %s", credentialsSecret.Name)
+	}
+	password, ok := credentialsSecret.Data["password"]
+	if !ok {
+		return "", "", fmt.Errorf("Could not find password key in credentials secret: %s", credentialsSecret.Name)
+	}
+
+	logger.Info("Retrieved credentials from Secret", "secretName", secretName, "retrievedUsername", string(username))
+	return string(username), string(password), nil
+}
+
 func (r *UserReconciler) setUserStatus(ctx context.Context, user *topologyv1alpha1.User) error {
 	logger := ctrl.LoggerFrom(ctx)
 
 	credentials := &corev1.LocalObjectReference{
-		Name: user.Spec.Name + "-user-credentials",
+		Name: user.Name + "-user-credentials",
 	}
 	user.Status.Credentials = credentials
 	if err := r.Status().Update(ctx, user); err != nil {
+		logger.Error(err, "Failed to update secret status credentials", "user", user.Name, "secretRef", credentials)
 		return err
 	}
 	logger.Info("Successfully updated secret status credentials", "user", user.Name, "secretRef", credentials)
@@ -167,27 +223,32 @@ func (r *UserReconciler) setUserStatus(ctx context.Context, user *topologyv1alph
 func (r *UserReconciler) declareUser(ctx context.Context, client *rabbithole.Client, user *topologyv1alpha1.User) error {
 	logger := ctrl.LoggerFrom(ctx)
 
-	credentials := &corev1.Secret{}
-	if err := r.Get(ctx, types.NamespacedName{Name: user.Status.Credentials.Name, Namespace: user.Namespace}, credentials); err != nil {
+	credentials, err := r.getUserCredentials(ctx, user)
+	if err != nil {
+		msg := "failed to retrieve user credentials secret from status"
+		r.Recorder.Event(user, corev1.EventTypeWarning, "FailedDeclare", msg)
+		logger.Error(err, msg, "user.status", user.Status)
 		return err
 	}
+	logger.Info("Retrieved credentials for user", "user", user.Name, "credentials", credentials.Name)
 
 	userSettings, err := internal.GenerateUserSettings(credentials, user.Spec.Tags)
 	if err != nil {
-		msg := "failed to generate user settings from credential in status"
+		msg := "failed to generate user settings from credential"
 		r.Recorder.Event(user, corev1.EventTypeWarning, "FailedDeclare", msg)
 		logger.Error(err, msg, "user.status", user.Status)
 		return err
 	}
+	logger.Info("Generated user settings", "user", user.Name, "settings", userSettings)
 
-	if err := validateResponse(client.PutUser(user.Spec.Name, userSettings)); err != nil {
+	if err := validateResponse(client.PutUser(userSettings.Name, userSettings)); err != nil {
 		msg := "failed to declare user"
 		r.Recorder.Event(user, corev1.EventTypeWarning, "FailedDeclare", msg)
-		logger.Error(err, msg, "user", user.Spec.Name)
+		logger.Error(err, msg, "user", user.Name)
 		return err
 	}
 
-	logger.Info("Successfully declared user", "user", user.Spec.Name)
+	logger.Info("Successfully declared user", "user", user.Name)
 	r.Recorder.Event(user, corev1.EventTypeNormal, "SuccessfulDeclare", "Successfully declared user")
 	return nil
 }
@@ -203,16 +264,40 @@ func (r *UserReconciler) addFinalizerIfNeeded(ctx context.Context, user *topolog
 	return nil
 }
 
+func (r *UserReconciler) getUserCredentials(ctx context.Context, user *topologyv1alpha1.User) (*corev1.Secret, error) {
+	logger := ctrl.LoggerFrom(ctx)
+	if user.Status.Credentials == nil {
+		return nil, fmt.Errorf("This User does not yet have a Credentials Secret created")
+	}
+
+	credentials := &corev1.Secret{}
+	if err := r.Get(ctx, types.NamespacedName{Name: user.Status.Credentials.Name, Namespace: user.Namespace}, credentials); err != nil {
+		logger.Error(err, "Failed to retrieve user credentials secret from status", "user", user.Name, "secretCredentials", user.Status.Credentials)
+		return nil, err
+	}
+
+	logger.Info("Successfully retrieved credentials", "user", user.Name, "secretCredentials", user.Status.Credentials)
+	return credentials, nil
+}
+
 func (r *UserReconciler) deleteUser(ctx context.Context, client *rabbithole.Client, user *topologyv1alpha1.User) error {
 	logger := ctrl.LoggerFrom(ctx)
 
-	err := validateResponseForDeletion(client.DeleteUser(user.Spec.Name))
+	credentials, err := r.getUserCredentials(ctx, user)
+	if err != nil {
+		msg := "failed to retrieve user credentials secret from status"
+		r.Recorder.Event(user, corev1.EventTypeWarning, "FailedDelete", msg)
+		logger.Error(err, msg, "user.status", user.Status)
+		return err
+	}
+
+	err = validateResponseForDeletion(client.DeleteUser(string(credentials.Data["username"])))
 	if errors.Is(err, NotFound) {
-		logger.Info("cannot find user in rabbitmq server; already deleted", "user", user.Spec.Name)
+		logger.Info("cannot find user in rabbitmq server; already deleted", "user", user.Name)
 	} else if err != nil {
 		msg := "failed to delete user"
 		r.Recorder.Event(user, corev1.EventTypeWarning, "FailedDelete", msg)
-		logger.Error(err, msg, "user", user.Spec.Name)
+		logger.Error(err, msg, "user", user.Name)
 		return err
 	}
 	return r.removeFinalizer(ctx, user)

docs/examples/users/user.yaml

@@ -4,7 +4,6 @@ metadata:
   name: user-sample
   namespace: rabbitmq-system
 spec:
-  name: test-user # name of the user
   tags:
   - management # available tags are 'management', 'policymaker', 'monitoring' and 'administrator'
   rabbitmqClusterReference:

docs/examples/users/userPreDefinedCreds.yaml

@@ -0,0 +1,22 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: credentials-secret
+type: Opaque
+data:
+  username: bXktc2VjcmV0LXVzZXI=
+  password: d2h5YXJleW91bG9va2luZ2hlcmU=
+---
+apiVersion: rabbitmq.com/v1alpha1
+kind: User
+metadata:
+  name: import-user-sample
+spec:
+  tags:
+  - management # available tags are 'management', 'policymaker', 'monitoring' and 'administrator'
+  - policymaker
+  rabbitmqClusterReference:
+    name: test
+    namespace: rabbitmq-system
+  importCredentialsSecret:
+    name: credentials-secret