diff --git a/pkg/webhook/parser.go b/pkg/webhook/parser.go index a4c5077fe..dda54dc93 100644 --- a/pkg/webhook/parser.go +++ b/pkg/webhook/parser.go @@ -19,7 +19,7 @@ limitations under the License. // // The markers take the form: // -// +kubebuilder:webhook:webhookVersions=<[]string>,failurePolicy=,matchPolicy=,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=,path=,mutating=,sideEffects= +// +kubebuilder:webhook:webhookVersions=<[]string>,failurePolicy=,matchPolicy=,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=,path=,mutating=,sideEffects=,admissionReviewVersions=<[]string> package webhook import ( @@ -107,6 +107,12 @@ type Config struct { // WebhookVersions specifies the target API versions of the {Mutating,Validating}WebhookConfiguration objects // itself to generate. Defaults to v1. WebhookVersions []string `marker:"webhookVersions,optional"` + + // AdmissionReviewVersions is an ordered list of preferred `AdmissionReview` + // versions the Webhook expects. + // For generating v1 {Mutating,Validating}WebhookConfiguration, this is mandatory. + // For generating v1beta1 {Mutating,Validating}WebhookConfiguration, this is optional, and default to v1beta1. + AdmissionReviewVersions []string `marker:"admissionReviewVersions,optional"` } // verbToAPIVariant converts a marker's verb to the proper value for the API. @@ -140,15 +146,13 @@ func (c Config) ToMutatingWebhook() (admissionregv1.MutatingWebhook, error) { } 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"}, + Name: c.Name, + Rules: c.rules(), + FailurePolicy: c.failurePolicy(), + MatchPolicy: matchPolicy, + ClientConfig: c.clientConfig(), + SideEffects: c.sideEffects(), + AdmissionReviewVersions: c.AdmissionReviewVersions, }, nil } @@ -164,15 +168,13 @@ func (c Config) ToValidatingWebhook() (admissionregv1.ValidatingWebhook, error) } 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"}, + Name: c.Name, + Rules: c.rules(), + FailurePolicy: c.failurePolicy(), + MatchPolicy: matchPolicy, + ClientConfig: c.clientConfig(), + SideEffects: c.sideEffects(), + AdmissionReviewVersions: c.AdmissionReviewVersions, }, nil } @@ -344,29 +346,26 @@ func (Generator) Generate(ctx *genall.GenerationContext) error { }, 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 { + // SideEffects is required in admissionregistration/v1, if this is not set or set to `Some` or `Known`, + // we return an error if err := checkSideEffectsForV1(objRaw.Webhooks[i].SideEffects); err != nil { return err } + // AdmissionReviewVersions is required in admissionregistration/v1, if this is not set, + // we return an error + if len(objRaw.Webhooks[i].AdmissionReviewVersions) == 0 { + return fmt.Errorf("AdmissionReviewVersions is mandatory for v1 {Mutating,Validating}WebhookConfiguration") + } } - } - // 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 { + versionedWebhooks[version] = append(versionedWebhooks[version], objRaw) + } else { 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) + versionedWebhooks[version] = append(versionedWebhooks[version], conv) } } @@ -381,29 +380,26 @@ func (Generator) Generate(ctx *genall.GenerationContext) error { }, 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 { + // SideEffects is required in admissionregistration/v1, if this is not set or set to `Some` or `Known`, + // we return an error if err := checkSideEffectsForV1(objRaw.Webhooks[i].SideEffects); err != nil { return err } + // AdmissionReviewVersions is required in admissionregistration/v1, if this is not set, + // we return an error + if len(objRaw.Webhooks[i].AdmissionReviewVersions) == 0 { + return fmt.Errorf("AdmissionReviewVersions is mandatory for v1 {Mutating,Validating}WebhookConfiguration") + } } - } - // 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 { + versionedWebhooks[version] = append(versionedWebhooks[version], objRaw) + } else { 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) + versionedWebhooks[version] = append(versionedWebhooks[version], conv) } } } diff --git a/pkg/webhook/testdata/manifests.v1beta1.yaml b/pkg/webhook/testdata/manifests.v1beta1.yaml index a6530e834..b8603f65f 100644 --- a/pkg/webhook/testdata/manifests.v1beta1.yaml +++ b/pkg/webhook/testdata/manifests.v1beta1.yaml @@ -6,7 +6,10 @@ metadata: creationTimestamp: null name: mutating-webhook-configuration webhooks: -- clientConfig: +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: caBundle: Cg== service: name: webhook-service diff --git a/pkg/webhook/testdata/webhook.go b/pkg/webhook/testdata/webhook.go index 9c00a3057..17ab8f415 100644 --- a/pkg/webhook/testdata/webhook.go +++ b/pkg/webhook/testdata/webhook.go @@ -28,8 +28,8 @@ func (c *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error { } // +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 +// +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,admissionReviewVersions=v1;v1beta1 +// +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,admissionReviewVersions=v1;v1beta1 var _ webhook.Defaulter = &CronJob{} var _ webhook.Validator = &CronJob{} diff --git a/pkg/webhook/zz_generated.markerhelp.go b/pkg/webhook/zz_generated.markerhelp.go index 92d179dcd..84e86cdbb 100644 --- a/pkg/webhook/zz_generated.markerhelp.go +++ b/pkg/webhook/zz_generated.markerhelp.go @@ -76,6 +76,10 @@ func (Config) Help() *markers.DefinitionHelp { Summary: "specifies the target API versions of the {Mutating,Validating}WebhookConfiguration objects itself to generate. Defaults to v1.", Details: "", }, + "AdmissionReviewVersions": markers.DetailedHelp{ + Summary: "is an ordered list of preferred `AdmissionReview` versions the Webhook expects. For generating v1 {Mutating,Validating}WebhookConfiguration, this is mandatory. For generating v1beta1 {Mutating,Validating}WebhookConfiguration, this is optional, and default to v1beta1.", + Details: "", + }, }, } }