From c5ee500a2ed295acf35e1cfd4e736b039e7c2130 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Mon, 14 Oct 2019 17:11:57 +0100 Subject: [PATCH] Add unit tests for validation webhook handler Signed-off-by: James Munnelly --- pkg/webhook/handlers/BUILD.bazel | 11 +- pkg/webhook/handlers/mutation_test.go | 8 +- .../testdata/apis/testgroup/BUILD.bazel | 1 + .../handlers/testdata/apis/testgroup/types.go | 5 + .../testdata/apis/testgroup/v1/types.go | 9 + .../testgroup/v1/zz_generated.conversion.go | 2 + .../apis/testgroup/validation/BUILD.bazel | 37 ++++ .../apis/testgroup/validation/validation.go | 47 ++++++ .../testgroup/validation/validation_test.go | 141 ++++++++++++++++ pkg/webhook/handlers/validation_test.go | 158 ++++++++++++++++++ 10 files changed, 414 insertions(+), 5 deletions(-) create mode 100644 pkg/webhook/handlers/testdata/apis/testgroup/validation/BUILD.bazel create mode 100644 pkg/webhook/handlers/testdata/apis/testgroup/validation/validation.go create mode 100644 pkg/webhook/handlers/testdata/apis/testgroup/validation/validation_test.go create mode 100644 pkg/webhook/handlers/validation_test.go diff --git a/pkg/webhook/handlers/BUILD.bazel b/pkg/webhook/handlers/BUILD.bazel index 1fe069013..bf626c5f7 100644 --- a/pkg/webhook/handlers/BUILD.bazel +++ b/pkg/webhook/handlers/BUILD.bazel @@ -24,14 +24,21 @@ go_library( go_test( name = "go_default_test", - srcs = ["mutation_test.go"], + srcs = [ + "mutation_test.go", + "validation_test.go", + ], embed = [":go_default_library"], deps = [ + "//pkg/webhook/handlers/testdata/apis/testgroup:go_default_library", "//pkg/webhook/handlers/testdata/apis/testgroup/install:go_default_library", + "//pkg/webhook/handlers/testdata/apis/testgroup/v1:go_default_library", + "//pkg/webhook/handlers/testdata/apis/testgroup/validation:go_default_library", "@com_github_mattbaird_jsonpatch//:go_default_library", "@io_k8s_api//admission/v1beta1:go_default_library", + "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_apimachinery//pkg/runtime:go_default_library", - "@io_k8s_klog//:go_default_library", + "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library", "@io_k8s_klog//klogr:go_default_library", "@io_k8s_utils//diff:go_default_library", ], diff --git a/pkg/webhook/handlers/mutation_test.go b/pkg/webhook/handlers/mutation_test.go index df759417b..a1dddf942 100644 --- a/pkg/webhook/handlers/mutation_test.go +++ b/pkg/webhook/handlers/mutation_test.go @@ -18,14 +18,12 @@ package handlers import ( "encoding/json" - "flag" "reflect" "testing" "github.com/mattbaird/jsonpatch" admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/klog" "k8s.io/klog/klogr" "k8s.io/utils/diff" @@ -50,7 +48,6 @@ func TestDefaultCertificate(t *testing.T) { install.Install(scheme) log := klogr.New() - klog.InitFlags(flag.CommandLine) c := NewSchemeBackedDefaulter(log, scheme) tests := map[string]testT{ "apply defaults to TestType": { @@ -77,6 +74,11 @@ func TestDefaultCertificate(t *testing.T) { Path: "/testField", Value: "", }, + jsonpatch.JsonPatchOperation{ + Operation: "add", + Path: "/testFieldImmutable", + Value: "", + }, jsonpatch.JsonPatchOperation{ Operation: "add", Path: "/testFieldPtr", diff --git a/pkg/webhook/handlers/testdata/apis/testgroup/BUILD.bazel b/pkg/webhook/handlers/testdata/apis/testgroup/BUILD.bazel index cce41eed4..30aef1a0a 100644 --- a/pkg/webhook/handlers/testdata/apis/testgroup/BUILD.bazel +++ b/pkg/webhook/handlers/testdata/apis/testgroup/BUILD.bazel @@ -31,6 +31,7 @@ filegroup( "//pkg/webhook/handlers/testdata/apis/testgroup/fuzzer:all-srcs", "//pkg/webhook/handlers/testdata/apis/testgroup/install:all-srcs", "//pkg/webhook/handlers/testdata/apis/testgroup/v1:all-srcs", + "//pkg/webhook/handlers/testdata/apis/testgroup/validation:all-srcs", ], tags = ["automanaged"], visibility = ["//visibility:public"], diff --git a/pkg/webhook/handlers/testdata/apis/testgroup/types.go b/pkg/webhook/handlers/testdata/apis/testgroup/types.go index 39d727b84..f1bb5f34a 100644 --- a/pkg/webhook/handlers/testdata/apis/testgroup/types.go +++ b/pkg/webhook/handlers/testdata/apis/testgroup/types.go @@ -26,6 +26,11 @@ type TestType struct { metav1.TypeMeta metav1.ObjectMeta + // TestField is used in tests. + // Validation doesn't allow this to be set to the value of TestFieldValueNotAllowed. TestField string TestFieldPtr *string + + // TestFieldImmutable cannot be changed after being set to a non-zero value + TestFieldImmutable string } diff --git a/pkg/webhook/handlers/testdata/apis/testgroup/v1/types.go b/pkg/webhook/handlers/testdata/apis/testgroup/v1/types.go index ac6c2758e..9d2c908b7 100644 --- a/pkg/webhook/handlers/testdata/apis/testgroup/v1/types.go +++ b/pkg/webhook/handlers/testdata/apis/testgroup/v1/types.go @@ -26,6 +26,15 @@ type TestType struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata"` + // TestField is used in tests. + // Validation doesn't allow this to be set to the value of TestFieldValueNotAllowed. TestField string `json:"testField"` TestFieldPtr *string `json:"testFieldPtr"` + + // TestFieldImmutable cannot be changed after being set to a non-zero value + TestFieldImmutable string `json:"testFieldImmutable"` } + +const ( + TestFieldValueNotAllowed = "not-allowed-value" +) diff --git a/pkg/webhook/handlers/testdata/apis/testgroup/v1/zz_generated.conversion.go b/pkg/webhook/handlers/testdata/apis/testgroup/v1/zz_generated.conversion.go index 6cf621bc6..8dc8c2d52 100644 --- a/pkg/webhook/handlers/testdata/apis/testgroup/v1/zz_generated.conversion.go +++ b/pkg/webhook/handlers/testdata/apis/testgroup/v1/zz_generated.conversion.go @@ -52,6 +52,7 @@ func autoConvert_v1_TestType_To_testgroup_TestType(in *TestType, out *testgroup. out.ObjectMeta = in.ObjectMeta out.TestField = in.TestField out.TestFieldPtr = (*string)(unsafe.Pointer(in.TestFieldPtr)) + out.TestFieldImmutable = in.TestFieldImmutable return nil } @@ -64,6 +65,7 @@ func autoConvert_testgroup_TestType_To_v1_TestType(in *testgroup.TestType, out * out.ObjectMeta = in.ObjectMeta out.TestField = in.TestField out.TestFieldPtr = (*string)(unsafe.Pointer(in.TestFieldPtr)) + out.TestFieldImmutable = in.TestFieldImmutable return nil } diff --git a/pkg/webhook/handlers/testdata/apis/testgroup/validation/BUILD.bazel b/pkg/webhook/handlers/testdata/apis/testgroup/validation/BUILD.bazel new file mode 100644 index 000000000..5feb53a62 --- /dev/null +++ b/pkg/webhook/handlers/testdata/apis/testgroup/validation/BUILD.bazel @@ -0,0 +1,37 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["validation.go"], + importpath = "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/validation", + visibility = ["//visibility:public"], + deps = [ + "//pkg/webhook/handlers/testdata/apis/testgroup/v1:go_default_library", + "@io_k8s_apimachinery//pkg/runtime:go_default_library", + "@io_k8s_apimachinery//pkg/util/validation/field:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["validation_test.go"], + embed = [":go_default_library"], + deps = [ + "//pkg/webhook/handlers/testdata/apis/testgroup/v1:go_default_library", + "@io_k8s_apimachinery//pkg/util/validation/field:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/webhook/handlers/testdata/apis/testgroup/validation/validation.go b/pkg/webhook/handlers/testdata/apis/testgroup/validation/validation.go new file mode 100644 index 000000000..0c93c4020 --- /dev/null +++ b/pkg/webhook/handlers/testdata/apis/testgroup/validation/validation.go @@ -0,0 +1,47 @@ +/* +Copyright 2019 The Jetstack cert-manager contributors. + +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 + +import ( + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + + v1 "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v1" +) + +func ValidateTestType(obj runtime.Object) field.ErrorList { + testType := obj.(*v1.TestType) + el := field.ErrorList{} + if testType.TestField == v1.TestFieldValueNotAllowed { + el = append(el, field.Invalid(field.NewPath("testField"), testType.TestField, "invalid value")) + } + return el +} + +func ValidateTestTypeUpdate(oldObj, newObj runtime.Object) field.ErrorList { + old, ok := oldObj.(*v1.TestType) + new := newObj.(*v1.TestType) + // if oldObj is not set, the Update operation is always valid. + if !ok || old == nil { + return nil + } + 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 +} diff --git a/pkg/webhook/handlers/testdata/apis/testgroup/validation/validation_test.go b/pkg/webhook/handlers/testdata/apis/testgroup/validation/validation_test.go new file mode 100644 index 000000000..dfda8f877 --- /dev/null +++ b/pkg/webhook/handlers/testdata/apis/testgroup/validation/validation_test.go @@ -0,0 +1,141 @@ +/* +Copyright 2019 The Jetstack cert-manager contributors. + +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 + +import ( + "reflect" + "testing" + + "k8s.io/apimachinery/pkg/util/validation/field" + + v1 "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v1" +) + +func TestValidateTestType(t *testing.T) { + scenarios := map[string]struct { + obj *v1.TestType + errs []*field.Error + }{ + "does not allow testField to be TestFieldValueNotAllowed": { + obj: &v1.TestType{ + TestField: v1.TestFieldValueNotAllowed, + }, + errs: []*field.Error{ + field.Invalid(field.NewPath("testField"), v1.TestFieldValueNotAllowed, "invalid value"), + }, + }, + } + for n, s := range scenarios { + t.Run(n, func(t *testing.T) { + errs := ValidateTestType(s.obj) + if len(errs) != len(s.errs) { + t.Errorf("Expected %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) + } + } + }) + } +} + +func TestValidateTestTypeUpdate(t *testing.T) { + testImmutableTestTypeField(t, field.NewPath("testFieldImmutable"), func(obj *v1.TestType, s testValue) { + obj.TestFieldImmutable = string(s) + }) + + scenarios := map[string]struct { + old, new *v1.TestType + errs []*field.Error + }{ + "allows all updates if old is nil": { + new: &v1.TestType{ + TestFieldImmutable: "abc", + }, + }, + } + for n, s := range scenarios { + t.Run(n, func(t *testing.T) { + errs := ValidateTestTypeUpdate(s.old, s.new) + if len(errs) != len(s.errs) { + t.Errorf("Expected %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) + } + } + }) + } +} + +type testValue string + +const ( + testValueNone = "" + testValueOptionOne = "one" + testValueOptionTwo = "two" +) + +// testImmutableOrderField will test that the field at path fldPath does +// not allow changes after being set, but does allow changes if the old field +// is not set. +func testImmutableTestTypeField(t *testing.T, fldPath *field.Path, setter func(*v1.TestType, testValue)) { + t.Run("should reject updates to "+fldPath.String(), func(t *testing.T) { + expectedErrs := []*field.Error{ + field.Forbidden(fldPath, "field is immutable once set"), + } + old := &v1.TestType{} + new := &v1.TestType{} + setter(old, testValueOptionOne) + setter(new, testValueOptionTwo) + errs := ValidateTestTypeUpdate(old, new) + if len(errs) != len(expectedErrs) { + t.Errorf("Expected %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.Run("should allow updates to "+fldPath.String()+" if not already set", func(t *testing.T) { + expectedErrs := []*field.Error{} + old := &v1.TestType{} + new := &v1.TestType{} + setter(old, testValueNone) + setter(new, testValueOptionOne) + errs := ValidateTestTypeUpdate(old, new) + if len(errs) != len(expectedErrs) { + t.Errorf("Expected %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) + } + } + }) +} diff --git a/pkg/webhook/handlers/validation_test.go b/pkg/webhook/handlers/validation_test.go new file mode 100644 index 000000000..42f15cb14 --- /dev/null +++ b/pkg/webhook/handlers/validation_test.go @@ -0,0 +1,158 @@ +/* +Copyright 2019 The Jetstack cert-manager contributors. + +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 handlers + +import ( + "fmt" + "net/http" + "testing" + + admissionv1beta1 "k8s.io/api/admission/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/klog/klogr" + + "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup" + "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/install" + "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v1" + "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/validation" +) + +func TestFuncBackedValidator(t *testing.T) { + scheme := runtime.NewScheme() + install.Install(scheme) + + log := klogr.New() + c := NewFuncBackedValidator(log, scheme, map[schema.GroupKind]Validator{ + {Group: testgroup.GroupName, Kind: "TestType"}: ValidatorFunc(&v1.TestType{}, validation.ValidateTestType, validation.ValidateTestTypeUpdate), + }) + testTypeGVK := metav1.GroupVersionKind{ + Group: v1.SchemeGroupVersion.Group, + Version: v1.SchemeGroupVersion.Version, + Kind: "TestType", + } + tests := map[string]testT{ + "should not allow invalid value for 'testField' field": { + inputRequest: admissionv1beta1.AdmissionRequest{ + Kind: testTypeGVK, + Object: runtime.RawExtension{ + Raw: []byte(fmt.Sprintf(` +{ + "apiVersion": "testgroup.testing.cert-manager.io/v1", + "kind": "TestType", + "metadata": { + "name": "testing", + "namespace": "abc", + "creationTimestamp": null + }, + "testField": "%s" +} +`, v1.TestFieldValueNotAllowed)), + }, + }, + expectedResponse: admissionv1beta1.AdmissionResponse{ + Allowed: false, + Result: &metav1.Status{ + Status: metav1.StatusFailure, Code: http.StatusNotAcceptable, Reason: metav1.StatusReasonNotAcceptable, + Message: "testField: Invalid value: \"not-allowed-value\": invalid value", + }, + }, + }, + "should allow setting immutable field if it is not already set": { + inputRequest: admissionv1beta1.AdmissionRequest{ + Kind: testTypeGVK, + OldObject: runtime.RawExtension{ + Raw: []byte(fmt.Sprintf(` +{ + "apiVersion": "testgroup.testing.cert-manager.io/v1", + "kind": "TestType", + "metadata": { + "name": "testing", + "namespace": "abc", + "creationTimestamp": null + } +} +`)), + }, + Object: runtime.RawExtension{ + Raw: []byte(fmt.Sprintf(` +{ + "apiVersion": "testgroup.testing.cert-manager.io/v1", + "kind": "TestType", + "metadata": { + "name": "testing", + "namespace": "abc", + "creationTimestamp": null + }, + "testFieldImmutable": "abc" +} +`)), + }, + }, + expectedResponse: admissionv1beta1.AdmissionResponse{ + Allowed: true, + }, + }, + "should not allow setting immutable field if it is already set": { + inputRequest: admissionv1beta1.AdmissionRequest{ + Kind: testTypeGVK, + OldObject: runtime.RawExtension{ + Raw: []byte(fmt.Sprintf(` +{ + "apiVersion": "testgroup.testing.cert-manager.io/v1", + "kind": "TestType", + "metadata": { + "name": "testing", + "namespace": "abc", + "creationTimestamp": null + }, + "testFieldImmutable": "oldvalue" +} +`)), + }, + Object: runtime.RawExtension{ + Raw: []byte(fmt.Sprintf(` +{ + "apiVersion": "testgroup.testing.cert-manager.io/v1", + "kind": "TestType", + "metadata": { + "name": "testing", + "namespace": "abc", + "creationTimestamp": null + }, + "testFieldImmutable": "abc" +} +`)), + }, + }, + expectedResponse: admissionv1beta1.AdmissionResponse{ + Allowed: false, + Result: &metav1.Status{ + Status: metav1.StatusFailure, Code: http.StatusNotAcceptable, Reason: metav1.StatusReasonNotAcceptable, + Message: "testFieldImmutable: Forbidden: field is immutable once set", + }, + }, + }, + } + + for n, test := range tests { + t.Run(n, func(t *testing.T) { + runTest(t, c.Validate, test) + }) + } +}