CLOUDP-277876: Reap orphan connection secrets (#1856)

* CLOUDP-277876: Reap orphan connection secrets

Signed-off-by: jose.vazquez <[email protected]>

* Use debug log

* Reuse local contains func

---------

Signed-off-by: jose.vazquez <[email protected]>

Author: josvazg

pkg/controller/atlasdatabaseuser/atlasdatabaseuser_controller_test.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"reflect"
 	"testing"
-	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -409,7 +408,7 @@ func TestReady(t *testing.T) {
 				},
 			},
 			passwordVersion: "1",
-			expectedResult:  workflow.Requeue(15 * time.Minute).ReconcileResult(),
+			expectedResult:  workflow.Requeue(workflow.StandaloneResourceRequeuePeriod).ReconcileResult(),
 			expectedConditions: []api.Condition{
 				api.TrueCondition(api.ReadyType),
 				api.TrueCondition(api.DatabaseUserReadyType),

pkg/controller/atlasdatabaseuser/databaseuser.go

@@ -188,6 +188,18 @@ func (r *AtlasDatabaseUserReconciler) readiness(ctx *workflow.Context, atlasProj
 		return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.Internal, true, err)
 	}
 
+	removedOrphanSecrets, err := connectionsecret.ReapOrphanConnectionSecrets(
+		ctx.Context, r.Client, atlasProject.ID, atlasDatabaseUser.Namespace, allDeploymentNames)
+	if err != nil {
+		return r.terminate(ctx, atlasDatabaseUser, api.DatabaseUserReadyType, workflow.Internal, true, err)
+	}
+	if len(removedOrphanSecrets) > 0 {
+		r.Log.Debug("Removed %d orphan secrets on project %s bound to an non existent deployment:")
+		for _, orphan := range removedOrphanSecrets {
+			r.Log.Debug("Removed orphan secret %q", orphan)
+		}
+	}
+
 	deploymentsToCheck := allDeploymentNames
 	if atlasDatabaseUser.Spec.Scopes != nil {
 		deploymentsToCheck = filterScopeDeployments(atlasDatabaseUser, allDeploymentNames)

pkg/controller/connectionsecret/connectionsecrets.go

@@ -7,6 +7,8 @@ import (
 
 	"go.mongodb.org/atlas/mongodbatlas"
 	"go.uber.org/zap"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -20,6 +22,35 @@ import (
 
 const ConnectionSecretsEnsuredEvent = "ConnectionSecretsEnsured"
 
+func ReapOrphanConnectionSecrets(ctx context.Context, k8sClient client.Client, projectID, namespace string, projectDeploymentNames []string) ([]string, error) {
+	secretList := &corev1.SecretList{}
+	labelSelector := labels.SelectorFromSet(labels.Set{TypeLabelKey: CredLabelVal, ProjectLabelKey: projectID})
+	err := k8sClient.List(context.Background(), secretList, &client.ListOptions{
+		LabelSelector: labelSelector,
+		Namespace:     namespace,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed listing possible orphan secrets: %w", err)
+	}
+
+	removedOrphanSecrets := []string{}
+	for _, secret := range secretList.Items {
+		clusterName, ok := secret.Labels[ClusterLabelKey]
+		if !ok {
+			continue
+		}
+		if clusterExists := stringutil.Contains(projectDeploymentNames, clusterName); clusterExists {
+			continue
+		}
+		if err := k8sClient.Delete(ctx, &secret); err != nil {
+			return nil, fmt.Errorf("failed to remove orphan connection Secret: %w", err)
+		} else {
+			removedOrphanSecrets = append(removedOrphanSecrets, fmt.Sprintf("%s/%s", namespace, secret.Name))
+		}
+	}
+	return removedOrphanSecrets, nil
+}
+
 func CreateOrUpdateConnectionSecrets(ctx *workflow.Context, k8sClient client.Client, ds deployment.AtlasDeploymentsService, recorder record.EventRecorder, project *project.Project, dbUser akov2.AtlasDatabaseUser) workflow.Result {
 	conns, err := ds.ListDeploymentConnections(ctx.Context, project.ID)
 	if err != nil {

pkg/controller/connectionsecret/connectionsecrets_test.go

@@ -0,0 +1,152 @@
+package connectionsecret_test
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/stretchr/testify/assert"
+
+	akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1"
+	"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/controller/connectionsecret"
+)
+
+const (
+	testProjectID = "123456"
+
+	testNamespace = "some-namespace"
+)
+
+func TestReapOrphanConnectionSecrets(t *testing.T) {
+	scheme := runtime.NewScheme()
+	utilruntime.Must(corev1.AddToScheme(scheme))
+	utilruntime.Must(akov2.AddToScheme(scheme))
+
+	for _, tc := range []struct {
+		title            string
+		deployments      []string
+		objects          []client.Object
+		expectedErr      error
+		expectedRemovals []string
+	}{
+		{
+			title:            "Empty list of secrets returns empty list of removals",
+			expectedRemovals: []string{},
+		},
+
+		{
+			title:            "Matching secrets do not get removed",
+			deployments:      sampleDeployments(),
+			objects:          matchingSecrets(),
+			expectedRemovals: []string{},
+		},
+
+		{
+			title:            "Secrets to non existing clusters get removed",
+			deployments:      sampleDeployments(),
+			objects:          merge(matchingSecrets(), nonMatchingSecrets()),
+			expectedRemovals: namesOf(nonMatchingSecrets()),
+		},
+	} {
+		t.Run(tc.title, func(t *testing.T) {
+			fakeClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(tc.objects...).Build()
+			removedOrphans, err := connectionsecret.ReapOrphanConnectionSecrets(
+				context.Background(),
+				fakeClient,
+				testProjectID,
+				testNamespace,
+				tc.deployments,
+			)
+			assert.Equal(t, tc.expectedErr, err)
+			assert.Equal(t, tc.expectedRemovals, removedOrphans)
+		})
+	}
+}
+
+func sampleDeployments() []string {
+	return []string{"cluster1", "serverless2"}
+}
+
+func matchingSecrets() []client.Object {
+	return []client.Object{
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "secret1",
+				Namespace: testNamespace,
+				Labels: map[string]string{
+					connectionsecret.ClusterLabelKey: "cluster1",
+					connectionsecret.ProjectLabelKey: testProjectID,
+					connectionsecret.TypeLabelKey:    connectionsecret.CredLabelVal,
+				},
+			},
+		},
+
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "secret2",
+				Namespace: testNamespace,
+				Labels: map[string]string{
+					connectionsecret.ClusterLabelKey: "serverless2",
+					connectionsecret.ProjectLabelKey: testProjectID,
+					connectionsecret.TypeLabelKey:    connectionsecret.CredLabelVal,
+				},
+			},
+		},
+	}
+}
+
+func nonMatchingSecrets() []client.Object {
+	return []client.Object{
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "secret3",
+				Namespace: testNamespace,
+				Labels: map[string]string{
+					connectionsecret.ClusterLabelKey: "cluster3",
+					connectionsecret.ProjectLabelKey: testProjectID,
+					connectionsecret.TypeLabelKey:    connectionsecret.CredLabelVal,
+				},
+			},
+		},
+
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "secret4",
+				Namespace: testNamespace,
+				Labels: map[string]string{
+					connectionsecret.ClusterLabelKey: "serverless4",
+					connectionsecret.ProjectLabelKey: testProjectID,
+					connectionsecret.TypeLabelKey:    connectionsecret.CredLabelVal,
+				},
+			},
+		},
+	}
+}
+
+func namesOf(objs []client.Object) []string {
+	names := make([]string, 0, len(objs))
+	for _, obj := range objs {
+		names = append(names, fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName()))
+	}
+	return names
+}
+
+func merge(objs ...[]client.Object) []client.Object {
+	if len(objs) == 0 {
+		return []client.Object{}
+	}
+	result := objs[0]
+	for i := 1; i < len(objs); i++ {
+		result = append(result, objs[i]...)
+	}
+	return result
+}