CLOUDP-196371: Delete users removed from the resource

.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

.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

.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

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,33 +31,50 @@ 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)
 		}
 	}
 }
 
-func getScramSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec) []string {
+// cleanupConnectionStringSecrets cleans up old scram secrets based on the last successful applied mongodb spec.
+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", s, err)
+		} else {
+			r.log.Debugf("Sucessfully cleaned up secret: %s", s)
+		}
+	}
+}
+
+func getScramSecretsToDelete(currentMDBSpec mdbv1.MongoDBCommunitySpec, lastAppliedMDBSpec mdbv1.MongoDBCommunitySpec) []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.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 {
@@ -73,3 +90,33 @@ func getScramSecretsToDelete(currentMDB mdbv1.MongoDBCommunitySpec, lastAppliedM
 	}
 	return secretsToDelete
 }
+
+func getConnectionStringSecretsToDelete(currentMDBSpec 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 currentMDBSpec.Users {
+		if mongoDBUser.DB == constants.ExternalDB {
+			continue
+		}
+		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 { // user was removed
+			secretsToDelete = append(secretsToDelete, mongoDBUser.GetConnectionStringSecretName(resourceName))
+		} else if currentConnectionStringSecretName != mongoDBUser.GetConnectionStringSecretName(resourceName) {
+			// this happens when a new ConnectionStringSecretName was set for the old user
+			secretsToDelete = append(secretsToDelete, mongoDBUser.GetConnectionStringSecretName(resourceName))
+		}
+	}
+	return secretsToDelete
+}

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(nil), 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(nil), actual)
 	})
 
 	t.Run("old user new secret", func(t *testing.T) {
@@ -154,3 +152,90 @@ 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")
+
+		assert.Equal(t, []string(nil), actual)
+	})
+
+	t.Run("new user does not require existing user cleanup", 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",
+			},
+		)
+
+		actual := getConnectionStringSecretsToDelete(current.Spec, lastApplied.Spec, "my-rs")
+
+		assert.Equal(t, []string(nil), 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 and changed secret of the other", 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)
+	})
+
+}

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: connectionStringSecret.Name, Namespace: connectionStringSecret.Namespace}
+		r.secretWatcher.Watch(ctx, secretNamespacedName, mdb.NamespacedName())
 	}
 
 	return nil

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,15 +332,15 @@ 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())
 	if err != nil && !apiErrors.IsNotFound(err) {
 		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}

docs/secure.md

@@ -36,20 +36,21 @@ 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 `<namespace>`
 
    ```
    helm upgrade --install community-operator mongodb/community-operator \
-   --namespace mongodb --set resource.tls.useCertManager=true \
+   --namespace <namespace> --set resource.tls.useCertManager=true \
    --set createResource=true --set resource.tls.enabled=true \
-   --set namespace=mongodb --create-namespace
+   --set namespace=<namespace>
    ```
 
   This creates a resource secured with TLS and generates the necessary
@@ -72,21 +73,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 <mongodb-replica-set-pod> -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 <mongodb-replica-set>.<mongodb-replica-set>-svc.<namespace>.svc.cluster.local
+   mongosh "<connection-string>" --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.
+   resource, `namespace` is the namespace of your deployment
+   and  `connection-string` is a connection string for your `<mongodb-replica-set>-svc` service.