Fix issue with conflicting User annotations (#899)

### Description
Correctly set annotations for Secret that stores multiple User
passwords.
`OsUserNameAnnotation` will be assigned only to the Secret that is
storing the password for a single user. Otherwise, if Secret is used by
multiple Users this annotation will be skipped.

To support correct reconciliation for multi-user Secret, I have added
iteration over `secretObj.Data`.
In the case of multi-user Secret, reconciliation will work only if the
Secret keys correspond to the User name. So in theory there could be a
breaking change here for people who are using multi-user Secret and have
different Secret keys and User names.

### Issues Resolved
Closes #884

### Check List
- [x] Commits are signed per the DCO using --signoff 
- [x] Unittest added for the new/changed functionality and all unit
tests are successful
- [x] Customer-visible features documented
- [x] No linter warnings (`make lint`)


Please refer to the [PR
guidelines](https://github.com/opensearch-project/opensearch-k8s-operator/blob/main/docs/developing.md#submitting-a-pr)
before submitting this pull request.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Yevhenii Tiutiunnyk <[email protected]>

docs/userguide/main.md

@@ -1146,6 +1146,10 @@ The namespace of the `OpenSearchUser` must be the namespace the OpenSearch clust
 
 Note that a secret called `sample-user-password` will need to exist in the `default` namespace with the base64 encoded password in the `password` key.
 
+Also, it is possible to store multiple Users password in the same Secret. To do this, you **must** create a secret where
+each key will be equal to a **User name** and value is a user password. **Otherwise, changes in the secret will not trigger User
+reconcile!**
+
 #### Opensearch Roles
 
 It is possible to manage Opensearch roles in Kubernetes with the operator. The operator will not modify roles that already exist. You can create an example role as follows:

opensearch-operator/controllers/opensearchuser_controller.go

@@ -90,29 +90,48 @@ func (r *OpensearchUserReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 }
 
 func (r *OpensearchUserReconciler) handleSecretEvent(_ context.Context, secret client.Object) []reconcile.Request {
-	reconcileRequests := []reconcile.Request{}
+	var reconcileRequests []reconcile.Request
 
 	if secret == nil {
 		return reconcileRequests
 	}
 
-	// Only check secrets with Opensearch User Annotations
+	// Only check secrets with OsUserNamespaceAnnotation and (optional) OsUserNameAnnotation
 	annotations := secret.GetAnnotations()
 
-	name, nameOk := annotations[helpers.OsUserNameAnnotation]
 	namespace, namespaceOk := annotations[helpers.OsUserNamespaceAnnotation]
 
-	if !nameOk || !namespaceOk {
+	if !namespaceOk {
 		return reconcileRequests
 	}
 
-	// Trigger reconcile for according OpenSearchUser
-	reconcileRequests = append(reconcileRequests, reconcile.Request{
-		NamespacedName: types.NamespacedName{
-			Name:      string(name),
-			Namespace: string(namespace),
-		},
-	})
+	name, nameOk := annotations[helpers.OsUserNameAnnotation]
+
+	if nameOk {
+		return append(reconcileRequests, reconcile.Request{
+			NamespacedName: types.NamespacedName{
+				Name:      name,
+				Namespace: namespace,
+			},
+		})
+	}
+
+	// For Secret that stores multiple User passwords
+	// Cast the client.Object to a *corev1.Secret
+	secretObj, ok := secret.(*corev1.Secret)
+	if !ok {
+		return reconcileRequests
+	}
+
+	for username := range secretObj.Data {
+		// Create a reconcile request for each user found in the Secret
+		reconcileRequests = append(reconcileRequests, reconcile.Request{
+			NamespacedName: types.NamespacedName{
+				Name:      username,
+				Namespace: namespace,
+			},
+		})
+	}
 
 	return reconcileRequests
 }

opensearch-operator/pkg/reconcilers/users.go

@@ -244,7 +244,10 @@ func (r *UserReconciler) managePasswordSecret(username string, namespace string)
 		secret.Annotations = make(map[string]string)
 	}
 
-	secret.Annotations[helpers.OsUserNameAnnotation] = username
+	// Only put OsUserNameAnnotation if Secret contain a single key, otherwise, consider as multi-user Secret
+	if len(secret.Data) == 1 {
+		secret.Annotations[helpers.OsUserNameAnnotation] = username
+	}
 	secret.Annotations[helpers.OsUserNamespaceAnnotation] = namespace
 
 	if _, err := r.client.CreateSecret(&secret); err != nil {

opensearch-operator/pkg/reconcilers/users_test.go

@@ -308,7 +308,6 @@ var _ = Describe("users reconciler", func() {
 		})
 		When("user exists and is different", func() {
 			BeforeEach(func() {
-				mockClient.EXPECT().GetSecret(mock.Anything, mock.Anything).Return(*password, nil)
 				userRequest := requests.User{
 					Attributes: map[string]string{
 						services.K8sAttributeField: "testuid",
@@ -342,6 +341,7 @@ var _ = Describe("users reconciler", func() {
 				)
 			})
 			It("should update the user", func() {
+				mockClient.EXPECT().GetSecret(mock.Anything, mock.Anything).Return(*password, nil)
 				var createdSecret *corev1.Secret
 				mockClient.On("CreateSecret", mock.Anything).
 					Return(func(secret *corev1.Secret) (*ctrl.Result, error) {
@@ -364,7 +364,8 @@ var _ = Describe("users reconciler", func() {
 				Expect(len(events)).To(Equal(1))
 				Expect(events[0]).To(Equal(fmt.Sprintf("Normal %s user updated in opensearch", opensearchAPIUpdated)))
 			})
-			It("should update the secret with opensearch annotations", func() {
+			It("should update the secret with User and Namespace annotations", func() {
+				mockClient.EXPECT().GetSecret(mock.Anything, mock.Anything).Return(*password, nil)
 				var createdSecret *corev1.Secret
 				mockClient.On("CreateSecret", mock.Anything).
 					Return(func(secret *corev1.Secret) (*ctrl.Result, error) {
@@ -385,6 +386,46 @@ var _ = Describe("users reconciler", func() {
 				Expect(actualName).To(Equal(expectedName))
 				Expect(actualNamespace).To(Equal(expectedNamespace))
 			})
+			It("should update the secret only with Namespace annotation", func() {
+				instance.Spec.PasswordFrom = corev1.SecretKeySelector{
+					Key: instance.Name,
+					LocalObjectReference: corev1.LocalObjectReference{
+						Name: "multi-user-secret",
+					},
+				}
+				password = &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "multi-user-secret",
+						Namespace: "test-user",
+					},
+					Data: map[string][]byte{
+						instance.Name: []byte("user1password"),
+						"test-user2":  []byte("user2password"),
+					},
+				}
+				mockClient.EXPECT().GetSecret(mock.Anything, mock.Anything).Return(*password, nil)
+
+				var createdSecret *corev1.Secret
+				mockClient.On("CreateSecret", mock.Anything).
+					Return(func(secret *corev1.Secret) (*ctrl.Result, error) {
+						createdSecret = secret
+						secret.Name = "multi-user-secret"
+						return &ctrl.Result{}, nil
+					})
+				defer close(recorder.Events)
+				_, err := reconciler.Reconcile()
+				Expect(err).ToNot(HaveOccurred())
+
+				annotations := createdSecret.GetAnnotations()
+
+				_, nameOk := annotations[helpers.OsUserNameAnnotation]
+				actualNamespace := annotations[helpers.OsUserNamespaceAnnotation]
+
+				expectedNamespace := "test-user"
+				Expect(actualNamespace).To(Equal(expectedNamespace))
+				Expect(nameOk).To(BeFalse(), "Expected Name annotation to be absent for Secret with multiple user passwords")
+				Expect(len(createdSecret.Data)).To(Equal(2), "Expected secret to contain 2 keys")
+			})
 		})
 		When("user does not exist", func() {
 			BeforeEach(func() {