CLOUDP-82591: support removing the database users (#151)

Author: Anton

pkg/controller/atlascluster/atlascluster_controller.go

@@ -165,7 +165,7 @@ func (r *AtlasClusterReconciler) Delete(e event.DeleteEvent) error {
 		return fmt.Errorf("cannot delete Atlas cluster: %w", err)
 	}
 
-	log.Infow("Started Atlas cluster deletion process", "projectID", project.Status.ID, "clusterName", cluster.Name)
+	log.Infow("Started Atlas cluster deletion process", "projectID", project.Status.ID, "clusterName", cluster.Spec.Name)
 
 	return nil
 }

pkg/controller/atlasdatabaseuser/atlasdatabaseuser_controller.go

@@ -18,6 +18,8 @@ package atlasdatabaseuser
 
 import (
 	"context"
+	"errors"
+	"fmt"
 
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -30,10 +32,12 @@ import (
 	mdbv1 "github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/status"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/atlas"
+	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/connectionsecret"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/customresource"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/statushandler"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/watch"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/workflow"
+	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/kube"
 )
 
 // AtlasDatabaseUserReconciler reconciles an AtlasDatabaseUser object
@@ -123,5 +127,57 @@ func (r *AtlasDatabaseUserReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func (r AtlasDatabaseUserReconciler) Delete(e event.DeleteEvent) error {
+	dbUser, ok := e.Object.(*mdbv1.AtlasDatabaseUser)
+	if !ok {
+		r.Log.Errorf("Ignoring malformed Delete() call (expected type %T, got %T)", &mdbv1.AtlasDatabaseUser{}, e.Object)
+		return nil
+	}
+
+	log := r.Log.With("atlasdatabaseuser", kube.ObjectKeyFromObject(dbUser))
+
+	log.Infow("-> Starting AtlasDatabaseUser deletion", "spec", dbUser.Spec)
+
+	project := &mdbv1.AtlasProject{}
+	if result := r.readProjectResource(dbUser, project); !result.IsOk() {
+		return errors.New("cannot read project resource")
+	}
+
+	connection, err := atlas.ReadConnection(log, r.Client, r.OperatorPod, project.ConnectionSecretObjectKey())
+	if err != nil {
+		return err
+	}
+
+	atlasClient, err := atlas.Client(r.AtlasDomain, connection, log)
+	if err != nil {
+		return fmt.Errorf("cannot build Atlas client: %w", err)
+	}
+
+	userName := dbUser.Spec.Username
+	_, err = atlasClient.DatabaseUsers.Delete(context.Background(), dbUser.Spec.DatabaseName, project.ID(), userName)
+	if err != nil {
+		return fmt.Errorf("cannot delete Database User in Atlas: %w", err)
+	}
+
+	log.Infow("Started DatabaseUser deletion process in Atlas", "projectID", project.ID(), "userName", userName)
+
+	secrets, err := connectionsecret.ListByUserName(r.Client, dbUser.Namespace, project.ID(), userName)
+	if err != nil {
+		return fmt.Errorf("failed to find connection secrets for the user: %w", err)
+	}
+
+	for _, secret := range secrets {
+		// Solves the "Implicit memory aliasing in for loop" linter error
+		s := secret.DeepCopy()
+		err = r.Client.Delete(context.Background(), s)
+		if err != nil {
+			log.Errorf("Failed to remove connection Secret: %v", err)
+		} else {
+			log.Debugw("Removed connection Secret", "secret", kube.ObjectKeyFromObject(s))
+		}
+	}
+	if len(secrets) > 0 {
+		log.Infof("Removed %d connection secrets", len(secrets))
+	}
+
 	return nil
 }

test/int/dbuser_test.go

@@ -3,6 +3,7 @@ package int
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"net/url"
 	"strings"
 	"time"
@@ -13,6 +14,7 @@ import (
 	"go.mongodb.org/mongo-driver/bson"
 	"go.mongodb.org/mongo-driver/mongo/options"
 	corev1 "k8s.io/api/core/v1"
+	apiErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -61,7 +63,7 @@ var _ = Describe("AtlasDatabaseUser", func() {
 				WithIPAccessList(project.NewIPAccessList().WithIP("0.0.0.0/0"))
 			if DevMode {
 				// While developing tests we need to reuse the same project
-				createdProject.Spec.Name = "dev-test-atlas-project"
+				createdProject.Spec.Name = "dev-test atlas-project"
 			}
 
 			Expect(k8sClient.Create(context.Background(), createdProject)).To(Succeed())
@@ -128,6 +130,10 @@ var _ = Describe("AtlasDatabaseUser", func() {
 		})
 	}
 
+	connSecretname := func(suffix string) string {
+		return kube.NormalizeIdentifier(createdProject.Spec.Name) + suffix
+	}
+
 	Describe("Create/Update the db user", func() {
 		It("Should be created successfully", func() {
 			createdDBUser = mdbv1.DefaultDBUser(namespace.Name, "test-db-user", createdProject.Name).WithPasswordSecret(UserPasswordSecret)
@@ -147,6 +153,12 @@ var _ = Describe("AtlasDatabaseUser", func() {
 					validateSecret(k8sClient, *createdProject, *createdClusterGCP, *createdDBUser)
 					validateSecret(k8sClient, *createdProject, *createdClusterAWS, *createdDBUser)
 					checkNumberOfConnectionSecrets(k8sClient, *createdProject, 2)
+
+					expectedSecretsInStatus := map[string]string{
+						"test-cluster-aws": connSecretname("-test-cluster-aws-test-db-user"),
+						"test-cluster-gcp": connSecretname("-test-cluster-gcp-test-db-user"),
+					}
+					Expect(createdDBUser.Status.ConnectionSecrets).To(Equal(expectedSecretsInStatus))
 				})
 				By("Checking connectivity to Clusters", func() {
 					// The user created lacks read/write roles
@@ -174,6 +186,12 @@ var _ = Describe("AtlasDatabaseUser", func() {
 					validateSecret(k8sClient, *createdProject, *createdClusterGCP, *createdDBUser)
 					validateSecret(k8sClient, *createdProject, *createdClusterAWS, *createdDBUser)
 					checkNumberOfConnectionSecrets(k8sClient, *createdProject, 2)
+
+					expectedSecretsInStatus := map[string]string{
+						"test-cluster-aws": connSecretname("-test-cluster-aws-test-db-user"),
+						"test-cluster-gcp": connSecretname("-test-cluster-gcp-test-db-user"),
+					}
+					Expect(createdDBUser.Status.ConnectionSecrets).To(Equal(expectedSecretsInStatus))
 				})
 
 				By("Checking write permissions for Clusters", func() {
@@ -201,6 +219,8 @@ var _ = Describe("AtlasDatabaseUser", func() {
 					validateSecret(k8sClient, *createdProject, *createdClusterAWS, *createdDBUser)
 					validateSecret(k8sClient, *createdProject, *createdClusterGCP, *secondDBUser)
 					checkNumberOfConnectionSecrets(k8sClient, *createdProject, 3)
+					expectedSecretsInStatus := map[string]string{"test-cluster-gcp": connSecretname("-test-cluster-gcp-second-db-user")}
+					Expect(secondDBUser.Status.ConnectionSecrets).To(Equal(expectedSecretsInStatus))
 				})
 
 				By("Checking write permissions for Clusters", func() {
@@ -216,7 +236,24 @@ var _ = Describe("AtlasDatabaseUser", func() {
 					Expect(err).To(HaveOccurred())
 					Expect(err.Error()).To(MatchRegexp("not authorized"))
 				})
+				By("Removing Second user", func() {
+					Expect(k8sClient.Delete(context.Background(), secondDBUser)).To(Succeed())
+					Eventually(checkAtlasDatabaseUserRemoved(createdProject.Status.ID, *secondDBUser), 50, interval).Should(BeTrue())
+
+					secretNames := []string{connSecretname("-test-cluster-gcp-second-db-user")}
+					Eventually(checkSecretsDontExist(namespace.Name, secretNames), 50, interval).Should(BeTrue())
+				})
 			})
+			By("Removing First user", func() {
+				Expect(k8sClient.Delete(context.Background(), createdDBUser)).To(Succeed())
+				Eventually(checkAtlasDatabaseUserRemoved(createdProject.Status.ID, *createdDBUser), 50, interval).Should(BeTrue())
+
+				secretNames := []string{connSecretname("-test-cluster-aws-test-db-user"), connSecretname("-test-cluster-gcp-test-db-user")}
+				Eventually(checkSecretsDontExist(namespace.Name, secretNames), 50, interval).Should(BeTrue())
+
+				checkNumberOfConnectionSecrets(k8sClient, *createdProject, 0)
+			})
+
 		})
 	})
 })
@@ -344,7 +381,7 @@ func validateSecret(k8sClient client.Client, project mdbv1.AtlasProject, cluster
 
 func checkNumberOfConnectionSecrets(k8sClient client.Client, project mdbv1.AtlasProject, length int) {
 	secretList := corev1.SecretList{}
-	Expect(k8sClient.List(context.Background(), &secretList)).To(Succeed())
+	Expect(k8sClient.List(context.Background(), &secretList, client.InNamespace(namespace.Name))).To(Succeed())
 
 	names := make([]string, 0)
 	for _, item := range secretList.Items {
@@ -360,3 +397,30 @@ func buildConnectionURL(connURL, userName, password string) string {
 	Expect(err).NotTo(HaveOccurred())
 	return u
 }
+
+func checkAtlasDatabaseUserRemoved(projectID string, user mdbv1.AtlasDatabaseUser) func() bool {
+	return func() bool {
+		_, r, err := atlasClient.DatabaseUsers.Get(context.Background(), user.Spec.DatabaseName, projectID, user.Spec.Username)
+		if err != nil {
+			if r != nil && r.StatusCode == http.StatusNotFound {
+				return true
+			}
+		}
+
+		return false
+	}
+}
+
+func checkSecretsDontExist(namespace string, secretNames []string) func() bool {
+	return func() bool {
+		nonExisting := 0
+		for _, name := range secretNames {
+			s := corev1.Secret{}
+			err := k8sClient.Get(context.Background(), kube.ObjectKey(namespace, name), &s)
+			if err != nil && apiErrors.IsNotFound(err) {
+				nonExisting++
+			}
+		}
+		return nonExisting == len(secretNames)
+	}
+}