Skip to content

Commit

Permalink
Replace webhook validations with CEL validation
Browse files Browse the repository at this point in the history 
Signed-off-by: Erik Godding Boye <[email protected]>
  • Loading branch information
@erikgb
erikgb committed Nov 28, 2024
1 parent f3cd2f5 commit 2af5468
Show file tree
Hide file tree
Showing 5 changed files with 316 additions and 340 deletions.
23 changes: 23 additions & 0 deletions deploy/charts/trust-manager/templates/crd-trust.cert-manager.io_bundles.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/trust/v1alpha1/types_bundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
114 changes: 1 addition & 113 deletions pkg/webhook/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package webhook
import (
"context"
"fmt"
"strconv"

"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/apis/meta/v1/validation"
Expand Down Expand Up @@ -86,88 +85,22 @@ 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...)
}

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 {
Expand All @@ -188,51 +121,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...)

Expand Down
Loading

0 comments on commit 2af5468

Please sign in to comment.