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 0c68c60d..91496b00 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 @@ -138,6 +138,11 @@ spec: x-kubernetes-map-type: atomic type: object x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: must specify one and only one of {name, selector} + rule: '[has(self.name), has(self.selector)].exists_one(x,x)' + - message: must specify key or includeAllKeys + rule: '[has(self.key), has(self.includeAllKeys) && self.includeAllKeys].exists_one(x,x)' inLine: description: InLine is a simple string to append as the source data. type: string @@ -209,6 +214,11 @@ spec: x-kubernetes-map-type: atomic type: object x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: must specify one and only one of {name, selector} + rule: '[has(self.name), has(self.selector)].exists_one(x,x)' + - message: must specify key or includeAllKeys + rule: '[has(self.key), has(self.includeAllKeys) && self.includeAllKeys].exists_one(x,x)' useDefaultCAs: description: |- UseDefaultCAs, when true, requests the default CA bundle to be used as a source. @@ -222,10 +232,16 @@ spec: type: boolean type: object x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: must define exactly one source + rule: '[has(self.configMap), has(self.secret), has(self.inLine), has(self.useDefaultCAs) && self.useDefaultCAs].exists_one(x,x)' maxItems: 100 minItems: 1 type: array x-kubernetes-list-type: atomic + x-kubernetes-validations: + - message: must request default CAs at most once + rule: size(self.filter(s, has(s.useDefaultCAs) && s.useDefaultCAs)) <= 1 target: description: Target is the target location in all namespaces to sync source data to. properties: @@ -343,6 +359,13 @@ spec: - key type: object type: object + x-kubernetes-validations: + - message: must define at least one target configMap/secret + rule: '[has(self.configMap), has(self.secret)].exists(x,x)' + - message: additional format keys must be unique + rule: '!has(self.additionalFormats) || ![has(self.additionalFormats.jks), has(self.additionalFormats.pkcs12)].all(x,x) || self.additionalFormats.jks.key != self.additionalFormats.pkcs12.key' + - message: additional format keys must be different from configMap/secret keys + rule: '!has(self.additionalFormats) || !((has(self.configMap) ? [self.configMap.key] : []) + (has(self.secret) ? [self.secret.key] : [])).exists(k,(has(self.additionalFormats.jks) && self.additionalFormats.jks.key == k) || (has(self.additionalFormats.pkcs12) && self.additionalFormats.pkcs12.key == k))' required: - sources - target diff --git a/docs/api/api.md b/docs/api/api.md index 16a1ee31..3e1ba85b 100644 --- a/docs/api/api.md +++ b/docs/api/api.md @@ -122,7 +122,7 @@ func Resource(resource string) schema.GroupResource Resource takes an unqualified resource and returns a Group qualified GroupResource -## type [AdditionalFormats]() +## type [AdditionalFormats]() AdditionalFormats specifies any additional formats to write to the target @@ -205,7 +205,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. @@ -312,9 +312,9 @@ func (in *BundleList) DeepCopyObject() runtime.Object DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -## type [BundleSource]() +## type [BundleSource]() -BundleSource is the set of sources whose data will be appended and synced to the BundleTarget in all Namespaces. \+structType=atomic +BundleSource is the set of sources whose data will be appended and synced to the BundleTarget in all Namespaces. \+structType=atomic \+kubebuilder:validation:XValidation:rule="\[has\(self.configMap\), has\(self.secret\), has\(self.inLine\), has\(self.useDefaultCAs\) && self.useDefaultCAs\].exists\_one\(x,x\)", message="must define exactly one source" ```go type BundleSource struct { @@ -364,7 +364,7 @@ func (in *BundleSource) DeepCopyInto(out *BundleSource) DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil. -## type [BundleSpec]() +## type [BundleSpec]() BundleSpec defines the desired state of a Bundle. @@ -374,6 +374,7 @@ type BundleSpec struct { // +listType=atomic // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=100 + // +kubebuilder:validation:XValidation:rule="size(self.filter(s, has(s.useDefaultCAs) && s.useDefaultCAs)) <= 1", message="must request default CAs at most once" Sources []BundleSource `json:"sources"` // Target is the target location in all namespaces to sync source data to. @@ -400,7 +401,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. @@ -441,9 +442,9 @@ func (in *BundleStatus) DeepCopyInto(out *BundleStatus) DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil. -## type [BundleTarget]() +## type [BundleTarget]() -BundleTarget is the target resource that the Bundle will sync all source data to. +BundleTarget is the target resource that the Bundle will sync all source data to. \+kubebuilder:validation:XValidation:rule="\[has\(self.configMap\), has\(self.secret\)\].exists\(x,x\)", message="must define at least one target configMap/secret" \+kubebuilder:validation:XValidation:rule="\!has\(self.additionalFormats\) || \!\[has\(self.additionalFormats.jks\), has\(self.additionalFormats.pkcs12\)\].all\(x,x\) || self.additionalFormats.jks.key \!= self.additionalFormats.pkcs12.key", message="additional format keys must be unique" \+kubebuilder:validation:XValidation:rule="\!has\(self.additionalFormats\) || \!\(\(has\(self.configMap\) ? \[self.configMap.key\] : \[\]\) \+ \(has\(self.secret\) ? \[self.secret.key\] : \[\]\)\).exists\(k,\(has\(self.additionalFormats.jks\) && self.additionalFormats.jks.key == k\) || \(has\(self.additionalFormats.pkcs12\) && self.additionalFormats.pkcs12.key == k\)\)", message="additional format keys must be different from configMap/secret keys" ```go type BundleTarget struct { @@ -488,7 +489,7 @@ func (in *BundleTarget) DeepCopyInto(out *BundleTarget) DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil. -## type [JKS]() +## type [JKS]() JKS specifies additional target JKS files \+structType=atomic @@ -524,7 +525,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. @@ -555,7 +556,7 @@ func (in *KeySelector) DeepCopyInto(out *KeySelector) DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil. -## type [PKCS12]() +## type [PKCS12]() PKCS12 specifies additional target PKCS\#12 files \+structType=atomic @@ -590,9 +591,9 @@ 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. \+structType=atomic +SourceObjectKeySelector is a reference to a source object and its \`data\` key\(s\) in the trust Namespace. \+structType=atomic \+kubebuilder:validation:XValidation:rule="\[has\(self.name\), has\(self.selector\)\].exists\_one\(x,x\)", message="must specify one and only one of \{name, selector\}" \+kubebuilder:validation:XValidation:rule="\[has\(self.key\), has\(self.includeAllKeys\) && self.includeAllKeys\].exists\_one\(x,x\)", message="must specify key or includeAllKeys" ```go type SourceObjectKeySelector struct { diff --git a/pkg/apis/trust/v1alpha1/types_bundle.go b/pkg/apis/trust/v1alpha1/types_bundle.go index e8e9d1cc..f75c12fd 100644 --- a/pkg/apis/trust/v1alpha1/types_bundle.go +++ b/pkg/apis/trust/v1alpha1/types_bundle.go @@ -62,6 +62,7 @@ type BundleSpec struct { // +listType=atomic // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=100 + // +kubebuilder:validation:XValidation:rule="size(self.filter(s, has(s.useDefaultCAs) && s.useDefaultCAs)) <= 1", message="must request default CAs at most once" Sources []BundleSource `json:"sources"` // Target is the target location in all namespaces to sync source data to. @@ -71,6 +72,7 @@ type BundleSpec struct { // BundleSource is the set of sources whose data will be appended and synced to // the BundleTarget in all Namespaces. // +structType=atomic +// +kubebuilder:validation:XValidation:rule="[has(self.configMap), has(self.secret), has(self.inLine), has(self.useDefaultCAs) && self.useDefaultCAs].exists_one(x,x)", message="must define exactly one source" type BundleSource struct { // ConfigMap is a reference (by name) to a ConfigMap's `data` key(s), or to a // list of ConfigMap's `data` key(s) using label selector, in the trust Namespace. @@ -100,6 +102,9 @@ type BundleSource struct { // BundleTarget is the target resource that the Bundle will sync all source // data to. +// +kubebuilder:validation:XValidation:rule="[has(self.configMap), has(self.secret)].exists(x,x)", message="must define at least one target configMap/secret" +// +kubebuilder:validation:XValidation:rule="!has(self.additionalFormats) || ![has(self.additionalFormats.jks), has(self.additionalFormats.pkcs12)].all(x,x) || self.additionalFormats.jks.key != self.additionalFormats.pkcs12.key", message="additional format keys must be unique" +// +kubebuilder:validation:XValidation:rule="!has(self.additionalFormats) || !((has(self.configMap) ? [self.configMap.key] : []) + (has(self.secret) ? [self.secret.key] : [])).exists(k,(has(self.additionalFormats.jks) && self.additionalFormats.jks.key == k) || (has(self.additionalFormats.pkcs12) && self.additionalFormats.pkcs12.key == k))", message="additional format keys must be different from configMap/secret keys" type BundleTarget struct { // ConfigMap is the target ConfigMap in Namespaces that all Bundle source // data will be synced to. @@ -163,6 +168,8 @@ type PKCS12 struct { // SourceObjectKeySelector is a reference to a source object and its `data` key(s) // in the trust Namespace. // +structType=atomic +// +kubebuilder:validation:XValidation:rule="[has(self.name), has(self.selector)].exists_one(x,x)", message="must specify one and only one of {name, selector}" +// +kubebuilder:validation:XValidation:rule="[has(self.key), has(self.includeAllKeys) && self.includeAllKeys].exists_one(x,x)", message="must specify key or includeAllKeys" type SourceObjectKeySelector struct { // Name is the name of the source object in the trust Namespace. // This field must be left empty when `selector` is set diff --git a/pkg/webhook/validation.go b/pkg/webhook/validation.go index 84c369d6..e88cef93 100644 --- a/pkg/webhook/validation.go +++ b/pkg/webhook/validation.go @@ -19,8 +19,6 @@ package webhook import ( "context" "fmt" - "strconv" - "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" @@ -86,31 +84,11 @@ func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) { path = field.NewPath("spec") ) - sourceCount := 0 - defaultCAsCount := 0 - for i, source := range bundle.Spec.Sources { - path := path.Child("sources").Child("[" + strconv.Itoa(i) + "]") - - unionCount := 0 + path := path.Child("sources").Index(i) if configMap := source.ConfigMap; configMap != nil { path := path.Child("configMap") - sourceCount++ - unionCount++ - - if len(configMap.Name) == 0 && configMap.Selector == nil { - el = append(el, field.Invalid(path, "name: ' ', selector: nil", "must validate one and only one schema (oneOf): [name, selector]. Found none valid")) - } - if len(configMap.Name) > 0 && configMap.Selector != nil { - el = append(el, field.Invalid(path, fmt.Sprintf("name: %s, selector: {}", configMap.Name), "must validate one and only one schema (oneOf): [name, selector]. Found both set")) - } - if len(configMap.Key) == 0 && !configMap.IncludeAllKeys { - el = append(el, field.Invalid(path, fmt.Sprintf("key: ' ', includeAllKeys: %t", configMap.IncludeAllKeys), "source configMap key must be defined when includeAllKeys is false")) - } - 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...) @@ -118,56 +96,10 @@ func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) { if secret := source.Secret; secret != nil { path := path.Child("secret") - sourceCount++ - unionCount++ - - if len(secret.Name) == 0 && secret.Selector == nil { - el = append(el, field.Invalid(path, "name: ' ', selector: nil", "must validate one and only one schema (oneOf): [name, selector]. Found none valid")) - } - if len(secret.Name) > 0 && secret.Selector != nil { - el = append(el, field.Invalid(path, fmt.Sprintf("name: %s, selector: {}", secret.Name), "must validate one and only one schema (oneOf): [name, selector]. Found both set")) - } - if len(secret.Key) == 0 && !secret.IncludeAllKeys { - el = append(el, field.Invalid(path, fmt.Sprintf("key: ' ', includeAllKeys: %t", secret.IncludeAllKeys), "source secret key must be defined when includeAllKeys is false")) - } - 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 { - sourceCount++ - unionCount++ - } - - if source.UseDefaultCAs != nil { - defaultCAsCount++ - unionCount++ - - if *source.UseDefaultCAs { - sourceCount++ - } - } - - if unionCount != 1 { - el = append(el, field.Forbidden( - path, fmt.Sprintf("must define exactly one source type for each item but found %d defined types", unionCount), - )) - } - } - - if sourceCount == 0 { - el = append(el, field.Forbidden(path.Child("sources"), "must define at least one source")) - } - - if defaultCAsCount > 1 { - el = append(el, field.Forbidden( - path.Child("sources"), - fmt.Sprintf("must request default CAs either once or not at all but got %d requests", defaultCAsCount), - )) } if target := bundle.Spec.Target.ConfigMap; target != nil { @@ -188,51 +120,6 @@ func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) { } } - configMap := bundle.Spec.Target.ConfigMap - secret := bundle.Spec.Target.Secret - - if configMap == nil && secret == nil { - el = append(el, field.Invalid(path.Child("target"), bundle.Spec.Target, "must define at least one target")) - } - - if configMap != nil && len(configMap.Key) == 0 { - el = append(el, field.Invalid(path.Child("target", "configMap", "key"), configMap.Key, "target configMap key must be defined")) - } - - if secret != nil && len(secret.Key) == 0 { - el = append(el, field.Invalid(path.Child("target", "secret", "key"), secret.Key, "target secret key must be defined")) - } - - if bundle.Spec.Target.AdditionalFormats != nil { - var formats = make(map[string]*trustapi.KeySelector) - targetKeys := map[string]struct{}{} - if configMap != nil { - targetKeys[configMap.Key] = struct{}{} - } - if secret != nil { - targetKeys[secret.Key] = struct{}{} - } - - // Checks for nil to avoid nil point dereference error - if bundle.Spec.Target.AdditionalFormats.JKS != nil { - formats["jks"] = &bundle.Spec.Target.AdditionalFormats.JKS.KeySelector - } - - // Checks for nil to avoid nil point dereference error - if bundle.Spec.Target.AdditionalFormats.PKCS12 != nil { - formats["pkcs12"] = &bundle.Spec.Target.AdditionalFormats.PKCS12.KeySelector - } - - for f, selector := range formats { - if selector != nil { - if _, ok := targetKeys[selector.Key]; ok { - el = append(el, field.Invalid(path.Child("target", "additionalFormats", f, "key"), selector.Key, "key must be unique in target configMap")) - } - targetKeys[selector.Key] = struct{}{} - } - } - } - errs := validation.ValidateLabelSelector(bundle.Spec.Target.NamespaceSelector, validation.LabelSelectorValidationOptions{}, path.Child("target", "namespaceSelector")) el = append(el, errs...) diff --git a/pkg/webhook/validation_test.go b/pkg/webhook/validation_test.go index 32e4ecdd..7ab9ca60 100644 --- a/pkg/webhook/validation_test.go +++ b/pkg/webhook/validation_test.go @@ -41,155 +41,6 @@ func Test_validate(t *testing.T) { bundle: &corev1.Pod{}, expErr: ptr.To("expected a Bundle, but got a *v1.Pod"), }, - "no sources, no target": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{}, - }, - expErr: ptr.To(field.ErrorList{ - field.Forbidden(field.NewPath("spec", "sources"), "must define at least one source"), - field.Invalid(field.NewPath("spec", "target"), trustapi.BundleTarget{}, "must define at least one target"), - }.ToAggregate().Error()), - }, - "sources with multiple types defined in items": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - { - ConfigMap: &trustapi.SourceObjectKeySelector{Name: "test", Key: "test"}, - InLine: ptr.To("test"), - }, - {InLine: ptr.To("test")}, - { - ConfigMap: &trustapi.SourceObjectKeySelector{Name: "test", Key: "test"}, - Secret: &trustapi.SourceObjectKeySelector{Name: "test", Key: "test"}, - }, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Forbidden(field.NewPath("spec", "sources", "[0]"), "must define exactly one source type for each item but found 2 defined types"), - field.Forbidden(field.NewPath("spec", "sources", "[2]"), "must define exactly one source type for each item but found 2 defined types"), - }.ToAggregate().Error()), - }, - "empty source with no defined types": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - {}, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Forbidden(field.NewPath("spec", "sources", "[0]"), "must define exactly one source type for each item but found 0 defined types"), - field.Forbidden(field.NewPath("spec", "sources"), "must define at least one source"), - }.ToAggregate().Error()), - }, - "useDefaultCAs false, with no other defined sources": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - { - UseDefaultCAs: ptr.To(false), - }, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Forbidden(field.NewPath("spec", "sources"), "must define at least one source"), - }.ToAggregate().Error()), - }, - "useDefaultCAs requested twice": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - { - UseDefaultCAs: ptr.To(true), - }, - { - UseDefaultCAs: ptr.To(true), - }, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Forbidden(field.NewPath("spec", "sources"), "must request default CAs either once or not at all but got 2 requests"), - }.ToAggregate().Error()), - }, - "useDefaultCAs requested three times": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - { - UseDefaultCAs: ptr.To(true), - }, - { - UseDefaultCAs: ptr.To(false), - }, - { - UseDefaultCAs: ptr.To(true), - }, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Forbidden(field.NewPath("spec", "sources"), "must request default CAs either once or not at all but got 3 requests"), - }.ToAggregate().Error()), - }, - "sources names, selectors and keys are empty": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "", Key: ""}}, - {InLine: ptr.To("test")}, - {Secret: &trustapi.SourceObjectKeySelector{Name: "", Key: ""}}, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Invalid(field.NewPath("spec", "sources", "[0]", "configMap"), "name: ' ', selector: nil", "must validate one and only one schema (oneOf): [name, selector]. Found none valid"), - field.Invalid(field.NewPath("spec", "sources", "[0]", "configMap"), "key: ' ', includeAllKeys: false", "source configMap key must be defined when includeAllKeys is false"), - field.Invalid(field.NewPath("spec", "sources", "[2]", "secret"), "name: ' ', selector: nil", "must validate one and only one schema (oneOf): [name, selector]. Found none valid"), - field.Invalid(field.NewPath("spec", "sources", "[2]", "secret"), "key: ' ', includeAllKeys: false", "source secret key must be defined when includeAllKeys is false"), - }.ToAggregate().Error()), - }, - "sources names and selectors are both set": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "some-config-map", Selector: &metav1.LabelSelector{}, Key: "test"}}, - {InLine: ptr.To("test")}, - {Secret: &trustapi.SourceObjectKeySelector{Name: "some-secret", Selector: &metav1.LabelSelector{}, Key: "test"}}, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Invalid(field.NewPath("spec", "sources", "[0]", "configMap"), "name: some-config-map, selector: {}", "must validate one and only one schema (oneOf): [name, selector]. Found both set"), - field.Invalid(field.NewPath("spec", "sources", "[2]", "secret"), "name: some-secret, selector: {}", "must validate one and only one schema (oneOf): [name, selector]. Found both set"), - }.ToAggregate().Error()), - }, - "sources key is set and includeAllKeys is true": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "some-config-map", Key: "test", IncludeAllKeys: true}}, - {InLine: ptr.To("test")}, - {Secret: &trustapi.SourceObjectKeySelector{Name: "some-secret", Key: "test", IncludeAllKeys: true}}, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Invalid(field.NewPath("spec", "sources", "[0]", "configMap"), "key: test, includeAllKeys: true", "source configMap key cannot be defined when includeAllKeys is true"), - field.Invalid(field.NewPath("spec", "sources", "[2]", "secret"), "key: test, includeAllKeys: true", "source secret key cannot be defined when includeAllKeys is true"), - }.ToAggregate().Error()), - }, "sources defines the same configMap target": { bundle: &trustapi.Bundle{ ObjectMeta: metav1.ObjectMeta{Name: "test-bundle"}, @@ -220,32 +71,6 @@ func Test_validate(t *testing.T) { field.Forbidden(field.NewPath("spec", "sources", "[1]", "secret", "test-bundle", "test"), "cannot define the same source as target"), }.ToAggregate().Error()), }, - "target configMap key not defined": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - {InLine: ptr.To("test")}, - }, - Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: ""}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Invalid(field.NewPath("spec", "target", "configMap", "key"), "", "target configMap key must be defined"), - }.ToAggregate().Error()), - }, - "target secret key not defined": { - bundle: &trustapi.Bundle{ - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - {InLine: ptr.To("test")}, - }, - Target: trustapi.BundleTarget{Secret: &trustapi.KeySelector{Key: ""}}, - }, - }, - expErr: ptr.To(field.ErrorList{ - field.Invalid(field.NewPath("spec", "target", "secret", "key"), "", "target secret key must be defined"), - }.ToAggregate().Error()), - }, "invalid namespace selector": { bundle: &trustapi.Bundle{ ObjectMeta: metav1.ObjectMeta{Name: "test-bundle-1"}, @@ -273,58 +98,6 @@ func Test_validate(t *testing.T) { 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": { - bundle: &trustapi.Bundle{ - ObjectMeta: metav1.ObjectMeta{Name: "testing"}, - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - {InLine: ptr.To("foo")}, - }, - Target: trustapi.BundleTarget{ - AdditionalFormats: &trustapi.AdditionalFormats{ - JKS: &trustapi.JKS{ - KeySelector: trustapi.KeySelector{ - Key: "bar", - }, - }, - }, - ConfigMap: &trustapi.KeySelector{ - Key: "bar", - }, - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"foo": "bar"}, - }, - }, - }, - }, - expErr: ptr.To("spec.target.additionalFormats.jks.key: Invalid value: \"bar\": key must be unique in target configMap"), - }, - "a Bundle with a duplicate target PKCS12 key should fail validation and return a denied response": { - bundle: &trustapi.Bundle{ - ObjectMeta: metav1.ObjectMeta{Name: "testing"}, - Spec: trustapi.BundleSpec{ - Sources: []trustapi.BundleSource{ - {InLine: ptr.To("foo")}, - }, - Target: trustapi.BundleTarget{ - AdditionalFormats: &trustapi.AdditionalFormats{ - PKCS12: &trustapi.PKCS12{ - KeySelector: trustapi.KeySelector{ - Key: "bar", - }, - }, - }, - ConfigMap: &trustapi.KeySelector{ - Key: "bar", - }, - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"foo": "bar"}, - }, - }, - }, - }, - expErr: ptr.To("spec.target.additionalFormats.pkcs12.key: Invalid value: \"bar\": key must be unique in target configMap"), - }, "valid Bundle": { bundle: &trustapi.Bundle{ ObjectMeta: metav1.ObjectMeta{Name: "test-bundle-1"}, diff --git a/test/integration/bundle/integration.go b/test/integration/bundle/integration.go index 665c368d..470a5110 100644 --- a/test/integration/bundle/integration.go +++ b/test/integration/bundle/integration.go @@ -18,6 +18,7 @@ package test import ( "os" + "path/filepath" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -38,14 +39,17 @@ var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) crdsYamlFile := os.Getenv("TRUST_MANAGER_CRDS") - Expect(crdsYamlFile).NotTo(BeEmpty(), "TRUST_MANAGER_CRDS must be set to the path of the CRDs to install") + if len(crdsYamlFile) == 0 { + crdsYamlFile = filepath.Join("..", "..", "..", "_bin", "scratch", "crds") + } env = &envtest.Environment{ UseExistingCluster: ptr.To(false), CRDDirectoryPaths: []string{ crdsYamlFile, }, - Scheme: trustapi.GlobalScheme, + ErrorIfCRDPathMissing: true, + Scheme: trustapi.GlobalScheme, } _, err := env.Start() diff --git a/test/integration/bundle/validation_test.go b/test/integration/bundle/validation_test.go new file mode 100644 index 00000000..88f5b8bb --- /dev/null +++ b/test/integration/bundle/validation_test.go @@ -0,0 +1,285 @@ +/* +Copyright 2021 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2/ktesting" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Bundle Validation", func() { + var ( + ctx context.Context + cl client.Client + bundle *trustapi.Bundle + ) + BeforeEach(func() { + _, ctx = ktesting.NewTestContext(GinkgoT()) + + var err error + cl, err = client.New(env.Config, client.Options{ + Scheme: trustapi.GlobalScheme, + }) + Expect(err).NotTo(HaveOccurred()) + + bundle = &trustapi.Bundle{} + bundle.GenerateName = "validation-" + bundle.Spec.Sources = []trustapi.BundleSource{{ + UseDefaultCAs: ptr.To(true), + }} + bundle.Spec.Target = trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "ca-bundle.crt"}} + }) + + Context("Sources", func() { + It("should require at least one source", func() { + bundle.Spec.Sources = make([]trustapi.BundleSource, 0) + + expectedErr := "spec.sources: Invalid value: 0: spec.sources in body should have at least 1 items" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr))) + }) + + It("should require at most one useDefaultCAs source", func() { + bundle.Spec.Sources = []trustapi.BundleSource{ + {UseDefaultCAs: ptr.To(true)}, + {UseDefaultCAs: ptr.To(true)}, + } + + expectedErr := "spec.sources: Invalid value: \"array\": must request default CAs at most once" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr))) + }) + }) + + Context("Source item", func() { + DescribeTable("should require exactly one source", + func(source trustapi.BundleSource, wantErr bool) { + bundle.Spec.Sources = []trustapi.BundleSource{source} + if wantErr { + expectedErr := "spec.sources[0]: Invalid value: \"object\": must define exactly one source" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr))) + } else { + Expect(cl.Create(ctx, bundle)).To(Succeed()) + } + }, + Entry("when none set", trustapi.BundleSource{}, true), + Entry("when configMap set", trustapi.BundleSource{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "ca", Key: "ca.crt"}}, false), + Entry("when secret set", trustapi.BundleSource{Secret: &trustapi.SourceObjectKeySelector{Name: "ca", Key: "ca.crt"}}, false), + Entry("when inLine set", trustapi.BundleSource{InLine: ptr.To("")}, false), + Entry("when useDefaultCAs=true set", trustapi.BundleSource{UseDefaultCAs: ptr.To(true)}, false), + Entry("when useDefaultCAs=false set", trustapi.BundleSource{UseDefaultCAs: ptr.To(false)}, true), + Entry("when multiple set", trustapi.BundleSource{InLine: ptr.To(""), UseDefaultCAs: ptr.To(true)}, true), + ) + }) + + Context("Source object item", func() { + var ( + selectorAccessor func(*trustapi.SourceObjectKeySelector) + field string + ) + + BeforeEach(func() { + bundle.Spec.Sources = []trustapi.BundleSource{{}} + }) + + sourceObjectAsserts := func() { + DescribeTable("should require exactly one object specifier", + func(selector *trustapi.SourceObjectKeySelector, wantErr bool) { + selector.Key = "ca.crt" + selectorAccessor(selector) + if wantErr { + expectedErr := "spec.sources[0].%s: Invalid value: \"object\": must specify one and only one of {name, selector}" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr, field))) + } else { + Expect(cl.Create(ctx, bundle)).To(Succeed()) + } + }, + Entry("when none set", &trustapi.SourceObjectKeySelector{}, true), + Entry("when name set", &trustapi.SourceObjectKeySelector{Name: "ca"}, false), + Entry("when selector set", &trustapi.SourceObjectKeySelector{Selector: &metav1.LabelSelector{}}, false), + Entry("when both set", &trustapi.SourceObjectKeySelector{Name: "ca", Selector: &metav1.LabelSelector{}}, true), + ) + + DescribeTable("should require exactly one key specifier", + func(selector *trustapi.SourceObjectKeySelector, wantErr bool) { + selector.Name = "ca" + selectorAccessor(selector) + if wantErr { + expectedErr := "spec.sources[0].%s: Invalid value: \"object\": must specify key or includeAllKeys" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr, field))) + } else { + Expect(cl.Create(ctx, bundle)).To(Succeed()) + } + }, + Entry("when none set", &trustapi.SourceObjectKeySelector{}, true), + Entry("when key set", &trustapi.SourceObjectKeySelector{Key: "ca.crt"}, false), + Entry("when selector set", &trustapi.SourceObjectKeySelector{IncludeAllKeys: true}, false), + Entry("when both set", &trustapi.SourceObjectKeySelector{Key: "ca.crt", IncludeAllKeys: true}, true), + ) + } + + Context("ConfigMap", func() { + BeforeEach(func() { + selectorAccessor = func(selector *trustapi.SourceObjectKeySelector) { + bundle.Spec.Sources[0].ConfigMap = selector + } + field = "configMap" + }) + + sourceObjectAsserts() + }) + + Context("Secret", func() { + BeforeEach(func() { + selectorAccessor = func(selector *trustapi.SourceObjectKeySelector) { + bundle.Spec.Sources[0].Secret = selector + } + field = "secret" + }) + + sourceObjectAsserts() + }) + }) + + Context("Target", func() { + var ( + selectorAccessor func(*trustapi.KeySelector) + field string + ) + + DescribeTable("should require at least one target configMap/secret", + func(target trustapi.BundleTarget, wantErr bool) { + bundle.Spec.Target = target + if wantErr { + expectedErr := "spec.target: Invalid value: \"object\": must define at least one target configMap/secret" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr))) + } else { + Expect(cl.Create(ctx, bundle)).To(Succeed()) + } + }, + Entry("when none set", trustapi.BundleTarget{NamespaceSelector: &metav1.LabelSelector{}}, true), + Entry("when configMap set", trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "ca-bundle.crt"}}, false), + Entry("when secret set", trustapi.BundleTarget{Secret: &trustapi.KeySelector{Key: "ca-bundle.crt"}}, false), + Entry("when both set", trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "ca-bundle.crt"}, Secret: &trustapi.KeySelector{Key: "ca-bundle.crt"}}, false), + ) + + type TargetKeySpec struct { + ConfigMapKey string + SecretKey string + JKSKey string + PKCS12Key string + } + + DescribeTable("should require additional format keys different from target keys", + func(keySpec TargetKeySpec, wantErr bool) { + target := trustapi.BundleTarget{AdditionalFormats: &trustapi.AdditionalFormats{}} + if keySpec.ConfigMapKey != "" { + target.ConfigMap = &trustapi.KeySelector{Key: keySpec.ConfigMapKey} + } + if keySpec.SecretKey != "" { + target.Secret = &trustapi.KeySelector{Key: keySpec.SecretKey} + } + if keySpec.JKSKey != "" { + target.AdditionalFormats.JKS = &trustapi.JKS{KeySelector: trustapi.KeySelector{Key: keySpec.JKSKey}} + } + if keySpec.PKCS12Key != "" { + target.AdditionalFormats.PKCS12 = &trustapi.PKCS12{KeySelector: trustapi.KeySelector{Key: keySpec.PKCS12Key}} + } + bundle.Spec.Target = target + + if wantErr { + expectedErr := "spec.target: Invalid value: \"object\": additional format keys must be different from configMap/secret keys" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr))) + } else { + Expect(cl.Create(ctx, bundle)).To(Succeed()) + } + }, + Entry(nil, TargetKeySpec{ConfigMapKey: "ca.crt"}, false), + Entry(nil, TargetKeySpec{SecretKey: "ca.crt"}, false), + Entry(nil, TargetKeySpec{ConfigMapKey: "ca.crt", SecretKey: "ca.crt"}, false), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s"}, false), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", JKSKey: "j"}, false), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", PKCS12Key: "p"}, false), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", JKSKey: "j", PKCS12Key: "p"}, false), + + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", JKSKey: "c"}, true), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", PKCS12Key: "c"}, true), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", JKSKey: "c", PKCS12Key: "p"}, true), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", JKSKey: "j", PKCS12Key: "c"}, true), + + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", JKSKey: "s"}, true), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", PKCS12Key: "s"}, true), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", JKSKey: "s", PKCS12Key: "p"}, true), + Entry(nil, TargetKeySpec{ConfigMapKey: "c", SecretKey: "s", JKSKey: "j", PKCS12Key: "s"}, true), + ) + + DescribeTable("should require unique additional format keys", + func(formats *trustapi.AdditionalFormats, wantErr bool) { + bundle.Spec.Target.AdditionalFormats = formats + if wantErr { + expectedErr := "spec.target: Invalid value: \"object\": additional format keys must be unique" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr))) + } else { + Expect(cl.Create(ctx, bundle)).To(Succeed()) + } + }, + Entry("when none set", &trustapi.AdditionalFormats{}, false), + Entry("when JKS key set", &trustapi.AdditionalFormats{JKS: &trustapi.JKS{KeySelector: trustapi.KeySelector{Key: "trust.jks"}}}, false), + Entry("when PKCS key set", &trustapi.AdditionalFormats{PKCS12: &trustapi.PKCS12{KeySelector: trustapi.KeySelector{Key: "trust.p12"}}}, false), + Entry("when both keys set, but different value", &trustapi.AdditionalFormats{JKS: &trustapi.JKS{KeySelector: trustapi.KeySelector{Key: "trust.jks"}}, PKCS12: &trustapi.PKCS12{KeySelector: trustapi.KeySelector{Key: "trust.p12"}}}, false), + Entry("when both keys set, same value", &trustapi.AdditionalFormats{JKS: &trustapi.JKS{KeySelector: trustapi.KeySelector{Key: "cacerts"}}, PKCS12: &trustapi.PKCS12{KeySelector: trustapi.KeySelector{Key: "cacerts"}}}, true), + ) + + targetObjectAsserts := func() { + It("should require target key", func() { + bundle.Spec.Target = trustapi.BundleTarget{} + selectorAccessor(&trustapi.KeySelector{}) + expectedErr := "spec.target.%s.key: Invalid value: \"\": spec.target.%s.key in body should be at least 1 chars long" + Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expectedErr, field, field))) + }) + } + + Context("ConfigMap", func() { + BeforeEach(func() { + selectorAccessor = func(selector *trustapi.KeySelector) { + bundle.Spec.Target.ConfigMap = selector + } + field = "configMap" + }) + + targetObjectAsserts() + }) + + Context("Secret", func() { + BeforeEach(func() { + selectorAccessor = func(selector *trustapi.KeySelector) { + bundle.Spec.Target.Secret = selector + } + field = "secret" + }) + + targetObjectAsserts() + }) + }) +})