fix: correctly convert matchExpressions to labelSelector (#1165)

Fixes #1155

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller.go

@@ -22,7 +22,6 @@ import (
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -43,16 +42,16 @@ type Reconciler struct {
 }
 
 const (
-	errGetCES              = "could not get ClusterExternalSecret"
-	errPatchStatus         = "unable to patch status"
-	errLabelMap            = "unable to get map from labels"
-	errNamespaces          = "could not get namespaces from selector"
-	errGetExistingES       = "could not get existing ExternalSecret"
-	errCreatingOrUpdating  = "could not create or update ExternalSecret"
-	errSetCtrlReference    = "could not set the controller owner reference"
-	errSecretAlreadyExists = "external secret already exists in namespace"
-	errNamespacesFailed    = "one or more namespaces failed"
-	errFailedToDelete      = "external secret in non matching namespace could not be deleted"
+	errGetCES               = "could not get ClusterExternalSecret"
+	errPatchStatus          = "unable to patch status"
+	errConvertLabelSelector = "unable to convert labelselector"
+	errNamespaces           = "could not get namespaces from selector"
+	errGetExistingES        = "could not get existing ExternalSecret"
+	errCreatingOrUpdating   = "could not create or update ExternalSecret"
+	errSetCtrlReference     = "could not set the controller owner reference"
+	errSecretAlreadyExists  = "external secret already exists in namespace"
+	errNamespacesFailed     = "one or more namespaces failed"
+	errFailedToDelete       = "external secret in non matching namespace could not be deleted"
 )
 
 func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
@@ -76,15 +75,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		refreshInt = clusterExternalSecret.Spec.RefreshInterval.Duration
 	}
 
-	labelMap, err := metav1.LabelSelectorAsMap(&clusterExternalSecret.Spec.NamespaceSelector)
+	labelSelector, err := metav1.LabelSelectorAsSelector(&clusterExternalSecret.Spec.NamespaceSelector)
 	if err != nil {
-		log.Error(err, errLabelMap)
+		log.Error(err, errConvertLabelSelector)
 		return ctrl.Result{RequeueAfter: refreshInt}, err
 	}
 
 	namespaceList := v1.NamespaceList{}
-
-	err = r.List(ctx, &namespaceList, &client.ListOptions{LabelSelector: labels.SelectorFromSet(labelMap)})
+	err = r.List(ctx, &namespaceList, &client.ListOptions{LabelSelector: labelSelector})
 	if err != nil {
 		log.Error(err, errNamespaces)
 		return ctrl.Result{RequeueAfter: refreshInt}, err

pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller_test.go

@@ -253,6 +253,46 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 		}
 	}
 
+	syncWithMatchExpressions := func(tc *testCase) {
+		tc.setup = func(tc *testCase) {
+			prefixes := []string{"foo", "bar", "baz"}
+			for _, prefix := range prefixes {
+				labels := map[string]string{
+					"e2e":    "with-label-selector",
+					"prefix": prefix,
+				}
+				ns, err := ctest.CreateNamespaceWithLabels(prefix, k8sClient, labels)
+				Expect(err).ToNot(HaveOccurred())
+				tc.externalSecretNamespaces = append(tc.externalSecretNamespaces, testNamespace{
+					namespace: v1.Namespace{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: ns,
+						},
+					},
+					containsES: true,
+				})
+			}
+			tc.clusterExternalSecret.Spec.NamespaceSelector.MatchExpressions = []metav1.LabelSelectorRequirement{
+				{
+					Key:      "prefix",
+					Operator: metav1.LabelSelectorOpIn,
+					Values:   prefixes,
+				},
+			}
+		}
+		tc.checkClusterExternalSecret = func(ces *esv1beta1.ClusterExternalSecret) {
+			for _, namespace := range tc.externalSecretNamespaces {
+				var es esv1beta1.ExternalSecret
+				err := k8sClient.Get(context.Background(), types.NamespacedName{
+					Namespace: namespace.namespace.Name,
+					Name:      ExternalSecretName,
+				}, &es)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(sliceContainsString(namespace.namespace.Name, ces.Status.ProvisionedNamespaces)).To(BeTrue())
+			}
+		}
+	}
+
 	DescribeTable("When reconciling a ClusterExternal Secret",
 		func(tweaks ...testTweaks) {
 			tc := makeDefaultTestCase()
@@ -326,7 +366,8 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 		Entry("Should use cluster external secret name if external secret name isn't defined", syncWithoutESName),
 		Entry("Should not overwrite existing external secrets and error out if one is present", doNotOverwriteExistingES),
 		Entry("Should have list of all provisioned namespaces", populatedProvisionedNamespaces),
-		Entry("Should delete external secrets when namespaces no longer match", deleteESInNonMatchingNS))
+		Entry("Should delete external secrets when namespaces no longer match", deleteESInNonMatchingNS),
+		Entry("Should sync with label selector", syncWithMatchExpressions))
 })
 
 func sliceContainsString(toFind string, collection []string) bool {