Support v1 admission webhooks

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

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,

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+

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+

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,

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",

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 {

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

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)

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

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{

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",

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)