Skip to content

Commit

Permalink
Make more use of api-server validation
Browse files Browse the repository at this point in the history
Signed-off-by: Erik Godding Boye <[email protected]>
  • Loading branch information
erikgb committed Nov 16, 2024
1 parent 41dc93d commit c7c15cb
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ spec:
defaultCAPackageVersion field of the Bundle's status field.
type: boolean
type: object
minItems: 1
type: array
target:
description: Target is the target location in all namespaces to sync source data to.
Expand All @@ -230,6 +231,7 @@ spec:
properties:
key:
description: Key is the key of the entry in the object's `data` field to be used.
minLength: 1
type: string
password:
default: changeit
Expand All @@ -247,6 +249,7 @@ spec:
properties:
key:
description: Key is the key of the entry in the object's `data` field to be used.
minLength: 1
type: string
password:
default: ""
Expand All @@ -264,6 +267,7 @@ spec:
properties:
key:
description: Key is the key of the entry in the object's `data` field to be used.
minLength: 1
type: string
required:
- key
Expand All @@ -289,11 +293,15 @@ spec:
properties:
key:
description: Key is the key of the entry in the object's `data` field to be used.
minLength: 1
type: string
required:
- 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)'
required:
- sources
- target
Expand Down
26 changes: 14 additions & 12 deletions docs/api/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func Resource(resource string) schema.GroupResource
Resource takes an unqualified resource and returns a Group qualified GroupResource

<a name="AdditionalFormats"></a>
## type [AdditionalFormats](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L120-L128>)
## type [AdditionalFormats](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L122-L130>)

AdditionalFormats specifies any additional formats to write to the target

Expand Down Expand Up @@ -206,7 +206,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#L209-L248>)

BundleCondition contains condition information for a Bundle.

Expand Down Expand Up @@ -313,7 +313,7 @@ func (in *BundleList) DeepCopyObject() runtime.Object
DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.

<a name="BundleSource"></a>
## type [BundleSource](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L70-L95>)
## type [BundleSource](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L71-L96>)

BundleSource is the set of sources whose data will be appended and synced to the BundleTarget in all Namespaces.

Expand Down Expand Up @@ -365,13 +365,14 @@ func (in *BundleSource) DeepCopyInto(out *BundleSource)
DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil.

<a name="BundleSpec"></a>
## type [BundleSpec](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L60-L66>)
## type [BundleSpec](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L60-L67>)

BundleSpec defines the desired state of a Bundle.

```go
type BundleSpec struct {
// Sources is a set of references to data whose data will sync to the target.
// +kubebuilder:validation:MinItems=1
Sources []BundleSource `json:"sources"`

// Target is the target location in all namespaces to sync source data to.
Expand All @@ -398,7 +399,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#L192-L206>)

BundleStatus defines the observed state of the Bundle.

Expand Down Expand Up @@ -439,9 +440,9 @@ func (in *BundleStatus) DeepCopyInto(out *BundleStatus)
DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil.

<a name="BundleTarget"></a>
## type [BundleTarget](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L99-L117>)
## type [BundleTarget](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L101-L119>)

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"

```go
type BundleTarget struct {
Expand Down Expand Up @@ -484,7 +485,7 @@ func (in *BundleTarget) DeepCopyInto(out *BundleTarget)
DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil.

<a name="JKS"></a>
## type [JKS](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L130-L139>)
## type [JKS](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L132-L141>)



Expand Down Expand Up @@ -520,13 +521,14 @@ 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#L185-L189>)

KeySelector is a reference to a key for some map data object.

```go
type KeySelector struct {
// Key is the key of the entry in the object's `data` field to be used.
// +kubebuilder:validation:MinLength=1
Key string `json:"key"`
}
```
Expand All @@ -550,7 +552,7 @@ 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>)
## type [NamespaceSelector](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L154-L159>)

NamespaceSelector defines selectors to match on Namespaces.

Expand Down Expand Up @@ -582,7 +584,7 @@ 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>)
## type [PKCS12](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/types_bundle.go#L143-L151>)



Expand Down Expand Up @@ -617,7 +619,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#L163-L182>)

SourceObjectKeySelector is a reference to a source object and its \`data\` key\(s\) in the trust Namespace.

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/trust/v1alpha1/types_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type BundleList struct {
// BundleSpec defines the desired state of a Bundle.
type BundleSpec struct {
// Sources is a set of references to data whose data will sync to the target.
// +kubebuilder:validation:MinItems=1
Sources []BundleSource `json:"sources"`

// Target is the target location in all namespaces to sync source data to.
Expand Down Expand Up @@ -96,6 +97,7 @@ 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"
type BundleTarget struct {
// ConfigMap is the target ConfigMap in Namespaces that all Bundle source
// data will be synced to.
Expand Down Expand Up @@ -182,6 +184,7 @@ type SourceObjectKeySelector struct {
// KeySelector is a reference to a key for some map data object.
type KeySelector struct {
// Key is the key of the entry in the object's `data` field to be used.
// +kubebuilder:validation:MinLength=1
Key string `json:"key"`
}

Expand Down
12 changes: 0 additions & 12 deletions pkg/webhook/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,18 +185,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{}{}
Expand Down
27 changes: 0 additions & 27 deletions pkg/webhook/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ func Test_validate(t *testing.T) {
},
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": {
Expand Down Expand Up @@ -220,32 +219,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"},
Expand Down
8 changes: 6 additions & 2 deletions test/integration/bundle/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package test

import (
"os"
"path/filepath"

"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/envtest"
Expand All @@ -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()
Expand Down
89 changes: 89 additions & 0 deletions test/integration/bundle/validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
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"
"k8s.io/utils/ptr"

"k8s.io/klog/v2/ktesting"
"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),
}}
})

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)))
})
})

Context("Target", func() {
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: &trustapi.NamespaceSelector{}}, 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),
)

DescribeTable("should require target key",
func(target trustapi.BundleTarget, expErr string) {
bundle.Spec.Target = target
Expect(cl.Create(ctx, bundle)).Should(MatchError(ContainSubstring(expErr)))
},
Entry("for configMap", trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{}}, "spec.target.configMap.key: Invalid value: \"\": spec.target.configMap.key in body should be at least 1 chars long"),
Entry("for secret", trustapi.BundleTarget{Secret: &trustapi.KeySelector{}}, "spec.target.secret.key: Invalid value: \"\": spec.target.secret.key in body should be at least 1 chars long"),
)
})
})

0 comments on commit c7c15cb

Please sign in to comment.