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

@@ -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
+}

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)
+	})
+
+}

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.

docs/users.md

@@ -84,6 +84,6 @@ You cannot disable SCRAM authentication.
 
 - To authenticate to your MongoDBCommunity resource, run the following command:
    ```
-   mongo "mongodb://<service-object-name>.<my-namespace>.svc.cluster.local:27017/?replicaSet=<replica-set-name>" --username <username> --password <password> --authenticationDatabase <authentication-database>
+   mongosh "mongodb://<replica-set-name>-svc.<my-namespace>.svc.cluster.local:27017/?replicaSet=<replica-set-name>" --username <username> --password <password> --authenticationDatabase <authentication-database>
    ```
 - 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.

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
+}