weiguoqiang/cert-manager
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Calls to validating webhook can now return warnings

Browse Source 
Adds warnings to the top level validating functions' signatures

Signed-off-by: irbekrm <irbekrm@gmail.com>
This commit is contained in:
irbekrm 2021-04-29 09:51:06 +01:00
parent 7ff54e61e9
commit bffebe2cb6
26 changed files with 366 additions and 193 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/api/util/BUILD.bazel
View File

@ -8,6 +8,7 @@ go_library(
"issuers.go",
"names.go",
"usages.go",
"warnings.go",
],
importpath = "github.com/jetstack/cert-manager/pkg/api/util",
visibility = ["//visibility:public"],

5
pkg/internal/api/validation/BUILD.bazel
View File

@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
srcs = ["registry.go"],
srcs = [
"registry.go",
"warning.go",
],
importpath = "github.com/jetstack/cert-manager/pkg/internal/api/validation",
visibility = ["//pkg:__subpackages__"],
deps = [

53
pkg/internal/api/validation/registry.go
View File

@ -35,8 +35,8 @@ type Registry struct {
validateUpdateRegister map[schema.GroupVersionKind]ValidateUpdateFunc
}
type ValidateFunc func(req *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList
type ValidateUpdateFunc func(req *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) field.ErrorList
type ValidateFunc func(req *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, WarningList)
type ValidateUpdateFunc func(req *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) (field.ErrorList, WarningList)
// NewRegistry creates a new empty registry, backed by the provided Scheme.
func NewRegistry(scheme *runtime.Scheme) *Registry {
@ -93,26 +93,29 @@ func (r *Registry) AddValidateUpdateFunc(obj runtime.Object, fn ValidateUpdateFu
// calling the validation functions.
// Any validation functions registered for the objects internal API version
// will be run against the object regardless of version.
func (r *Registry) Validate(req *admissionv1.AdmissionRequest, obj runtime.Object, requestVersion schema.GroupVersionKind) field.ErrorList {
func (r *Registry) Validate(req *admissionv1.AdmissionRequest, obj runtime.Object, requestVersion schema.GroupVersionKind) (field.ErrorList, WarningList) {
versioned, internal := r.lookupValidateFuncs(requestVersion)
if versioned == nil && internal == nil {
return nil
return nil, nil
}
targetObj, internalObj, err := r.convert(obj, requestVersion)
if err != nil {
return internalError(err)
return internalError(err), nil
}
el := field.ErrorList{}
warnings := WarningList{}
if versioned != nil {
el = append(el, versioned(req, targetObj)...)
e, w := versioned(req, targetObj)
el, warnings = append(el, e...), append(warnings, w...)
}
if internal != nil {
el = append(el, internal(req, internalObj)...)
e, w := internal(req, internalObj)
el, warnings = append(el, e...), append(warnings, w...)
}
return el
return el, warnings
}
// ValidateUpdate will run all update validation functions registered for the
@ -122,31 +125,34 @@ func (r *Registry) Validate(req *admissionv1.AdmissionRequest, obj runtime.Objec
// calling the validation functions.
// Any validation functions registered for the objects internal API version
// will be run against the object regardless of version.
func (r *Registry) ValidateUpdate(req *admissionv1.AdmissionRequest, oldObj, obj runtime.Object, requestVersion schema.GroupVersionKind) field.ErrorList {
func (r *Registry) ValidateUpdate(req *admissionv1.AdmissionRequest, oldObj, obj runtime.Object, requestVersion schema.GroupVersionKind) (field.ErrorList, []string) {
versioned, internal := r.lookupValidateUpdateFuncs(requestVersion)
if versioned == nil && internal == nil {
return nil
return nil, nil
}
targetOldObj, internalOldObj, err := r.convert(oldObj, requestVersion)
if err != nil {
return internalError(err)
return internalError(err), nil
}
targetObj, internalObj, err := r.convert(obj, requestVersion)
if err != nil {
return internalError(err)
return internalError(err), nil
}
el := field.ErrorList{}
warnings := WarningList{}
if versioned != nil {
el = append(el, versioned(req, targetOldObj, targetObj)...)
e, w := versioned(req, targetOldObj, targetObj)
el, warnings = append(el, e...), append(warnings, w...)
}
if internal != nil {
el = append(el, internal(req, internalOldObj, internalObj)...)
e, w := internal(req, internalOldObj, internalObj)
el, warnings = append(el, e...), append(warnings, w...)
}
return el
return el, warnings
}
func (r *Registry) lookupValidateFuncs(gvk schema.GroupVersionKind) (versioned ValidateFunc, internal ValidateFunc) {
@ -170,8 +176,12 @@ func (r *Registry) appendValidate(gvk schema.GroupVersionKind, fn ValidateFunc)
return
}
r.validateRegister[gvk] = func(req *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
return append(existing(req, obj), fn(req, obj)...)
// If a ValidateFunc for GVK already exists, build a new ValidateFunc that
// will return both the results of the new and old ValidateFunc.
r.validateRegister[gvk] = func(req *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, WarningList) {
e, w := existing(req, obj)
newE, newW := fn(req, obj)
return append(e, newE...), append(w, newW...)
}
}
@ -182,8 +192,13 @@ func (r *Registry) appendValidateUpdate(gvk schema.GroupVersionKind, fn Validate
return
}
r.validateUpdateRegister[gvk] = func(req *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) field.ErrorList {
return append(existing(req, oldObj, obj), fn(req, oldObj, obj)...)
// If a ValidateUpdateFunc for GVK already exists, build a new
// ValidateUpdateFunc that will return both the results of the new and old
// ValidateUpdateFunc.
r.validateUpdateRegister[gvk] = func(req *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) (field.ErrorList, WarningList) {
e, w := existing(req, oldObj, obj)
newE, newW := fn(req, oldObj, obj)
return append(e, newE...), append(w, newW...)
}
}

92
pkg/internal/api/validation/registry_test.go
View File

@ -18,6 +18,7 @@ package validation_test
import (
"fmt"
"reflect"
"testing"
admissionv1 "k8s.io/api/admission/v1"
@ -41,14 +42,17 @@ var (
func TestValidateType(t *testing.T) {
reg := validation.NewRegistry(scheme)
called := false
utilruntime.Must(reg.AddValidateFunc(&cmapi.Certificate{}, func(req *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateFunc(&cmapi.Certificate{}, func(req *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
called = true
return nil
return nil, nil
}))
errs := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
errs, warnings := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
if len(errs) > 0 {
t.Errorf("expected to not get an error but got: %v", errs.ToAggregate())
}
if len(warnings) > 0 {
t.Errorf("expected no warnings but got: %+v", warnings)
}
if !called {
t.Errorf("expected registered validation function to run but it did not")
}
@ -59,22 +63,25 @@ func TestValidateTypeMultiple(t *testing.T) {
called1 := false
called2 := false
calledInternal := false
utilruntime.Must(reg.AddValidateFunc(&cmapi.Certificate{}, func(req *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateFunc(&cmapi.Certificate{}, func(req *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
called1 = true
return nil
return nil, nil
}))
utilruntime.Must(reg.AddValidateFunc(&cmapi.Certificate{}, func(req *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateFunc(&cmapi.Certificate{}, func(req *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
called2 = true
return nil
return nil, nil
}))
utilruntime.Must(reg.AddValidateFunc(&cmapiinternal.Certificate{}, func(req *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateFunc(&cmapiinternal.Certificate{}, func(req *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
calledInternal = true
return nil
return nil, nil
}))
errs := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
errs, warnings := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
if len(errs) > 0 {
t.Errorf("expected to not get an error but got: %v", errs.ToAggregate())
}
if len(warnings) > 0 {
t.Errorf("expected to not get any warnings but got %+v", warnings)
}
if !called1 || !called2 {
t.Errorf("expected registered validation function to run but it did not")
}
@ -88,22 +95,25 @@ func TestValidateUpdateTypeMultiple(t *testing.T) {
called1 := false
called2 := false
calledInternal := false
utilruntime.Must(reg.AddValidateUpdateFunc(&cmapi.Certificate{}, func(_ *admissionv1.AdmissionRequest, _, _ runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateUpdateFunc(&cmapi.Certificate{}, func(_ *admissionv1.AdmissionRequest, _, _ runtime.Object) (field.ErrorList, validation.WarningList) {
called1 = true
return nil
return nil, nil
}))
utilruntime.Must(reg.AddValidateUpdateFunc(&cmapi.Certificate{}, func(_ *admissionv1.AdmissionRequest, _, _ runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateUpdateFunc(&cmapi.Certificate{}, func(_ *admissionv1.AdmissionRequest, _, _ runtime.Object) (field.ErrorList, validation.WarningList) {
called2 = true
return nil
return nil, nil
}))
utilruntime.Must(reg.AddValidateUpdateFunc(&cmapiinternal.Certificate{}, func(_ *admissionv1.AdmissionRequest, _, _ runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateUpdateFunc(&cmapiinternal.Certificate{}, func(_ *admissionv1.AdmissionRequest, _, _ runtime.Object) (field.ErrorList, validation.WarningList) {
calledInternal = true
return nil
return nil, nil
}))
errs := reg.ValidateUpdate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
errs, warnings := reg.ValidateUpdate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
if len(errs) > 0 {
t.Errorf("expected to not get an error but got: %v", errs.ToAggregate())
}
if len(warnings) > 0 {
t.Errorf("expected to not get any warnings but got: %v", warnings)
}
if !called1 || !called2 {
t.Errorf("expected registered validation function to run but it did not")
}
@ -115,33 +125,40 @@ func TestValidateUpdateTypeMultiple(t *testing.T) {
func TestValidateUpdateType(t *testing.T) {
reg := validation.NewRegistry(scheme)
called := false
utilruntime.Must(reg.AddValidateUpdateFunc(&cmapi.Certificate{}, func(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateUpdateFunc(&cmapi.Certificate{}, func(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) (field.ErrorList, validation.WarningList) {
called = true
return nil
return nil, nil
}))
errs := reg.ValidateUpdate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
errs, warnings := reg.ValidateUpdate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
if len(errs) > 0 {
t.Errorf("expected to not get an error but got: %v", errs.ToAggregate())
}
if len(warnings) > 0 {
t.Errorf("expected to not get any warnings but got %+v", warnings)
}
if !called {
t.Errorf("expected registered validation function to run but it did not")
}
}
func TestValidateTypeReturnsErrors(t *testing.T) {
func TestValidateTypeReturnsErrorsAndWarnings(t *testing.T) {
reg := validation.NewRegistry(scheme)
called := false
expectedErr := field.InternalError(nil, fmt.Errorf("failed"))
utilruntime.Must(reg.AddValidateFunc(&cmapi.Certificate{}, func(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
expectedWarnings := validation.WarningList{"test warning"}
utilruntime.Must(reg.AddValidateFunc(&cmapi.Certificate{}, func(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
called = true
return field.ErrorList{expectedErr}
return field.ErrorList{expectedErr}, expectedWarnings
}))
errs := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
errs, warnings := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
if len(errs) != 1 {
t.Error("expected to get an error but got none")
} else if err := errs[0]; err.Error() != expectedErr.Error() {
t.Errorf("expected error to be %q but got %q", expectedErr.Error(), err.Error())
}
if !reflect.DeepEqual(warnings, expectedWarnings) {
t.Errorf("expected warnings %+#v got %+#v", expectedWarnings, warnings)
}
if !called {
t.Errorf("expected registered validation function to run but it did not")
}
@ -150,14 +167,17 @@ func TestValidateTypeReturnsErrors(t *testing.T) {
func TestValidateInternalType(t *testing.T) {
reg := validation.NewRegistry(scheme)
called := false
utilruntime.Must(reg.AddValidateFunc(&cmapiinternal.Certificate{}, func(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
utilruntime.Must(reg.AddValidateFunc(&cmapiinternal.Certificate{}, func(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
called = true
return nil
return nil, nil
}))
errs := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
errs, warnings := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
if len(errs) > 0 {
t.Errorf("expected to not get an error but got: %v", errs.ToAggregate())
}
if len(warnings) > 0 {
t.Errorf("expected to not get any warnings but got %+v", warnings)
}
if !called {
t.Errorf("expected registered internal validation function to run for external type but it did not")
}
@ -165,24 +185,30 @@ func TestValidateInternalType(t *testing.T) {
func TestValidateNoErrorNoneRegistered(t *testing.T) {
reg := validation.NewRegistry(scheme)
errs := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
errs, warnings := reg.Validate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
if len(errs) > 0 {
t.Errorf("expected to not get an error but got: %v", errs.ToAggregate())
}
if len(warnings) > 0 {
t.Errorf("expected to not get any warnings but got: %+v", warnings)
}
}
func TestValidateUpdateNoErrorNoneRegistered(t *testing.T) {
reg := validation.NewRegistry(scheme)
errs := reg.ValidateUpdate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
errs, warnings := reg.ValidateUpdate(&admissionv1.AdmissionRequest{}, &cmapi.Certificate{}, &cmapi.Certificate{}, cmapi.SchemeGroupVersion.WithKind("Certificate"))
if len(errs) > 0 {
t.Errorf("expected to not get an error but got: %v", errs.ToAggregate())
}
if len(warnings) > 0 {
t.Errorf("exptected to not get any warnings but got: %+v", warnings)
}
}
func TestValidateUnrecognisedType(t *testing.T) {
reg := validation.NewRegistry(scheme)
err := reg.AddValidateFunc(&corev1.Pod{}, func(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
return nil
err := reg.AddValidateFunc(&corev1.Pod{}, func(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
return nil, nil
})
if err == nil {
t.Errorf("expected to get an error but did not")
@ -191,8 +217,8 @@ func TestValidateUnrecognisedType(t *testing.T) {
func TestValidateUpdateUnrecognisedType(t *testing.T) {
reg := validation.NewRegistry(scheme)
err := reg.AddValidateUpdateFunc(&corev1.Pod{}, func(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) field.ErrorList {
return nil
err := reg.AddValidateUpdateFunc(&corev1.Pod{}, func(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) (field.ErrorList, validation.WarningList) {
return nil, nil
})
if err == nil {
t.Errorf("expected to get an error but did not")

21
pkg/internal/api/validation/warning.go Normal file
View File

@ -0,0 +1,21 @@
/*
Copyright 2020 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 validation
// WarningList is a list of warnings that will be returned by the validating webhook.
// See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#webhook-request-and-response
type WarningList []string

1
pkg/internal/apis/acme/validation/BUILD.bazel
View File

@ -26,6 +26,7 @@ go_test(
],
embed = [":go_default_library"],
deps = [
"//pkg/internal/api/validation:go_default_library",
"//pkg/internal/apis/acme:go_default_library",
"@io_k8s_apimachinery//pkg/util/validation/field:go_default_library",
"@io_k8s_utils//pointer:go_default_library",

8
pkg/internal/apis/acme/validation/challenge.go
View File

@ -23,21 +23,23 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmacme "github.com/jetstack/cert-manager/pkg/internal/apis/acme"
)
func ValidateChallengeUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) field.ErrorList {
func ValidateChallengeUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, validation.WarningList) {
old, ok := oldObj.(*cmacme.Challenge)
new := newObj.(*cmacme.Challenge)
// if oldObj is not set, the Update operation is always valid.
if !ok || old == nil {
return nil
return nil, nil
}
var warnings validation.WarningList
el := field.ErrorList{}
if !reflect.DeepEqual(old.Spec, new.Spec) {
el = append(el, field.Forbidden(field.NewPath("spec"), "challenge spec is immutable after creation"))
}
return el
return el, warnings
}

9
pkg/internal/apis/acme/validation/challenge_test.go
View File

@ -22,6 +22,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmacme "github.com/jetstack/cert-manager/pkg/internal/apis/acme"
)
@ -29,6 +30,7 @@ func TestValidateChallengeUpdate(t *testing.T) {
scenarios := map[string]struct {
old, new *cmacme.Challenge
errs []*field.Error
warnings validation.WarningList
}{
"allows setting challenge spec for the first time": {
new: &cmacme.Challenge{
@ -67,7 +69,7 @@ func TestValidateChallengeUpdate(t *testing.T) {
}
for n, s := range scenarios {
t.Run(n, func(t *testing.T) {
errs := ValidateChallengeUpdate(nil, s.old, s.new)
errs, warnings := ValidateChallengeUpdate(nil, s.old, s.new)
if len(errs) != len(s.errs) {
t.Errorf("Expected %v but got %v", s.errs, errs)
return
@ -75,9 +77,12 @@ func TestValidateChallengeUpdate(t *testing.T) {
for i, e := range errs {
expectedErr := s.errs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected errors %v but got %v", expectedErr, e)
}
}
if !reflect.DeepEqual(warnings, s.warnings) {
t.Errorf("Expected warnings %+#v but got %+#v", s.warnings, warnings)
}
})
}
}

8
pkg/internal/apis/acme/validation/order.go
View File

@ -23,21 +23,23 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmacme "github.com/jetstack/cert-manager/pkg/internal/apis/acme"
)
func ValidateOrderUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) field.ErrorList {
func ValidateOrderUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, validation.WarningList) {
old, ok := oldObj.(*cmacme.Order)
new := newObj.(*cmacme.Order)
// if oldObj is not set, the Update operation is always valid.
if !ok || old == nil {
return nil
return nil, nil
}
el := field.ErrorList{}
var warnings validation.WarningList
el = append(el, ValidateOrderSpecUpdate(old.Spec, new.Spec, field.NewPath("spec"))...)
el = append(el, ValidateOrderStatusUpdate(old.Status, new.Status, field.NewPath("status"))...)
return el
return el, warnings
}
func ValidateOrderSpecUpdate(old, new cmacme.OrderSpec, fldPath *field.Path) field.ErrorList {

29
pkg/internal/apis/acme/validation/order_test.go
View File

@ -24,6 +24,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmacme "github.com/jetstack/cert-manager/pkg/internal/apis/acme"
)
@ -43,39 +44,47 @@ func testImmutableOrderField(t *testing.T, fldPath *field.Path, setter func(*cma
expectedErrs := []*field.Error{
field.Forbidden(fldPath, "field is immutable once set"),
}
var expectedWarnings validation.WarningList
old := &cmacme.Order{}
new := &cmacme.Order{}
setter(old, testValueOptionOne)
setter(new, testValueOptionTwo)
errs := ValidateOrderUpdate(nil, old, new)
errs, warnings := ValidateOrderUpdate(nil, old, new)
if len(errs) != len(expectedErrs) {
t.Errorf("Expected %v but got %v", expectedErrs, errs)
t.Errorf("Expected errors %v but got %v", expectedErrs, errs)
return
}
for i, e := range errs {
expectedErr := expectedErrs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected error %v but got %v", expectedErr, e)
}
}
if !reflect.DeepEqual(warnings, expectedWarnings) {
t.Errorf("Expected warnings %+#v but got %+#v", expectedWarnings, warnings)
}
})
t.Run("should allow updates to "+fldPath.String()+" if not already set", func(t *testing.T) {
expectedErrs := []*field.Error{}
var expectedWarnings validation.WarningList
old := &cmacme.Order{}
new := &cmacme.Order{}
setter(old, testValueNone)
setter(new, testValueOptionOne)
errs := ValidateOrderUpdate(nil, old, new)
errs, warnings := ValidateOrderUpdate(nil, old, new)
if len(errs) != len(expectedErrs) {
t.Errorf("Expected %v but got %v", expectedErrs, errs)
t.Errorf("Expected errors %v but got %v", expectedErrs, errs)
return
}
for i, e := range errs {
expectedErr := expectedErrs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected error %v but got %v", expectedErr, e)
}
}
if !reflect.DeepEqual(warnings, expectedWarnings) {
t.Errorf("Expected warnings %+#v but got %+#v", expectedWarnings, warnings)
}
})
}
@ -200,6 +209,7 @@ func TestValidateCertificateUpdate(t *testing.T) {
scenarios := map[string]struct {
old, new *cmacme.Order
errs []*field.Error
warnings validation.WarningList
}{
"allows all updates if old is nil": {
new: &cmacme.Order{
@ -211,7 +221,7 @@ func TestValidateCertificateUpdate(t *testing.T) {
}
for n, s := range scenarios {
t.Run(n, func(t *testing.T) {
errs := ValidateOrderUpdate(nil, s.old, s.new)
errs, warnings := ValidateOrderUpdate(nil, s.old, s.new)
if len(errs) != len(s.errs) {
t.Errorf("Expected %v but got %v", s.errs, errs)
return
@ -219,9 +229,12 @@ func TestValidateCertificateUpdate(t *testing.T) {
for i, e := range errs {
expectedErr := s.errs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected errors %v but got %v", expectedErr, e)
}
}
if !reflect.DeepEqual(warnings, s.warnings) {
t.Errorf("Expected warnings %+#v but got %+#v", s.warnings, warnings)
}
})
}
}

2
pkg/internal/apis/certmanager/identity/certificaterequests/BUILD.bazel
View File

@ -6,6 +6,7 @@ go_library(
importpath = "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager/identity/certificaterequests",
visibility = ["//pkg:__subpackages__"],
deps = [
"//pkg/internal/api/validation:go_default_library",
"//pkg/internal/apis/certmanager:go_default_library",
"//pkg/util:go_default_library",
"@io_k8s_api//admission/v1:go_default_library",
@ -34,6 +35,7 @@ go_test(
srcs = ["certificaterequests_test.go"],
embed = [":go_default_library"],
deps = [
"//pkg/internal/api/validation:go_default_library",
"//pkg/internal/apis/certmanager:go_default_library",
"@io_k8s_api//admission/v1:go_default_library",
"@io_k8s_api//authentication/v1:go_default_library",

11
pkg/internal/apis/certmanager/identity/certificaterequests/certificaterequests.go
View File

@ -26,11 +26,13 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmapi "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
"github.com/jetstack/cert-manager/pkg/util"
)
func ValidateCreate(req *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
func ValidateCreate(req *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
cr := obj.(*cmapi.CertificateRequest)
fldPath := field.NewPath("spec")
@ -48,7 +50,7 @@ func ValidateCreate(req *admissionv1.AdmissionRequest, obj runtime.Object) field
el = append(el, field.Forbidden(fldPath.Child("extra"), "extra identity must be that of the requester"))
}
return el
return el, warnings
}
func extrasMatch(crExtra map[string][]string, reqExtra map[string]authenticationv1.ExtraValue) bool {
@ -70,7 +72,8 @@ func extrasMatch(crExtra map[string][]string, reqExtra map[string]authentication
return true
}
func ValidateUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) field.ErrorList {
func ValidateUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
oldCR, newCR := oldObj.(*cmapi.CertificateRequest), newObj.(*cmapi.CertificateRequest)
fldPath := field.NewPath("spec")
@ -88,7 +91,7 @@ func ValidateUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Obje
el = append(el, field.Forbidden(fldPath.Child("extra"), "extra identity cannot be changed once set"))
}
return el
return el, warnings
}
func MutateCreate(req *admissionv1.AdmissionRequest, obj runtime.Object) {

37
pkg/internal/apis/certmanager/identity/certificaterequests/certificaterequests_test.go
View File

@ -24,6 +24,7 @@ import (
authenticationv1 "k8s.io/api/authentication/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmapi "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
)
@ -31,9 +32,10 @@ func TestValidateCreate(t *testing.T) {
fldPath := field.NewPath("spec")
tests := map[string]struct {
req *admissionv1.AdmissionRequest
cr *cmapi.CertificateRequest
want field.ErrorList
req *admissionv1.AdmissionRequest
cr *cmapi.CertificateRequest
wantE field.ErrorList
wantW validation.WarningList
}{
"if identity fields don't match that of requester, should fail": {
req: &admissionv1.AdmissionRequest{
@ -58,7 +60,7 @@ func TestValidateCreate(t *testing.T) {
},
},
},
want: field.ErrorList{
wantE: field.ErrorList{
field.Forbidden(fldPath.Child("uid"), "uid identity must be that of the requester"),
field.Forbidden(fldPath.Child("username"), "username identity must be that of the requester"),
field.Forbidden(fldPath.Child("groups"), "groups identity must be that of the requester"),
@ -88,15 +90,18 @@ func TestValidateCreate(t *testing.T) {
},
},
},
want: nil,
wantE: nil,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := ValidateCreate(test.req, test.cr)
if !reflect.DeepEqual(got, test.want) {
t.Errorf("ValidateCreate() = %v, want %v", got, test.want)
gotE, gotW := ValidateCreate(test.req, test.cr)
if !reflect.DeepEqual(gotE, test.wantE) {
t.Errorf("errors from ValidateCreate() = %v, want %v", gotE, test.wantE)
}
if !reflect.DeepEqual(gotW, test.wantW) {
t.Errorf("warnings from ValidateCreate() = %v, want %v", gotW, test.wantW)
}
})
}
@ -107,7 +112,8 @@ func TestValidateUpdate(t *testing.T) {
tests := map[string]struct {
oldCR, newCR *cmapi.CertificateRequest
want field.ErrorList
wantE field.ErrorList
wantW validation.WarningList
}{
"if identity fields don't match that of the old CertificateRequest, should fail": {
oldCR: &cmapi.CertificateRequest{
@ -132,7 +138,7 @@ func TestValidateUpdate(t *testing.T) {
},
},
},
want: field.ErrorList{
wantE: field.ErrorList{
field.Forbidden(fldPath.Child("uid"), "uid identity cannot be changed once set"),
field.Forbidden(fldPath.Child("username"), "username identity cannot be changed once set"),
field.Forbidden(fldPath.Child("groups"), "groups identity cannot be changed once set"),
@ -162,15 +168,18 @@ func TestValidateUpdate(t *testing.T) {
},
},
},
want: nil,
wantE: nil,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := ValidateUpdate(nil, test.newCR, test.oldCR)
if !reflect.DeepEqual(got, test.want) {
t.Errorf("ValidateUpdate() = %v, want %v", got, test.want)
gotE, gotW := ValidateUpdate(nil, test.newCR, test.oldCR)
if !reflect.DeepEqual(gotE, test.wantE) {
t.Errorf("errors from ValidateUpdate() = %v, want %v", gotE, test.wantE)
}
if !reflect.DeepEqual(gotW, test.wantW) {
t.Errorf("warnings from ValidateUpdate() = %v, want %v", gotW, test.wantW)
}
})
}

2
pkg/internal/apis/certmanager/validation/BUILD.bazel
View File

@ -42,7 +42,9 @@ go_test(
],
embed = [":go_default_library"],
deps = [
"//pkg/api/util:go_default_library",
"//pkg/apis/certmanager/v1:go_default_library",
"//pkg/internal/api/validation:go_default_library",
"//pkg/internal/apis/acme:go_default_library",
"//pkg/internal/apis/certmanager:go_default_library",
"//pkg/internal/apis/meta:go_default_library",

11
pkg/internal/apis/certmanager/validation/certificate.go
View File

@ -27,6 +27,7 @@ import (
"github.com/jetstack/cert-manager/pkg/api/util"
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
internalcmapi "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
cmmeta "github.com/jetstack/cert-manager/pkg/internal/apis/meta"
)
@ -86,16 +87,18 @@ func ValidateCertificateSpec(crt *internalcmapi.CertificateSpec, fldPath *field.
return el
}
func ValidateCertificate(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
func ValidateCertificate(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
crt := obj.(*internalcmapi.Certificate)
allErrs := ValidateCertificateSpec(&crt.Spec, field.NewPath("spec"))
return allErrs
return allErrs, warnings
}
func ValidateUpdateCertificate(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) field.ErrorList {
func ValidateUpdateCertificate(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
crt := obj.(*internalcmapi.Certificate)
allErrs := ValidateCertificateSpec(&crt.Spec, field.NewPath("spec"))
return allErrs
return allErrs, warnings
}
func validateIssuerRef(issuerRef cmmeta.ObjectReference, fldPath *field.Path) field.ErrorList {

21
pkg/internal/apis/certmanager/validation/certificate_test.go
View File

@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
internalcmapi "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
cmmeta "github.com/jetstack/cert-manager/pkg/internal/apis/meta"
)
@ -48,8 +49,9 @@ func int32Ptr(i int32) *int32 {
func TestValidateCertificate(t *testing.T) {
fldPath := field.NewPath("spec")
scenarios := map[string]struct {
cfg *internalcmapi.Certificate
errs []*field.Error
cfg *internalcmapi.Certificate
errs []*field.Error
warnings validation.WarningList
}{
"valid basic certificate": {
cfg: &internalcmapi.Certificate{
@ -514,15 +516,24 @@ func TestValidateCertificate(t *testing.T) {
}
for n, s := range scenarios {
t.Run(n, func(t *testing.T) {
errs := ValidateCertificate(nil, s.cfg)
errs, warnings := ValidateCertificate(nil, s.cfg)
if len(errs) != len(s.errs) {
t.Errorf("Expected %v but got %v", s.errs, errs)
t.Errorf("Expected errors %v but got %v", s.errs, errs)
return
}
if len(warnings) != len(s.warnings) {
t.Errorf("Expected warnings %v but got %v", s.warnings, warnings)
}
for i, e := range errs {
expectedErr := s.errs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected error %v but got %v", expectedErr, e)
}
}
for i, w := range warnings {
expectedWarning := s.warnings[i]
if w != expectedWarning {
t.Errorf("Expected warning %q but got %q", expectedWarning, w)
}
}
})

11
pkg/internal/apis/certmanager/validation/certificaterequest.go
View File

@ -30,6 +30,7 @@ import (
"github.com/jetstack/cert-manager/pkg/apis/acme"
"github.com/jetstack/cert-manager/pkg/apis/certmanager"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmapi "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
cmmeta "github.com/jetstack/cert-manager/pkg/internal/apis/meta"
"github.com/jetstack/cert-manager/pkg/util"
@ -38,16 +39,18 @@ import (
var defaultInternalKeyUsages = []cmapi.KeyUsage{cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment}
func ValidateCertificateRequest(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
func ValidateCertificateRequest(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
cr := obj.(*cmapi.CertificateRequest)
allErrs := ValidateCertificateRequestSpec(&cr.Spec, field.NewPath("spec"), true)
allErrs = append(allErrs,
ValidateCertificateRequestApprovalCondition(cr.Status.Conditions, field.NewPath("status", "conditions"))...)
return allErrs
return allErrs, warnings
}
func ValidateUpdateCertificateRequest(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) field.ErrorList {
func ValidateUpdateCertificateRequest(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
oldCR, newCR := oldObj.(*cmapi.CertificateRequest), newObj.(*cmapi.CertificateRequest)
var el field.ErrorList
@ -66,7 +69,7 @@ func ValidateUpdateCertificateRequest(_ *admissionv1.AdmissionRequest, oldObj, n
el = append(el, field.Forbidden(field.NewPath("spec"), "cannot change spec after creation"))
}
return el
return el, warnings
}
func validateCertificateRequestAnnotations(objA, objB *cmapi.CertificateRequest, fieldPath *field.Path) field.ErrorList {

99
pkg/internal/apis/certmanager/validation/certificaterequest_test.go
View File

@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cminternal "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
cminternalmeta "github.com/jetstack/cert-manager/pkg/internal/apis/meta"
"github.com/jetstack/cert-manager/pkg/util/pki"
@ -62,7 +63,8 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
tests := map[string]struct {
oldCR, newCR *cminternal.CertificateRequest
want field.ErrorList
wantE field.ErrorList
wantW validation.WarningList
}{
"if CertificateRequest spec and cert-manager.io annotations change, error": {
oldCR: baseCR.DeepCopy(),
@ -77,7 +79,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"))),
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(field.NewPath("metadata", "annotations", "cert-manager.io/foo"), "cannot change cert-manager annotation after creation"),
field.Forbidden(field.NewPath("spec"), "cannot change spec after creation"),
},
@ -95,7 +97,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com"))),
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(field.NewPath("metadata", "annotations", "acme.cert-manager.io/bar"), "cannot change cert-manager annotation after creation"),
field.Forbidden(field.NewPath("spec"), "cannot change spec after creation"),
},
@ -103,7 +105,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
"if CertificateRequest spec and annotations do not change, don't error": {
oldCR: baseCR.DeepCopy(),
newCR: baseCR.DeepCopy(),
want: nil,
wantE: nil,
},
"CertificateRequest with single Approved=true condition that doesn't change, shouldn't error": {
oldCR: &cminternal.CertificateRequest{
@ -134,7 +136,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
},
},
},
want: nil,
wantE: nil,
},
"CertificateRequest with single Denied=true condition that doesn't change, shouldn't error": {
oldCR: &cminternal.CertificateRequest{
@ -165,7 +167,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
},
},
},
want: nil,
wantE: nil,
},
"CertificateRequest with single Approved=false condition that changes, should error": {
oldCR: &cminternal.CertificateRequest{
@ -198,7 +200,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, "'Approved' condition may not be modified once set"),
},
},
@ -233,7 +235,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, "'Denied' condition may not be modified once set"),
field.Invalid(fldPathConditions.Child("Denied"), nil, `"Denied" condition may only be set to True`),
},
@ -269,7 +271,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, "'Denied' condition may not be modified once set"),
},
},
@ -304,7 +306,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, "'Approved' condition may not be modified once set"),
},
},
@ -333,7 +335,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
},
},
},
want: nil,
wantE: nil,
},
"CertificateRequest with no condition that changes to Denied=true, shouldn't error": {
oldCR: &cminternal.CertificateRequest{
@ -360,7 +362,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
},
},
},
want: nil,
wantE: nil,
},
"CertificateRequest with single Approved=true condition that is removed, should error": {
oldCR: &cminternal.CertificateRequest{
@ -386,7 +388,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
Conditions: []cminternal.CertificateRequestCondition{},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, "'Approved' condition may not be modified once set"),
},
},
@ -414,7 +416,7 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
Conditions: []cminternal.CertificateRequestCondition{},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, "'Denied' condition may not be modified once set"),
},
},
@ -422,16 +424,19 @@ func TestValidateCertificateRequestUpdate(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := ValidateUpdateCertificateRequest(nil, test.oldCR, test.newCR)
for i := range got {
if got[i].Type != field.ErrorTypeForbidden {
gotE, gotW := ValidateUpdateCertificateRequest(nil, test.oldCR, test.newCR)
for i := range gotE {
if gotE[i].Type != field.ErrorTypeForbidden {
// filter out the value so it does not print the full CSR in tests
got[i].BadValue = nil
gotE[i].BadValue = nil
}
}
if !reflect.DeepEqual(got, test.want) {
t.Errorf("ValidateUpdateCertificateRequest() = %v, want %v", got, test.want)
if !reflect.DeepEqual(gotE, test.wantE) {
t.Errorf("errors from ValidateUpdateCertificateRequest() = %v, want %v", gotE, test.wantE)
}
if !reflect.DeepEqual(gotW, test.wantW) {
t.Errorf("warnings from ValidateUpdateCertificateRequest() = %#+v, want %#+v", gotW, test.wantW)
}
})
}
@ -442,8 +447,9 @@ func TestValidateCertificateRequest(t *testing.T) {
fldPathConditions := field.NewPath("status", "conditions")
tests := map[string]struct {
cr *cminternal.CertificateRequest
want field.ErrorList
cr *cminternal.CertificateRequest
wantE field.ErrorList
wantW validation.WarningList
}{
"Test csr with no usages": {
cr: &cminternal.CertificateRequest{
@ -453,7 +459,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: nil,
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"Test csr with double signature usages": {
cr: &cminternal.CertificateRequest{
@ -463,7 +469,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment},
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"Test csr with double extended usages": {
cr: &cminternal.CertificateRequest{
@ -473,7 +479,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageServerAuth, cminternal.UsageClientAuth},
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"Test csr with reordered usages": {
cr: &cminternal.CertificateRequest{
@ -483,7 +489,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: []cminternal.KeyUsage{cminternal.UsageServerAuth, cminternal.UsageClientAuth, cminternal.UsageKeyEncipherment, cminternal.UsageDigitalSignature},
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"Test csr that is CA with usages set": {
cr: &cminternal.CertificateRequest{
@ -494,7 +500,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageDigitalSignature, cminternal.UsageKeyEncipherment, cminternal.UsageCertSign},
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"Test csr that is CA but no cert sign in usages": {
cr: &cminternal.CertificateRequest{
@ -505,7 +511,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageDigitalSignature, cminternal.UsageKeyEncipherment, cminternal.UsageClientAuth, cminternal.UsageServerAuth},
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"Error on csr not having all usages": {
cr: &cminternal.CertificateRequest{
@ -515,7 +521,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageServerAuth, cminternal.UsageClientAuth},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Invalid(fldPath.Child("request"), nil, "csr key usages do not match specified usages, these should match if both are set: [[]certmanager.KeyUsage[3] != []certmanager.KeyUsage[4]]"),
},
},
@ -527,7 +533,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: []cminternal.KeyUsage{cminternal.UsageSigning, cminternal.UsageKeyEncipherment},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Invalid(fldPath.Child("request"), nil, "csr key usages do not match specified usages, these should match if both are set: [[]certmanager.KeyUsage[4] != []certmanager.KeyUsage[2]]"),
},
},
@ -540,7 +546,7 @@ func TestValidateCertificateRequest(t *testing.T) {
Usages: []cminternal.KeyUsage{cminternal.UsageAny, cminternal.UsageSigning, cminternal.UsageKeyEncipherment, cminternal.UsageClientAuth, cminternal.UsageServerAuth},
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"CertificateRequest with single Approved=true condition, shouldn't error": {
cr: &cminternal.CertificateRequest{
@ -559,7 +565,7 @@ func TestValidateCertificateRequest(t *testing.T) {
},
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"CertificateRequest with single Denied=true condition, shouldn't error": {
cr: &cminternal.CertificateRequest{
@ -578,7 +584,7 @@ func TestValidateCertificateRequest(t *testing.T) {
},
},
},
want: []*field.Error{},
wantE: []*field.Error{},
},
"CertificateRequest with single Approved=false condition, should error": {
cr: &cminternal.CertificateRequest{
@ -598,7 +604,7 @@ func TestValidateCertificateRequest(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Invalid(fldPathConditions.Child("Approved"), nil,
`"Approved" condition may only be set to True`),
},
@ -621,7 +627,7 @@ func TestValidateCertificateRequest(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Invalid(fldPathConditions.Child("Denied"), nil,
`"Denied" condition may only be set to True`),
},
@ -649,7 +655,7 @@ func TestValidateCertificateRequest(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Invalid(field.NewPath("status", "conditions", "Approved"), nil,
`"Approved" condition may only be set to True`),
field.Invalid(field.NewPath("status", "conditions", "Denied"), nil,
@ -680,7 +686,7 @@ func TestValidateCertificateRequest(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, "both 'Denied' and 'Approved' conditions cannot coexist"),
},
},
@ -707,7 +713,7 @@ func TestValidateCertificateRequest(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, `multiple "Approved" conditions present`),
},
},
@ -734,22 +740,25 @@ func TestValidateCertificateRequest(t *testing.T) {
},
},
},
want: []*field.Error{
wantE: []*field.Error{
field.Forbidden(fldPathConditions, `multiple "Denied" conditions present`),
},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := ValidateCertificateRequest(nil, test.cr)
for i := range got {
if got[i].Type != field.ErrorTypeForbidden {
gotE, gotW := ValidateCertificateRequest(nil, test.cr)
for i := range gotE {
if gotE[i].Type != field.ErrorTypeForbidden {
// filter out the value so it does not print the full CSR in tests
got[i].BadValue = nil
gotE[i].BadValue = nil
}
}
if !reflect.DeepEqual(got, test.want) {
t.Errorf("ValidateCertificateRequest() = %v, want %v", got, test.want)
if !reflect.DeepEqual(gotE, test.wantE) {
t.Errorf("errors from ValidateCertificateRequest() = %v, want %v", gotE, test.wantE)
}
if !reflect.DeepEqual(test.wantW, gotW) {
t.Errorf("warnings from ValidateCertificateRequest() = %v, want %v", gotW, test.wantW)
}
})
}

15
pkg/internal/apis/certmanager/validation/clusterissuer.go
View File

@ -21,19 +21,20 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmapi "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
)
// Validation functions for cert-manager v1alpha2 ClusterIssuer types
// Validation functions for cert-manager ClusterIssuer types.
func ValidateClusterIssuer(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
func ValidateClusterIssuer(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
iss := obj.(*cmapi.ClusterIssuer)
allErrs := ValidateIssuerSpec(&iss.Spec, field.NewPath("spec"))
return allErrs
allErrs, warnings := ValidateIssuerSpec(&iss.Spec, field.NewPath("spec"))
return allErrs, warnings
}
func ValidateUpdateClusterIssuer(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) field.ErrorList {
func ValidateUpdateClusterIssuer(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) (field.ErrorList, validation.WarningList) {
iss := obj.(*cmapi.ClusterIssuer)
allErrs := ValidateIssuerSpec(&iss.Spec, field.NewPath("spec"))
return allErrs
allErrs, warnings := ValidateIssuerSpec(&iss.Spec, field.NewPath("spec"))
return allErrs, warnings
}

29
pkg/internal/apis/certmanager/validation/issuer.go
View File

@ -26,6 +26,8 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
apiutil "github.com/jetstack/cert-manager/pkg/api/util"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmacme "github.com/jetstack/cert-manager/pkg/internal/apis/acme"
"github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
"github.com/jetstack/cert-manager/pkg/internal/apis/certmanager/validation/util"
@ -34,23 +36,24 @@ import (
// Validation functions for cert-manager Issuer types.
func ValidateIssuer(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
func ValidateIssuer(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
iss := obj.(*certmanager.Issuer)
allErrs := ValidateIssuerSpec(&iss.Spec, field.NewPath("spec"))
return allErrs
allErrs, warnings := ValidateIssuerSpec(&iss.Spec, field.NewPath("spec"))
return allErrs, warnings
}
func ValidateUpdateIssuer(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) field.ErrorList {
func ValidateUpdateIssuer(_ *admissionv1.AdmissionRequest, oldObj, obj runtime.Object) (field.ErrorList, validation.WarningList) {
iss := obj.(*certmanager.Issuer)
allErrs := ValidateIssuerSpec(&iss.Spec, field.NewPath("spec"))
return allErrs
allErrs, warnings := ValidateIssuerSpec(&iss.Spec, field.NewPath("spec"))
return allErrs, warnings
}
func ValidateIssuerSpec(iss *certmanager.IssuerSpec, fldPath *field.Path) field.ErrorList {
func ValidateIssuerSpec(iss *certmanager.IssuerSpec, fldPath *field.Path) (field.ErrorList, validation.WarningList) {
return ValidateIssuerConfig(&iss.IssuerConfig, fldPath)
}
func ValidateIssuerConfig(iss *certmanager.IssuerConfig, fldPath *field.Path) field.ErrorList {
func ValidateIssuerConfig(iss *certmanager.IssuerConfig, fldPath *field.Path) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
numConfigs := 0
el := field.ErrorList{}
if iss.ACME != nil {
@ -58,7 +61,8 @@ func ValidateIssuerConfig(iss *certmanager.IssuerConfig, fldPath *field.Path) fi
el = append(el, field.Forbidden(fldPath.Child("acme"), "may not specify more than one issuer type"))
} else {
numConfigs++
el = append(el, ValidateACMEIssuerConfig(iss.ACME, fldPath.Child("acme"))...)
e, w := ValidateACMEIssuerConfig(iss.ACME, fldPath.Child("acme"))
el, warnings = append(el, e...), append(warnings, w...)
}
}
if iss.CA != nil {
@ -97,10 +101,11 @@ func ValidateIssuerConfig(iss *certmanager.IssuerConfig, fldPath *field.Path) fi
el = append(el, field.Required(fldPath, "at least one issuer must be configured"))
}
return el
return el, warnings
}
func ValidateACMEIssuerConfig(iss *cmacme.ACMEIssuer, fldPath *field.Path) field.ErrorList {
func ValidateACMEIssuerConfig(iss *cmacme.ACMEIssuer, fldPath *field.Path) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
el := field.ErrorList{}
if len(iss.PrivateKey.Name) == 0 {
el = append(el, field.Required(fldPath.Child("privateKeySecretRef", "name"), "private key secret name is a required field"))
@ -122,7 +127,7 @@ func ValidateACMEIssuerConfig(iss *cmacme.ACMEIssuer, fldPath *field.Path) field
el = append(el, ValidateACMEIssuerChallengeSolverConfig(&sol, fldPath.Child("solvers").Index(i))...)
}
return el
return el, warnings
}
func ValidateACMEIssuerChallengeSolverConfig(sol *cmacme.ACMEChallengeSolver, fldPath *field.Path) field.ErrorList {

18
pkg/internal/apis/certmanager/validation/issuer_test.go
View File

@ -25,6 +25,8 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
apiutil "github.com/jetstack/cert-manager/pkg/api/util"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
cmacme "github.com/jetstack/cert-manager/pkg/internal/apis/acme"
cmapi "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager"
cmmeta "github.com/jetstack/cert-manager/pkg/internal/apis/meta"
@ -106,8 +108,9 @@ func TestValidateVaultIssuerConfig(t *testing.T) {
func TestValidateACMEIssuerConfig(t *testing.T) {
fldPath := field.NewPath("")
scenarios := map[string]struct {
spec *cmacme.ACMEIssuer
errs []*field.Error
spec *cmacme.ACMEIssuer
errs []*field.Error
warnings validation.WarningList
}{
"valid acme issuer": {
spec: &validACMEIssuer,
@ -316,7 +319,7 @@ func TestValidateACMEIssuerConfig(t *testing.T) {
}
for n, s := range scenarios {
t.Run(n, func(t *testing.T) {
errs := ValidateACMEIssuerConfig(s.spec, fldPath)
errs, warnings := ValidateACMEIssuerConfig(s.spec, fldPath)
if len(errs) != len(s.errs) {
t.Errorf("Expected %v but got %v", s.errs, errs)
return
@ -327,6 +330,7 @@ func TestValidateACMEIssuerConfig(t *testing.T) {
t.Errorf("Expected %v but got %v", expectedErr, e)
}
}
assert.Equal(t, s.warnings, warnings)
})
}
}
@ -334,8 +338,9 @@ func TestValidateACMEIssuerConfig(t *testing.T) {
func TestValidateIssuerSpec(t *testing.T) {
fldPath := field.NewPath("")
scenarios := map[string]struct {
spec *cmapi.IssuerSpec
errs field.ErrorList
spec *cmapi.IssuerSpec
errs field.ErrorList
warnings validation.WarningList
}{
"valid ca issuer": {
spec: &cmapi.IssuerSpec{
@ -427,8 +432,9 @@ func TestValidateIssuerSpec(t *testing.T) {
}
for n, s := range scenarios {
t.Run(n, func(t *testing.T) {
gotErrs := ValidateIssuerSpec(s.spec, fldPath)
gotErrs, warnings := ValidateIssuerSpec(s.spec, fldPath)
assert.Equal(t, s.errs, gotErrs)
assert.Equal(t, s.warnings, warnings)
})
}
}

7
pkg/webhook/handlers/testdata/apis/testgroup/v2/validation.go vendored
View File

@ -20,13 +20,16 @@ import (
admissionv1 "k8s.io/api/admission/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
)
func ValidateTestType(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
func ValidateTestType(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
el := field.ErrorList{}
warnings := validation.WarningList{}
tt := obj.(*TestType)
if tt.TestField == DisallowedTestFieldValue {
el = append(el, field.Invalid(field.NewPath("testField"), tt.TestField, "value not allowed"))
}
return el
return el, warnings
}

1
pkg/webhook/handlers/testdata/apis/testgroup/validation/BUILD.bazel vendored
View File

@ -23,6 +23,7 @@ go_test(
srcs = ["validation_test.go"],
embed = [":go_default_library"],
deps = [
"//pkg/internal/api/validation:go_default_library",
"//pkg/webhook/handlers/testdata/apis/testgroup:go_default_library",
"//pkg/webhook/handlers/testdata/apis/testgroup/v1:go_default_library",
"@io_k8s_apimachinery//pkg/util/validation/field:go_default_library",

13
pkg/webhook/handlers/testdata/apis/testgroup/validation/validation.go vendored
View File

@ -21,29 +21,32 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
"github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup"
v1 "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v1"
)
func ValidateTestType(_ *admissionv1.AdmissionRequest, obj runtime.Object) field.ErrorList {
func ValidateTestType(_ *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
testType := obj.(*testgroup.TestType)
el := field.ErrorList{}
if testType.TestField == v1.TestFieldValueNotAllowed {
el = append(el, field.Invalid(field.NewPath("testField"), testType.TestField, "invalid value"))
}
return el
return el, warnings
}
func ValidateTestTypeUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) field.ErrorList {
func ValidateTestTypeUpdate(_ *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, validation.WarningList) {
var warnings validation.WarningList
old, ok := oldObj.(*testgroup.TestType)
new := newObj.(*testgroup.TestType)
// if oldObj is not set, the Update operation is always valid.
if !ok || old == nil {
return nil
return nil, warnings
}
el := field.ErrorList{}
if old.TestFieldImmutable != "" && old.TestFieldImmutable != new.TestFieldImmutable {
el = append(el, field.Forbidden(field.NewPath("testFieldImmutable"), "field is immutable once set"))
}
return el
return el, warnings
}

45
pkg/webhook/handlers/testdata/apis/testgroup/validation/validation_test.go vendored
View File

@ -22,14 +22,16 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"github.com/jetstack/cert-manager/pkg/internal/api/validation"
"github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup"
v1 "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v1"
)
func TestValidateTestType(t *testing.T) {
scenarios := map[string]struct {
obj *testgroup.TestType
errs []*field.Error
obj *testgroup.TestType
errs []*field.Error
warnings validation.WarningList
}{
"does not allow testField to be TestFieldValueNotAllowed": {
obj: &testgroup.TestType{
@ -42,17 +44,20 @@ func TestValidateTestType(t *testing.T) {
}
for n, s := range scenarios {
t.Run(n, func(t *testing.T) {
errs := ValidateTestType(nil, s.obj)
errs, warnings := ValidateTestType(nil, s.obj)
if len(errs) != len(s.errs) {
t.Errorf("Expected %v but got %v", s.errs, errs)
t.Errorf("Expected errors %v but got %v", s.errs, errs)
return
}
for i, e := range errs {
expectedErr := s.errs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected error %v but got %v", expectedErr, e)
}
}
if !reflect.DeepEqual(warnings, s.warnings) {
t.Errorf("Expected warnings %+#v but got %+#v", s.warnings, warnings)
}
})
}
}
@ -65,6 +70,7 @@ func TestValidateTestTypeUpdate(t *testing.T) {
scenarios := map[string]struct {
old, new *testgroup.TestType
errs []*field.Error
warnings validation.WarningList
}{
"allows all updates if old is nil": {
new: &testgroup.TestType{
@ -74,17 +80,20 @@ func TestValidateTestTypeUpdate(t *testing.T) {
}
for n, s := range scenarios {
t.Run(n, func(t *testing.T) {
errs := ValidateTestTypeUpdate(nil, s.old, s.new)
errs, warnings := ValidateTestTypeUpdate(nil, s.old, s.new)
if len(errs) != len(s.errs) {
t.Errorf("Expected %v but got %v", s.errs, errs)
t.Errorf("Expected errors %v but got %v", s.errs, errs)
return
}
for i, e := range errs {
expectedErr := s.errs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected error %v but got %v", expectedErr, e)
}
}
if !reflect.DeepEqual(warnings, s.warnings) {
t.Errorf("Expected warnings %+#v but got %+#v", s.warnings, warnings)
}
})
}
}
@ -102,6 +111,7 @@ const (
// is not set.
func testImmutableTestTypeField(t *testing.T, fldPath *field.Path, setter func(*testgroup.TestType, testValue)) {
t.Run("should reject updates to "+fldPath.String(), func(t *testing.T) {
var expectedWarnings validation.WarningList
expectedErrs := []*field.Error{
field.Forbidden(fldPath, "field is immutable once set"),
}
@ -109,34 +119,41 @@ func testImmutableTestTypeField(t *testing.T, fldPath *field.Path, setter func(*
new := &testgroup.TestType{}
setter(old, testValueOptionOne)
setter(new, testValueOptionTwo)
errs := ValidateTestTypeUpdate(nil, old, new)
errs, warnings := ValidateTestTypeUpdate(nil, old, new)
if len(errs) != len(expectedErrs) {
t.Errorf("Expected %v but got %v", expectedErrs, errs)
t.Errorf("Expected errors %v but got %v", expectedErrs, errs)
return
}
for i, e := range errs {
expectedErr := expectedErrs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected error %v but got %v", expectedErr, e)
}
}
if !reflect.DeepEqual(warnings, expectedWarnings) {
t.Errorf("Expected warnings %+#v got %+#v", expectedWarnings, warnings)
}
})
t.Run("should allow updates to "+fldPath.String()+" if not already set", func(t *testing.T) {
var expectedWarnings validation.WarningList
expectedErrs := []*field.Error{}
old := &testgroup.TestType{}
new := &testgroup.TestType{}
setter(old, testValueNone)
setter(new, testValueOptionOne)
errs := ValidateTestTypeUpdate(nil, old, new)
errs, warnings := ValidateTestTypeUpdate(nil, old, new)
if len(errs) != len(expectedErrs) {
t.Errorf("Expected %v but got %v", expectedErrs, errs)
t.Errorf("Expected errors %v but got %v", expectedErrs, errs)
return
}
for i, e := range errs {
expectedErr := expectedErrs[i]
if !reflect.DeepEqual(e, expectedErr) {
t.Errorf("Expected %v but got %v", expectedErr, e)
t.Errorf("Expected error %v but got %v", expectedErr, e)
}
}
if !reflect.DeepEqual(warnings, expectedWarnings) {
t.Errorf("Expected warnings %+#v but got %+#v", expectedWarnings, warnings)
}
})
}

10
pkg/webhook/handlers/validation.go
View File

@ -101,15 +101,19 @@ func (r *registryBackedValidator) Validate(ctx context.Context, admissionSpec *a
}
}
errs := field.ErrorList{}
var warnings validation.WarningList
if admissionSpec.Operation == admissionv1.Create {
// perform validation on new version of resource
errs = append(errs, r.registry.Validate(admissionSpec, obj, gvk)...)
e, w := r.registry.Validate(admissionSpec, obj, gvk)
errs, warnings = append(errs, e...), append(warnings, w...)
} else if admissionSpec.Operation == admissionv1.Update {
// perform update validation on resource
errs = append(errs, r.registry.ValidateUpdate(admissionSpec, oldObj, obj, gvk)...)
e, w := r.registry.ValidateUpdate(admissionSpec, oldObj, obj, gvk)
errs, warnings = append(errs, e...), append(warnings, w...)
}
// TODO: implement warnings for Plugin interface
// If no validation errors occurred, perform plugin checks.
if len(errs) == 0 {
for _, plugin := range r.plugins {
@ -119,6 +123,8 @@ func (r *registryBackedValidator) Validate(ctx context.Context, admissionSpec *a
}
}
status.Warnings = warnings
// return with allowed = false if any errors occurred
if err := errs.ToAggregate(); err != nil {
status.Allowed = false