Fix ClusterExternalSecret FailedNamespaces and ProvisionedNamespaces (#2506)

Signed-off-by: shuheiktgw <s-kitagawa@mercari.com>

pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller.go

@@ -132,12 +132,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	condition := NewClusterExternalSecretCondition(failedNamespaces, &namespaceList)
 	SetClusterExternalSecretCondition(&clusterExternalSecret, *condition)
-	setFailedNamespaces(&clusterExternalSecret, failedNamespaces)
 
-	if len(provisionedNamespaces) > 0 {
-		sort.Strings(provisionedNamespaces)
-		clusterExternalSecret.Status.ProvisionedNamespaces = provisionedNamespaces
-	}
+	clusterExternalSecret.Status.FailedNamespaces = toNamespaceFailures(failedNamespaces)
+	sort.Strings(provisionedNamespaces)
+	clusterExternalSecret.Status.ProvisionedNamespaces = provisionedNamespaces
 
 	return ctrl.Result{RequeueAfter: refreshInt}, nil
 }
@@ -243,30 +241,35 @@ func checkForError(getError error, existingES *metav1.PartialObjectMetadata) str
 }
 
 func getRemovedNamespaces(nsList v1.NamespaceList, provisionedNs []string) []string {
-	result := []string{}
+	var removedNamespaces []string
+
+	nsSet := map[string]struct{}{}
+	for i := range nsList.Items {
+		nsSet[nsList.Items[i].Name] = struct{}{}
+	}
 
 	for _, ns := range provisionedNs {
-		if !ContainsNamespace(nsList, ns) {
-			result = append(result, ns)
+		if _, ok := nsSet[ns]; !ok {
+			removedNamespaces = append(removedNamespaces, ns)
 		}
 	}
 
-	return result
+	return removedNamespaces
 }
 
-func setFailedNamespaces(ces *esv1beta1.ClusterExternalSecret, failedNamespaces map[string]string) {
-	if len(failedNamespaces) == 0 {
-		return
-	}
-
-	ces.Status.FailedNamespaces = []esv1beta1.ClusterExternalSecretNamespaceFailure{}
+func toNamespaceFailures(failedNamespaces map[string]string) []esv1beta1.ClusterExternalSecretNamespaceFailure {
+	namespaceFailures := make([]esv1beta1.ClusterExternalSecretNamespaceFailure, len(failedNamespaces))
 
+	i := 0
 	for namespace, message := range failedNamespaces {
-		ces.Status.FailedNamespaces = append(ces.Status.FailedNamespaces, esv1beta1.ClusterExternalSecretNamespaceFailure{
+		namespaceFailures[i] = esv1beta1.ClusterExternalSecretNamespaceFailure{
 			Namespace: namespace,
 			Reason:    message,
-		})
+		}
+		i++
 	}
+	sort.Slice(namespaceFailures, func(i, j int) bool { return namespaceFailures[i].Namespace < namespaceFailures[j].Namespace })
+	return namespaceFailures
 }
 
 // SetupWithManager sets up the controller with the Manager.

pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller_test.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
+	"sort"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -38,7 +39,7 @@ func init() {
 }
 
 var (
-	timeout  = time.Second * 3
+	timeout  = time.Second * 10
 	interval = time.Millisecond * 250
 )
 
@@ -291,6 +292,67 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 				}
 			},
 		}),
+		Entry("Should crate an external secret and if one with the same name has been deleted", testCase{
+			namespaces: []v1.Namespace{
+				{ObjectMeta: metav1.ObjectMeta{Name: randomNamespaceName()}},
+			},
+			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
+				ces := defaultClusterExternalSecret()
+				ces.Spec.NamespaceSelector.MatchLabels = map[string]string{"kubernetes.io/metadata.name": namespaces[0].Name}
+				return *ces
+			},
+			beforeCheck: func(ctx context.Context, namespaces []v1.Namespace, created esv1beta1.ClusterExternalSecret) {
+				es := &esv1beta1.ExternalSecret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      created.Name,
+						Namespace: namespaces[0].Name,
+					},
+				}
+				Expect(k8sClient.Create(ctx, es)).ShouldNot(HaveOccurred())
+
+				ces := esv1beta1.ClusterExternalSecret{}
+				Eventually(func(g Gomega) {
+					key := types.NamespacedName{Namespace: created.Namespace, Name: created.Name}
+					g.Expect(k8sClient.Get(ctx, key, &ces)).ShouldNot(HaveOccurred())
+					g.Expect(len(ces.Status.FailedNamespaces)).Should(Equal(1))
+				}).WithTimeout(timeout).WithPolling(interval).Should(Succeed())
+
+				Expect(k8sClient.Delete(ctx, es)).ShouldNot(HaveOccurred())
+			},
+			expectedClusterExternalSecret: func(namespaces []v1.Namespace, created esv1beta1.ClusterExternalSecret) esv1beta1.ClusterExternalSecret {
+				return esv1beta1.ClusterExternalSecret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: created.Name,
+					},
+					Spec: created.Spec,
+					Status: esv1beta1.ClusterExternalSecretStatus{
+						ProvisionedNamespaces: []string{namespaces[0].Name},
+						Conditions: []esv1beta1.ClusterExternalSecretStatusCondition{
+							{
+								Type:    esv1beta1.ClusterExternalSecretNotReady,
+								Status:  v1.ConditionTrue,
+								Message: "one or more namespaces failed",
+							},
+							{
+								Type:   esv1beta1.ClusterExternalSecretReady,
+								Status: v1.ConditionTrue,
+							},
+						},
+					},
+				}
+			},
+			expectedExternalSecrets: func(namespaces []v1.Namespace, created esv1beta1.ClusterExternalSecret) []esv1beta1.ExternalSecret {
+				return []esv1beta1.ExternalSecret{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: namespaces[0].Name,
+							Name:      created.Name,
+						},
+						Spec: created.Spec.ExternalSecretSpec,
+					},
+				}
+			},
+		}),
 		Entry("Should delete external secrets when namespaces no longer match", testCase{
 			namespaces: []v1.Namespace{
 				{
@@ -333,7 +395,6 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 					},
 					Spec: created.Spec,
 					Status: esv1beta1.ClusterExternalSecretStatus{
-						ProvisionedNamespaces: []string{namespaces[0].Name, namespaces[1].Name},
 						Conditions: []esv1beta1.ClusterExternalSecretStatusCondition{
 							{
 								Type:   esv1beta1.ClusterExternalSecretReady,
@@ -381,13 +442,15 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 				return *ces
 			},
 			expectedClusterExternalSecret: func(namespaces []v1.Namespace, created esv1beta1.ClusterExternalSecret) esv1beta1.ClusterExternalSecret {
+				provisionedNamespaces := []string{namespaces[0].Name, namespaces[1].Name}
+				sort.Strings(provisionedNamespaces)
 				return esv1beta1.ClusterExternalSecret{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: created.Name,
 					},
 					Spec: created.Spec,
 					Status: esv1beta1.ClusterExternalSecretStatus{
-						ProvisionedNamespaces: []string{namespaces[0].Name, namespaces[1].Name},
+						ProvisionedNamespaces: provisionedNamespaces,
 						Conditions: []esv1beta1.ClusterExternalSecretStatusCondition{
 							{
 								Type:   esv1beta1.ClusterExternalSecretReady,