From 91ac04d440c2d22e4fb8391336f792100c09655c Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Fri, 5 Jul 2024 16:44:14 +0100 Subject: [PATCH 01/17] Modify missleading or outdated documentation --- docs/secure.md | 26 +++++++++++++------------- docs/users.md | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/secure.md b/docs/secure.md index f2b7d877a..094ce49fc 100644 --- a/docs/secure.md +++ b/docs/secure.md @@ -36,20 +36,20 @@ To secure connections to MongoDBCommunity resources with TLS using `cert-manager helm repo update ``` -1. Install `cert-manager`: +2. Install `cert-manager`: ``` - helm install cert-manager jetstack/cert-manager --namespace cert-manager \ - --create-namespace --set installCRDs=true + helm install cert-manager jetstack/cert-manager --namespace cert-manager --create-namespace --set crds.enabled=true ``` -1. Create a TLS-secured MongoDBCommunity resource: +3. Create a TLS-secured MongoDBCommunity resource: +This assumes you already have the operator installed in namespace `` ``` helm upgrade --install community-operator mongodb/community-operator \ - --namespace mongodb --set resource.tls.useCertManager=true \ + --namespace --set resource.tls.useCertManager=true \ --set createResource=true --set resource.tls.enabled=true \ - --set namespace=mongodb --create-namespace + --set namespace= --create-namespace ``` This creates a resource secured with TLS and generates the necessary @@ -72,21 +72,21 @@ To secure connections to MongoDBCommunity resources with TLS using `cert-manager 1. Test your connection over TLS by - - Connecting to a `mongod` container using `kubectl`: + - Connecting to a `mongod` container inside a pod using `kubectl`: ``` - kubectl exec -it mongodb-replica-set -c mongod -- bash + kubectl exec -it -c mongod -- bash ``` - Where `mongodb-replica-set` is the name of your MongoDBCommunity resource + Where `mongodb-replica-set-pod` is the name of a pod from your MongoDBCommunity resource - Then, use `mongosh` to connect over TLS: + For how to get the connection string look at [Deploy A Replica Set](deploy-configure.md#deploy-a-replica-set) ``` - mongosh --tls --tlsCAFile /var/lib/tls/ca/ca.crt --tlsCertificateKeyFile \ - /var/lib/tls/server/*.pem \ - --host .-svc..svc.cluster.local + mongosh "" --tls --tlsCAFile /var/lib/tls/ca/ca.crt --tlsCertificateKeyFile /var/lib/tls/server/*.pem ``` Where `mongodb-replica-set` is the name of your MongoDBCommunity - resource and `namespace` is the namespace of your deployment. \ No newline at end of file + resource, `namespace` is the namespace of your deployment + and `connection-string` is a connection string for your `-svc` service. \ No newline at end of file diff --git a/docs/users.md b/docs/users.md index a1980e4fb..96a44570a 100644 --- a/docs/users.md +++ b/docs/users.md @@ -84,6 +84,6 @@ You cannot disable SCRAM authentication. - To authenticate to your MongoDBCommunity resource, run the following command: ``` - mongo "mongodb://..svc.cluster.local:27017/?replicaSet=" --username --password --authenticationDatabase + mongosh "mongodb://-svc..svc.cluster.local:27017/?replicaSet=" --username --password --authenticationDatabase ``` - To change a user's password, create and apply a new secret resource definition with a `metadata.name` that is the same as the name specified in `passwordSecretRef.name` of the MongoDB CRD. The Operator will automatically regenerate credentials. From c56b2f0a7437abbdf0bb453689693f701a20d164 Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Fri, 5 Jul 2024 17:05:32 +0100 Subject: [PATCH 02/17] Fix indentation --- docs/secure.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/secure.md b/docs/secure.md index 094ce49fc..d937d3ea0 100644 --- a/docs/secure.md +++ b/docs/secure.md @@ -43,7 +43,8 @@ To secure connections to MongoDBCommunity resources with TLS using `cert-manager ``` 3. Create a TLS-secured MongoDBCommunity resource: -This assumes you already have the operator installed in namespace `` + + This assumes you already have the operator installed in namespace `` ``` helm upgrade --install community-operator mongodb/community-operator \ From 74cc2c558e6e2e4b8386f4bb3eea680ffb59fdca Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Fri, 5 Jul 2024 17:19:06 +0100 Subject: [PATCH 03/17] Removed redundant flag --- docs/secure.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/secure.md b/docs/secure.md index d937d3ea0..e1a1e8631 100644 --- a/docs/secure.md +++ b/docs/secure.md @@ -50,7 +50,7 @@ To secure connections to MongoDBCommunity resources with TLS using `cert-manager helm upgrade --install community-operator mongodb/community-operator \ --namespace --set resource.tls.useCertManager=true \ --set createResource=true --set resource.tls.enabled=true \ - --set namespace= --create-namespace + --set namespace= ``` This creates a resource secured with TLS and generates the necessary From 232a38fbd5d915931e99fc16df291bb21e5eaf7f Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 9 Jul 2024 15:39:51 +0100 Subject: [PATCH 04/17] Mofify Makefile to fix make deploy --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index a03dfb9bd..61d6670a2 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ AGENT_IMAGE_NAME := $(shell jq -r .agent_image_ubi < $(MONGODB_COMMUNITY_CONFIG) HELM_CHART ?= ./helm-charts/charts/community-operator -STRING_SET_VALUES := --set namespace=$(NAMESPACE),versionUpgradeHook.name=$(UPGRADE_HOOK_IMG),readinessProbe.name=$(READINESS_PROBE_IMG),registry.operator=$(REPO_URL),operator.operatorImageName=$(OPERATOR_IMAGE),operator.version=latest,registry.agent=$(REGISTRY),registry.versionUpgradeHook=$(REGISTRY),registry.readinessProbe=$(REGISTRY),registry.operator=$(REGISTRY),versionUpgradeHook.version=latest,readinessProbe.version=latest,agent.version=latest,agent.name=$(AGENT_IMAGE_NAME) +STRING_SET_VALUES := --set namespace=$(NAMESPACE),versionUpgradeHook.name=$(UPGRADE_HOOK_IMG),readinessProbe.name=$(READINESS_PROBE_IMG),registry.operator=$(REPO_URL),operator.operatorImageName=$(OPERATOR_IMAGE),operator.version=0.9.0-arm64,registry.agent=$(REGISTRY),registry.versionUpgradeHook=$(REGISTRY),registry.readinessProbe=$(REGISTRY),registry.operator=$(REGISTRY),versionUpgradeHook.version=1.0.8-arm64,readinessProbe.version=1.0.19-arm64,agent.version=13.19.0.8951-1-arm64,agent.name=$(AGENT_IMAGE_NAME) STRING_SET_VALUES_LOCAL := $(STRING_SET_VALUES) --set operator.replicas=0 DOCKERFILE ?= operator @@ -134,7 +134,7 @@ install-rbac: $(HELM) template $(STRING_SET_VALUES) -s templates/operator_roles.yaml $(HELM_CHART) | kubectl apply -f - uninstall-crd: - kubectl delete crd mongodbcommunity.mongodbcommunity.mongodb.com + kubectl delete crd --ignore-not-found=true mongodbcommunity.mongodbcommunity.mongodb.com uninstall-chart: $(HELM) uninstall $(RELEASE_NAME_HELM) -n $(NAMESPACE) From 8ebccf6c9ea1b1e3f10903a668114260b175bb26 Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 16 Jul 2024 11:43:34 +0100 Subject: [PATCH 05/17] Cleanup connection string secrets of removed users --- controllers/mongodb_cleanup.go | 44 ++++++++++++++ controllers/mongodb_cleanup_test.go | 89 +++++++++++++++++++++++++++++ controllers/mongodb_users.go | 3 + 3 files changed, 136 insertions(+) diff --git a/controllers/mongodb_cleanup.go b/controllers/mongodb_cleanup.go index 85d720ec0..6cff0d86d 100644 --- a/controllers/mongodb_cleanup.go +++ b/controllers/mongodb_cleanup.go @@ -46,6 +46,22 @@ func (r *ReplicaSetReconciler) cleanupScramSecrets(ctx context.Context, currentM } } +// cleanupConnectionStringSecrets cleans up old scram secrets based on the last successful applied mongodb spec. +func (r *ReplicaSetReconciler) cleanupConnectionStringSecrets(ctx context.Context, currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, namespace string, resourceName string) { + secretsToDelete := getConnectionStringSecretsToDelete(currentMDB, lastAppliedMDBSpec, resourceName) + + for _, s := range secretsToDelete { + if err := r.client.DeleteSecret(ctx, types.NamespacedName{ + Name: s, + Namespace: namespace, + }); err != nil { + r.log.Warnf("Could not cleanup old secret %s", s) + } else { + r.log.Debugf("Sucessfully cleaned up secret: %s", s) + } + } +} + func getScramSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec) []string { type user struct { db string @@ -73,3 +89,31 @@ func getScramSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedM } return secretsToDelete } + +func getConnectionStringSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, resourceName string) []string { + type user struct { + db string + name string + } + m := map[user]string{} + var secretsToDelete []string + + for _, mongoDBUser := range currentMDB.Users { + if mongoDBUser.DB != constants.ExternalDB { + m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] = mongoDBUser.GetConnectionStringSecretName(resourceName) + } + } + + for _, mongoDBUser := range lastAppliedMDBSpec.Users { + if mongoDBUser.DB == constants.ExternalDB { + continue + } + currentConnectionStringSecretName, ok := m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] + if !ok { // not used anymore + secretsToDelete = append(secretsToDelete, mongoDBUser.GetConnectionStringSecretName(resourceName)) + } else if currentConnectionStringSecretName != mongoDBUser.GetConnectionStringSecretName(resourceName) { // have changed + secretsToDelete = append(secretsToDelete, mongoDBUser.GetConnectionStringSecretName(resourceName)) + } + } + return secretsToDelete +} diff --git a/controllers/mongodb_cleanup_test.go b/controllers/mongodb_cleanup_test.go index 472e77266..267590014 100644 --- a/controllers/mongodb_cleanup_test.go +++ b/controllers/mongodb_cleanup_test.go @@ -154,3 +154,92 @@ func TestReplicaSetReconcilerCleanupPemSecret(t *testing.T) { _, err = r.client.GetSecret(ctx, mdb.AgentCertificatePemSecretNamespacedName()) assert.Error(t, err) } + +func TestReplicaSetReconcilerCleanupConnectionStringSecrets(t *testing.T) { + lastApplied := newScramReplicaSet(mdbv1.MongoDBUser{ + Name: "testUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret", + }) + + t.Run("no change same resource", func(t *testing.T) { + actual := getConnectionStringSecretsToDelete(lastApplied.Spec, lastApplied.Spec, "my-rs") + + var expected []string + assert.Equal(t, expected, actual) + }) + + t.Run("new user new secret", func(t *testing.T) { + current := newScramReplicaSet( + mdbv1.MongoDBUser{ + Name: "testUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret", + }, + mdbv1.MongoDBUser{ + Name: "newUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret-2", + }, + ) + + var expected []string + actual := getConnectionStringSecretsToDelete(current.Spec, lastApplied.Spec, "my-rs") + + assert.Equal(t, expected, actual) + }) + + t.Run("old user new secret", func(t *testing.T) { + current := newScramReplicaSet(mdbv1.MongoDBUser{ + Name: "testUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret-2", + }) + + expected := []string{"connection-string-secret"} + actual := getConnectionStringSecretsToDelete(current.Spec, lastApplied.Spec, "my-rs") + + assert.Equal(t, expected, actual) + }) + + t.Run("removed one user", func(t *testing.T) { + lastApplied = newScramReplicaSet( + mdbv1.MongoDBUser{ + Name: "testUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret", + }, + mdbv1.MongoDBUser{ + Name: "anotherUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret-2", + }, + ) + + current := newScramReplicaSet(mdbv1.MongoDBUser{ + Name: "testUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret-1", + }) + + expected := []string{"connection-string-secret", "connection-string-secret-2"} + actual := getConnectionStringSecretsToDelete(current.Spec, lastApplied.Spec, "my-rs") + + assert.Equal(t, expected, actual) + }) + +} diff --git a/controllers/mongodb_users.go b/controllers/mongodb_users.go index 2af312d2c..154282810 100644 --- a/controllers/mongodb_users.go +++ b/controllers/mongodb_users.go @@ -82,6 +82,9 @@ func (r ReplicaSetReconciler) updateConnectionStringSecrets(ctx context.Context, if err := secret.CreateOrUpdate(ctx, r.client, connectionStringSecret); err != nil { return err } + + secretNamespacedName := types.NamespacedName{Name: user.PasswordSecretName, Namespace: secretNamespace} + r.secretWatcher.Watch(ctx, secretNamespacedName, mdb.NamespacedName()) } return nil From 5d3047a492721f67d3d43d3f466aee5213c1da8c Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 16 Jul 2024 11:45:33 +0100 Subject: [PATCH 06/17] Add removed users to automation config --- controllers/replica_set_controller.go | 21 +++-- pkg/authentication/authentication.go | 33 ++++++++ pkg/authentication/authentication_test.go | 94 +++++++++++++++++++++++ pkg/automationconfig/automation_config.go | 9 +++ 4 files changed, 149 insertions(+), 8 deletions(-) diff --git a/controllers/replica_set_controller.go b/controllers/replica_set_controller.go index 2d9355a57..ea29b69b0 100644 --- a/controllers/replica_set_controller.go +++ b/controllers/replica_set_controller.go @@ -173,7 +173,7 @@ func (r ReplicaSetReconciler) Reconcile(ctx context.Context, request reconcile.R withFailedPhase()) } - ready, err := r.deployMongoDBReplicaSet(ctx, mdb) + ready, err := r.deployMongoDBReplicaSet(ctx, mdb, lastAppliedSpec) if err != nil { return status.Update(ctx, r.client.Status(), &mdb, statusOptions(). withMessage(Error, fmt.Sprintf("Error deploying MongoDB ReplicaSet: %s", err)). @@ -225,6 +225,7 @@ func (r ReplicaSetReconciler) Reconcile(ctx context.Context, request reconcile.R if lastAppliedSpec != nil { r.cleanupScramSecrets(ctx, mdb.Spec, *lastAppliedSpec, mdb.Namespace) r.cleanupPemSecret(ctx, mdb.Spec, *lastAppliedSpec, mdb.Namespace) + r.cleanupConnectionStringSecrets(ctx, mdb.Spec, *lastAppliedSpec, mdb.Namespace, mdb.Name) } if err := r.updateLastSuccessfulConfiguration(ctx, mdb); err != nil { @@ -331,7 +332,7 @@ func (r *ReplicaSetReconciler) deployStatefulSet(ctx context.Context, mdb mdbv1. // deployAutomationConfig deploys the AutomationConfig for the MongoDBCommunity resource. // The returned boolean indicates whether or not that Agents have all reached goal state. -func (r *ReplicaSetReconciler) deployAutomationConfig(ctx context.Context, mdb mdbv1.MongoDBCommunity) (bool, error) { +func (r *ReplicaSetReconciler) deployAutomationConfig(ctx context.Context, mdb mdbv1.MongoDBCommunity, lastAppliedSpec *mdbv1.MongoDBCommunitySpec) (bool, error) { r.log.Infof("Creating/Updating AutomationConfig") sts, err := r.client.GetStatefulSet(ctx, mdb.NamespacedName()) @@ -339,7 +340,7 @@ func (r *ReplicaSetReconciler) deployAutomationConfig(ctx context.Context, mdb m return false, fmt.Errorf("failed to get StatefulSet: %s", err) } - ac, err := r.ensureAutomationConfig(mdb, ctx) + ac, err := r.ensureAutomationConfig(mdb, ctx, lastAppliedSpec) if err != nil { return false, fmt.Errorf("failed to ensure AutomationConfig: %s", err) } @@ -408,10 +409,10 @@ func (r *ReplicaSetReconciler) shouldRunInOrder(ctx context.Context, mdb mdbv1.M // deployMongoDBReplicaSet will ensure that both the AutomationConfig secret and backing StatefulSet // have been successfully created. A boolean is returned indicating if the process is complete // and an error if there was one. -func (r *ReplicaSetReconciler) deployMongoDBReplicaSet(ctx context.Context, mdb mdbv1.MongoDBCommunity) (bool, error) { +func (r *ReplicaSetReconciler) deployMongoDBReplicaSet(ctx context.Context, mdb mdbv1.MongoDBCommunity, lastAppliedSpec *mdbv1.MongoDBCommunitySpec) (bool, error) { return functions.RunSequentially(r.shouldRunInOrder(ctx, mdb), func() (bool, error) { - return r.deployAutomationConfig(ctx, mdb) + return r.deployAutomationConfig(ctx, mdb, lastAppliedSpec) }, func() (bool, error) { return r.deployStatefulSet(ctx, mdb) @@ -489,8 +490,8 @@ func (r *ReplicaSetReconciler) createOrUpdateStatefulSet(ctx context.Context, md // ensureAutomationConfig makes sure the AutomationConfig secret has been successfully created. The automation config // that was updated/created is returned. -func (r ReplicaSetReconciler) ensureAutomationConfig(mdb mdbv1.MongoDBCommunity, ctx context.Context) (automationconfig.AutomationConfig, error) { - ac, err := r.buildAutomationConfig(ctx, mdb) +func (r ReplicaSetReconciler) ensureAutomationConfig(mdb mdbv1.MongoDBCommunity, ctx context.Context, lastAppliedSpec *mdbv1.MongoDBCommunitySpec) (automationconfig.AutomationConfig, error) { + ac, err := r.buildAutomationConfig(ctx, mdb, lastAppliedSpec) if err != nil { return automationconfig.AutomationConfig{}, fmt.Errorf("could not build automation config: %s", err) } @@ -622,7 +623,7 @@ func getCustomRolesModification(mdb mdbv1.MongoDBCommunity) (automationconfig.Mo }, nil } -func (r ReplicaSetReconciler) buildAutomationConfig(ctx context.Context, mdb mdbv1.MongoDBCommunity) (automationconfig.AutomationConfig, error) { +func (r ReplicaSetReconciler) buildAutomationConfig(ctx context.Context, mdb mdbv1.MongoDBCommunity, lastAppliedSpec *mdbv1.MongoDBCommunitySpec) (automationconfig.AutomationConfig, error) { tlsModification, err := getTLSConfigModification(ctx, r.client, r.client, mdb) if err != nil { return automationconfig.AutomationConfig{}, fmt.Errorf("could not configure TLS modification: %s", err) @@ -643,6 +644,10 @@ func (r ReplicaSetReconciler) buildAutomationConfig(ctx context.Context, mdb mdb return automationconfig.AutomationConfig{}, err } + if lastAppliedSpec != nil { + authentication.AddRemovedUsers(&auth, mdb, lastAppliedSpec) + } + prometheusModification := automationconfig.NOOP() if mdb.Spec.Prometheus != nil { secretNamespacedName := types.NamespacedName{Name: mdb.Spec.Prometheus.PasswordSecretRef.Name, Namespace: mdb.Namespace} diff --git a/pkg/authentication/authentication.go b/pkg/authentication/authentication.go index b73c97898..0a0041ce5 100644 --- a/pkg/authentication/authentication.go +++ b/pkg/authentication/authentication.go @@ -3,6 +3,7 @@ package authentication import ( "context" "fmt" + mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1" "k8s.io/apimachinery/pkg/types" @@ -33,3 +34,35 @@ func Enable(ctx context.Context, auth *automationconfig.Auth, secretGetUpdateCre } return nil } + +func AddRemovedUsers(auth *automationconfig.Auth, mdb mdbv1.MongoDBCommunity, lastAppliedSpec *mdbv1.MongoDBCommunitySpec) { + deletedUsers := getDeletedUsers(mdb.Spec, lastAppliedSpec) + + auth.UsersDeleted = append(auth.UsersDeleted, deletedUsers...) +} + +func getDeletedUsers(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec *mdbv1.MongoDBCommunitySpec) []automationconfig.DeletedUser { + type user struct { + db string + name string + } + m := map[user]bool{} + var deletedUsers []automationconfig.DeletedUser + + for _, mongoDBUser := range currentMDB.Users { + if mongoDBUser.DB != constants.ExternalDB { + m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] = true + } + } + + for _, mongoDBUser := range lastAppliedMDBSpec.Users { + if mongoDBUser.DB == constants.ExternalDB { + continue + } + _, ok := m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] + if !ok { + deletedUsers = append(deletedUsers, automationconfig.DeletedUser{User: mongoDBUser.Name, Dbs: []string{mongoDBUser.DB}}) + } + } + return deletedUsers +} diff --git a/pkg/authentication/authentication_test.go b/pkg/authentication/authentication_test.go index 8521cbf1c..ee9f49995 100644 --- a/pkg/authentication/authentication_test.go +++ b/pkg/authentication/authentication_test.go @@ -2,6 +2,7 @@ package authentication import ( "context" + mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -132,6 +133,99 @@ func TestEnable(t *testing.T) { } +func TestGetDeletedUsers(t *testing.T) { + lastAppliedSpec := mdbv1.MongoDBCommunitySpec{ + Members: 3, + Type: "ReplicaSet", + Version: "7.0.2", + Arbiters: 0, + Security: mdbv1.Security{ + Authentication: mdbv1.Authentication{ + Modes: []mdbv1.AuthMode{"SCRAM"}, + }, + }, + Users: []mdbv1.MongoDBUser{ + { + Name: "testUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret", + DB: "admin", + }, + }, + } + + t.Run("no change same resource", func(t *testing.T) { + actual := getDeletedUsers(lastAppliedSpec, &lastAppliedSpec) + + var expected []automationconfig.DeletedUser + assert.Equal(t, expected, actual) + }) + + t.Run("new user", func(t *testing.T) { + current := mdbv1.MongoDBCommunitySpec{ + Members: 3, + Type: "ReplicaSet", + Version: "7.0.2", + Arbiters: 0, + Security: mdbv1.Security{ + Authentication: mdbv1.Authentication{ + Modes: []mdbv1.AuthMode{"SCRAM"}, + }, + }, + Users: []mdbv1.MongoDBUser{ + { + Name: "testUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "password-secret-name", + }, + ConnectionStringSecretName: "connection-string-secret", + DB: "admin", + }, + { + Name: "newUser", + PasswordSecretRef: mdbv1.SecretKeyReference{ + Name: "new-password-secret-name", + }, + ConnectionStringSecretName: "new-connection-string-secret", + DB: "admin", + }, + }, + } + + var expected []automationconfig.DeletedUser + actual := getDeletedUsers(current, &lastAppliedSpec) + + assert.Equal(t, expected, actual) + }) + + t.Run("removed one user", func(t *testing.T) { + current := mdbv1.MongoDBCommunitySpec{ + Members: 3, + Type: "ReplicaSet", + Version: "7.0.2", + Arbiters: 0, + Security: mdbv1.Security{ + Authentication: mdbv1.Authentication{ + Modes: []mdbv1.AuthMode{"SCRAM"}, + }, + }, + Users: []mdbv1.MongoDBUser{}, + } + + expected := []automationconfig.DeletedUser{ + { + User: "testUser", + Dbs: []string{"admin"}, + }, + } + actual := getDeletedUsers(current, &lastAppliedSpec) + + assert.Equal(t, expected, actual) + }) +} + func buildConfigurable(name string, auth []string, agent string, users ...authtypes.User) mocks.MockConfigurable { return mocks.NewMockConfigurable( authtypes.Options{ diff --git a/pkg/automationconfig/automation_config.go b/pkg/automationconfig/automation_config.go index b3dc5a36b..6195dd4f8 100644 --- a/pkg/automationconfig/automation_config.go +++ b/pkg/automationconfig/automation_config.go @@ -327,6 +327,15 @@ type Auth struct { KeyFileWindows string `json:"keyfileWindows,omitempty"` // AutoPwd is a required field when going from `Disabled=false` to `Disabled=true` AutoPwd string `json:"autoPwd,omitempty"` + // UsersDeleted is an array of DeletedUser objects that define the authenticated users to be deleted from specified databases + UsersDeleted []DeletedUser `json:"usersDeleted,omitempty"` +} + +type DeletedUser struct { + // User is the username that should be deleted + User string `json:"user",omitempty` + // Dbs is the array of database names from which the authenticated user should be deleted + Dbs []string `json:"dbs",omitempty` } type Prometheus struct { From 0bddbc3a0c10c177020f427c15a7e7c5abd22586 Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 16 Jul 2024 11:46:55 +0100 Subject: [PATCH 07/17] Add e2e tests for cleaning up users --- test/e2e/mongodbtests/mongodbtests.go | 31 ++++++ .../replica_set_remove_user_test.go | 105 ++++++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 test/e2e/replica_set_remove_user/replica_set_remove_user_test.go diff --git a/test/e2e/mongodbtests/mongodbtests.go b/test/e2e/mongodbtests/mongodbtests.go index d4bfdd010..43ff0670a 100644 --- a/test/e2e/mongodbtests/mongodbtests.go +++ b/test/e2e/mongodbtests/mongodbtests.go @@ -771,3 +771,34 @@ func assertEqualOwnerReference(t *testing.T, resourceType string, resourceNamesp assert.Equal(t, expectedOwnerReference.Name, ownerReferences[0].Name) assert.Equal(t, expectedOwnerReference.UID, ownerReferences[0].UID) } + +func RemoveUserFromResource(ctx context.Context, mdb *mdbv1.MongoDBCommunity) func(*testing.T) { + return func(t *testing.T) { + err := e2eutil.UpdateMongoDBResource(ctx, mdb, func(db *mdbv1.MongoDBCommunity) { + db.Spec.Users = []mdbv1.MongoDBUser{} + }) + + if err != nil { + t.Fatal(err) + } + } +} + +func ConnectionStringSecretsAreCleanedUp(ctx context.Context, mdb *mdbv1.MongoDBCommunity, removedConnectionString string) func(t *testing.T) { + return func(t *testing.T) { + connectionStringSecret := corev1.Secret{} + newErr := e2eutil.TestClient.Get(ctx, types.NamespacedName{Name: removedConnectionString, Namespace: mdb.Namespace}, &connectionStringSecret) + + assert.Error(t, newErr) + } +} + +func AuthUsersDeletedIsUpdated(ctx context.Context, mdb *mdbv1.MongoDBCommunity, mdbUser mdbv1.MongoDBUser) func(t *testing.T) { + return func(t *testing.T) { + deletedUser := automationconfig.DeletedUser{User: mdbUser.Name, Dbs: []string{"admin"}} + + currentAc := getAutomationConfig(ctx, t, mdb) + + assert.Contains(t, currentAc.Auth.UsersDeleted, deletedUser) + } +} diff --git a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go new file mode 100644 index 000000000..cf0351635 --- /dev/null +++ b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go @@ -0,0 +1,105 @@ +package replica_set_remove_user + +import ( + "context" + "fmt" + v1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1" + "github.com/mongodb/mongodb-kubernetes-operator/pkg/automationconfig" + e2eutil "github.com/mongodb/mongodb-kubernetes-operator/test/e2e" + "github.com/mongodb/mongodb-kubernetes-operator/test/e2e/mongodbtests" + "github.com/mongodb/mongodb-kubernetes-operator/test/e2e/setup" + . "github.com/mongodb/mongodb-kubernetes-operator/test/e2e/util/mongotester" + "os" + "testing" +) + +func TestMain(m *testing.M) { + code, err := e2eutil.RunTest(m) + if err != nil { + fmt.Println(err) + } + os.Exit(code) +} + +func intPtr(x int) *int { return &x } +func strPtr(s string) *string { return &s } + +func TestCleanupUsers(t *testing.T) { + ctx := context.Background() + testCtx := setup.Setup(ctx, t) + defer testCtx.Teardown() + + mdb, user := e2eutil.NewTestMongoDB(testCtx, "mdb0", "") + + _, err := setup.GeneratePasswordForUser(testCtx, user, "") + if err != nil { + t.Fatal(err) + } + + lcr := automationconfig.CrdLogRotate{ + // fractional values are supported + SizeThresholdMB: "0.1", + LogRotate: automationconfig.LogRotate{ + TimeThresholdHrs: 1, + NumUncompressed: 10, + NumTotal: 10, + IncludeAuditLogsWithMongoDBLogs: false, + }, + PercentOfDiskspace: "1", + } + + systemLog := automationconfig.SystemLog{ + Destination: automationconfig.File, + Path: "/tmp/mongod.log", + LogAppend: false, + } + + // logRotate can only be configured if systemLog to file has been configured + mdb.Spec.AgentConfiguration.LogRotate = &lcr + mdb.Spec.AgentConfiguration.SystemLog = &systemLog + + // config member options + memberOptions := []automationconfig.MemberOptions{ + { + Votes: intPtr(1), + Tags: map[string]string{"foo1": "bar1"}, + Priority: strPtr("1.5"), + }, + { + Votes: intPtr(1), + Tags: map[string]string{"foo2": "bar2"}, + Priority: strPtr("1"), + }, + { + Votes: intPtr(1), + Tags: map[string]string{"foo3": "bar3"}, + Priority: strPtr("2.5"), + }, + } + mdb.Spec.MemberConfig = memberOptions + + settings := map[string]interface{}{ + "electionTimeoutMillis": float64(20), + } + mdb.Spec.AutomationConfigOverride = &v1.AutomationConfigOverride{ + ReplicaSet: v1.OverrideReplicaSet{Settings: v1.MapWrapper{Object: settings}}, + } + + tester, err := FromResource(ctx, t, mdb) + if err != nil { + t.Fatal(err) + } + + t.Run("Create MongoDB Resource", mongodbtests.CreateMongoDBResource(&mdb, testCtx)) + t.Run("Basic tests", mongodbtests.BasicFunctionality(ctx, &mdb)) + t.Run("Keyfile authentication is configured", tester.HasKeyfileAuth(3)) + t.Run("AutomationConfig has the correct logRotateConfig", mongodbtests.AutomationConfigHasLogRotationConfig(ctx, &mdb, &lcr)) + t.Run("Test Basic Connectivity", tester.ConnectivitySucceeds()) + t.Run("Test SRV Connectivity", tester.ConnectivitySucceeds(WithURI(mdb.MongoSRVURI("")), WithoutTls(), WithReplicaSet(mdb.Name))) + deletedUser := mdb.Spec.Users[0] + t.Run("Delete user from MongoDB Resource", mongodbtests.RemoveUserFromResource(ctx, &mdb)) + t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb)) + t.Run("Removed users are added to automation config", mongodbtests.AuthUsersDeletedIsUpdated(ctx, &mdb, user)) + t.Run("Connection string secrets are cleaned up", mongodbtests.ConnectionStringSecretsAreCleanedUp(ctx, &mdb, deletedUser.GetConnectionStringSecretName(mdb.Name))) + t.Run("Delete MongoDB Resource", mongodbtests.DeleteMongoDBResource(&mdb, testCtx)) +} From 92b4ab1511164495a4c4c3670029b27775cbe4fd Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 16 Jul 2024 12:06:21 +0100 Subject: [PATCH 08/17] Add the connection string secret to the watcher --- controllers/mongodb_users.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/mongodb_users.go b/controllers/mongodb_users.go index 154282810..e9c28f0af 100644 --- a/controllers/mongodb_users.go +++ b/controllers/mongodb_users.go @@ -83,7 +83,7 @@ func (r ReplicaSetReconciler) updateConnectionStringSecrets(ctx context.Context, return err } - secretNamespacedName := types.NamespacedName{Name: user.PasswordSecretName, Namespace: secretNamespace} + secretNamespacedName := types.NamespacedName{Name: connectionStringSecret.Name, Namespace: connectionStringSecret.Namespace} r.secretWatcher.Watch(ctx, secretNamespacedName, mdb.NamespacedName()) } From 9accbc7c792637bbd8b07690cf723ada7e1c5dee Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 16 Jul 2024 13:53:39 +0100 Subject: [PATCH 09/17] Remove Makefile changes --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 61d6670a2..a03dfb9bd 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ AGENT_IMAGE_NAME := $(shell jq -r .agent_image_ubi < $(MONGODB_COMMUNITY_CONFIG) HELM_CHART ?= ./helm-charts/charts/community-operator -STRING_SET_VALUES := --set namespace=$(NAMESPACE),versionUpgradeHook.name=$(UPGRADE_HOOK_IMG),readinessProbe.name=$(READINESS_PROBE_IMG),registry.operator=$(REPO_URL),operator.operatorImageName=$(OPERATOR_IMAGE),operator.version=0.9.0-arm64,registry.agent=$(REGISTRY),registry.versionUpgradeHook=$(REGISTRY),registry.readinessProbe=$(REGISTRY),registry.operator=$(REGISTRY),versionUpgradeHook.version=1.0.8-arm64,readinessProbe.version=1.0.19-arm64,agent.version=13.19.0.8951-1-arm64,agent.name=$(AGENT_IMAGE_NAME) +STRING_SET_VALUES := --set namespace=$(NAMESPACE),versionUpgradeHook.name=$(UPGRADE_HOOK_IMG),readinessProbe.name=$(READINESS_PROBE_IMG),registry.operator=$(REPO_URL),operator.operatorImageName=$(OPERATOR_IMAGE),operator.version=latest,registry.agent=$(REGISTRY),registry.versionUpgradeHook=$(REGISTRY),registry.readinessProbe=$(REGISTRY),registry.operator=$(REGISTRY),versionUpgradeHook.version=latest,readinessProbe.version=latest,agent.version=latest,agent.name=$(AGENT_IMAGE_NAME) STRING_SET_VALUES_LOCAL := $(STRING_SET_VALUES) --set operator.replicas=0 DOCKERFILE ?= operator @@ -134,7 +134,7 @@ install-rbac: $(HELM) template $(STRING_SET_VALUES) -s templates/operator_roles.yaml $(HELM_CHART) | kubectl apply -f - uninstall-crd: - kubectl delete crd --ignore-not-found=true mongodbcommunity.mongodbcommunity.mongodb.com + kubectl delete crd mongodbcommunity.mongodbcommunity.mongodb.com uninstall-chart: $(HELM) uninstall $(RELEASE_NAME_HELM) -n $(NAMESPACE) From 999f2c6d20d887b05ff7d7934884c84efbc4dbf0 Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 16 Jul 2024 14:32:02 +0100 Subject: [PATCH 10/17] Update method name --- test/e2e/mongodbtests/mongodbtests.go | 4 ++-- .../replica_set_remove_user/replica_set_remove_user_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/mongodbtests/mongodbtests.go b/test/e2e/mongodbtests/mongodbtests.go index 43ff0670a..b5faa9b6a 100644 --- a/test/e2e/mongodbtests/mongodbtests.go +++ b/test/e2e/mongodbtests/mongodbtests.go @@ -784,7 +784,7 @@ func RemoveUserFromResource(ctx context.Context, mdb *mdbv1.MongoDBCommunity) fu } } -func ConnectionStringSecretsAreCleanedUp(ctx context.Context, mdb *mdbv1.MongoDBCommunity, removedConnectionString string) func(t *testing.T) { +func ConnectionStringSecretIsCleanedUp(ctx context.Context, mdb *mdbv1.MongoDBCommunity, removedConnectionString string) func(t *testing.T) { return func(t *testing.T) { connectionStringSecret := corev1.Secret{} newErr := e2eutil.TestClient.Get(ctx, types.NamespacedName{Name: removedConnectionString, Namespace: mdb.Namespace}, &connectionStringSecret) @@ -795,7 +795,7 @@ func ConnectionStringSecretsAreCleanedUp(ctx context.Context, mdb *mdbv1.MongoDB func AuthUsersDeletedIsUpdated(ctx context.Context, mdb *mdbv1.MongoDBCommunity, mdbUser mdbv1.MongoDBUser) func(t *testing.T) { return func(t *testing.T) { - deletedUser := automationconfig.DeletedUser{User: mdbUser.Name, Dbs: []string{"admin"}} + deletedUser := automationconfig.DeletedUser{User: mdbUser.Name, Dbs: []string{mdbUser.DB}} currentAc := getAutomationConfig(ctx, t, mdb) diff --git a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go index cf0351635..28796677b 100644 --- a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go +++ b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go @@ -99,7 +99,7 @@ func TestCleanupUsers(t *testing.T) { deletedUser := mdb.Spec.Users[0] t.Run("Delete user from MongoDB Resource", mongodbtests.RemoveUserFromResource(ctx, &mdb)) t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb)) - t.Run("Removed users are added to automation config", mongodbtests.AuthUsersDeletedIsUpdated(ctx, &mdb, user)) - t.Run("Connection string secrets are cleaned up", mongodbtests.ConnectionStringSecretsAreCleanedUp(ctx, &mdb, deletedUser.GetConnectionStringSecretName(mdb.Name))) + t.Run("Removed users are added to automation config", mongodbtests.AuthUsersDeletedIsUpdated(ctx, &mdb, deletedUser)) + t.Run("Connection string secrets are cleaned up", mongodbtests.ConnectionStringSecretIsCleanedUp(ctx, &mdb, deletedUser.GetConnectionStringSecretName(mdb.Name))) t.Run("Delete MongoDB Resource", mongodbtests.DeleteMongoDBResource(&mdb, testCtx)) } From 0995b72dd0f44b64aa61e719dc78067d93756fd9 Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 16 Jul 2024 14:52:38 +0100 Subject: [PATCH 11/17] Fix linting --- pkg/automationconfig/automation_config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/automationconfig/automation_config.go b/pkg/automationconfig/automation_config.go index 6195dd4f8..74697c79d 100644 --- a/pkg/automationconfig/automation_config.go +++ b/pkg/automationconfig/automation_config.go @@ -333,9 +333,9 @@ type Auth struct { type DeletedUser struct { // User is the username that should be deleted - User string `json:"user",omitempty` + User string `json:"user,omitempty"` // Dbs is the array of database names from which the authenticated user should be deleted - Dbs []string `json:"dbs",omitempty` + Dbs []string `json:"dbs,omitempty"` } type Prometheus struct { From 105acba3cd9213c9ee55517a9cf5a83d836ccf7f Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 16 Jul 2024 16:32:46 +0100 Subject: [PATCH 12/17] Add new e2e test to workflow --- .github/workflows/e2e.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 90e2013c6..4f43cf203 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -153,6 +153,8 @@ jobs: distro: ubi - test-name: replica_set_x509 distro: ubi + - test-name: replica_set_remove_user + distro: ubi steps: # template: .action_templates/steps/cancel-previous.yaml - name: Cancel Previous Runs From 9a926d933a8193ad93045f8164a80a0e7a3694b1 Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Thu, 18 Jul 2024 14:52:01 +0100 Subject: [PATCH 13/17] Add new e2e test to github workflow --- .action_templates/jobs/tests.yaml | 2 ++ .github/workflows/e2e-fork.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/.action_templates/jobs/tests.yaml b/.action_templates/jobs/tests.yaml index 6128df9ed..400b47d77 100644 --- a/.action_templates/jobs/tests.yaml +++ b/.action_templates/jobs/tests.yaml @@ -62,3 +62,5 @@ tests: distro: ubi - test-name: replica_set_x509 distro: ubi + - test-name: replica_set_remove_user + distro: ubi diff --git a/.github/workflows/e2e-fork.yml b/.github/workflows/e2e-fork.yml index c5e0f4ef1..e3d465118 100644 --- a/.github/workflows/e2e-fork.yml +++ b/.github/workflows/e2e-fork.yml @@ -147,6 +147,8 @@ jobs: distro: ubi - test-name: replica_set_x509 distro: ubi + - test-name: replica_set_remove_user + distro: ubi steps: # template: .action_templates/steps/cancel-previous.yaml - name: Cancel Previous Runs From 259f2882f1b65bc1f6961456e76a14bcdbd4401a Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Mon, 22 Jul 2024 09:58:58 +0100 Subject: [PATCH 14/17] Fix failing test --- .../replica_set_remove_user/replica_set_remove_user_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go index 28796677b..0fbf86831 100644 --- a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go +++ b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go @@ -98,8 +98,9 @@ func TestCleanupUsers(t *testing.T) { t.Run("Test SRV Connectivity", tester.ConnectivitySucceeds(WithURI(mdb.MongoSRVURI("")), WithoutTls(), WithReplicaSet(mdb.Name))) deletedUser := mdb.Spec.Users[0] t.Run("Delete user from MongoDB Resource", mongodbtests.RemoveUserFromResource(ctx, &mdb)) - t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb)) + t.Run("MongoDB reaches Pending phase", mongodbtests.MongoDBReachesPendingPhase(ctx, &mdb)) t.Run("Removed users are added to automation config", mongodbtests.AuthUsersDeletedIsUpdated(ctx, &mdb, deletedUser)) + t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb)) t.Run("Connection string secrets are cleaned up", mongodbtests.ConnectionStringSecretIsCleanedUp(ctx, &mdb, deletedUser.GetConnectionStringSecretName(mdb.Name))) t.Run("Delete MongoDB Resource", mongodbtests.DeleteMongoDBResource(&mdb, testCtx)) } From 83decf60cac7a0c62bd084225e18d5768916cad6 Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Mon, 22 Jul 2024 15:20:55 +0100 Subject: [PATCH 15/17] Implement suggested changes --- controllers/mongodb_cleanup.go | 41 ++++++++++--------- controllers/mongodb_cleanup_test.go | 16 +++----- pkg/authentication/authentication.go | 9 ++-- pkg/authentication/authentication_test.go | 6 +-- test/e2e/mongodbtests/mongodbtests.go | 3 +- .../replica_set_remove_user_test.go | 2 +- 6 files changed, 39 insertions(+), 38 deletions(-) diff --git a/controllers/mongodb_cleanup.go b/controllers/mongodb_cleanup.go index 6cff0d86d..d13b0426d 100644 --- a/controllers/mongodb_cleanup.go +++ b/controllers/mongodb_cleanup.go @@ -10,12 +10,12 @@ import ( ) // cleanupPemSecret cleans up the old pem secret generated for the agent certificate. -func (r *ReplicaSetReconciler) cleanupPemSecret(ctx context.Context, currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, namespace string) { - if currentMDB.GetAgentAuthMode() == lastAppliedMDBSpec.GetAgentAuthMode() { +func (r *ReplicaSetReconciler) cleanupPemSecret(ctx context.Context, currentMDBSpec mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, namespace string) { + if currentMDBSpec.GetAgentAuthMode() == lastAppliedMDBSpec.GetAgentAuthMode() { return } - if !currentMDB.IsAgentX509() && lastAppliedMDBSpec.IsAgentX509() { + if !currentMDBSpec.IsAgentX509() && lastAppliedMDBSpec.IsAgentX509() { agentCertSecret := lastAppliedMDBSpec.GetAgentCertificateRef() if err := r.client.DeleteSecret(ctx, types.NamespacedName{ Namespace: namespace, @@ -31,15 +31,15 @@ func (r *ReplicaSetReconciler) cleanupPemSecret(ctx context.Context, currentMDB } // cleanupScramSecrets cleans up old scram secrets based on the last successful applied mongodb spec. -func (r *ReplicaSetReconciler) cleanupScramSecrets(ctx context.Context, currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, namespace string) { - secretsToDelete := getScramSecretsToDelete(currentMDB, lastAppliedMDBSpec) +func (r *ReplicaSetReconciler) cleanupScramSecrets(ctx context.Context, currentMDBSpec mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, namespace string) { + secretsToDelete := getScramSecretsToDelete(currentMDBSpec, lastAppliedMDBSpec) for _, s := range secretsToDelete { if err := r.client.DeleteSecret(ctx, types.NamespacedName{ Name: s, Namespace: namespace, }); err != nil { - r.log.Warnf("Could not cleanup old secret %s", s) + r.log.Warnf("Could not cleanup old secret %s: %s", s, err) } else { r.log.Debugf("Sucessfully cleaned up secret: %s", s) } @@ -47,22 +47,22 @@ func (r *ReplicaSetReconciler) cleanupScramSecrets(ctx context.Context, currentM } // cleanupConnectionStringSecrets cleans up old scram secrets based on the last successful applied mongodb spec. -func (r *ReplicaSetReconciler) cleanupConnectionStringSecrets(ctx context.Context, currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, namespace string, resourceName string) { - secretsToDelete := getConnectionStringSecretsToDelete(currentMDB, lastAppliedMDBSpec, resourceName) +func (r *ReplicaSetReconciler) cleanupConnectionStringSecrets(ctx context.Context, currentMDBSpec mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, namespace string, resourceName string) { + secretsToDelete := getConnectionStringSecretsToDelete(currentMDBSpec, lastAppliedMDBSpec, resourceName) for _, s := range secretsToDelete { if err := r.client.DeleteSecret(ctx, types.NamespacedName{ Name: s, Namespace: namespace, }); err != nil { - r.log.Warnf("Could not cleanup old secret %s", s) + r.log.Warnf("Could not cleanup old secret %s: %s", s, err) } else { r.log.Debugf("Sucessfully cleaned up secret: %s", s) } } } -func getScramSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec) []string { +func getScramSecretsToDelete(currentMDBSpec mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec) []string { type user struct { db string name string @@ -70,10 +70,11 @@ func getScramSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedM m := map[user]string{} var secretsToDelete []string - for _, mongoDBUser := range currentMDB.Users { - if mongoDBUser.DB != constants.ExternalDB { - m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] = mongoDBUser.GetScramCredentialsSecretName() + for _, mongoDBUser := range currentMDBSpec.Users { + if mongoDBUser.DB == constants.ExternalDB { + continue } + m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] = mongoDBUser.GetScramCredentialsSecretName() } for _, mongoDBUser := range lastAppliedMDBSpec.Users { @@ -90,7 +91,7 @@ func getScramSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedM return secretsToDelete } -func getConnectionStringSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, resourceName string) []string { +func getConnectionStringSecretsToDelete(currentMDBSpec mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec, resourceName string) []string { type user struct { db string name string @@ -98,10 +99,11 @@ func getConnectionStringSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, l m := map[user]string{} var secretsToDelete []string - for _, mongoDBUser := range currentMDB.Users { - if mongoDBUser.DB != constants.ExternalDB { - m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] = mongoDBUser.GetConnectionStringSecretName(resourceName) + for _, mongoDBUser := range currentMDBSpec.Users { + if mongoDBUser.DB == constants.ExternalDB { + continue } + m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] = mongoDBUser.GetConnectionStringSecretName(resourceName) } for _, mongoDBUser := range lastAppliedMDBSpec.Users { @@ -109,9 +111,10 @@ func getConnectionStringSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, l continue } currentConnectionStringSecretName, ok := m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] - if !ok { // not used anymore + if !ok { // user was removed secretsToDelete = append(secretsToDelete, mongoDBUser.GetConnectionStringSecretName(resourceName)) - } else if currentConnectionStringSecretName != mongoDBUser.GetConnectionStringSecretName(resourceName) { // have changed + } else if currentConnectionStringSecretName != mongoDBUser.GetConnectionStringSecretName(resourceName) { + // this happens when a new ConnectionStringSecretName was set for the old user secretsToDelete = append(secretsToDelete, mongoDBUser.GetConnectionStringSecretName(resourceName)) } } diff --git a/controllers/mongodb_cleanup_test.go b/controllers/mongodb_cleanup_test.go index 267590014..d5dc49914 100644 --- a/controllers/mongodb_cleanup_test.go +++ b/controllers/mongodb_cleanup_test.go @@ -22,8 +22,7 @@ func TestReplicaSetReconcilerCleanupScramSecrets(t *testing.T) { t.Run("no change same resource", func(t *testing.T) { actual := getScramSecretsToDelete(lastApplied.Spec, lastApplied.Spec) - var expected []string - assert.Equal(t, expected, actual) + assert.Equal(t, []string{}, actual) }) t.Run("new user new secret", func(t *testing.T) { @@ -44,10 +43,9 @@ func TestReplicaSetReconcilerCleanupScramSecrets(t *testing.T) { }, ) - var expected []string actual := getScramSecretsToDelete(current.Spec, lastApplied.Spec) - assert.Equal(t, expected, actual) + assert.Equal(t, []string{}, actual) }) t.Run("old user new secret", func(t *testing.T) { @@ -167,11 +165,10 @@ func TestReplicaSetReconcilerCleanupConnectionStringSecrets(t *testing.T) { t.Run("no change same resource", func(t *testing.T) { actual := getConnectionStringSecretsToDelete(lastApplied.Spec, lastApplied.Spec, "my-rs") - var expected []string - assert.Equal(t, expected, actual) + assert.Equal(t, []string{}, actual) }) - t.Run("new user new secret", func(t *testing.T) { + t.Run("new user does not require existing user cleanup", func(t *testing.T) { current := newScramReplicaSet( mdbv1.MongoDBUser{ Name: "testUser", @@ -189,10 +186,9 @@ func TestReplicaSetReconcilerCleanupConnectionStringSecrets(t *testing.T) { }, ) - var expected []string actual := getConnectionStringSecretsToDelete(current.Spec, lastApplied.Spec, "my-rs") - assert.Equal(t, expected, actual) + assert.Equal(t, []string{}, actual) }) t.Run("old user new secret", func(t *testing.T) { @@ -210,7 +206,7 @@ func TestReplicaSetReconcilerCleanupConnectionStringSecrets(t *testing.T) { assert.Equal(t, expected, actual) }) - t.Run("removed one user", func(t *testing.T) { + t.Run("removed one user and changed secret of the other", func(t *testing.T) { lastApplied = newScramReplicaSet( mdbv1.MongoDBUser{ Name: "testUser", diff --git a/pkg/authentication/authentication.go b/pkg/authentication/authentication.go index 0a0041ce5..a856bda66 100644 --- a/pkg/authentication/authentication.go +++ b/pkg/authentication/authentication.go @@ -36,12 +36,12 @@ func Enable(ctx context.Context, auth *automationconfig.Auth, secretGetUpdateCre } func AddRemovedUsers(auth *automationconfig.Auth, mdb mdbv1.MongoDBCommunity, lastAppliedSpec *mdbv1.MongoDBCommunitySpec) { - deletedUsers := getDeletedUsers(mdb.Spec, lastAppliedSpec) + deletedUsers := getRemovedUsersFromSpec(mdb.Spec, lastAppliedSpec) auth.UsersDeleted = append(auth.UsersDeleted, deletedUsers...) } -func getDeletedUsers(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec *mdbv1.MongoDBCommunitySpec) []automationconfig.DeletedUser { +func getRemovedUsersFromSpec(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec *mdbv1.MongoDBCommunitySpec) []automationconfig.DeletedUser { type user struct { db string name string @@ -50,9 +50,10 @@ func getDeletedUsers(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec * var deletedUsers []automationconfig.DeletedUser for _, mongoDBUser := range currentMDB.Users { - if mongoDBUser.DB != constants.ExternalDB { - m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] = true + if mongoDBUser.DB == constants.ExternalDB { + continue } + m[user{db: mongoDBUser.DB, name: mongoDBUser.Name}] = true } for _, mongoDBUser := range lastAppliedMDBSpec.Users { diff --git a/pkg/authentication/authentication_test.go b/pkg/authentication/authentication_test.go index ee9f49995..edfef363d 100644 --- a/pkg/authentication/authentication_test.go +++ b/pkg/authentication/authentication_test.go @@ -157,7 +157,7 @@ func TestGetDeletedUsers(t *testing.T) { } t.Run("no change same resource", func(t *testing.T) { - actual := getDeletedUsers(lastAppliedSpec, &lastAppliedSpec) + actual := getRemovedUsersFromSpec(lastAppliedSpec, &lastAppliedSpec) var expected []automationconfig.DeletedUser assert.Equal(t, expected, actual) @@ -195,7 +195,7 @@ func TestGetDeletedUsers(t *testing.T) { } var expected []automationconfig.DeletedUser - actual := getDeletedUsers(current, &lastAppliedSpec) + actual := getRemovedUsersFromSpec(current, &lastAppliedSpec) assert.Equal(t, expected, actual) }) @@ -220,7 +220,7 @@ func TestGetDeletedUsers(t *testing.T) { Dbs: []string{"admin"}, }, } - actual := getDeletedUsers(current, &lastAppliedSpec) + actual := getRemovedUsersFromSpec(current, &lastAppliedSpec) assert.Equal(t, expected, actual) }) diff --git a/test/e2e/mongodbtests/mongodbtests.go b/test/e2e/mongodbtests/mongodbtests.go index b5faa9b6a..fbe24f21e 100644 --- a/test/e2e/mongodbtests/mongodbtests.go +++ b/test/e2e/mongodbtests/mongodbtests.go @@ -772,7 +772,7 @@ func assertEqualOwnerReference(t *testing.T, resourceType string, resourceNamesp assert.Equal(t, expectedOwnerReference.UID, ownerReferences[0].UID) } -func RemoveUserFromResource(ctx context.Context, mdb *mdbv1.MongoDBCommunity) func(*testing.T) { +func RemoveAllUsersFromResource(ctx context.Context, mdb *mdbv1.MongoDBCommunity) func(*testing.T) { return func(t *testing.T) { err := e2eutil.UpdateMongoDBResource(ctx, mdb, func(db *mdbv1.MongoDBCommunity) { db.Spec.Users = []mdbv1.MongoDBUser{} @@ -789,6 +789,7 @@ func ConnectionStringSecretIsCleanedUp(ctx context.Context, mdb *mdbv1.MongoDBCo connectionStringSecret := corev1.Secret{} newErr := e2eutil.TestClient.Get(ctx, types.NamespacedName{Name: removedConnectionString, Namespace: mdb.Namespace}, &connectionStringSecret) + fmt.Println(newErr) assert.Error(t, newErr) } } diff --git a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go index 0fbf86831..6aac46a82 100644 --- a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go +++ b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go @@ -97,7 +97,7 @@ func TestCleanupUsers(t *testing.T) { t.Run("Test Basic Connectivity", tester.ConnectivitySucceeds()) t.Run("Test SRV Connectivity", tester.ConnectivitySucceeds(WithURI(mdb.MongoSRVURI("")), WithoutTls(), WithReplicaSet(mdb.Name))) deletedUser := mdb.Spec.Users[0] - t.Run("Delete user from MongoDB Resource", mongodbtests.RemoveUserFromResource(ctx, &mdb)) + t.Run("Delete user from MongoDB Resource", mongodbtests.RemoveAllUsersFromResource(ctx, &mdb)) t.Run("MongoDB reaches Pending phase", mongodbtests.MongoDBReachesPendingPhase(ctx, &mdb)) t.Run("Removed users are added to automation config", mongodbtests.AuthUsersDeletedIsUpdated(ctx, &mdb, deletedUser)) t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb)) From 396c8124db30338e29baea0ada1919a9925996bd Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 23 Jul 2024 10:43:12 +0100 Subject: [PATCH 16/17] Update test to check for more complex behavior --- controllers/mongodb_cleanup_test.go | 8 +- test/e2e/mongodbtests/mongodbtests.go | 29 ++++++- .../replica_set_remove_user_test.go | 77 ++++++++++++------- 3 files changed, 79 insertions(+), 35 deletions(-) diff --git a/controllers/mongodb_cleanup_test.go b/controllers/mongodb_cleanup_test.go index d5dc49914..249556061 100644 --- a/controllers/mongodb_cleanup_test.go +++ b/controllers/mongodb_cleanup_test.go @@ -22,7 +22,7 @@ func TestReplicaSetReconcilerCleanupScramSecrets(t *testing.T) { t.Run("no change same resource", func(t *testing.T) { actual := getScramSecretsToDelete(lastApplied.Spec, lastApplied.Spec) - assert.Equal(t, []string{}, actual) + assert.Equal(t, []string(nil), actual) }) t.Run("new user new secret", func(t *testing.T) { @@ -45,7 +45,7 @@ func TestReplicaSetReconcilerCleanupScramSecrets(t *testing.T) { actual := getScramSecretsToDelete(current.Spec, lastApplied.Spec) - assert.Equal(t, []string{}, actual) + assert.Equal(t, []string(nil), actual) }) t.Run("old user new secret", func(t *testing.T) { @@ -165,7 +165,7 @@ func TestReplicaSetReconcilerCleanupConnectionStringSecrets(t *testing.T) { t.Run("no change same resource", func(t *testing.T) { actual := getConnectionStringSecretsToDelete(lastApplied.Spec, lastApplied.Spec, "my-rs") - assert.Equal(t, []string{}, actual) + assert.Equal(t, []string(nil), actual) }) t.Run("new user does not require existing user cleanup", func(t *testing.T) { @@ -188,7 +188,7 @@ func TestReplicaSetReconcilerCleanupConnectionStringSecrets(t *testing.T) { actual := getConnectionStringSecretsToDelete(current.Spec, lastApplied.Spec, "my-rs") - assert.Equal(t, []string{}, actual) + assert.Equal(t, []string(nil), actual) }) t.Run("old user new secret", func(t *testing.T) { diff --git a/test/e2e/mongodbtests/mongodbtests.go b/test/e2e/mongodbtests/mongodbtests.go index fbe24f21e..976d2d4c6 100644 --- a/test/e2e/mongodbtests/mongodbtests.go +++ b/test/e2e/mongodbtests/mongodbtests.go @@ -772,10 +772,22 @@ func assertEqualOwnerReference(t *testing.T, resourceType string, resourceNamesp assert.Equal(t, expectedOwnerReference.UID, ownerReferences[0].UID) } -func RemoveAllUsersFromResource(ctx context.Context, mdb *mdbv1.MongoDBCommunity) func(*testing.T) { +func RemoveLastUserFromMongoDBCommunity(ctx context.Context, mdb *mdbv1.MongoDBCommunity) func(*testing.T) { return func(t *testing.T) { err := e2eutil.UpdateMongoDBResource(ctx, mdb, func(db *mdbv1.MongoDBCommunity) { - db.Spec.Users = []mdbv1.MongoDBUser{} + db.Spec.Users = db.Spec.Users[:len(db.Spec.Users)-1] + }) + + if err != nil { + t.Fatal(err) + } + } +} + +func EditConnectionStringSecretNameOfLastUser(ctx context.Context, mdb *mdbv1.MongoDBCommunity, newSecretName string) func(*testing.T) { + return func(t *testing.T) { + err := e2eutil.UpdateMongoDBResource(ctx, mdb, func(db *mdbv1.MongoDBCommunity) { + db.Spec.Users[len(db.Spec.Users)-1].ConnectionStringSecretName = newSecretName }) if err != nil { @@ -790,7 +802,7 @@ func ConnectionStringSecretIsCleanedUp(ctx context.Context, mdb *mdbv1.MongoDBCo newErr := e2eutil.TestClient.Get(ctx, types.NamespacedName{Name: removedConnectionString, Namespace: mdb.Namespace}, &connectionStringSecret) fmt.Println(newErr) - assert.Error(t, newErr) + assert.EqualError(t, newErr, fmt.Sprintf("secrets \"%s\" not found", removedConnectionString)) } } @@ -803,3 +815,14 @@ func AuthUsersDeletedIsUpdated(ctx context.Context, mdb *mdbv1.MongoDBCommunity, assert.Contains(t, currentAc.Auth.UsersDeleted, deletedUser) } } + +func AddUserToMongoDBCommunity(ctx context.Context, mdb *mdbv1.MongoDBCommunity, newUser mdbv1.MongoDBUser) func(t *testing.T) { + return func(t *testing.T) { + err := e2eutil.UpdateMongoDBResource(ctx, mdb, func(db *mdbv1.MongoDBCommunity) { + db.Spec.Users = append(db.Spec.Users, newUser) + }) + if err != nil { + t.Fatal(err) + } + } +} diff --git a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go index 6aac46a82..2abeb93c3 100644 --- a/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go +++ b/test/e2e/replica_set_remove_user/replica_set_remove_user_test.go @@ -3,7 +3,7 @@ package replica_set_remove_user import ( "context" "fmt" - v1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1" + mdbv1 "github.com/mongodb/mongodb-kubernetes-operator/api/v1" "github.com/mongodb/mongodb-kubernetes-operator/pkg/automationconfig" e2eutil "github.com/mongodb/mongodb-kubernetes-operator/test/e2e" "github.com/mongodb/mongodb-kubernetes-operator/test/e2e/mongodbtests" @@ -36,28 +36,6 @@ func TestCleanupUsers(t *testing.T) { t.Fatal(err) } - lcr := automationconfig.CrdLogRotate{ - // fractional values are supported - SizeThresholdMB: "0.1", - LogRotate: automationconfig.LogRotate{ - TimeThresholdHrs: 1, - NumUncompressed: 10, - NumTotal: 10, - IncludeAuditLogsWithMongoDBLogs: false, - }, - PercentOfDiskspace: "1", - } - - systemLog := automationconfig.SystemLog{ - Destination: automationconfig.File, - Path: "/tmp/mongod.log", - LogAppend: false, - } - - // logRotate can only be configured if systemLog to file has been configured - mdb.Spec.AgentConfiguration.LogRotate = &lcr - mdb.Spec.AgentConfiguration.SystemLog = &systemLog - // config member options memberOptions := []automationconfig.MemberOptions{ { @@ -81,8 +59,46 @@ func TestCleanupUsers(t *testing.T) { settings := map[string]interface{}{ "electionTimeoutMillis": float64(20), } - mdb.Spec.AutomationConfigOverride = &v1.AutomationConfigOverride{ - ReplicaSet: v1.OverrideReplicaSet{Settings: v1.MapWrapper{Object: settings}}, + mdb.Spec.AutomationConfigOverride = &mdbv1.AutomationConfigOverride{ + ReplicaSet: mdbv1.OverrideReplicaSet{Settings: mdbv1.MapWrapper{Object: settings}}, + } + + newUser := mdbv1.MongoDBUser{ + Name: fmt.Sprintf("%s-user-2", "mdb-0"), + PasswordSecretRef: mdbv1.SecretKeyReference{ + Key: fmt.Sprintf("%s-password-2", "mdb-0"), + Name: fmt.Sprintf("%s-%s-password-secret-2", "mdb-0", testCtx.ExecutionId), + }, + Roles: []mdbv1.Role{ + // roles on testing db for general connectivity + { + DB: "testing", + Name: "readWrite", + }, + { + DB: "testing", + Name: "clusterAdmin", + }, + // admin roles for reading FCV + { + DB: "admin", + Name: "readWrite", + }, + { + DB: "admin", + Name: "clusterAdmin", + }, + { + DB: "admin", + Name: "userAdmin", + }, + }, + ScramCredentialsSecretName: fmt.Sprintf("%s-my-scram-2", "mdb-0"), + } + + _, err = setup.GeneratePasswordForUser(testCtx, newUser, "") + if err != nil { + t.Fatal(err) } tester, err := FromResource(ctx, t, mdb) @@ -93,11 +109,16 @@ func TestCleanupUsers(t *testing.T) { t.Run("Create MongoDB Resource", mongodbtests.CreateMongoDBResource(&mdb, testCtx)) t.Run("Basic tests", mongodbtests.BasicFunctionality(ctx, &mdb)) t.Run("Keyfile authentication is configured", tester.HasKeyfileAuth(3)) - t.Run("AutomationConfig has the correct logRotateConfig", mongodbtests.AutomationConfigHasLogRotationConfig(ctx, &mdb, &lcr)) t.Run("Test Basic Connectivity", tester.ConnectivitySucceeds()) t.Run("Test SRV Connectivity", tester.ConnectivitySucceeds(WithURI(mdb.MongoSRVURI("")), WithoutTls(), WithReplicaSet(mdb.Name))) - deletedUser := mdb.Spec.Users[0] - t.Run("Delete user from MongoDB Resource", mongodbtests.RemoveAllUsersFromResource(ctx, &mdb)) + t.Run("Add new user to MongoDB Resource", mongodbtests.AddUserToMongoDBCommunity(ctx, &mdb, newUser)) + t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb)) + editedUser := mdb.Spec.Users[1] + t.Run("Edit connection string secret name of the added user", mongodbtests.EditConnectionStringSecretNameOfLastUser(ctx, &mdb, "other-secret-name")) + t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb)) + t.Run("Old connection string secret is cleaned up", mongodbtests.ConnectionStringSecretIsCleanedUp(ctx, &mdb, editedUser.GetConnectionStringSecretName(mdb.Name))) + deletedUser := mdb.Spec.Users[1] + t.Run("Remove last user from MongoDB Resource", mongodbtests.RemoveLastUserFromMongoDBCommunity(ctx, &mdb)) t.Run("MongoDB reaches Pending phase", mongodbtests.MongoDBReachesPendingPhase(ctx, &mdb)) t.Run("Removed users are added to automation config", mongodbtests.AuthUsersDeletedIsUpdated(ctx, &mdb, deletedUser)) t.Run("MongoDB reaches Running phase", mongodbtests.MongoDBReachesRunningPhase(ctx, &mdb)) From 5aeacf79a2a267b589c99b40fd61d96d0b5ee717 Mon Sep 17 00:00:00 2001 From: Matei Grigore Date: Tue, 23 Jul 2024 12:23:59 +0100 Subject: [PATCH 17/17] Remove lftover debuggign print statement --- test/e2e/mongodbtests/mongodbtests.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e/mongodbtests/mongodbtests.go b/test/e2e/mongodbtests/mongodbtests.go index 976d2d4c6..be14a3c30 100644 --- a/test/e2e/mongodbtests/mongodbtests.go +++ b/test/e2e/mongodbtests/mongodbtests.go @@ -800,8 +800,7 @@ func ConnectionStringSecretIsCleanedUp(ctx context.Context, mdb *mdbv1.MongoDBCo return func(t *testing.T) { connectionStringSecret := corev1.Secret{} newErr := e2eutil.TestClient.Get(ctx, types.NamespacedName{Name: removedConnectionString, Namespace: mdb.Namespace}, &connectionStringSecret) - - fmt.Println(newErr) + assert.EqualError(t, newErr, fmt.Sprintf("secrets \"%s\" not found", removedConnectionString)) } }