Automatically update User password on Secret change (#922)

### Description
Solving a problem with updating User password if it was changed in the
Secret by adding an additional attribute to the Opensearch user -
`secret-version`. The logic relies on a default Kubernetes
`ResourceVersion` behavior.

### Issues Resolved
Closes #908 

### 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`)

If CRDs are changed:
- [x] CRD YAMLs updated (`make manifests`) and also copied into the helm
chart
- [x] Changes to CRDs documented

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]>

opensearch-operator/opensearch-gateway/services/os_security_service.go

@@ -13,7 +13,8 @@ import (
 )
 
 const (
-	K8sAttributeField = "k8s-uid"
+	K8sAttributeField              = "k8s-uid"
+	K8sAttributeSecretVersionField = "secret-version"
 
 	ROLES         = "roles"
 	INTERNALUSERS = "internalusers"

opensearch-operator/pkg/reconcilers/users.go

@@ -3,6 +3,7 @@ package reconcilers
 import (
 	"context"
 	"fmt"
+	v1 "k8s.io/api/core/v1"
 	"time"
 
 	opsterv1 "github.com/Opster/opensearch-k8s-operator/opensearch-operator/api/v1"
@@ -141,28 +142,35 @@ func (r *UserReconciler) Reconcile() (retResult ctrl.Result, retErr error) {
 		return
 	}
 
-	userPassword, retErr := r.managePasswordSecret(r.instance.Name, r.instance.Namespace)
+	userSecret, retErr := r.managePasswordSecret(r.instance.Name, r.instance.Namespace)
 
 	if retErr != nil {
 		// Event and logging handled in fetch function
-		reason = "failed to get password from secret"
+		reason = "failed to get user secret"
 		return
 	}
+
+	userPassword, ok := userSecret.Data[r.instance.Spec.PasswordFrom.Key]
+	if !ok {
+		retErr = fmt.Errorf("key %s does not exist in secret", r.instance.Spec.PasswordFrom.Key)
+		r.logger.V(1).Error(retErr, "failed to get password from secret")
+		r.recorder.Event(r.instance, "Warning", passwordError, fmt.Sprintf("key %s does not exist in secret", r.instance.Spec.PasswordFrom.Key))
+		return
+	}
+
 	user := requests.User{
-		Password:                userPassword,
+		Password:                string(userPassword),
 		OpendistroSecurityRoles: r.instance.Spec.OpendistroSecurityRoles,
 		BackendRoles:            r.instance.Spec.BackendRoles,
 		Attributes:              r.instance.Spec.Attributes,
 	}
 
 	// Instantiate the map first
 	if user.Attributes == nil {
-		user.Attributes = map[string]string{
-			services.K8sAttributeField: string(r.instance.GetUID()),
-		}
-	} else {
-		user.Attributes[services.K8sAttributeField] = string(r.instance.GetUID())
+		user.Attributes = make(map[string]string)
 	}
+	user.Attributes[services.K8sAttributeField] = string(r.instance.GetUID())
+	user.Attributes[services.K8sAttributeSecretVersionField] = userSecret.ResourceVersion
 
 	update, retErr := services.ShouldUpdateUser(r.ctx, r.osClient, r.instance.Name, user)
 	if retErr != nil {
@@ -231,12 +239,12 @@ func (r *UserReconciler) Delete() error {
 	return services.DeleteUser(r.ctx, r.osClient, r.instance.Name)
 }
 
-func (r *UserReconciler) managePasswordSecret(username string, namespace string) (string, error) {
+func (r *UserReconciler) managePasswordSecret(username string, namespace string) (v1.Secret, error) {
 	secret, err := r.client.GetSecret(r.instance.Spec.PasswordFrom.Name, r.instance.Namespace)
 	if err != nil {
 		r.logger.V(1).Error(err, "failed to fetch password secret")
 		r.recorder.Event(r.instance, "Warning", passwordError, "error fetching password secret")
-		return "", err
+		return v1.Secret{}, err
 	}
 
 	// Patch OpenSearch Annotations onto secret
@@ -250,19 +258,11 @@ func (r *UserReconciler) managePasswordSecret(username string, namespace string)
 	}
 	secret.Annotations[helpers.OsUserNamespaceAnnotation] = namespace
 
-	if _, err := r.client.CreateSecret(&secret); err != nil {
+	if _, err = r.client.CreateSecret(&secret); err != nil {
 		r.logger.V(1).Error(err, "failed to patch opensearch username onto password secret")
 		r.recorder.Event(r.instance, "Warning", passwordError, "error patching opensearch username onto password secret")
-		return "", err
-	}
-
-	userPassword, ok := secret.Data[r.instance.Spec.PasswordFrom.Key]
-	if !ok {
-		err = fmt.Errorf("key %s does not exist in secret", r.instance.Spec.PasswordFrom.Key)
-		r.logger.V(1).Error(err, "failed to get password from secret")
-		r.recorder.Event(r.instance, "Warning", passwordError, fmt.Sprintf("key %s does not exist in secret", r.instance.Spec.PasswordFrom.Key))
-		return "", err
+		return v1.Secret{}, err
 	}
 
-	return string(userPassword), nil
+	return secret, nil
 }

opensearch-operator/pkg/reconcilers/users_test.go

@@ -60,8 +60,9 @@ var _ = Describe("users reconciler", func() {
 		}
 		password = &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test-password",
-				Namespace: "test-user",
+				Name:            "test-password",
+				Namespace:       "test-user",
+				ResourceVersion: "123456789",
 			},
 			Data: map[string][]byte{
 				"password": []byte("testpassword"),
@@ -231,7 +232,8 @@ var _ = Describe("users reconciler", func() {
 				mockClient.EXPECT().GetSecret(mock.Anything, mock.Anything).Return(*password, nil)
 				userRequest := requests.User{
 					Attributes: map[string]string{
-						services.K8sAttributeField: "testuid",
+						services.K8sAttributeField:              "testuid",
+						services.K8sAttributeSecretVersionField: "123456789",
 					},
 				}
 				transport.RegisterResponder(
@@ -310,7 +312,8 @@ var _ = Describe("users reconciler", func() {
 			BeforeEach(func() {
 				userRequest := requests.User{
 					Attributes: map[string]string{
-						services.K8sAttributeField: "testuid",
+						services.K8sAttributeField:              "testuid",
+						services.K8sAttributeSecretVersionField: "123456789",
 					},
 				}
 				recorder = record.NewFakeRecorder(1)
@@ -364,6 +367,34 @@ 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 user password", func() {
+				instance.Spec.BackendRoles = []string{}
+				password.ObjectMeta.ResourceVersion = "987654321"
+				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
+						return &ctrl.Result{}, nil
+					})
+
+				go func() {
+					defer GinkgoRecover()
+					defer close(recorder.Events)
+
+					_, err := reconciler.Reconcile()
+					Expect(err).ToNot(HaveOccurred())
+				}()
+
+				var events []string
+				for msg := range recorder.Events {
+					events = append(events, msg)
+				}
+
+				Expect(createdSecret).ToNot(BeNil())
+				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 User and Namespace annotations", func() {
 				mockClient.EXPECT().GetSecret(mock.Anything, mock.Anything).Return(*password, nil)
 				var createdSecret *corev1.Secret