diff --git a/opensearch-operator/opensearch-gateway/services/os_security_service.go b/opensearch-operator/opensearch-gateway/services/os_security_service.go index 220236ff..5bd92962 100644 --- a/opensearch-operator/opensearch-gateway/services/os_security_service.go +++ b/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" diff --git a/opensearch-operator/pkg/reconcilers/users.go b/opensearch-operator/pkg/reconcilers/users.go index 01af78ea..95bcd2f4 100644 --- a/opensearch-operator/pkg/reconcilers/users.go +++ b/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,15 +142,24 @@ 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, @@ -157,12 +167,10 @@ func (r *UserReconciler) Reconcile() (retResult ctrl.Result, retErr error) { // 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 } diff --git a/opensearch-operator/pkg/reconcilers/users_test.go b/opensearch-operator/pkg/reconcilers/users_test.go index a9971734..28780b1f 100644 --- a/opensearch-operator/pkg/reconcilers/users_test.go +++ b/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