feat: support namespaceSelector matchExpressions

Signed-off-by: Erik Godding Boye <egboye@gmail.com>
cert-manager · Nov 17, 2024 · cde993f · cde993f
1 parent 41dc93d
commit cde993f
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 97 deletions.
diff --git a/deploy/charts/trust-manager/templates/crd-trust.cert-manager.io_bundles.yaml b/deploy/charts/trust-manager/templates/crd-trust.cert-manager.io_bundles.yaml
@@ -273,14 +273,47 @@ spec:
                         NamespaceSelector will, if set, only sync the target resource in
                         Namespaces which match the selector.
                       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
+                                x-kubernetes-list-type: atomic
+                            required:
+                              - key
+                              - operator
+                            type: object
+                          type: array
+                          x-kubernetes-list-type: atomic
                         matchLabels:
                           additionalProperties:
                             type: string
                           description: |-
-                            MatchLabels matches on the set of labels that must be present on a
-                            Namespace for the Bundle target to be synced there.
+                            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
                     secret:
                       description: |-
                         Secret is the target Secret that all Bundle source data will be synced to.

diff --git a/docs/api/api.md b/docs/api/api.md
@@ -45,9 +45,6 @@ import "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 - [type KeySelector](<#KeySelector>)
   - [func \(in \*KeySelector\) DeepCopy\(\) \*KeySelector](<#KeySelector.DeepCopy>)
   - [func \(in \*KeySelector\) DeepCopyInto\(out \*KeySelector\)](<#KeySelector.DeepCopyInto>)
-- [type NamespaceSelector](<#NamespaceSelector>)
-  - [func \(in \*NamespaceSelector\) DeepCopy\(\) \*NamespaceSelector](<#NamespaceSelector.DeepCopy>)
-  - [func \(in \*NamespaceSelector\) DeepCopyInto\(out \*NamespaceSelector\)](<#NamespaceSelector.DeepCopyInto>)
 - [type PKCS12](<#PKCS12>)
   - [func \(in \*PKCS12\) DeepCopy\(\) \*PKCS12](<#PKCS12.DeepCopy>)
   - [func \(in \*PKCS12\) DeepCopyInto\(out \*PKCS12\)](<#PKCS12.DeepCopyInto>)
@@ -206,7 +203,7 @@ func (in *Bundle) DeepCopyObject() runtime.Object
 DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
 
 <a name="BundleCondition"></a>
-## type [BundleCondition](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L206-L245>)
+## type [BundleCondition](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L198-L237>)
 
 BundleCondition contains condition information for a Bundle.
 
@@ -398,7 +395,7 @@ func (in *BundleSpec) DeepCopyInto(out *BundleSpec)
 DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil.
 
 <a name="BundleStatus"></a>
-## type [BundleStatus](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L189-L203>)
+## type [BundleStatus](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L181-L195>)
 
 BundleStatus defines the observed state of the Bundle.
 
@@ -461,7 +458,7 @@ type BundleTarget struct {
     // NamespaceSelector will, if set, only sync the target resource in
     // Namespaces which match the selector.
     // +optional
-    NamespaceSelector *NamespaceSelector `json:"namespaceSelector,omitempty"`
+    NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"`
 }
 ```
 
@@ -520,7 +517,7 @@ func (in *JKS) DeepCopyInto(out *JKS)
 DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil.
 
 <a name="KeySelector"></a>
-## type [KeySelector](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L183-L186>)
+## type [KeySelector](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L175-L178>)
 
 KeySelector is a reference to a key for some map data object.
 
@@ -549,38 +546,6 @@ func (in *KeySelector) DeepCopyInto(out *KeySelector)
 
 DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil.
 
-<a name="NamespaceSelector"></a>
-## type [NamespaceSelector](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L152-L157>)
-
-NamespaceSelector defines selectors to match on Namespaces.
-
-```go
-type NamespaceSelector struct {
-    // MatchLabels matches on the set of labels that must be present on a
-    // Namespace for the Bundle target to be synced there.
-    // +optional
-    MatchLabels map[string]string `json:"matchLabels,omitempty"`
-}
-```
-
-<a name="NamespaceSelector.DeepCopy"></a>
-### func \(\*NamespaceSelector\) [DeepCopy](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L297>)
-
-```go
-func (in *NamespaceSelector) DeepCopy() *NamespaceSelector
-```
-
-DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceSelector.
-
-<a name="NamespaceSelector.DeepCopyInto"></a>
-### func \(\*NamespaceSelector\) [DeepCopyInto](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L285>)
-
-```go
-func (in *NamespaceSelector) DeepCopyInto(out *NamespaceSelector)
-```
-
-DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil.
-
 <a name="PKCS12"></a>
 ## type [PKCS12](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L141-L149>)
 
@@ -599,7 +564,7 @@ type PKCS12 struct {
 ```
 
 <a name="PKCS12.DeepCopy"></a>
-### func \(\*PKCS12\) [DeepCopy](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L318>)
+### func \(\*PKCS12\) [DeepCopy](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L296>)
 
 ```go
 func (in *PKCS12) DeepCopy() *PKCS12
@@ -608,7 +573,7 @@ func (in *PKCS12) DeepCopy() *PKCS12
 DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PKCS12.
 
 <a name="PKCS12.DeepCopyInto"></a>
-### func \(\*PKCS12\) [DeepCopyInto](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L307>)
+### func \(\*PKCS12\) [DeepCopyInto](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L285>)
 
 ```go
 func (in *PKCS12) DeepCopyInto(out *PKCS12)
@@ -617,7 +582,7 @@ func (in *PKCS12) DeepCopyInto(out *PKCS12)
 DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil.
 
 <a name="SourceObjectKeySelector"></a>
-## type [SourceObjectKeySelector](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L161-L180>)
+## type [SourceObjectKeySelector](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L153-L172>)
 
 SourceObjectKeySelector is a reference to a source object and its \`data\` key\(s\) in the trust Namespace.
 
@@ -645,7 +610,7 @@ type SourceObjectKeySelector struct {
 ```
 
 <a name="SourceObjectKeySelector.DeepCopy"></a>
-### func \(\*SourceObjectKeySelector\) [DeepCopy](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L338>)
+### func \(\*SourceObjectKeySelector\) [DeepCopy](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L316>)
 
 ```go
 func (in *SourceObjectKeySelector) DeepCopy() *SourceObjectKeySelector
@@ -654,7 +619,7 @@ func (in *SourceObjectKeySelector) DeepCopy() *SourceObjectKeySelector
 DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SourceObjectKeySelector.
 
 <a name="SourceObjectKeySelector.DeepCopyInto"></a>
-### func \(\*SourceObjectKeySelector\) [DeepCopyInto](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L328>)
+### func \(\*SourceObjectKeySelector\) [DeepCopyInto](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L306>)
 
 ```go
 func (in *SourceObjectKeySelector) DeepCopyInto(out *SourceObjectKeySelector)

diff --git a/pkg/apis/trust/v1alpha1/types_bundle.go b/pkg/apis/trust/v1alpha1/types_bundle.go
@@ -113,7 +113,7 @@ type BundleTarget struct {
 	// NamespaceSelector will, if set, only sync the target resource in
 	// Namespaces which match the selector.
 	// +optional
-	NamespaceSelector *NamespaceSelector `json:"namespaceSelector,omitempty"`
+	NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"`
 }
 
 // AdditionalFormats specifies any additional formats to write to the target
@@ -148,14 +148,6 @@ type PKCS12 struct {
 	Password *string `json:"password,omitempty"`
 }
 
-// NamespaceSelector defines selectors to match on Namespaces.
-type NamespaceSelector struct {
-	// MatchLabels matches on the set of labels that must be present on a
-	// Namespace for the Bundle target to be synced there.
-	// +optional
-	MatchLabels map[string]string `json:"matchLabels,omitempty"`
-}
-
 // SourceObjectKeySelector is a reference to a source object and its `data` key(s)
 // in the trust Namespace.
 type SourceObjectKeySelector struct {

diff --git a/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go
@@ -370,9 +370,12 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
 func (b *bundle) bundleTargetNamespaceSelector(bundleObj *trustapi.Bundle) (labels.Selector, error) {
 	nsSelector := bundleObj.Spec.Target.NamespaceSelector
 
-	if nsSelector == nil || nsSelector.MatchLabels == nil {
+	// metav1.LabelSelectorAsSelector returns a Selector selecting nothing if LabelSelector is nil,
+	// while our current default is to select everything. But this is subject to change.
+	// See https://github.com/cert-manager/trust-manager/issues/39
+	if nsSelector == nil {
 		return labels.Everything(), nil
 	}
 
-	return metav1.LabelSelectorAsSelector(&metav1.LabelSelector{MatchLabels: nsSelector.MatchLabels})
+	return metav1.LabelSelectorAsSelector(nsSelector)
 }
diff --git a/pkg/webhook/validation.go b/pkg/webhook/validation.go
@@ -22,7 +22,7 @@ import (
 	"strconv"
 
 	"github.com/go-logr/logr"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/validation"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -111,6 +111,9 @@ func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) {
 			if len(configMap.Key) > 0 && configMap.IncludeAllKeys {
 				el = append(el, field.Invalid(path, fmt.Sprintf("key: %s, includeAllKeys: %t", configMap.Key, configMap.IncludeAllKeys), "source configMap key cannot be defined when includeAllKeys is true"))
 			}
+
+			errs := validation.ValidateLabelSelector(configMap.Selector, validation.LabelSelectorValidationOptions{}, path.Child("selector"))
+			el = append(el, errs...)
 		}
 
 		if secret := source.Secret; secret != nil {
@@ -130,6 +133,9 @@ func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) {
 			if len(secret.Key) > 0 && secret.IncludeAllKeys {
 				el = append(el, field.Invalid(path, fmt.Sprintf("key: %s, includeAllKeys: %t", secret.Key, secret.IncludeAllKeys), "source secret key cannot be defined when includeAllKeys is true"))
 			}
+
+			errs := validation.ValidateLabelSelector(secret.Selector, validation.LabelSelectorValidationOptions{}, path.Child("selector"))
+			el = append(el, errs...)
 		}
 
 		if source.InLine != nil {
@@ -227,11 +233,8 @@ func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) {
 		}
 	}
 
-	if nsSel := bundle.Spec.Target.NamespaceSelector; nsSel != nil && len(nsSel.MatchLabels) > 0 {
-		if _, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{MatchLabels: nsSel.MatchLabels}); err != nil {
-			el = append(el, field.Invalid(path.Child("target", "namespaceSelector", "matchLabels"), nsSel.MatchLabels, err.Error()))
-		}
-	}
+	errs := validation.ValidateLabelSelector(bundle.Spec.Target.NamespaceSelector, validation.LabelSelectorValidationOptions{}, path.Child("target", "namespaceSelector"))
+	el = append(el, errs...)
 
 	return warnings, el.ToAggregate()
 

diff --git a/pkg/webhook/validation_test.go b/pkg/webhook/validation_test.go
@@ -255,7 +255,7 @@ func Test_validate(t *testing.T) {
 					},
 					Target: trustapi.BundleTarget{
 						ConfigMap: &trustapi.KeySelector{Key: "test-1"},
-						NamespaceSelector: &trustapi.NamespaceSelector{
+						NamespaceSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{"@@@@": ""},
 						},
 					},
@@ -270,7 +270,7 @@ func Test_validate(t *testing.T) {
 				},
 			},
 			expErr: ptr.To(field.ErrorList{
-				field.Invalid(field.NewPath("spec", "target", "namespaceSelector", "matchLabels"), map[string]string{"@@@@": ""}, `key: Invalid value: "@@@@": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')`),
+				field.Invalid(field.NewPath("spec", "target", "namespaceSelector", "matchLabels"), "@@@@", `name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')`),
 			}.ToAggregate().Error()),
 		},
 		"a Bundle with a duplicate target JKS key should fail validation and return a denied response": {
@@ -291,7 +291,7 @@ func Test_validate(t *testing.T) {
 						ConfigMap: &trustapi.KeySelector{
 							Key: "bar",
 						},
-						NamespaceSelector: &trustapi.NamespaceSelector{
+						NamespaceSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{"foo": "bar"},
 						},
 					},
@@ -317,7 +317,7 @@ func Test_validate(t *testing.T) {
 						ConfigMap: &trustapi.KeySelector{
 							Key: "bar",
 						},
-						NamespaceSelector: &trustapi.NamespaceSelector{
+						NamespaceSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{"foo": "bar"},
 						},
 					},
@@ -334,7 +334,7 @@ func Test_validate(t *testing.T) {
 					},
 					Target: trustapi.BundleTarget{
 						ConfigMap: &trustapi.KeySelector{Key: "test-1"},
-						NamespaceSelector: &trustapi.NamespaceSelector{
+						NamespaceSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{"foo": "bar"},
 						},
 					},
@@ -372,7 +372,7 @@ func Test_validate(t *testing.T) {
 						ConfigMap: &trustapi.KeySelector{
 							Key: "bar",
 						},
-						NamespaceSelector: &trustapi.NamespaceSelector{
+						NamespaceSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{"foo": "bar"},
 						},
 					},
@@ -398,7 +398,7 @@ func Test_validate(t *testing.T) {
 						ConfigMap: &trustapi.KeySelector{
 							Key: "bar",
 						},
-						NamespaceSelector: &trustapi.NamespaceSelector{
+						NamespaceSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{"foo": "bar"},
 						},
 					},
@@ -416,7 +416,7 @@ func Test_validate(t *testing.T) {
 					},
 					Target: trustapi.BundleTarget{
 						ConfigMap: &trustapi.KeySelector{Key: "test-1"},
-						NamespaceSelector: &trustapi.NamespaceSelector{
+						NamespaceSelector: &metav1.LabelSelector{
 							MatchLabels: map[string]string{"foo": "bar"},
 						},
 					},

diff --git a/test/gen/bundle.go b/test/gen/bundle.go
@@ -79,7 +79,7 @@ func SetBundleResourceVersion(resourceVersion string) BundleModifier {
 // target namespace selector.
 func SetBundleTargetNamespaceSelectorMatchLabels(matchLabels map[string]string) BundleModifier {
 	return func(bundle *trustapi.Bundle) {
-		bundle.Spec.Target.NamespaceSelector = &trustapi.NamespaceSelector{
+		bundle.Spec.Target.NamespaceSelector = &metav1.LabelSelector{
 			MatchLabels: matchLabels,
 		}
 	}

diff --git a/test/integration/bundle/suite.go b/test/integration/bundle/suite.go
@@ -645,7 +645,7 @@ var _ = Describe("Integration", func() {
 
 		// add a label selector to the Bundle which should exclude all namespaces
 		Expect(komega.Update(testBundle, func() {
-			testBundle.Spec.Target.NamespaceSelector = &trustapi.NamespaceSelector{
+			testBundle.Spec.Target.NamespaceSelector = &metav1.LabelSelector{
 				MatchLabels: map[string]string{"foo": "bar"},
 			}
 		})()).To(Succeed())

diff --git a/test/smoke/suite_test.go b/test/smoke/suite_test.go
@@ -153,7 +153,7 @@ func testBundleCommon(ctx context.Context, cl client.Client, testBundle *trustap
 	By("Setting Namespace Selector should remove Secrets from Namespaces that do not have a match")
 	Expect(cl.Get(ctx, client.ObjectKey{Name: testBundle.Name}, testBundle)).NotTo(HaveOccurred())
 
-	testBundle.Spec.Target.NamespaceSelector = &trustapi.NamespaceSelector{
+	testBundle.Spec.Target.NamespaceSelector = &metav1.LabelSelector{
 		MatchLabels: map[string]string{"foo": "bar"},
 	}
 	Expect(cl.Update(ctx, testBundle)).NotTo(HaveOccurred())