From d5ffbc7818609493f03ddf1bda646c40617d33f8 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sat, 16 Nov 2024 21:49:40 +0100 Subject: [PATCH] feat: support namespaceSelector matchExpressions Signed-off-by: Erik Godding Boye --- .../crd-trust.cert-manager.io_bundles.yaml | 37 ++++++++++++- docs/api/api.md | 53 ++++--------------- pkg/apis/trust/v1alpha1/types_bundle.go | 10 +--- .../trust/v1alpha1/zz_generated.deepcopy.go | 24 +-------- pkg/bundle/bundle.go | 4 +- pkg/webhook/validation.go | 19 +++++-- pkg/webhook/validation_test.go | 16 +++--- test/gen/bundle.go | 2 +- test/integration/bundle/suite.go | 2 +- test/smoke/suite_test.go | 2 +- 10 files changed, 73 insertions(+), 96 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 index 5211aab6..dddad443 100644 --- 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 index 5b913c14..f1630113 100644 --- 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. -## type [BundleCondition]() +## type [BundleCondition]() 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. -## type [BundleStatus]() +## type [BundleStatus]() 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. -## type [KeySelector]() +## type [KeySelector]() 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. - -## type [NamespaceSelector]() - -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"` -} -``` - - -### func \(\*NamespaceSelector\) [DeepCopy]() - -```go -func (in *NamespaceSelector) DeepCopy() *NamespaceSelector -``` - -DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceSelector. - - -### func \(\*NamespaceSelector\) [DeepCopyInto]() - -```go -func (in *NamespaceSelector) DeepCopyInto(out *NamespaceSelector) -``` - -DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil. - ## type [PKCS12]() @@ -599,7 +564,7 @@ type PKCS12 struct { ``` -### func \(\*PKCS12\) [DeepCopy]() +### func \(\*PKCS12\) [DeepCopy]() ```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. -### func \(\*PKCS12\) [DeepCopyInto]() +### func \(\*PKCS12\) [DeepCopyInto]() ```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. -## type [SourceObjectKeySelector]() +## type [SourceObjectKeySelector]() SourceObjectKeySelector is a reference to a source object and its \`data\` key\(s\) in the trust Namespace. @@ -645,7 +610,7 @@ type SourceObjectKeySelector struct { ``` -### func \(\*SourceObjectKeySelector\) [DeepCopy]() +### func \(\*SourceObjectKeySelector\) [DeepCopy]() ```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. -### func \(\*SourceObjectKeySelector\) [DeepCopyInto]() +### func \(\*SourceObjectKeySelector\) [DeepCopyInto]() ```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 index 014839ad..b49c8465 100644 --- 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 index 6264b3ae..b6e43412 100644 --- a/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go @@ -230,7 +230,7 @@ func (in *BundleTarget) DeepCopyInto(out *BundleTarget) { } if in.NamespaceSelector != nil { in, out := &in.NamespaceSelector, &out.NamespaceSelector - *out = new(NamespaceSelector) + *out = new(v1.LabelSelector) (*in).DeepCopyInto(*out) } } @@ -281,28 +281,6 @@ func (in *KeySelector) DeepCopy() *KeySelector { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *NamespaceSelector) DeepCopyInto(out *NamespaceSelector) { - *out = *in - if in.MatchLabels != nil { - in, out := &in.MatchLabels, &out.MatchLabels - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceSelector. -func (in *NamespaceSelector) DeepCopy() *NamespaceSelector { - if in == nil { - return nil - } - out := new(NamespaceSelector) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PKCS12) DeepCopyInto(out *PKCS12) { *out = *in diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go index 892eb3bb..b0096633 100644 --- a/pkg/bundle/bundle.go +++ b/pkg/bundle/bundle.go @@ -370,9 +370,9 @@ 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 { + 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 index c5f21059..ecaa211b 100644 --- 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,11 @@ 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")) } + + if configMap.Selector != nil { + errs := validation.ValidateLabelSelector(configMap.Selector, validation.LabelSelectorValidationOptions{}, path.Child("selector")) + el = append(el, errs...) + } } if secret := source.Secret; secret != nil { @@ -130,6 +135,11 @@ 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")) } + + if secret.Selector != nil { + errs := validation.ValidateLabelSelector(secret.Selector, validation.LabelSelectorValidationOptions{}, path.Child("selector")) + el = append(el, errs...) + } } if source.InLine != nil { @@ -227,10 +237,9 @@ 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())) - } + if nsSel := bundle.Spec.Target.NamespaceSelector; nsSel != nil { + errs := validation.ValidateLabelSelector(nsSel, 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 index 68fe2cb7..32e4ecdd 100644 --- 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 index ef00d0e8..cd965c0c 100644 --- 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 index 93efb13c..00e26928 100644 --- 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 index 556267c7..b4e1bcd8 100644 --- 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())