Add NamespaceSelectors field to ClusterExternalSecret (#3268)

https://github.com/external-secrets/external-secrets/issues/3257

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

apis/externalsecrets/v1beta1/clusterexternalsecret_types.go

@@ -33,9 +33,14 @@ type ClusterExternalSecretSpec struct {
 	ExternalSecretMetadata ExternalSecretMetadata `json:"externalSecretMetadata,omitempty"`
 
 	// The labels to select by to find the Namespaces to create the ExternalSecrets in.
+	// Deprecated: Use NamespaceSelectors instead.
 	// +optional
 	NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"`
 
+	// A list of labels to select by to find the Namespaces to create the ExternalSecrets in. The selectors are ORed.
+	// +optional
+	NamespaceSelectors []*metav1.LabelSelector `json:"namespaceSelectors,omitempty"`
+
 	// Choose namespaces by name. This field is ORed with anything that NamespaceSelector ends up choosing.
 	// +optional
 	Namespaces []string `json:"namespaces,omitempty"`

apis/externalsecrets/v1beta1/zz_generated.deepcopy.go

@@ -554,6 +554,17 @@ func (in *ClusterExternalSecretSpec) DeepCopyInto(out *ClusterExternalSecretSpec
 		*out = new(v1.LabelSelector)
 		(*in).DeepCopyInto(*out)
 	}
+	if in.NamespaceSelectors != nil {
+		in, out := &in.NamespaceSelectors, &out.NamespaceSelectors
+		*out = make([]*v1.LabelSelector, len(*in))
+		for i := range *in {
+			if (*in)[i] != nil {
+				in, out := &(*in)[i], &(*out)[i]
+				*out = new(v1.LabelSelector)
+				(*in).DeepCopyInto(*out)
+			}
+		}
+	}
 	if in.Namespaces != nil {
 		in, out := &in.Namespaces, &out.Namespaces
 		*out = make([]string, len(*in))

config/crds/bases/external-secrets.io_clusterexternalsecrets.yaml

@@ -517,8 +517,9 @@ spec:
                     type: object
                 type: object
               namespaceSelector:
-                description: The labels to select by to find the Namespaces to create
-                  the ExternalSecrets in.
+                description: |-
+                  The labels to select by to find the Namespaces to create the ExternalSecrets in.
+                  Deprecated: Use NamespaceSelectors instead.
                 properties:
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.
@@ -561,6 +562,57 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
+              namespaceSelectors:
+                description: A list of labels to select by to find the Namespaces
+                  to create the ExternalSecrets in. The selectors are ORed.
+                items:
+                  description: |-
+                    A label selector is a label query over a set of resources. The result of matchLabels and
+                    matchExpressions are ANDed. An empty label selector matches all objects. A null
+                    label selector matches no objects.
+                  properties:
+                    matchExpressions:
+                      description: matchExpressions is a list of label selector requirements.
+                        The requirements are ANDed.
+                      items:
+                        description: |-
+                          A label selector requirement is a selector that contains values, a key, and an operator that
+                          relates the key and values.
+                        properties:
+                          key:
+                            description: key is the label key that the selector applies
+                              to.
+                            type: string
+                          operator:
+                            description: |-
+                              operator represents a key's relationship to a set of values.
+                              Valid operators are In, NotIn, Exists and DoesNotExist.
+                            type: string
+                          values:
+                            description: |-
+                              values is an array of string values. If the operator is In or NotIn,
+                              the values array must be non-empty. If the operator is Exists or DoesNotExist,
+                              the values array must be empty. This array is replaced during a strategic
+                              merge patch.
+                            items:
+                              type: string
+                            type: array
+                        required:
+                        - key
+                        - operator
+                        type: object
+                      type: array
+                    matchLabels:
+                      additionalProperties:
+                        type: string
+                      description: |-
+                        matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels
+                        map is equivalent to an element of matchExpressions, whose key field is "key", the
+                        operator is "In", and the values array contains only "value". The requirements are ANDed.
+                      type: object
+                  type: object
+                  x-kubernetes-map-type: atomic
+                type: array
               namespaces:
                 description: Choose namespaces by name. This field is ORed with anything
                   that NamespaceSelector ends up choosing.

deploy/crds/bundle.yaml

@@ -491,7 +491,9 @@ spec:
                       type: object
                   type: object
                 namespaceSelector:
-                  description: The labels to select by to find the Namespaces to create the ExternalSecrets in.
+                  description: |-
+                    The labels to select by to find the Namespaces to create the ExternalSecrets in.
+                    Deprecated: Use NamespaceSelectors instead.
                   properties:
                     matchExpressions:
                       description: matchExpressions is a list of label selector requirements. The requirements are ANDed.
@@ -532,6 +534,54 @@ spec:
                       type: object
                   type: object
                   x-kubernetes-map-type: atomic
+                namespaceSelectors:
+                  description: A list of labels to select by to find the Namespaces to create the ExternalSecrets in. The selectors are ORed.
+                  items:
+                    description: |-
+                      A label selector is a label query over a set of resources. The result of matchLabels and
+                      matchExpressions are ANDed. An empty label selector matches all objects. A null
+                      label selector matches no objects.
+                    properties:
+                      matchExpressions:
+                        description: matchExpressions is a list of label selector requirements. The requirements are ANDed.
+                        items:
+                          description: |-
+                            A label selector requirement is a selector that contains values, a key, and an operator that
+                            relates the key and values.
+                          properties:
+                            key:
+                              description: key is the label key that the selector applies to.
+                              type: string
+                            operator:
+                              description: |-
+                                operator represents a key's relationship to a set of values.
+                                Valid operators are In, NotIn, Exists and DoesNotExist.
+                              type: string
+                            values:
+                              description: |-
+                                values is an array of string values. If the operator is In or NotIn,
+                                the values array must be non-empty. If the operator is Exists or DoesNotExist,
+                                the values array must be empty. This array is replaced during a strategic
+                                merge patch.
+                              items:
+                                type: string
+                              type: array
+                          required:
+                            - key
+                            - operator
+                          type: object
+                        type: array
+                      matchLabels:
+                        additionalProperties:
+                          type: string
+                        description: |-
+                          matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels
+                          map is equivalent to an element of matchExpressions, whose key field is "key", the
+                          operator is "In", and the values array contains only "value". The requirements are ANDed.
+                        type: object
+                    type: object
+                    x-kubernetes-map-type: atomic
+                  type: array
                 namespaces:
                   description: Choose namespaces by name. This field is ORed with anything that NamespaceSelector ends up choosing.
                   items:

docs/api/spec.md

@@ -1315,7 +1315,22 @@ Kubernetes meta/v1.LabelSelector
 </td>
 <td>
 <em>(Optional)</em>
-<p>The labels to select by to find the Namespaces to create the ExternalSecrets in.</p>
+<p>The labels to select by to find the Namespaces to create the ExternalSecrets in.
+Deprecated: Use NamespaceSelectors instead.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>namespaceSelectors</code></br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#*k8s.io/apimachinery/pkg/apis/meta/v1.labelselector--">
+[]*k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>A list of labels to select by to find the Namespaces to create the ExternalSecrets in. The selectors are ORed.</p>
 </td>
 </tr>
 <tr>
@@ -1488,7 +1503,22 @@ Kubernetes meta/v1.LabelSelector
 </td>
 <td>
 <em>(Optional)</em>
-<p>The labels to select by to find the Namespaces to create the ExternalSecrets in.</p>
+<p>The labels to select by to find the Namespaces to create the ExternalSecrets in.
+Deprecated: Use NamespaceSelectors instead.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>namespaceSelectors</code></br>
+<em>
+<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#*k8s.io/apimachinery/pkg/apis/meta/v1.labelselector--">
+[]*k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>A list of labels to select by to find the Namespaces to create the ExternalSecrets in. The selectors are ORed.</p>
 </td>
 </tr>
 <tr>

pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller.go

@@ -56,7 +56,6 @@ const (
 	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"
 	errNamespacesFailed     = "one or more namespaces failed"
 )
@@ -96,47 +95,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		refreshInt = clusterExternalSecret.Spec.RefreshInterval.Duration
 	}
 
-	namespaceList := v1.NamespaceList{}
-
-	if clusterExternalSecret.Spec.NamespaceSelector != nil {
-		labelSelector, err := metav1.LabelSelectorAsSelector(clusterExternalSecret.Spec.NamespaceSelector)
-		if err != nil {
-			log.Error(err, errConvertLabelSelector)
-			return ctrl.Result{}, err
-		}
-
-		err = r.List(ctx, &namespaceList, &client.ListOptions{LabelSelector: labelSelector})
-		if err != nil {
-			log.Error(err, errNamespaces)
-			return ctrl.Result{}, err
-		}
-	}
-
-	if len(clusterExternalSecret.Spec.Namespaces) > 0 {
-		var additionalNamespace []v1.Namespace
-
-		for _, ns := range clusterExternalSecret.Spec.Namespaces {
-			namespace := &v1.Namespace{}
-			if err = r.Get(ctx, types.NamespacedName{Name: ns}, namespace); err != nil {
-				if apierrors.IsNotFound(err) {
-					continue
-				}
-
-				log.Error(err, errNamespaces)
-				return ctrl.Result{}, err
-			}
-
-			additionalNamespace = append(additionalNamespace, *namespace)
-		}
-
-		namespaceList.Items = append(namespaceList.Items, additionalNamespace...)
-	}
-
 	esName := clusterExternalSecret.Spec.ExternalSecretName
 	if esName == "" {
 		esName = clusterExternalSecret.ObjectMeta.Name
 	}
-
 	if prevName := clusterExternalSecret.Status.ExternalSecretName; prevName != esName {
 		// ExternalSecretName has changed, so remove the old ones
 		for _, ns := range clusterExternalSecret.Status.ProvisionedNamespaces {
@@ -146,13 +108,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			}
 		}
 	}
-
 	clusterExternalSecret.Status.ExternalSecretName = esName
 
-	failedNamespaces := r.deleteOutdatedExternalSecrets(ctx, namespaceList, esName, clusterExternalSecret.Name, clusterExternalSecret.Status.ProvisionedNamespaces)
+	namespaces, err := r.getTargetNamespaces(ctx, &clusterExternalSecret)
+	if err != nil {
+		log.Error(err, "failed to get target Namespaces")
+		return ctrl.Result{}, err
+	}
+
+	failedNamespaces := r.deleteOutdatedExternalSecrets(ctx, namespaces, esName, clusterExternalSecret.Name, clusterExternalSecret.Status.ProvisionedNamespaces)
 
 	provisionedNamespaces := []string{}
-	for _, namespace := range namespaceList.Items {
+	for _, namespace := range namespaces {
 		var existingES esv1beta1.ExternalSecret
 		err = r.Get(ctx, types.NamespacedName{
 			Name:      esName,
@@ -188,6 +155,46 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	return ctrl.Result{RequeueAfter: refreshInt}, nil
 }
 
+func (r *Reconciler) getTargetNamespaces(ctx context.Context, ces *esv1beta1.ClusterExternalSecret) ([]v1.Namespace, error) {
+	selectors := []*metav1.LabelSelector{}
+	if s := ces.Spec.NamespaceSelector; s != nil {
+		selectors = append(selectors, s)
+	}
+	for _, ns := range ces.Spec.Namespaces {
+		selectors = append(selectors, &metav1.LabelSelector{
+			MatchLabels: map[string]string{
+				"kubernetes.io/metadata.name": ns,
+			},
+		})
+	}
+	selectors = append(selectors, ces.Spec.NamespaceSelectors...)
+
+	var namespaces []v1.Namespace
+	namespaceSet := make(map[string]struct{})
+	for _, selector := range selectors {
+		labelSelector, err := metav1.LabelSelectorAsSelector(selector)
+		if err != nil {
+			return nil, fmt.Errorf("failed to convert label selector %s: %w", selector, err)
+		}
+
+		var nl v1.NamespaceList
+		err = r.List(ctx, &nl, &client.ListOptions{LabelSelector: labelSelector})
+		if err != nil {
+			return nil, fmt.Errorf("failed to list namespaces by label selector %s: %w", selector, err)
+		}
+
+		for _, n := range nl.Items {
+			if _, exist := namespaceSet[n.Name]; exist {
+				continue
+			}
+			namespaceSet[n.Name] = struct{}{}
+			namespaces = append(namespaces, n)
+		}
+	}
+
+	return namespaces, nil
+}
+
 func (r *Reconciler) createOrUpdateExternalSecret(ctx context.Context, clusterExternalSecret *esv1beta1.ClusterExternalSecret, namespace v1.Namespace, esName string, esMetadata esv1beta1.ExternalSecretMetadata) error {
 	externalSecret := &esv1beta1.ExternalSecret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -247,10 +254,10 @@ func (r *Reconciler) deferPatch(ctx context.Context, log logr.Logger, clusterExt
 	}
 }
 
-func (r *Reconciler) deleteOutdatedExternalSecrets(ctx context.Context, namespaceList v1.NamespaceList, esName, cesName string, provisionedNamespaces []string) map[string]error {
+func (r *Reconciler) deleteOutdatedExternalSecrets(ctx context.Context, namespaces []v1.Namespace, esName, cesName string, provisionedNamespaces []string) map[string]error {
 	failedNamespaces := map[string]error{}
 	// Loop through existing namespaces first to make sure they still have our labels
-	for _, namespace := range getRemovedNamespaces(namespaceList, provisionedNamespaces) {
+	for _, namespace := range getRemovedNamespaces(namespaces, provisionedNamespaces) {
 		err := r.deleteExternalSecret(ctx, esName, cesName, namespace)
 		if err != nil {
 			r.Log.Error(err, "unable to delete external secret")
@@ -266,10 +273,10 @@ func isExternalSecretOwnedBy(es *esv1beta1.ExternalSecret, cesName string) bool
 	return owner != nil && owner.APIVersion == esv1beta1.SchemeGroupVersion.String() && owner.Kind == esv1beta1.ClusterExtSecretKind && owner.Name == cesName
 }
 
-func getRemovedNamespaces(currentNSs v1.NamespaceList, provisionedNSs []string) []string {
+func getRemovedNamespaces(currentNSs []v1.Namespace, provisionedNSs []string) []string {
 	currentNSSet := map[string]struct{}{}
-	for i := range currentNSs.Items {
-		currentNSSet[currentNSs.Items[i].Name] = struct{}{}
+	for _, currentNs := range currentNSs {
+		currentNSSet[currentNs.Name] = struct{}{}
 	}
 
 	var removedNSs []string
@@ -321,11 +328,18 @@ func (r *Reconciler) findObjectsForNamespace(ctx context.Context, namespace clie
 	var requests []reconcile.Request
 	for i := range clusterExternalSecrets.Items {
 		clusterExternalSecret := &clusterExternalSecrets.Items[i]
-		if clusterExternalSecret.Spec.NamespaceSelector != nil {
-			labelSelector, err := metav1.LabelSelectorAsSelector(clusterExternalSecret.Spec.NamespaceSelector)
+		var selectors []*metav1.LabelSelector
+		if s := clusterExternalSecret.Spec.NamespaceSelector; s != nil {
+			selectors = append(selectors, s)
+		}
+		selectors = append(selectors, clusterExternalSecret.Spec.NamespaceSelectors...)
+
+		var selected bool
+		for _, selector := range selectors {
+			labelSelector, err := metav1.LabelSelectorAsSelector(selector)
 			if err != nil {
 				r.Log.Error(err, errConvertLabelSelector)
-				return []reconcile.Request{}
+				continue
 			}
 
 			if labelSelector.Matches(labels.Set(namespace.GetLabels())) {
@@ -335,22 +349,24 @@ func (r *Reconciler) findObjectsForNamespace(ctx context.Context, namespace clie
 						Namespace: clusterExternalSecret.GetNamespace(),
 					},
 				})
-
-				// Prevent the object from being added twice if it happens to be listed
-				// by Namespaces selector as well.
-				continue
+				selected = true
+				break
 			}
 		}
 
-		if len(clusterExternalSecret.Spec.Namespaces) > 0 {
-			if slices.Contains(clusterExternalSecret.Spec.Namespaces, namespace.GetName()) {
-				requests = append(requests, reconcile.Request{
-					NamespacedName: types.NamespacedName{
-						Name:      clusterExternalSecret.GetName(),
-						Namespace: clusterExternalSecret.GetNamespace(),
-					},
-				})
-			}
+		// Prevent the object from being added twice if it happens to be listed
+		// by Namespaces selector as well.
+		if selected {
+			continue
+		}
+
+		if slices.Contains(clusterExternalSecret.Spec.Namespaces, namespace.GetName()) {
+			requests = append(requests, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      clusterExternalSecret.GetName(),
+					Namespace: clusterExternalSecret.GetNamespace(),
+				},
+			})
 		}
 	}
 

pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller_test.go

@@ -670,7 +670,86 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 				return []esv1beta1.ExternalSecret{}
 			},
 		}),
-		Entry("Should be ready if namespace is selected via the namespace selector", testCase{
+		Entry("Should be ready if namespace is selected via the namespace selectors", testCase{
+			namespaces: []v1.Namespace{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "namespace1",
+						Labels: map[string]string{
+							"key": "value1",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "namespace2",
+						Labels: map[string]string{
+							"key": "value2",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "namespace3",
+						Labels: map[string]string{
+							"key": "value3",
+						},
+					},
+				},
+			},
+			clusterExternalSecret: func(namespaces []v1.Namespace) esv1beta1.ClusterExternalSecret {
+				ces := defaultClusterExternalSecret()
+				ces.Spec.NamespaceSelectors = []*metav1.LabelSelector{
+					{
+						MatchLabels: map[string]string{"key": "value1"},
+					},
+					{
+						MatchLabels: map[string]string{"key": "value2"},
+					},
+				}
+				return *ces
+			},
+			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{
+						ExternalSecretName: created.Name,
+						ProvisionedNamespaces: []string{
+							"namespace1",
+							"namespace2",
+						},
+						Conditions: []esv1beta1.ClusterExternalSecretStatusCondition{
+							{
+								Type:   esv1beta1.ClusterExternalSecretReady,
+								Status: v1.ConditionTrue,
+							},
+						},
+					},
+				}
+			},
+			expectedExternalSecrets: func(namespaces []v1.Namespace, created esv1beta1.ClusterExternalSecret) []esv1beta1.ExternalSecret {
+				return []esv1beta1.ExternalSecret{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: "namespace1",
+							Name:      created.Name,
+						},
+						Spec: created.Spec.ExternalSecretSpec,
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: "namespace2",
+							Name:      created.Name,
+						},
+						Spec: created.Spec.ExternalSecretSpec,
+					},
+				}
+			},
+		}),
+		Entry("Should be ready if namespace is selected via namespaces", testCase{
 			namespaces: []v1.Namespace{
 				{
 					ObjectMeta: metav1.ObjectMeta{