Repository: kubernetes-sigs/controller-tools (go)

Issue:
contoller-gen: webhook generator support for admissionregistration.k8s.io/v1
Hi folks,
Since `admissionregistration.k8s.io/v1beta1` is deprecated in kubernetes v1.16 in favor of `admissionregistration.k8s.io/v1`, do you have a plan for adding support to generate webhook configurations (Validating/MutatingWebhokConfiguration) for `admissionregistration.k8s.io/v1`?

I can work on this if it is needed.

Pull request description:
⚠ Add admissionregisteration/v1 support for webhook
<!-- please add a icon to the title of this PR (see VERSIONING.md), and delete this line and similar ones -->
<!-- the icon will be either ⚠ (:warning:, major), ✨ (:sparkles:, minor), 🐛 (:bug:, patch), 📖 (:book:, docs), or 🏃 (:running:, other) -->

<!-- What does this do, and why do we need it? -->
This PR implements support to generate {Mutating, Validating}WebhookConfiguration with version `v1`.
Fixes #419

The first intention was to add a package marker `webhookVersions=<[]string>` for the user to specify the versions that want to generate. It will default to `v1` if the `webhookVersions` is not specified. However, after discussion with @estroz (https://github.com/kubernetes-sigs/controller-tools/pull/469#discussion_r469346611), we found out this cannot work nicely because, for different versions of WebhookConfiguraiton, Some fields (SideEffects, AdmissionReviewVersions) can have different available values, and if one value in that marker is invalid for one version then you can't generate the config for either version.
So we decided to use an optional field marker `+kubebuilder:webhookVersions=<[]string>` to specify the versions in the `+kubebuilder:webhookVersions` directly, it works as follows:
- We default to generate `v1` configuration if the marker is not specified.
- Generate `v1` if the marker is `+kubebuilder:webhook:webhookVersions=v1`
- Generate `v1beta1` if the marker is `+kubebuilder:webhook:webhookVersions=v1beta1`
- Generate both `v1` and `v1beta1` if the marker is `+kubebuilder:webhook:webhookVersions=v1;v1beta1` or `+kubebuilder:webhook:webhookVersions=v1beta1;v1`, we throw error out if the `sideEffects` (later we also needed to handle `AdmissionReviewVersions`) cannot meet requirement for either versions.

This is a breaking change since we generate `v1` version configurations by default if the marker is not specified.

Patch:
diff --git a/pkg/webhook/conv.go b/pkg/webhook/conv.go
new file mode 100644
index 00000000..e85b06b1
--- /dev/null
+++ b/pkg/webhook/conv.go
@@ -0,0 +1,44 @@
+/*
+Copyright 2020 The Kubernetes 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 webhook
+
+import (
+	admissionregv1 "k8s.io/api/admissionregistration/v1"
+	admissionregv1beta1 "k8s.io/api/admissionregistration/v1beta1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+)
+
+var (
+	conversionScheme = runtime.NewScheme()
+)
+
+func init() {
+	utilruntime.Must(admissionregv1.AddToScheme(conversionScheme))
+	utilruntime.Must(admissionregv1beta1.AddToScheme(conversionScheme))
+}
+
+// MutatingWebhookConfigurationAsVersion converts a MutatingWebhookConfiguration from the canonical internal form (currently v1) to some external form.
+func MutatingWebhookConfigurationAsVersion(original *admissionregv1.MutatingWebhookConfiguration, gv schema.GroupVersion) (runtime.Object, error) {
+	return conversionScheme.ConvertToVersion(original, gv)
+}
+
+// ValidatingWebhookConfigurationAsVersion converts a ValidatingWebhookConfiguration from the canonical internal form (currently v1) to some external form.
+func ValidatingWebhookConfigurationAsVersion(original *admissionregv1.ValidatingWebhookConfiguration, gv schema.GroupVersion) (runtime.Object, error) {
+	return conversionScheme.ConvertToVersion(original, gv)
+}
diff --git a/pkg/webhook/parser.go b/pkg/webhook/parser.go
index 21eaa90c..a4c5077f 100644
--- a/pkg/webhook/parser.go
+++ b/pkg/webhook/parser.go
@@ -19,26 +19,38 @@ limitations under the License.
 //
 // The markers take the form:
 //
-//  +kubebuilder:webhook:failurePolicy=<string>,matchPolicy=<string>,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=<string>,path=<string>,mutating=<bool>,sideEffects=<string>
+//  +kubebuilder:webhook:webhookVersions=<[]string>,failurePolicy=<string>,matchPolicy=<string>,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=<string>,path=<string>,mutating=<bool>,sideEffects=<string>
 package webhook
 
 import (
 	"fmt"
 	"strings"
 
-	admissionreg "k8s.io/api/admissionregistration/v1beta1"
+	admissionregv1 "k8s.io/api/admissionregistration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/sets"
 
 	"sigs.k8s.io/controller-tools/pkg/genall"
 	"sigs.k8s.io/controller-tools/pkg/markers"
 )
 
+// The default {Mutating,Validating}WebhookConfiguration version to generate.
+const (
+	defaultWebhookVersion = "v1"
+)
+
 var (
 	// ConfigDefinition s a marker for defining Webhook manifests.
 	// Call ToWebhook on the value to get a Kubernetes Webhook.
 	ConfigDefinition = markers.Must(markers.MakeDefinition("kubebuilder:webhook", markers.DescribesPackage, Config{}))
 )
 
+// supportedWebhookVersions returns currently supported API version of {Mutating,Validating}WebhookConfiguration.
+func supportedWebhookVersions() []string {
+	return []string{defaultWebhookVersion, "v1beta1"}
+}
+
 // +controllertools:marker:generateHelp:category=Webhook
 
 // Config specifies how a webhook should be served.
@@ -91,79 +103,89 @@ type Config struct {
 	// batch.tutorial.kubebuilder.io/v1,Kind=CronJob would be
 	// /validate-batch-tutorial-kubebuilder-io-v1-cronjob
 	Path string
+
+	// WebhookVersions specifies the target API versions of the {Mutating,Validating}WebhookConfiguration objects
+	// itself to generate.  Defaults to v1.
+	WebhookVersions []string `marker:"webhookVersions,optional"`
 }
 
 // verbToAPIVariant converts a marker's verb to the proper value for the API.
 // Unrecognized verbs are passed through.
-func verbToAPIVariant(verbRaw string) admissionreg.OperationType {
+func verbToAPIVariant(verbRaw string) admissionregv1.OperationType {
 	switch strings.ToLower(verbRaw) {
-	case strings.ToLower(string(admissionreg.Create)):
-		return admissionreg.Create
-	case strings.ToLower(string(admissionreg.Update)):
-		return admissionreg.Update
-	case strings.ToLower(string(admissionreg.Delete)):
-		return admissionreg.Delete
-	case strings.ToLower(string(admissionreg.Connect)):
-		return admissionreg.Connect
-	case strings.ToLower(string(admissionreg.OperationAll)):
-		return admissionreg.OperationAll
+	case strings.ToLower(string(admissionregv1.Create)):
+		return admissionregv1.Create
+	case strings.ToLower(string(admissionregv1.Update)):
+		return admissionregv1.Update
+	case strings.ToLower(string(admissionregv1.Delete)):
+		return admissionregv1.Delete
+	case strings.ToLower(string(admissionregv1.Connect)):
+		return admissionregv1.Connect
+	case strings.ToLower(string(admissionregv1.OperationAll)):
+		return admissionregv1.OperationAll
 	default:
-		return admissionreg.OperationType(verbRaw)
+		return admissionregv1.OperationType(verbRaw)
 	}
 }
 
 // ToMutatingWebhook converts this rule to its Kubernetes API form.
-func (c Config) ToMutatingWebhook() (admissionreg.MutatingWebhook, error) {
+func (c Config) ToMutatingWebhook() (admissionregv1.MutatingWebhook, error) {
 	if !c.Mutating {
-		return admissionreg.MutatingWebhook{}, fmt.Errorf("%s is a validating webhook", c.Name)
+		return admissionregv1.MutatingWebhook{}, fmt.Errorf("%s is a validating webhook", c.Name)
 	}
 
 	matchPolicy, err := c.matchPolicy()
 	if err != nil {
-		return admissionreg.MutatingWebhook{}, err
+		return admissionregv1.MutatingWebhook{}, err
 	}
 
-	return admissionreg.MutatingWebhook{
+	return admissionregv1.MutatingWebhook{
 		Name:          c.Name,
 		Rules:         c.rules(),
 		FailurePolicy: c.failurePolicy(),
 		MatchPolicy:   matchPolicy,
 		ClientConfig:  c.clientConfig(),
 		SideEffects:   c.sideEffects(),
+		// TODO(jiachengxu): AdmissionReviewVersions becomes required in admissionregistration/v1, here we default it
+		// to `v1` and `v1beta1`, and we should support to config the `AdmissionReviewVersions` as a marker.
+		AdmissionReviewVersions: []string{defaultWebhookVersion, "v1beta1"},
 	}, nil
 }
 
 // ToValidatingWebhook converts this rule to its Kubernetes API form.
-func (c Config) ToValidatingWebhook() (admissionreg.ValidatingWebhook, error) {
+func (c Config) ToValidatingWebhook() (admissionregv1.ValidatingWebhook, error) {
 	if c.Mutating {
-		return admissionreg.ValidatingWebhook{}, fmt.Errorf("%s is a mutating webhook", c.Name)
+		return admissionregv1.ValidatingWebhook{}, fmt.Errorf("%s is a mutating webhook", c.Name)
 	}
 
 	matchPolicy, err := c.matchPolicy()
 	if err != nil {
-		return admissionreg.ValidatingWebhook{}, err
+		return admissionregv1.ValidatingWebhook{}, err
 	}
 
-	return admissionreg.ValidatingWebhook{
+	return admissionregv1.ValidatingWebhook{
 		Name:          c.Name,
 		Rules:         c.rules(),
 		FailurePolicy: c.failurePolicy(),
 		MatchPolicy:   matchPolicy,
 		ClientConfig:  c.clientConfig(),
 		SideEffects:   c.sideEffects(),
+		// TODO(jiachengxu): AdmissionReviewVersions becomes required in admissionregistration/v1, here we default it
+		// to `v1` and `v1beta1`, and we should support to config the `AdmissionReviewVersions` as a marker.
+		AdmissionReviewVersions: []string{defaultWebhookVersion, "v1beta1"},
 	}, nil
 }
 
 // rules returns the configuration of what operations on what
 // resources/subresources a webhook should care about.
-func (c Config) rules() []admissionreg.RuleWithOperations {
-	whConfig := admissionreg.RuleWithOperations{
-		Rule: admissionreg.Rule{
+func (c Config) rules() []admissionregv1.RuleWithOperations {
+	whConfig := admissionregv1.RuleWithOperations{
+		Rule: admissionregv1.Rule{
 			APIGroups:   c.Groups,
 			APIVersions: c.Versions,
 			Resources:   c.Resources,
 		},
-		Operations: make([]admissionreg.OperationType, len(c.Verbs)),
+		Operations: make([]admissionregv1.OperationType, len(c.Verbs)),
 	}
 
 	for i, verbRaw := range c.Verbs {
@@ -177,32 +199,32 @@ func (c Config) rules() []admissionreg.RuleWithOperations {
 		}
 	}
 
-	return []admissionreg.RuleWithOperations{whConfig}
+	return []admissionregv1.RuleWithOperations{whConfig}
 }
 
 // failurePolicy converts the string value to the proper value for the API.
 // Unrecognized values are passed through.
-func (c Config) failurePolicy() *admissionreg.FailurePolicyType {
-	var failurePolicy admissionreg.FailurePolicyType
+func (c Config) failurePolicy() *admissionregv1.FailurePolicyType {
+	var failurePolicy admissionregv1.FailurePolicyType
 	switch strings.ToLower(c.FailurePolicy) {
-	case strings.ToLower(string(admissionreg.Ignore)):
-		failurePolicy = admissionreg.Ignore
-	case strings.ToLower(string(admissionreg.Fail)):
-		failurePolicy = admissionreg.Fail
+	case strings.ToLower(string(admissionregv1.Ignore)):
+		failurePolicy = admissionregv1.Ignore
+	case strings.ToLower(string(admissionregv1.Fail)):
+		failurePolicy = admissionregv1.Fail
 	default:
-		failurePolicy = admissionreg.FailurePolicyType(c.FailurePolicy)
+		failurePolicy = admissionregv1.FailurePolicyType(c.FailurePolicy)
 	}
 	return &failurePolicy
 }
 
 // matchPolicy converts the string value to the proper value for the API.
-func (c Config) matchPolicy() (*admissionreg.MatchPolicyType, error) {
-	var matchPolicy admissionreg.MatchPolicyType
+func (c Config) matchPolicy() (*admissionregv1.MatchPolicyType, error) {
+	var matchPolicy admissionregv1.MatchPolicyType
 	switch strings.ToLower(c.MatchPolicy) {
-	case strings.ToLower(string(admissionreg.Exact)):
-		matchPolicy = admissionreg.Exact
-	case strings.ToLower(string(admissionreg.Equivalent)):
-		matchPolicy = admissionreg.Equivalent
+	case strings.ToLower(string(admissionregv1.Exact)):
+		matchPolicy = admissionregv1.Exact
+	case strings.ToLower(string(admissionregv1.Equivalent)):
+		matchPolicy = admissionregv1.Equivalent
 	case "":
 		return nil, nil
 	default:
@@ -212,10 +234,10 @@ func (c Config) matchPolicy() (*admissionreg.MatchPolicyType, error) {
 }
 
 // clientConfig returns the client config for a webhook.
-func (c Config) clientConfig() admissionreg.WebhookClientConfig {
+func (c Config) clientConfig() admissionregv1.WebhookClientConfig {
 	path := c.Path
-	return admissionreg.WebhookClientConfig{
-		Service: &admissionreg.ServiceReference{
+	return admissionregv1.WebhookClientConfig{
+		Service: &admissionregv1.ServiceReference{
 			Name:      "webhook-service",
 			Namespace: "system",
 			Path:      &path,
@@ -228,15 +250,15 @@ func (c Config) clientConfig() admissionreg.WebhookClientConfig {
 }
 
 // sideEffects returns the sideEffects config for a webhook.
-func (c Config) sideEffects() *admissionreg.SideEffectClass {
-	var sideEffects admissionreg.SideEffectClass
+func (c Config) sideEffects() *admissionregv1.SideEffectClass {
+	var sideEffects admissionregv1.SideEffectClass
 	switch strings.ToLower(c.SideEffects) {
-	case strings.ToLower(string(admissionreg.SideEffectClassNone)):
-		sideEffects = admissionreg.SideEffectClassNone
-	case strings.ToLower(string(admissionreg.SideEffectClassNoneOnDryRun)):
-		sideEffects = admissionreg.SideEffectClassNoneOnDryRun
-	case strings.ToLower(string(admissionreg.SideEffectClassSome)):
-		sideEffects = admissionreg.SideEffectClassSome
+	case strings.ToLower(string(admissionregv1.SideEffectClassNone)):
+		sideEffects = admissionregv1.SideEffectClassNone
+	case strings.ToLower(string(admissionregv1.SideEffectClassNoneOnDryRun)):
+		sideEffects = admissionregv1.SideEffectClassNoneOnDryRun
+	case strings.ToLower(string(admissionregv1.SideEffectClassSome)):
+		sideEffects = admissionregv1.SideEffectClassSome
 	case "":
 		return nil
 	default:
@@ -245,6 +267,21 @@ func (c Config) sideEffects() *admissionreg.SideEffectClass {
 	return &sideEffects
 }
 
+// webhookVersions returns the target API versions of the {Mutating,Validating}WebhookConfiguration objects for a webhook.
+func (c Config) webhookVersions() ([]string, error) {
+	// If WebhookVersions is not specified, we default it to `v1`.
+	if len(c.WebhookVersions) == 0 {
+		return []string{defaultWebhookVersion}, nil
+	}
+	supportedWebhookVersions := sets.NewString(supportedWebhookVersions()...)
+	for _, version := range c.WebhookVersions {
+		if !supportedWebhookVersions.Has(version) {
+			return nil, fmt.Errorf("unsupported webhook version: %s", version)
+		}
+	}
+	return sets.NewString(c.WebhookVersions...).UnsortedList(), nil
+}
+
 // +controllertools:marker:generateHelp
 
 // Generator generates (partial) {Mutating,Validating}WebhookConfiguration objects.
@@ -259,8 +296,9 @@ func (Generator) RegisterMarkers(into *markers.Registry) error {
 }
 
 func (Generator) Generate(ctx *genall.GenerationContext) error {
-	var mutatingCfgs []admissionreg.MutatingWebhook
-	var validatingCfgs []admissionreg.ValidatingWebhook
+	supportedWebhookVersions := supportedWebhookVersions()
+	mutatingCfgs := make(map[string][]admissionregv1.MutatingWebhook, len(supportedWebhookVersions))
+	validatingCfgs := make(map[string][]admissionregv1.ValidatingWebhook, len(supportedWebhookVersions))
 	for _, root := range ctx.Roots {
 		markerSet, err := markers.PackageMarkers(ctx.Collector, root)
 		if err != nil {
@@ -269,47 +307,128 @@ func (Generator) Generate(ctx *genall.GenerationContext) error {
 
 		for _, cfg := range markerSet[ConfigDefinition.Name] {
 			cfg := cfg.(Config)
+			webhookVersions, err := cfg.webhookVersions()
+			if err != nil {
+				return err
+			}
 			if cfg.Mutating {
-				w, _ := cfg.ToMutatingWebhook()
-				mutatingCfgs = append(mutatingCfgs, w)
+				w, err := cfg.ToMutatingWebhook()
+				if err != nil {
+					return err
+				}
+				for _, webhookVersion := range webhookVersions {
+					mutatingCfgs[webhookVersion] = append(mutatingCfgs[webhookVersion], w)
+				}
 			} else {
-				w, _ := cfg.ToValidatingWebhook()
-				validatingCfgs = append(validatingCfgs, w)
+				w, err := cfg.ToValidatingWebhook()
+				if err != nil {
+					return err
+				}
+				for _, webhookVersion := range webhookVersions {
+					validatingCfgs[webhookVersion] = append(validatingCfgs[webhookVersion], w)
+				}
 			}
 		}
 	}
 
-	var objs []interface{}
-	if len(mutatingCfgs) > 0 {
-		objs = append(objs, &admissionreg.MutatingWebhookConfiguration{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       "MutatingWebhookConfiguration",
-				APIVersion: admissionreg.SchemeGroupVersion.String(),
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "mutating-webhook-configuration",
-			},
-			Webhooks: mutatingCfgs,
-		})
-	}
-
-	if len(validatingCfgs) > 0 {
-		objs = append(objs, &admissionreg.ValidatingWebhookConfiguration{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       "ValidatingWebhookConfiguration",
-				APIVersion: admissionreg.SchemeGroupVersion.String(),
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "validating-webhook-configuration",
-			},
-			Webhooks: validatingCfgs,
-		})
+	versionedWebhooks := make(map[string][]interface{}, len(supportedWebhookVersions))
+	for _, version := range supportedWebhookVersions {
+		if cfgs, ok := mutatingCfgs[version]; ok {
+			objRaw := &admissionregv1.MutatingWebhookConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "MutatingWebhookConfiguration",
+					APIVersion: admissionregv1.SchemeGroupVersion.String(),
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "mutating-webhook-configuration",
+				},
+				Webhooks: cfgs,
+			}
+			// SideEffects in required in admissionregistration/v1, if this is not set or set to `Some` or `Known`,
+			// we return an error
+			if version == defaultWebhookVersion {
+				for i := range objRaw.Webhooks {
+					if err := checkSideEffectsForV1(objRaw.Webhooks[i].SideEffects); err != nil {
+						return err
+					}
+				}
+			}
+			// AdmissionReviewVersions is optional in admissionregistration/v1beta1, so let kubernetes to default it.
+			if version == "v1beta1" {
+				for i := range objRaw.Webhooks {
+					objRaw.Webhooks[i].AdmissionReviewVersions = nil
+				}
+			}
+			if version != defaultWebhookVersion {
+				conv, err := MutatingWebhookConfigurationAsVersion(objRaw, schema.GroupVersion{Group: admissionregv1.SchemeGroupVersion.Group, Version: version})
+				versionedWebhooks[version] = append(versionedWebhooks[version], conv)
+				if err != nil {
+					return err
+				}
+			} else {
+				versionedWebhooks[version] = append(versionedWebhooks[version], objRaw)
+			}
+		}
 
+		if cfgs, ok := validatingCfgs[version]; ok {
+			objRaw := &admissionregv1.ValidatingWebhookConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "ValidatingWebhookConfiguration",
+					APIVersion: admissionregv1.SchemeGroupVersion.String(),
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "validating-webhook-configuration",
+				},
+				Webhooks: cfgs,
+			}
+			// SideEffects in required in admissionregistration/v1, if this is not set or set to `Some` or `Known`,
+			// we return an error
+			if version == defaultWebhookVersion {
+				for i := range objRaw.Webhooks {
+					if err := checkSideEffectsForV1(objRaw.Webhooks[i].SideEffects); err != nil {
+						return err
+					}
+				}
+			}
+			// AdmissionReviewVersions is optional in admissionregistration/v1beta1, so let kubernetes to default it.
+			if version == "v1beta1" {
+				for i := range objRaw.Webhooks {
+					objRaw.Webhooks[i].AdmissionReviewVersions = nil
+				}
+			}
+			if version != defaultWebhookVersion {
+				conv, err := ValidatingWebhookConfigurationAsVersion(objRaw, schema.GroupVersion{Group: admissionregv1.SchemeGroupVersion.Group, Version: version})
+				versionedWebhooks[version] = append(versionedWebhooks[version], conv)
+				if err != nil {
+					return err
+				}
+			} else {
+				versionedWebhooks[version] = append(versionedWebhooks[version], objRaw)
+			}
+		}
 	}
 
-	if len(objs) > 0 {
-		return ctx.WriteYAML("manifests.yaml", objs...)
+	for k, v := range versionedWebhooks {
+		var fileName string
+		if k == defaultWebhookVersion {
+			fileName = fmt.Sprintf("manifests.yaml")
+		} else {
+			fileName = fmt.Sprintf("manifests.%s.yaml", k)
+		}
+		if err := ctx.WriteYAML(fileName, v...); err != nil {
+			return err
+		}
 	}
+	return nil
+}
 
+func checkSideEffectsForV1(sideEffects *admissionregv1.SideEffectClass) error {
+	if sideEffects == nil {
+		return fmt.Errorf("SideEffects is required for creating v1 {Mutating,Validating}WebhookConfiguration")
+	}
+	if *sideEffects == admissionregv1.SideEffectClassUnknown ||
+		*sideEffects == admissionregv1.SideEffectClassSome {
+		return fmt.Errorf("SideEffects should not be set to `Some` or `Unknown` for v1 {Mutating,Validating}WebhookConfiguration")
+	}
 	return nil
 }
diff --git a/pkg/webhook/zz_generated.markerhelp.go b/pkg/webhook/zz_generated.markerhelp.go
index cf477210..92d179dc 100644
--- a/pkg/webhook/zz_generated.markerhelp.go
+++ b/pkg/webhook/zz_generated.markerhelp.go
@@ -72,6 +72,10 @@ func (Config) Help() *markers.DefinitionHelp {
 				Summary: "specifies that path that the API server should connect to this webhook on. Must be prefixed with a '/validate-' or '/mutate-' depending on the type, and followed by $GROUP-$VERSION-$KIND where all values are lower-cased and the periods in the group are substituted for hyphens. For example, a validating webhook path for type batch.tutorial.kubebuilder.io/v1,Kind=CronJob would be /validate-batch-tutorial-kubebuilder-io-v1-cronjob",
 				Details: "",
 			},
+			"WebhookVersions": markers.DetailedHelp{
+				Summary: "specifies the target API versions of the {Mutating,Validating}WebhookConfiguration objects itself to generate.  Defaults to v1.",
+				Details: "",
+			},
 		},
 	}
 }


Test patch:
diff --git a/pkg/webhook/parser_integration_test.go b/pkg/webhook/parser_integration_test.go
index 4e9f6c57..a065ae17 100644
--- a/pkg/webhook/parser_integration_test.go
+++ b/pkg/webhook/parser_integration_test.go
@@ -25,7 +25,8 @@ import (
 	"github.com/google/go-cmp/cmp"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	admissionreg "k8s.io/api/admissionregistration/v1beta1"
+	admissionregv1 "k8s.io/api/admissionregistration/v1"
+	admissionregv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	"sigs.k8s.io/yaml"
 
 	"sigs.k8s.io/controller-tools/pkg/genall"
@@ -35,6 +36,10 @@ import (
 )
 
 var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition", func() {
+	assertSame := func(actual, expected interface{}) {
+		ExpectWithOffset(1, actual).To(Equal(expected), "type not as expected, check pkg/webhook/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(actual, expected))
+	}
+
 	It("should properly generate the webhook definition", func() {
 		// TODO(directxman12): test generation across multiple versions (right
 		// now, we're trusting k/k's conversion code, though, which is probably
@@ -64,26 +69,46 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
 			OutputRule: genall.OutputToDirectory(outputDir),
 		}))
 
-		By("loading the generated YAML")
+		By("loading the generated v1 YAML")
 		actualFile, err := ioutil.ReadFile(path.Join(outputDir, "manifests.yaml"))
 		Expect(err).NotTo(HaveOccurred())
-		actualMutating, actualValidating := unmarshalBoth(actualFile)
+		actualMutating, actualValidating := unmarshalBothV1(actualFile)
 
-		By("loading the desired YAML")
+		By("loading the desired v1 YAML")
 		expectedFile, err := ioutil.ReadFile("manifests.yaml")
 		Expect(err).NotTo(HaveOccurred())
-		expectedMutating, expectedValidating := unmarshalBoth(expectedFile)
+		expectedMutating, expectedValidating := unmarshalBothV1(expectedFile)
 
 		By("comparing the two")
-		assertSame := func(actual, expected interface{}) {
-			ExpectWithOffset(1, actual).To(Equal(expected), "type not as expected, check pkg/webhook/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(actual, expected))
-		}
 		assertSame(actualMutating, expectedMutating)
 		assertSame(actualValidating, expectedValidating)
+
+		By("loading the generated v1beta1 YAML")
+		actualFile, err = ioutil.ReadFile(path.Join(outputDir, "manifests.v1beta1.yaml"))
+		Expect(err).NotTo(HaveOccurred())
+		actualMutatingV1beta1, actualValidatingV1beta1 := unmarshalBothV1beta1(actualFile)
+
+		By("loading the desired v1beta1 YAML")
+		expectedFile, err = ioutil.ReadFile("manifests.v1beta1.yaml")
+		Expect(err).NotTo(HaveOccurred())
+		expectedMutatingV1beta1, expectedValidatingV1beta1 := unmarshalBothV1beta1(expectedFile)
+
+		By("comparing the two")
+		assertSame(actualMutatingV1beta1, expectedMutatingV1beta1)
+		assertSame(actualValidatingV1beta1, expectedValidatingV1beta1)
 	})
 })
 
-func unmarshalBoth(in []byte) (mutating admissionreg.MutatingWebhookConfiguration, validating admissionreg.ValidatingWebhookConfiguration) {
+func unmarshalBothV1beta1(in []byte) (mutating admissionregv1beta1.MutatingWebhookConfiguration, validating admissionregv1beta1.ValidatingWebhookConfiguration) {
+	documents := bytes.Split(in, []byte("\n---\n"))[1:]
+	ExpectWithOffset(1, documents).To(HaveLen(2), "expected two documents in file, found %d", len(documents))
+
+	ExpectWithOffset(1, yaml.UnmarshalStrict(documents[0], &mutating)).To(Succeed(), "expected the first document in the file to be a mutating webhook configuration")
+	ExpectWithOffset(1, yaml.UnmarshalStrict(documents[1], &validating)).To(Succeed(), "expected the second document in the file to be a validating webhook configuration")
+	return
+}
+
+func unmarshalBothV1(in []byte) (mutating admissionregv1.MutatingWebhookConfiguration, validating admissionregv1.ValidatingWebhookConfiguration) {
 	documents := bytes.Split(in, []byte("\n---\n"))[1:]
 	ExpectWithOffset(1, documents).To(HaveLen(2), "expected two documents in file, found %d", len(documents))
 
diff --git a/pkg/webhook/testdata/manifests.v1beta1.yaml b/pkg/webhook/testdata/manifests.v1beta1.yaml
new file mode 100644
index 00000000..a6530e83
--- /dev/null
+++ b/pkg/webhook/testdata/manifests.v1beta1.yaml
@@ -0,0 +1,56 @@
+
+---
+apiVersion: admissionregistration.k8s.io/v1beta1
+kind: MutatingWebhookConfiguration
+metadata:
+  creationTimestamp: null
+  name: mutating-webhook-configuration
+webhooks:
+- clientConfig:
+    caBundle: Cg==
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-testdata-kubebuilder-io-v1-cronjob
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: default.cronjob.testdata.kubebuilder.io
+  rules:
+  - apiGroups:
+    - testdata.kubebuiler.io
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - cronjobs
+  sideEffects: None
+
+---
+apiVersion: admissionregistration.k8s.io/v1beta1
+kind: ValidatingWebhookConfiguration
+metadata:
+  creationTimestamp: null
+  name: validating-webhook-configuration
+webhooks:
+- clientConfig:
+    caBundle: Cg==
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-testdata-kubebuilder-io-v1-cronjob
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: validation.cronjob.testdata.kubebuilder.io
+  rules:
+  - apiGroups:
+    - testdata.kubebuiler.io
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - cronjobs
+  sideEffects: Some
diff --git a/pkg/webhook/testdata/manifests.yaml b/pkg/webhook/testdata/manifests.yaml
index a6530e83..6f6f92b5 100644
--- a/pkg/webhook/testdata/manifests.yaml
+++ b/pkg/webhook/testdata/manifests.yaml
@@ -1,12 +1,15 @@
 
 ---
-apiVersion: admissionregistration.k8s.io/v1beta1
+apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
 metadata:
   creationTimestamp: null
   name: mutating-webhook-configuration
 webhooks:
-- clientConfig:
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
     caBundle: Cg==
     service:
       name: webhook-service
@@ -28,13 +31,16 @@ webhooks:
   sideEffects: None
 
 ---
-apiVersion: admissionregistration.k8s.io/v1beta1
+apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   creationTimestamp: null
   name: validating-webhook-configuration
 webhooks:
-- clientConfig:
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
     caBundle: Cg==
     service:
       name: webhook-service
@@ -53,4 +59,4 @@ webhooks:
     - UPDATE
     resources:
     - cronjobs
-  sideEffects: Some
+  sideEffects: NoneOnDryRun
diff --git a/pkg/webhook/testdata/webhook.go b/pkg/webhook/testdata/webhook.go
index 48b1185b..9c00a305 100644
--- a/pkg/webhook/testdata/webhook.go
+++ b/pkg/webhook/testdata/webhook.go
@@ -27,8 +27,9 @@ func (c *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		Complete()
 }
 
-// +kubebuilder:webhook:verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=Some
-// +kubebuilder:webhook:verbs=create;update,path=/mutate-testdata-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=default.cronjob.testdata.kubebuilder.io,sideEffects=None
+// +kubebuilder:webhook:webhookVersions=v1beta1,verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=Some
+// +kubebuilder:webhook:verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=NoneOnDryRun
+// +kubebuilder:webhook:webhookVersions=v1;v1beta1,verbs=create;update,path=/mutate-testdata-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=default.cronjob.testdata.kubebuilder.io,sideEffects=None
 
 var _ webhook.Defaulter = &CronJob{}
 var _ webhook.Validator = &CronJob{}