From 50686d3e267bd7d5fe1329f593fe734cb6f336cc Mon Sep 17 00:00:00 2001 From: Maartje Eyskens Date: Mon, 10 Aug 2020 16:39:49 +0200 Subject: [PATCH] Support v1 admission webhooks Signed-off-by: Maartje Eyskens --- build/helm.bzl | 2 + .../templates/webhook-mutating-webhook.yaml | 9 ++++ .../templates/webhook-validating-webhook.yaml | 9 ++++ deploy/manifests/BUILD.bazel | 3 ++ pkg/webhook/handlers/BUILD.bazel | 4 +- pkg/webhook/handlers/interfaces.go | 6 +-- pkg/webhook/handlers/mutation.go | 8 +-- pkg/webhook/handlers/mutation_test.go | 14 ++--- pkg/webhook/handlers/validation.go | 6 +-- pkg/webhook/handlers/validation_test.go | 34 ++++++------- pkg/webhook/server/BUILD.bazel | 1 + pkg/webhook/server/server.go | 51 +++++++++++++++---- 12 files changed, 101 insertions(+), 46 deletions(-) diff --git a/build/helm.bzl b/build/helm.bzl index 400b105af..7d76794fd 100644 --- a/build/helm.bzl +++ b/build/helm.bzl @@ -94,6 +94,7 @@ def helm_tmpl( helm_pkg, release_namespace, release_name, + additional_api_versions = "", values = {}, helm_cmd = "//hack/bin:helm", **kwargs, @@ -105,6 +106,7 @@ def helm_tmpl( tmpl_cmd = [ "$(location %s)" % helm_cmd, "template", + "--api-versions=\"%s\"" % additional_api_versions, "--namespace=%s" % release_namespace, release_name, "$(location %s)" % helm_pkg, diff --git a/deploy/charts/cert-manager/templates/webhook-mutating-webhook.yaml b/deploy/charts/cert-manager/templates/webhook-mutating-webhook.yaml index 511371805..494aaf02f 100644 --- a/deploy/charts/cert-manager/templates/webhook-mutating-webhook.yaml +++ b/deploy/charts/cert-manager/templates/webhook-mutating-webhook.yaml @@ -1,4 +1,10 @@ +{{- $isV1AdmissionRegistration := false -}} +{{- if (or (not (.Capabilities.APIVersions.Has "admissionregistration.k8s.io/v1")) (.Capabilities.APIVersions.Has "hacking-helm.i-wish-this-wasnt-required.cert-manager.io/use-v1beta1-webhooks") ) }} apiVersion: admissionregistration.k8s.io/v1beta1 +{{- else }} +{{- $isV1AdmissionRegistration = true -}} +apiVersion: admissionregistration.k8s.io/v1 +{{- end }} kind: MutatingWebhookConfiguration metadata: name: {{ include "webhook.fullname" . }} @@ -27,6 +33,9 @@ webhooks: - UPDATE resources: - "*/*" + {{- if $isV1AdmissionRegistration }} + admissionReviewVersions: ["v1", "v1beta1"] + {{- end }} failurePolicy: Fail {{- if (semverCompare ">=1.12-0" .Capabilities.KubeVersion.GitVersion) }} # Only include 'sideEffects' field in Kubernetes 1.12+ diff --git a/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml b/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml index 9184c65e4..be7952a10 100644 --- a/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml +++ b/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml @@ -1,4 +1,10 @@ +{{- $isV1AdmissionRegistration := false -}} +{{- if (or (not (.Capabilities.APIVersions.Has "admissionregistration.k8s.io/v1")) (.Capabilities.APIVersions.Has "hacking-helm.i-wish-this-wasnt-required.cert-manager.io/use-v1beta1-webhooks") ) }} apiVersion: admissionregistration.k8s.io/v1beta1 +{{- else }} +{{- $isV1AdmissionRegistration = true -}} +apiVersion: admissionregistration.k8s.io/v1 +{{- end }} kind: ValidatingWebhookConfiguration metadata: name: {{ include "webhook.fullname" . }} @@ -37,6 +43,9 @@ webhooks: - UPDATE resources: - "*/*" + {{- if $isV1AdmissionRegistration }} + admissionReviewVersions: ["v1", "v1beta1"] + {{- end }} failurePolicy: Fail {{- if (semverCompare ">=1.12-0" .Capabilities.KubeVersion.GitVersion) }} # Only include 'sideEffects' field in Kubernetes 1.12+ diff --git a/deploy/manifests/BUILD.bazel b/deploy/manifests/BUILD.bazel index 51652bd91..d9d7aa1c4 100644 --- a/deploy/manifests/BUILD.bazel +++ b/deploy/manifests/BUILD.bazel @@ -13,12 +13,14 @@ RELEASE_NAMESPACE = "cert-manager" VARIANTS = { "cert-manager": { "crd_variant": "regular", + "additional_api_versions": "", "values": { "installCRDs": "false", }, }, "cert-manager-legacy": { "crd_variant": "legacy", + "additional_api_versions": "hacking-helm.i-wish-this-wasnt-required.cert-manager.io/use-v1beta1-webhooks", "values": { "installCRDs": "false", }, @@ -27,6 +29,7 @@ VARIANTS = { [helm_tmpl( name = "%s.manifests" % name, + additional_api_versions = meta["additional_api_versions"], helm_pkg = "//deploy/charts/cert-manager", release_name = RELEASE_NAME, release_namespace = RELEASE_NAMESPACE, diff --git a/pkg/webhook/handlers/BUILD.bazel b/pkg/webhook/handlers/BUILD.bazel index 6001ab6fd..ae6b9c22e 100644 --- a/pkg/webhook/handlers/BUILD.bazel +++ b/pkg/webhook/handlers/BUILD.bazel @@ -15,7 +15,7 @@ go_library( "//pkg/logs:go_default_library", "@com_github_go_logr_logr//:go_default_library", "@com_github_mattbaird_jsonpatch//:go_default_library", - "@io_k8s_api//admission/v1beta1:go_default_library", + "@io_k8s_api//admission/v1:go_default_library", "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1beta1:go_default_library", "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_apimachinery//pkg/runtime:go_default_library", @@ -42,7 +42,7 @@ go_test( "//pkg/webhook/handlers/testdata/apis/testgroup/v1:go_default_library", "//pkg/webhook/handlers/testdata/apis/testgroup/v2:go_default_library", "@com_github_mattbaird_jsonpatch//:go_default_library", - "@io_k8s_api//admission/v1beta1:go_default_library", + "@io_k8s_api//admission/v1:go_default_library", "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1beta1:go_default_library", "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_apimachinery//pkg/runtime:go_default_library", diff --git a/pkg/webhook/handlers/interfaces.go b/pkg/webhook/handlers/interfaces.go index 58c252ad2..391af5c02 100644 --- a/pkg/webhook/handlers/interfaces.go +++ b/pkg/webhook/handlers/interfaces.go @@ -17,20 +17,20 @@ limitations under the License. package handlers import ( - admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionv1 "k8s.io/api/admission/v1" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" ) type ValidatingAdmissionHook interface { // Validate is called to decide whether to accept the admission request. The returned AdmissionResponse // must not use the Patch field. - Validate(admissionSpec *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse + Validate(admissionSpec *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse } type MutatingAdmissionHook interface { // Admit is called to decide whether to accept the admission request. The returned AdmissionResponse may // use the Patch field to mutate the object from the passed AdmissionRequest. - Mutate(admissionSpec *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse + Mutate(admissionSpec *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse } type ConversionHook interface { diff --git a/pkg/webhook/handlers/mutation.go b/pkg/webhook/handlers/mutation.go index ee2e454df..d3a6e50c6 100644 --- a/pkg/webhook/handlers/mutation.go +++ b/pkg/webhook/handlers/mutation.go @@ -25,7 +25,7 @@ import ( "github.com/go-logr/logr" "github.com/mattbaird/jsonpatch" - admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -52,8 +52,8 @@ func NewSchemeBackedDefaulter(log logr.Logger, scheme *runtime.Scheme) *SchemeBa } } -func (c *SchemeBackedDefaulter) Mutate(admissionSpec *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { - status := &admissionv1beta1.AdmissionResponse{} +func (c *SchemeBackedDefaulter) Mutate(admissionSpec *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { + status := &admissionv1.AdmissionResponse{} status.UID = admissionSpec.UID // decode the raw object data @@ -101,7 +101,7 @@ func (c *SchemeBackedDefaulter) Mutate(admissionSpec *admissionv1beta1.Admission } // set the AdmissionReview status - jsonPatchType := admissionv1beta1.PatchTypeJSONPatch + jsonPatchType := admissionv1.PatchTypeJSONPatch status.Patch = patch status.PatchType = &jsonPatchType status.Allowed = true diff --git a/pkg/webhook/handlers/mutation_test.go b/pkg/webhook/handlers/mutation_test.go index e6ad49320..52d9f39bd 100644 --- a/pkg/webhook/handlers/mutation_test.go +++ b/pkg/webhook/handlers/mutation_test.go @@ -22,7 +22,7 @@ import ( "testing" "github.com/mattbaird/jsonpatch" - admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/klog/klogr" @@ -32,7 +32,7 @@ import ( ) var ( - jsonPatchType = admissionv1beta1.PatchTypeJSONPatch + jsonPatchType = admissionv1.PatchTypeJSONPatch ) func responseForOperations(ops ...jsonpatch.JsonPatchOperation) []byte { @@ -52,7 +52,7 @@ func TestDefaultCertificate(t *testing.T) { c := NewSchemeBackedDefaulter(log, scheme) tests := map[string]admissionTestT{ "apply defaults to TestType": { - inputRequest: admissionv1beta1.AdmissionRequest{ + inputRequest: admissionv1.AdmissionRequest{ UID: types.UID("abc"), Object: runtime.RawExtension{ Raw: []byte(` @@ -68,7 +68,7 @@ func TestDefaultCertificate(t *testing.T) { `), }, }, - expectedResponse: admissionv1beta1.AdmissionResponse{ + expectedResponse: admissionv1.AdmissionResponse{ UID: types.UID("abc"), Allowed: true, Patch: responseForOperations( @@ -101,11 +101,11 @@ func TestDefaultCertificate(t *testing.T) { } type admissionTestT struct { - inputRequest admissionv1beta1.AdmissionRequest - expectedResponse admissionv1beta1.AdmissionResponse + inputRequest admissionv1.AdmissionRequest + expectedResponse admissionv1.AdmissionResponse } -type admissionFn func(request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse +type admissionFn func(request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse func runAdmissionTest(t *testing.T, fn admissionFn, test admissionTestT) { resp := fn(&test.inputRequest) diff --git a/pkg/webhook/handlers/validation.go b/pkg/webhook/handlers/validation.go index dce77bcbd..049f5373a 100644 --- a/pkg/webhook/handlers/validation.go +++ b/pkg/webhook/handlers/validation.go @@ -20,7 +20,7 @@ import ( "net/http" "github.com/go-logr/logr" - admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -45,8 +45,8 @@ func NewRegistryBackedValidator(log logr.Logger, scheme *runtime.Scheme, registr } } -func (r *registryBackedValidator) Validate(admissionSpec *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse { - status := &admissionv1beta1.AdmissionResponse{} +func (r *registryBackedValidator) Validate(admissionSpec *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { + status := &admissionv1.AdmissionResponse{} status.UID = admissionSpec.UID // decode new version of object diff --git a/pkg/webhook/handlers/validation_test.go b/pkg/webhook/handlers/validation_test.go index cf512a743..d3d9d4daa 100644 --- a/pkg/webhook/handlers/validation_test.go +++ b/pkg/webhook/handlers/validation_test.go @@ -21,7 +21,7 @@ import ( "net/http" "testing" - admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -29,8 +29,8 @@ import ( "github.com/jetstack/cert-manager/pkg/internal/api/validation" "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/v2" + v1 "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v1" + v2 "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v2" ) func TestRegistryBackedValidator(t *testing.T) { @@ -53,7 +53,7 @@ func TestRegistryBackedValidator(t *testing.T) { } tests := map[string]admissionTestT{ "should not allow invalid value for 'testField' field": { - inputRequest: admissionv1beta1.AdmissionRequest{ + inputRequest: admissionv1.AdmissionRequest{ UID: types.UID("abc"), RequestKind: testTypeGVK, Object: runtime.RawExtension{ @@ -71,7 +71,7 @@ func TestRegistryBackedValidator(t *testing.T) { `, v1.TestFieldValueNotAllowed)), }, }, - expectedResponse: admissionv1beta1.AdmissionResponse{ + expectedResponse: admissionv1.AdmissionResponse{ UID: types.UID("abc"), Allowed: false, Result: &metav1.Status{ @@ -81,7 +81,7 @@ func TestRegistryBackedValidator(t *testing.T) { }, }, "should allow setting immutable field if it is not already set": { - inputRequest: admissionv1beta1.AdmissionRequest{ + inputRequest: admissionv1.AdmissionRequest{ RequestKind: testTypeGVK, OldObject: runtime.RawExtension{ Raw: []byte(fmt.Sprintf(` @@ -111,12 +111,12 @@ func TestRegistryBackedValidator(t *testing.T) { `)), }, }, - expectedResponse: admissionv1beta1.AdmissionResponse{ + expectedResponse: admissionv1.AdmissionResponse{ Allowed: true, }, }, "should not allow setting immutable field if it is already set": { - inputRequest: admissionv1beta1.AdmissionRequest{ + inputRequest: admissionv1.AdmissionRequest{ RequestKind: testTypeGVK, OldObject: runtime.RawExtension{ Raw: []byte(fmt.Sprintf(` @@ -147,7 +147,7 @@ func TestRegistryBackedValidator(t *testing.T) { `)), }, }, - expectedResponse: admissionv1beta1.AdmissionResponse{ + expectedResponse: admissionv1.AdmissionResponse{ Allowed: false, Result: &metav1.Status{ Status: metav1.StatusFailure, Code: http.StatusNotAcceptable, Reason: metav1.StatusReasonNotAcceptable, @@ -156,7 +156,7 @@ func TestRegistryBackedValidator(t *testing.T) { }, }, "should not allow setting immutable field if it is already set (v2)": { - inputRequest: admissionv1beta1.AdmissionRequest{ + inputRequest: admissionv1.AdmissionRequest{ RequestKind: testTypeGVKV2, OldObject: runtime.RawExtension{ Raw: []byte(fmt.Sprintf(` @@ -187,7 +187,7 @@ func TestRegistryBackedValidator(t *testing.T) { `)), }, }, - expectedResponse: admissionv1beta1.AdmissionResponse{ + expectedResponse: admissionv1.AdmissionResponse{ Allowed: false, Result: &metav1.Status{ Status: metav1.StatusFailure, Code: http.StatusNotAcceptable, Reason: metav1.StatusReasonNotAcceptable, @@ -196,7 +196,7 @@ func TestRegistryBackedValidator(t *testing.T) { }, }, "should not allow invalid value for 'testField' field in v2": { - inputRequest: admissionv1beta1.AdmissionRequest{ + inputRequest: admissionv1.AdmissionRequest{ UID: types.UID("abc"), RequestKind: testTypeGVKV2, Object: runtime.RawExtension{ @@ -214,7 +214,7 @@ func TestRegistryBackedValidator(t *testing.T) { `, v2.DisallowedTestFieldValue)), }, }, - expectedResponse: admissionv1beta1.AdmissionResponse{ + expectedResponse: admissionv1.AdmissionResponse{ UID: types.UID("abc"), Allowed: false, Result: &metav1.Status{ @@ -224,7 +224,7 @@ func TestRegistryBackedValidator(t *testing.T) { }, }, "should allow value for 'testField' field in v2 if requestKind is v1": { - inputRequest: admissionv1beta1.AdmissionRequest{ + inputRequest: admissionv1.AdmissionRequest{ UID: types.UID("abc"), RequestKind: testTypeGVK, Object: runtime.RawExtension{ @@ -242,13 +242,13 @@ func TestRegistryBackedValidator(t *testing.T) { `, v2.DisallowedTestFieldValue)), }, }, - expectedResponse: admissionv1beta1.AdmissionResponse{ + expectedResponse: admissionv1.AdmissionResponse{ UID: types.UID("abc"), Allowed: true, }, }, "should validate in the current APIVersion if RequestKind is not set (for Kubernetes <1.15 support)": { - inputRequest: admissionv1beta1.AdmissionRequest{ + inputRequest: admissionv1.AdmissionRequest{ UID: types.UID("abc"), Kind: *testTypeGVKV2, Object: runtime.RawExtension{ @@ -266,7 +266,7 @@ func TestRegistryBackedValidator(t *testing.T) { `, v2.DisallowedTestFieldValue)), }, }, - expectedResponse: admissionv1beta1.AdmissionResponse{ + expectedResponse: admissionv1.AdmissionResponse{ UID: types.UID("abc"), Allowed: false, Result: &metav1.Status{ diff --git a/pkg/webhook/server/BUILD.bazel b/pkg/webhook/server/BUILD.bazel index ff47f2d6b..38e76bbf3 100644 --- a/pkg/webhook/server/BUILD.bazel +++ b/pkg/webhook/server/BUILD.bazel @@ -10,6 +10,7 @@ go_library( "//pkg/webhook/handlers:go_default_library", "//pkg/webhook/server/tls:go_default_library", "@com_github_go_logr_logr//:go_default_library", + "@io_k8s_api//admission/v1:go_default_library", "@io_k8s_api//admission/v1beta1:go_default_library", "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1beta1:go_default_library", "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", diff --git a/pkg/webhook/server/server.go b/pkg/webhook/server/server.go index f93a4a45a..63a85b8e4 100644 --- a/pkg/webhook/server/server.go +++ b/pkg/webhook/server/server.go @@ -27,6 +27,7 @@ import ( "time" "github.com/go-logr/logr" + admissionv1 "k8s.io/api/admission/v1" admissionv1beta1 "k8s.io/api/admission/v1beta1" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -50,6 +51,7 @@ var ( func init() { admissionv1beta1.AddToScheme(defaultScheme) + admissionv1.AddToScheme(defaultScheme) apiextensionsv1beta1.AddToScheme(defaultScheme) // we need to add the options to empty v1 @@ -283,28 +285,52 @@ func (s *Server) scheme() *runtime.Scheme { return s.Scheme } -func (s *Server) validate(obj runtime.Object) runtime.Object { - review := obj.(*admissionv1beta1.AdmissionReview) +func (s *Server) validate(obj runtime.Object) (runtime.Object, error) { + outputVersion := admissionv1.SchemeGroupVersion + review, isV1 := obj.(*admissionv1.AdmissionReview) + if !isV1 { + outputVersion = admissionv1beta1.SchemeGroupVersion + reviewv1beta1 := obj.(*admissionv1beta1.AdmissionReview) + convertedReview, err := defaultScheme.ConvertToVersion(reviewv1beta1, admissionv1.SchemeGroupVersion) + if err != nil { + return nil, err + } + review = convertedReview.(*admissionv1.AdmissionReview) + } resp := s.ValidationWebhook.Validate(review.Request) review.Response = resp - return review + + versionedOutput, err := defaultScheme.ConvertToVersion(review, outputVersion) + return versionedOutput, err } -func (s *Server) mutate(obj runtime.Object) runtime.Object { - review := obj.(*admissionv1beta1.AdmissionReview) +func (s *Server) mutate(obj runtime.Object) (runtime.Object, error) { + outputVersion := admissionv1.SchemeGroupVersion + review, isV1 := obj.(*admissionv1.AdmissionReview) + if !isV1 { + outputVersion = admissionv1beta1.SchemeGroupVersion + reviewv1beta1 := obj.(*admissionv1beta1.AdmissionReview) + convertedReview, err := defaultScheme.ConvertToVersion(reviewv1beta1, admissionv1.SchemeGroupVersion) + if err != nil { + return nil, err + } + review = convertedReview.(*admissionv1.AdmissionReview) + } resp := s.MutationWebhook.Mutate(review.Request) review.Response = resp - return review + + versionedOutput, err := defaultScheme.ConvertToVersion(review, outputVersion) + return versionedOutput, err } -func (s *Server) convert(obj runtime.Object) runtime.Object { +func (s *Server) convert(obj runtime.Object) (runtime.Object, error) { review := obj.(*apiextensionsv1beta1.ConversionReview) resp := s.ConversionWebhook.Convert(review.Request) review.Response = resp - return review + return review, nil } -func (s *Server) handle(inner func(runtime.Object) runtime.Object) func(w http.ResponseWriter, req *http.Request) { +func (s *Server) handle(inner func(runtime.Object) (runtime.Object, error)) func(w http.ResponseWriter, req *http.Request) { return func(w http.ResponseWriter, req *http.Request) { defer req.Body.Close() @@ -326,7 +352,12 @@ func (s *Server) handle(inner func(runtime.Object) runtime.Object) func(w http.R return } - result := inner(obj) + result, err := inner(obj) + if err != nil { + s.Log.Error(err, "failed to process webhook request") + w.WriteHeader(http.StatusInternalServerError) + return + } if err := codec.Encode(result, w); err != nil { s.Log.Error(err, "failed to encode response body") w.WriteHeader(http.StatusInternalServerError)