From f7dff4725f8dc694a852e7fdbdde2c8a6dd5b7d4 Mon Sep 17 00:00:00 2001 From: Mehdy Bohlool Date: Mon, 4 Mar 2019 20:52:57 -0800 Subject: [PATCH] Add AdmissionReviewVersions to admissionregistration and default it --- .../admissionregistration/fuzzer/fuzzer.go | 1 + pkg/apis/admissionregistration/types.go | 9 + .../admissionregistration/v1beta1/defaults.go | 4 + .../validation/validation.go | 77 +- .../validation/validation_test.go | 876 ++++++++++-------- .../mutatingwebhookconfiguration/strategy.go | 4 +- .../strategy.go | 7 +- .../admissionregistration/v1beta1/types.go | 11 + .../plugin/webhook/mutating/dispatcher.go | 6 + .../plugin/webhook/testing/testcase.go | 334 ++++--- .../plugin/webhook/util/client_config.go | 10 + .../plugin/webhook/validating/dispatcher.go | 6 + 12 files changed, 812 insertions(+), 533 deletions(-) diff --git a/pkg/apis/admissionregistration/fuzzer/fuzzer.go b/pkg/apis/admissionregistration/fuzzer/fuzzer.go index 5b5b84f2e8..0f54c54730 100644 --- a/pkg/apis/admissionregistration/fuzzer/fuzzer.go +++ b/pkg/apis/admissionregistration/fuzzer/fuzzer.go @@ -43,6 +43,7 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { i := int32(30) obj.TimeoutSeconds = &i } + obj.AdmissionReviewVersions = []string{"v1beta1"} }, } } diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index a296590e5a..09a5df6113 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/pkg/apis/admissionregistration/types.go @@ -239,6 +239,15 @@ type Webhook struct { // The timeout value must be between 1 and 30 seconds. // +optional TimeoutSeconds *int32 + + // AdmissionReviewVersions is an ordered list of preferred `AdmissionReview` + // versions the Webhook expects. API server will try to use first version in + // the list which it supports. If none of the versions specified in this list + // supported by API server, validation will fail for this object. + // If the webhook configuration has already been persisted with a version apiserver + // does not understand, calls to the webhook will fail and be subject to the failure policy. + // +optional + AdmissionReviewVersions []string } // RuleWithOperations is a tuple of Operations and Resources. It is recommended to make diff --git a/pkg/apis/admissionregistration/v1beta1/defaults.go b/pkg/apis/admissionregistration/v1beta1/defaults.go index 20a318956c..a5da0bfbc8 100644 --- a/pkg/apis/admissionregistration/v1beta1/defaults.go +++ b/pkg/apis/admissionregistration/v1beta1/defaults.go @@ -44,6 +44,10 @@ func SetDefaults_Webhook(obj *admissionregistrationv1beta1.Webhook) { obj.TimeoutSeconds = new(int32) *obj.TimeoutSeconds = 30 } + + if len(obj.AdmissionReviewVersions) == 0 { + obj.AdmissionReviewVersions = []string{admissionregistrationv1beta1.SchemeGroupVersion.Version} + } } func SetDefaults_Rule(obj *admissionregistrationv1beta1.Rule) { diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index 955d5d7ad0..9cf318a2c7 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -24,9 +24,11 @@ import ( metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/kubernetes/pkg/apis/admissionregistration" + "k8s.io/kubernetes/pkg/apis/admissionregistration/v1beta1" ) func hasWildcard(slice []string) bool { @@ -150,18 +152,71 @@ func validateRule(rule *admissionregistration.Rule, fldPath *field.Path, allowSu return allErrors } +var AcceptedAdmissionReviewVersions = []string{v1beta1.SchemeGroupVersion.Version} + +func isAcceptedAdmissionReviewVersion(v string) bool { + for _, version := range AcceptedAdmissionReviewVersions { + if v == version { + return true + } + } + return false +} + +func validateAdmissionReviewVersions(versions []string, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList { + allErrors := field.ErrorList{} + + // Currently only v1beta1 accepted in AdmissionReviewVersions + if len(versions) < 1 { + allErrors = append(allErrors, field.Required(fldPath, "")) + } else { + seen := map[string]bool{} + hasAcceptedVersion := false + for i, v := range versions { + if seen[v] { + allErrors = append(allErrors, field.Invalid(fldPath.Index(i), v, "duplicate version")) + continue + } + seen[v] = true + for _, errString := range utilvalidation.IsDNS1035Label(v) { + allErrors = append(allErrors, field.Invalid(fldPath.Index(i), v, errString)) + } + if isAcceptedAdmissionReviewVersion(v) { + hasAcceptedVersion = true + } + } + if requireRecognizedVersion && !hasAcceptedVersion { + allErrors = append(allErrors, field.Invalid( + fldPath, versions, + fmt.Sprintf("none of the versions accepted by this server. accepted version(s) are %v", + strings.Join(AcceptedAdmissionReviewVersions, ", ")))) + } + } + return allErrors +} + func ValidateValidatingWebhookConfiguration(e *admissionregistration.ValidatingWebhookConfiguration) field.ErrorList { + return validateValidatingWebhookConfiguration(e, true) +} + +func validateValidatingWebhookConfiguration(e *admissionregistration.ValidatingWebhookConfiguration, requireRecognizedVersion bool) field.ErrorList { allErrors := genericvalidation.ValidateObjectMeta(&e.ObjectMeta, false, genericvalidation.NameIsDNSSubdomain, field.NewPath("metadata")) for i, hook := range e.Webhooks { allErrors = append(allErrors, validateWebhook(&hook, field.NewPath("webhooks").Index(i))...) + allErrors = append(allErrors, validateAdmissionReviewVersions(hook.AdmissionReviewVersions, requireRecognizedVersion, field.NewPath("webhooks").Index(i).Child("admissionReviewVersions"))...) } return allErrors } func ValidateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebhookConfiguration) field.ErrorList { + return validateMutatingWebhookConfiguration(e, true) +} + +func validateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebhookConfiguration, requireRecognizedVersion bool) field.ErrorList { allErrors := genericvalidation.ValidateObjectMeta(&e.ObjectMeta, false, genericvalidation.NameIsDNSSubdomain, field.NewPath("metadata")) for i, hook := range e.Webhooks { allErrors = append(allErrors, validateWebhook(&hook, field.NewPath("webhooks").Index(i))...) + allErrors = append(allErrors, validateAdmissionReviewVersions(hook.AdmissionReviewVersions, requireRecognizedVersion, field.NewPath("webhooks").Index(i).Child("admissionReviewVersions"))...) } return allErrors } @@ -247,10 +302,28 @@ func validateRuleWithOperations(ruleWithOperations *admissionregistration.RuleWi return allErrors } +// hasAcceptedAdmissionReviewVersions returns true if all webhooks have at least one +// admission review version this apiserver accepts. +func hasAcceptedAdmissionReviewVersions(webhooks []admissionregistration.Webhook) bool { + for _, hook := range webhooks { + hasRecognizedVersion := false + for _, version := range hook.AdmissionReviewVersions { + if isAcceptedAdmissionReviewVersion(version) { + hasRecognizedVersion = true + break + } + } + if !hasRecognizedVersion && len(hook.AdmissionReviewVersions) > 0 { + return false + } + } + return true +} + func ValidateValidatingWebhookConfigurationUpdate(newC, oldC *admissionregistration.ValidatingWebhookConfiguration) field.ErrorList { - return ValidateValidatingWebhookConfiguration(newC) + return validateValidatingWebhookConfiguration(newC, hasAcceptedAdmissionReviewVersions(oldC.Webhooks)) } func ValidateMutatingWebhookConfigurationUpdate(newC, oldC *admissionregistration.MutatingWebhookConfiguration) field.ErrorList { - return ValidateMutatingWebhookConfiguration(newC) + return validateMutatingWebhookConfiguration(newC, hasAcceptedAdmissionReviewVersions(oldC.Webhooks)) } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index 90d31fd419..61e8457539 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -28,7 +28,14 @@ func strPtr(s string) *string { return &s } func int32Ptr(i int32) *int32 { return &i } -func newValidatingWebhookConfiguration(hooks []admissionregistration.Webhook) *admissionregistration.ValidatingWebhookConfiguration { +func newValidatingWebhookConfiguration(hooks []admissionregistration.Webhook, defaultAdmissionReviewVersions bool) *admissionregistration.ValidatingWebhookConfiguration { + // If the test case did not specify an AdmissionReviewVersions, default it so the test passes as + // this field will be defaulted in production code. + for i := range hooks { + if defaultAdmissionReviewVersions && len(hooks[i].AdmissionReviewVersions) == 0 { + hooks[i].AdmissionReviewVersions = []string{"v1beta1"} + } + } return &admissionregistration.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: "config", @@ -48,560 +55,582 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) { config *admissionregistration.ValidatingWebhookConfiguration expectedError string }{ + { + name: "should fail on bad AdmissionReviewVersion value", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"0v"}, + }, + }, true), + expectedError: `Invalid value: "0v": a DNS-1035 label`, + }, + { + name: "should pass on valid AdmissionReviewVersion", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"v1beta1"}, + }, + }, true), + expectedError: ``, + }, + { + name: "should pass on mix of accepted and unaccepted AdmissionReviewVersion", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"v1beta1", "invalid-version"}, + }, + }, true), + expectedError: ``, + }, + { + name: "should fail on invalid AdmissionReviewVersion", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"invalidVersion"}, + }, + }, true), + expectedError: `Invalid value: []string{"invalidVersion"}`, + }, + { + name: "should fail on duplicate AdmissionReviewVersion", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"v1beta1", "v1beta1"}, + }, + }, true), + expectedError: `Invalid value: "v1beta1": duplicate version`, + }, { name: "all Webhooks must have a fully qualified name", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - }, - { - Name: "k8s.io", - ClientConfig: validClientConfig, - }, - { - Name: "", - ClientConfig: validClientConfig, - }, - }), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + }, + { + Name: "k8s.io", + ClientConfig: validClientConfig, + }, + { + Name: "", + ClientConfig: validClientConfig, + }, + }, true), expectedError: `webhooks[1].name: Invalid value: "k8s.io": should be a domain with at least three segments separated by dots, webhooks[2].name: Required value`, }, { name: "Operations must not be empty or nil", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"a"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a"}, }, - { - Operations: nil, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"a"}, - }, + }, + { + Operations: nil, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a"}, }, }, }, - }), + }, + }, true), expectedError: `webhooks[0].rules[0].operations: Required value, webhooks[0].rules[1].operations: Required value`, }, { name: "\"\" is NOT a valid operation", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"CREATE", ""}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"a"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"CREATE", ""}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a"}, }, }, }, - }), + }, + }, true), expectedError: `Unsupported value: ""`, }, { name: "operation must be either create/update/delete/connect", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"PATCH"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"a"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"PATCH"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a"}, }, }, }, - }), + }, + }, true), expectedError: `Unsupported value: "PATCH"`, }, { name: "wildcard operation cannot be mixed with other strings", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"CREATE", "*"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"a"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"CREATE", "*"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a"}, }, }, }, - }), + }, + }, true), expectedError: `if '*' is present, must not specify other operations`, }, { name: `resource "*" can co-exist with resources that have subresources`, - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"CREATE"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"*", "a/b", "a/*", "*/b"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"CREATE"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*", "a/b", "a/*", "*/b"}, }, }, }, - }), + }, + }, true), }, { name: `resource "*" cannot mix with resources that don't have subresources`, - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"CREATE"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"*", "a"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"CREATE"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*", "a"}, }, }, }, - }), + }, + }, true), expectedError: `if '*' is present, must not specify other resources without subresources`, }, { name: "resource a/* cannot mix with a/x", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"CREATE"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"a/*", "a/x"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"CREATE"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a/*", "a/x"}, }, }, }, - }), + }, + }, true), expectedError: `webhooks[0].rules[0].resources[1]: Invalid value: "a/x": if 'a/*' is present, must not specify a/x`, }, { name: "resource a/* can mix with a", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"CREATE"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"a/*", "a"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"CREATE"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a/*", "a"}, }, }, }, - }), + }, + }, true), }, { name: "resource */a cannot mix with x/a", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"CREATE"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"*/a", "x/a"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"CREATE"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*/a", "x/a"}, }, }, }, - }), + }, + }, true), expectedError: `webhooks[0].rules[0].resources[1]: Invalid value: "x/a": if '*/a' is present, must not specify x/a`, }, { name: "resource */* cannot mix with other resources", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - Rules: []admissionregistration.RuleWithOperations{ - { - Operations: []admissionregistration.OperationType{"CREATE"}, - Rule: admissionregistration.Rule{ - APIGroups: []string{"a"}, - APIVersions: []string{"a"}, - Resources: []string{"*/*", "a"}, - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + Rules: []admissionregistration.RuleWithOperations{ + { + Operations: []admissionregistration.OperationType{"CREATE"}, + Rule: admissionregistration.Rule{ + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*/*", "a"}, }, }, }, - }), + }, + }, true), expectedError: `webhooks[0].rules[0].resources: Invalid value: []string{"*/*", "a"}: if '*/*' is present, must not specify other resources`, }, { name: "FailurePolicy can only be \"Ignore\" or \"Fail\"", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - FailurePolicy: func() *admissionregistration.FailurePolicyType { - r := admissionregistration.FailurePolicyType("other") - return &r - }(), - }, - }), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + FailurePolicy: func() *admissionregistration.FailurePolicyType { + r := admissionregistration.FailurePolicyType("other") + return &r + }(), + }, + }, true), expectedError: `webhooks[0].failurePolicy: Unsupported value: "other": supported values: "Fail", "Ignore"`, }, { name: "SideEffects can only be \"Unknown\", \"None\", \"Some\", or \"NoneOnDryRun\"", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - SideEffects: func() *admissionregistration.SideEffectClass { - r := admissionregistration.SideEffectClass("other") - return &r - }(), - }, - }), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: func() *admissionregistration.SideEffectClass { + r := admissionregistration.SideEffectClass("other") + return &r + }(), + }, + }, true), expectedError: `webhooks[0].sideEffects: Unsupported value: "other": supported values: "None", "NoneOnDryRun", "Some", "Unknown"`, }, { name: "both service and URL missing", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{}, - }, - }), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{}, + }, + }, true), expectedError: `exactly one of`, }, { name: "both service and URL provided", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - Service: &admissionregistration.ServiceReference{ - Namespace: "ns", - Name: "n", - }, - URL: strPtr("example.com/k8s/webhook"), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + Service: &admissionregistration.ServiceReference{ + Namespace: "ns", + Name: "n", }, + URL: strPtr("example.com/k8s/webhook"), }, - }), + }, + }, true), expectedError: `[0].clientConfig: Required value: exactly one of url or service is required`, }, { name: "blank URL", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - URL: strPtr(""), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr(""), }, - }), + }, + }, true), expectedError: `[0].clientConfig.url: Invalid value: "": host must be provided`, }, { name: "wrong scheme", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - URL: strPtr("http://example.com"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("http://example.com"), }, - }), + }, + }, true), expectedError: `https`, }, { name: "missing host", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - URL: strPtr("https:///fancy/webhook"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("https:///fancy/webhook"), }, - }), + }, + }, true), expectedError: `host must be provided`, }, { name: "fragment", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - URL: strPtr("https://example.com/#bookmark"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("https://example.com/#bookmark"), }, - }), + }, + }, true), expectedError: `"bookmark": fragments are not permitted`, }, { name: "query", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - URL: strPtr("https://example.com?arg=value"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("https://example.com?arg=value"), }, - }), + }, + }, true), expectedError: `"arg=value": query parameters are not permitted`, }, { name: "user", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - URL: strPtr("https://harry.potter@example.com/"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("https://harry.potter@example.com/"), }, - }), + }, + }, true), expectedError: `"harry.potter": user information is not permitted`, }, { name: "just totally wrong", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - URL: strPtr("arg#backwards=thisis?html.index/port:host//:https"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("arg#backwards=thisis?html.index/port:host//:https"), }, - }), + }, + }, true), expectedError: `host must be provided`, }, { name: "path must start with slash", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - Service: &admissionregistration.ServiceReference{ - Namespace: "ns", - Name: "n", - Path: strPtr("foo/"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + Service: &admissionregistration.ServiceReference{ + Namespace: "ns", + Name: "n", + Path: strPtr("foo/"), }, }, - }), + }, + }, true), expectedError: `clientConfig.service.path: Invalid value: "foo/": must start with a '/'`, }, { name: "path accepts slash", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - Service: &admissionregistration.ServiceReference{ - Namespace: "ns", - Name: "n", - Path: strPtr("/"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + Service: &admissionregistration.ServiceReference{ + Namespace: "ns", + Name: "n", + Path: strPtr("/"), }, }, - }), + }, + }, true), expectedError: ``, }, { name: "path accepts no trailing slash", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - Service: &admissionregistration.ServiceReference{ - Namespace: "ns", - Name: "n", - Path: strPtr("/foo"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + Service: &admissionregistration.ServiceReference{ + Namespace: "ns", + Name: "n", + Path: strPtr("/foo"), }, }, - }), + }, + }, true), expectedError: ``, }, { name: "path fails //", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - Service: &admissionregistration.ServiceReference{ - Namespace: "ns", - Name: "n", - Path: strPtr("//"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + Service: &admissionregistration.ServiceReference{ + Namespace: "ns", + Name: "n", + Path: strPtr("//"), }, }, - }), + }, + }, true), expectedError: `clientConfig.service.path: Invalid value: "//": segment[0] may not be empty`, }, { name: "path no empty step", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - Service: &admissionregistration.ServiceReference{ - Namespace: "ns", - Name: "n", - Path: strPtr("/foo//bar/"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + Service: &admissionregistration.ServiceReference{ + Namespace: "ns", + Name: "n", + Path: strPtr("/foo//bar/"), }, }, - }), + }, + }, true), expectedError: `clientConfig.service.path: Invalid value: "/foo//bar/": segment[1] may not be empty`, }, { name: "path no empty step 2", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - Service: &admissionregistration.ServiceReference{ - Namespace: "ns", - Name: "n", - Path: strPtr("/foo/bar//"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + Service: &admissionregistration.ServiceReference{ + Namespace: "ns", + Name: "n", + Path: strPtr("/foo/bar//"), }, }, - }), + }, + }, true), expectedError: `clientConfig.service.path: Invalid value: "/foo/bar//": segment[2] may not be empty`, }, { name: "path no non-subdomain", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: admissionregistration.WebhookClientConfig{ - Service: &admissionregistration.ServiceReference{ - Namespace: "ns", - Name: "n", - Path: strPtr("/apis/foo.bar/v1alpha1/--bad"), - }, + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + Service: &admissionregistration.ServiceReference{ + Namespace: "ns", + Name: "n", + Path: strPtr("/apis/foo.bar/v1alpha1/--bad"), }, }, - }), + }, + }, true), expectedError: `clientConfig.service.path: Invalid value: "/apis/foo.bar/v1alpha1/--bad": segment[3]: a DNS-1123 subdomain`, }, { name: "timeout seconds cannot be greater than 30", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - TimeoutSeconds: int32Ptr(31), - }, - }), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(31), + }, + }, true), expectedError: `webhooks[0].timeoutSeconds: Invalid value: 31: the timeout value must be between 1 and 30 seconds`, }, { name: "timeout seconds cannot be smaller than 1", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - TimeoutSeconds: int32Ptr(0), - }, - }), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(0), + }, + }, true), expectedError: `webhooks[0].timeoutSeconds: Invalid value: 0: the timeout value must be between 1 and 30 seconds`, }, { name: "timeout seconds must be positive", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - TimeoutSeconds: int32Ptr(-1), - }, - }), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(-1), + }, + }, true), expectedError: `webhooks[0].timeoutSeconds: Invalid value: -1: the timeout value must be between 1 and 30 seconds`, }, { name: "valid timeout seconds", - config: newValidatingWebhookConfiguration( - []admissionregistration.Webhook{ - { - Name: "webhook.k8s.io", - ClientConfig: validClientConfig, - TimeoutSeconds: int32Ptr(1), - }, - { - Name: "webhook2.k8s.io", - ClientConfig: validClientConfig, - TimeoutSeconds: int32Ptr(15), - }, - { - Name: "webhook3.k8s.io", - ClientConfig: validClientConfig, - TimeoutSeconds: int32Ptr(30), - }, - }), + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(1), + }, + { + Name: "webhook2.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(15), + }, + { + Name: "webhook3.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(30), + }, + }, true), }, } for _, test := range tests { @@ -621,3 +650,102 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) { } } + +func TestValidateValidatingWebhookConfigurationUpdate(t *testing.T) { + validClientConfig := admissionregistration.WebhookClientConfig{ + URL: strPtr("https://example.com"), + } + tests := []struct { + name string + oldconfig *admissionregistration.ValidatingWebhookConfiguration + config *admissionregistration.ValidatingWebhookConfiguration + expectedError string + }{ + { + name: "should pass on valid new AdmissionReviewVersion", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"v1beta1"}, + }, + }, true), + oldconfig: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + }, + }, true), + expectedError: ``, + }, + { + name: "should pass on invalid AdmissionReviewVersion with invalid previous versions", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"invalid-v1", "invalid-v2"}, + }, + }, true), + oldconfig: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"invalid-v0"}, + }, + }, true), + expectedError: ``, + }, + { + name: "should fail on invalid AdmissionReviewVersion with valid previous versions", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"invalid-v1"}, + }, + }, true), + oldconfig: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"v1beta1", "invalid-v1"}, + }, + }, true), + expectedError: `Invalid value: []string{"invalid-v1"}`, + }, + { + name: "should fail on invalid AdmissionReviewVersion with missing previous versions", + config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + AdmissionReviewVersions: []string{"invalid-v1"}, + }, + }, true), + oldconfig: newValidatingWebhookConfiguration([]admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + }, + }, false), + expectedError: `Invalid value: []string{"invalid-v1"}`, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + errs := ValidateValidatingWebhookConfigurationUpdate(test.config, test.oldconfig) + err := errs.ToAggregate() + if err != nil { + if e, a := test.expectedError, err.Error(); !strings.Contains(a, e) || e == "" { + t.Errorf("expected to contain %s, got %s", e, a) + } + } else { + if test.expectedError != "" { + t.Errorf("unexpected no error, expected to contain %s", test.expectedError) + } + } + }) + + } +} diff --git a/pkg/registry/admissionregistration/mutatingwebhookconfiguration/strategy.go b/pkg/registry/admissionregistration/mutatingwebhookconfiguration/strategy.go index ae0f8b7f96..9c476102ee 100644 --- a/pkg/registry/admissionregistration/mutatingwebhookconfiguration/strategy.go +++ b/pkg/registry/admissionregistration/mutatingwebhookconfiguration/strategy.go @@ -78,9 +78,7 @@ func (mutatingWebhookConfigurationStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (mutatingWebhookConfigurationStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - validationErrorList := validation.ValidateMutatingWebhookConfiguration(obj.(*admissionregistration.MutatingWebhookConfiguration)) - updateErrorList := validation.ValidateMutatingWebhookConfigurationUpdate(obj.(*admissionregistration.MutatingWebhookConfiguration), old.(*admissionregistration.MutatingWebhookConfiguration)) - return append(validationErrorList, updateErrorList...) + return validation.ValidateMutatingWebhookConfigurationUpdate(obj.(*admissionregistration.MutatingWebhookConfiguration), old.(*admissionregistration.MutatingWebhookConfiguration)) } // AllowUnconditionalUpdate is the default update policy for mutatingWebhookConfiguration objects. Status update should diff --git a/pkg/registry/admissionregistration/validatingwebhookconfiguration/strategy.go b/pkg/registry/admissionregistration/validatingwebhookconfiguration/strategy.go index ee696717bf..6100ecc0cb 100644 --- a/pkg/registry/admissionregistration/validatingwebhookconfiguration/strategy.go +++ b/pkg/registry/admissionregistration/validatingwebhookconfiguration/strategy.go @@ -63,8 +63,7 @@ func (validatingWebhookConfigurationStrategy) PrepareForUpdate(ctx context.Conte // Validate validates a new validatingWebhookConfiguration. func (validatingWebhookConfigurationStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { - ic := obj.(*admissionregistration.ValidatingWebhookConfiguration) - return validation.ValidateValidatingWebhookConfiguration(ic) + return validation.ValidateValidatingWebhookConfiguration(obj.(*admissionregistration.ValidatingWebhookConfiguration)) } // Canonicalize normalizes the object after validation. @@ -78,9 +77,7 @@ func (validatingWebhookConfigurationStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (validatingWebhookConfigurationStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - validationErrorList := validation.ValidateValidatingWebhookConfiguration(obj.(*admissionregistration.ValidatingWebhookConfiguration)) - updateErrorList := validation.ValidateValidatingWebhookConfigurationUpdate(obj.(*admissionregistration.ValidatingWebhookConfiguration), old.(*admissionregistration.ValidatingWebhookConfiguration)) - return append(validationErrorList, updateErrorList...) + return validation.ValidateValidatingWebhookConfigurationUpdate(obj.(*admissionregistration.ValidatingWebhookConfiguration), old.(*admissionregistration.ValidatingWebhookConfiguration)) } // AllowUnconditionalUpdate is the default update policy for validatingWebhookConfiguration objects. Status update should diff --git a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go b/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go index 48c3826121..f7025f6080 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go +++ b/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go @@ -248,6 +248,17 @@ type Webhook struct { // Default to 30 seconds. // +optional TimeoutSeconds *int32 `json:"timeoutSeconds,omitempty" protobuf:"varint,7,opt,name=timeoutSeconds"` + + // AdmissionReviewVersions is an ordered list of preferred `AdmissionReview` + // versions the Webhook expects. API server will try to use first version in + // the list which it supports. If none of the versions specified in this list + // supported by API server, validation will fail for this object. + // If a persisted webhook configuration specifies allowed versions and does not + // include any versions known to the API Server, calls to the webhook will fail + // and be subject to the failure policy. + // Default to `['v1beta1']`. + // +optional + AdmissionReviewVersions []string `json:"admissionReviewVersions,omitempty" protobuf:"bytes,8,rep,name=admissionReviewVersions"` } // RuleWithOperations is a tuple of Operations and Resources. It is recommended to make diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go index a2b3674955..441e64f838 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go @@ -94,6 +94,12 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta } } + // Currently dispatcher only supports `v1beta1` AdmissionReview + // TODO: Make the dispatcher capable of sending multiple AdmissionReview versions + if !util.HasAdmissionReviewVersion(v1beta1.SchemeGroupVersion.Version, h) { + return &webhook.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("webhook does not accept v1beta1 AdmissionReview")} + } + // Make the webhook request request := request.CreateAdmissionReview(attr) client, err := a.cm.HookClient(util.HookClientConfigForWebhook(h)) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go index a09fc8da3f..7a82411744 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go @@ -211,17 +211,19 @@ func NewNonMutatingTestCases(url *url.URL) []Test { Rules: []registrationv1beta1.RuleWithOperations{{ Operations: []registrationv1beta1.OperationType{registrationv1beta1.Create}, }}, - NamespaceSelector: &metav1.LabelSelector{}, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, }, { Name: "match & allow", Webhooks: []registrationv1beta1.Webhook{{ - Name: "allow.example.com", - ClientConfig: ccfgSVC("allow"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "allow.example.com", + ClientConfig: ccfgSVC("allow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, ExpectAnnotations: map[string]string{"allow.example.com/key1": "value1"}, @@ -229,20 +231,22 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match & disallow", Webhooks: []registrationv1beta1.Webhook{{ - Name: "disallow", - ClientConfig: ccfgSVC("disallow"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "disallow", + ClientConfig: ccfgSVC("disallow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ErrorContains: "without explanation", }, { Name: "match & disallow ii", Webhooks: []registrationv1beta1.Webhook{{ - Name: "disallowReason", - ClientConfig: ccfgSVC("disallowReason"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "disallowReason", + ClientConfig: ccfgSVC("disallowReason"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ErrorContains: "you shall not pass", @@ -260,6 +264,7 @@ func NewNonMutatingTestCases(url *url.URL) []Test { Operator: metav1.LabelSelectorOpIn, }}, }, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, @@ -277,29 +282,33 @@ func NewNonMutatingTestCases(url *url.URL) []Test { Operator: metav1.LabelSelectorOpNotIn, }}, }, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, }, { Name: "match & fail (but allow because fail open)", Webhooks: []registrationv1beta1.Webhook{{ - Name: "internalErr A", - ClientConfig: ccfgSVC("internalErr"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyIgnore, + Name: "internalErr A", + ClientConfig: ccfgSVC("internalErr"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyIgnore, + AdmissionReviewVersions: []string{"v1beta1"}, }, { - Name: "internalErr B", - ClientConfig: ccfgSVC("internalErr"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyIgnore, + Name: "internalErr B", + ClientConfig: ccfgSVC("internalErr"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyIgnore, + AdmissionReviewVersions: []string{"v1beta1"}, }, { - Name: "internalErr C", - ClientConfig: ccfgSVC("internalErr"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyIgnore, + Name: "internalErr C", + ClientConfig: ccfgSVC("internalErr"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyIgnore, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, @@ -307,53 +316,60 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match & fail (but disallow because fail close on nil FailurePolicy)", Webhooks: []registrationv1beta1.Webhook{{ - Name: "internalErr A", - ClientConfig: ccfgSVC("internalErr"), - NamespaceSelector: &metav1.LabelSelector{}, - Rules: matchEverythingRules, + Name: "internalErr A", + ClientConfig: ccfgSVC("internalErr"), + NamespaceSelector: &metav1.LabelSelector{}, + Rules: matchEverythingRules, + AdmissionReviewVersions: []string{"v1beta1"}, }, { - Name: "internalErr B", - ClientConfig: ccfgSVC("internalErr"), - NamespaceSelector: &metav1.LabelSelector{}, - Rules: matchEverythingRules, + Name: "internalErr B", + ClientConfig: ccfgSVC("internalErr"), + NamespaceSelector: &metav1.LabelSelector{}, + Rules: matchEverythingRules, + AdmissionReviewVersions: []string{"v1beta1"}, }, { - Name: "internalErr C", - ClientConfig: ccfgSVC("internalErr"), - NamespaceSelector: &metav1.LabelSelector{}, - Rules: matchEverythingRules, + Name: "internalErr C", + ClientConfig: ccfgSVC("internalErr"), + NamespaceSelector: &metav1.LabelSelector{}, + Rules: matchEverythingRules, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: false, }, { Name: "match & fail (but fail because fail closed)", Webhooks: []registrationv1beta1.Webhook{{ - Name: "internalErr A", - ClientConfig: ccfgSVC("internalErr"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyFail, + Name: "internalErr A", + ClientConfig: ccfgSVC("internalErr"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyFail, + AdmissionReviewVersions: []string{"v1beta1"}, }, { - Name: "internalErr B", - ClientConfig: ccfgSVC("internalErr"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyFail, + Name: "internalErr B", + ClientConfig: ccfgSVC("internalErr"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyFail, + AdmissionReviewVersions: []string{"v1beta1"}, }, { - Name: "internalErr C", - ClientConfig: ccfgSVC("internalErr"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyFail, + Name: "internalErr C", + ClientConfig: ccfgSVC("internalErr"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyFail, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: false, }, { Name: "match & allow (url)", Webhooks: []registrationv1beta1.Webhook{{ - Name: "allow.example.com", - ClientConfig: ccfgURL("allow"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "allow.example.com", + ClientConfig: ccfgURL("allow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, ExpectAnnotations: map[string]string{"allow.example.com/key1": "value1"}, @@ -361,31 +377,34 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match & disallow (url)", Webhooks: []registrationv1beta1.Webhook{{ - Name: "disallow", - ClientConfig: ccfgURL("disallow"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "disallow", + ClientConfig: ccfgURL("disallow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ErrorContains: "without explanation", }, { Name: "absent response and fail open", Webhooks: []registrationv1beta1.Webhook{{ - Name: "nilResponse", - ClientConfig: ccfgURL("nilResponse"), - FailurePolicy: &policyIgnore, - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "nilResponse", + ClientConfig: ccfgURL("nilResponse"), + FailurePolicy: &policyIgnore, + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, }, { Name: "absent response and fail closed", Webhooks: []registrationv1beta1.Webhook{{ - Name: "nilResponse", - ClientConfig: ccfgURL("nilResponse"), - FailurePolicy: &policyFail, - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "nilResponse", + ClientConfig: ccfgURL("nilResponse"), + FailurePolicy: &policyFail, + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ErrorContains: "Webhook response was absent", }, @@ -397,8 +416,9 @@ func NewNonMutatingTestCases(url *url.URL) []Test { Rules: []registrationv1beta1.RuleWithOperations{{ Operations: []registrationv1beta1.OperationType{registrationv1beta1.Create}, }}, - NamespaceSelector: &metav1.LabelSelector{}, - SideEffects: &sideEffectsSome, + NamespaceSelector: &metav1.LabelSelector{}, + SideEffects: &sideEffectsSome, + AdmissionReviewVersions: []string{"v1beta1"}, }}, IsDryRun: true, ExpectAllow: true, @@ -406,11 +426,12 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match dry run side effects Unknown", Webhooks: []registrationv1beta1.Webhook{{ - Name: "allow", - ClientConfig: ccfgSVC("allow"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - SideEffects: &sideEffectsUnknown, + Name: "allow", + ClientConfig: ccfgSVC("allow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + SideEffects: &sideEffectsUnknown, + AdmissionReviewVersions: []string{"v1beta1"}, }}, IsDryRun: true, ErrorContains: "does not support dry run", @@ -418,11 +439,12 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match dry run side effects None", Webhooks: []registrationv1beta1.Webhook{{ - Name: "allow", - ClientConfig: ccfgSVC("allow"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - SideEffects: &sideEffectsNone, + Name: "allow", + ClientConfig: ccfgSVC("allow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + SideEffects: &sideEffectsNone, + AdmissionReviewVersions: []string{"v1beta1"}, }}, IsDryRun: true, ExpectAllow: true, @@ -431,11 +453,12 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match dry run side effects Some", Webhooks: []registrationv1beta1.Webhook{{ - Name: "allow", - ClientConfig: ccfgSVC("allow"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - SideEffects: &sideEffectsSome, + Name: "allow", + ClientConfig: ccfgSVC("allow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + SideEffects: &sideEffectsSome, + AdmissionReviewVersions: []string{"v1beta1"}, }}, IsDryRun: true, ErrorContains: "does not support dry run", @@ -443,11 +466,12 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match dry run side effects NoneOnDryRun", Webhooks: []registrationv1beta1.Webhook{{ - Name: "allow", - ClientConfig: ccfgSVC("allow"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - SideEffects: &sideEffectsNoneOnDryRun, + Name: "allow", + ClientConfig: ccfgSVC("allow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + SideEffects: &sideEffectsNoneOnDryRun, + AdmissionReviewVersions: []string{"v1beta1"}, }}, IsDryRun: true, ExpectAllow: true, @@ -456,10 +480,11 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "illegal annotation format", Webhooks: []registrationv1beta1.Webhook{{ - Name: "invalidAnnotation", - ClientConfig: ccfgURL("invalidAnnotation"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "invalidAnnotation", + ClientConfig: ccfgURL("invalidAnnotation"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, }, @@ -476,10 +501,11 @@ func NewMutatingTestCases(url *url.URL) []Test { { Name: "match & remove label", Webhooks: []registrationv1beta1.Webhook{{ - Name: "removelabel.example.com", - ClientConfig: ccfgSVC("removeLabel"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "removelabel.example.com", + ClientConfig: ccfgSVC("removeLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, AdditionalLabels: map[string]string{"remove": "me"}, @@ -489,10 +515,11 @@ func NewMutatingTestCases(url *url.URL) []Test { { Name: "match & add label", Webhooks: []registrationv1beta1.Webhook{{ - Name: "addLabel", - ClientConfig: ccfgSVC("addLabel"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "addLabel", + ClientConfig: ccfgSVC("addLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, ExpectLabels: map[string]string{"pod.name": "my-pod", "added": "test"}, @@ -500,10 +527,11 @@ func NewMutatingTestCases(url *url.URL) []Test { { Name: "match CRD & add label", Webhooks: []registrationv1beta1.Webhook{{ - Name: "addLabel", - ClientConfig: ccfgSVC("addLabel"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "addLabel", + ClientConfig: ccfgSVC("addLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, IsCRD: true, ExpectAllow: true, @@ -512,10 +540,11 @@ func NewMutatingTestCases(url *url.URL) []Test { { Name: "match CRD & remove label", Webhooks: []registrationv1beta1.Webhook{{ - Name: "removelabel.example.com", - ClientConfig: ccfgSVC("removeLabel"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "removelabel.example.com", + ClientConfig: ccfgSVC("removeLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, IsCRD: true, ExpectAllow: true, @@ -526,21 +555,23 @@ func NewMutatingTestCases(url *url.URL) []Test { { Name: "match & invalid mutation", Webhooks: []registrationv1beta1.Webhook{{ - Name: "invalidMutation", - ClientConfig: ccfgSVC("invalidMutation"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, + Name: "invalidMutation", + ClientConfig: ccfgSVC("invalidMutation"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ErrorContains: "invalid character", }, { Name: "match & remove label dry run unsupported", Webhooks: []registrationv1beta1.Webhook{{ - Name: "removeLabel", - ClientConfig: ccfgSVC("removeLabel"), - Rules: matchEverythingRules, - NamespaceSelector: &metav1.LabelSelector{}, - SideEffects: &sideEffectsUnknown, + Name: "removeLabel", + ClientConfig: ccfgSVC("removeLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + SideEffects: &sideEffectsUnknown, + AdmissionReviewVersions: []string{"v1beta1"}, }}, IsDryRun: true, ErrorContains: "does not support dry run", @@ -567,11 +598,12 @@ func NewCachedClientTestcases(url *url.URL) []CachedTest { { Name: "uncached: service webhook, path 'allow'", Webhooks: []registrationv1beta1.Webhook{{ - Name: "cache1", - ClientConfig: ccfgSVC("allow"), - Rules: newMatchEverythingRules(), - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyIgnore, + Name: "cache1", + ClientConfig: ccfgSVC("allow"), + Rules: newMatchEverythingRules(), + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyIgnore, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, ExpectCacheMiss: true, @@ -579,11 +611,12 @@ func NewCachedClientTestcases(url *url.URL) []CachedTest { { Name: "uncached: service webhook, path 'internalErr'", Webhooks: []registrationv1beta1.Webhook{{ - Name: "cache2", - ClientConfig: ccfgSVC("internalErr"), - Rules: newMatchEverythingRules(), - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyIgnore, + Name: "cache2", + ClientConfig: ccfgSVC("internalErr"), + Rules: newMatchEverythingRules(), + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyIgnore, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, ExpectCacheMiss: true, @@ -591,11 +624,12 @@ func NewCachedClientTestcases(url *url.URL) []CachedTest { { Name: "cached: service webhook, path 'allow'", Webhooks: []registrationv1beta1.Webhook{{ - Name: "cache3", - ClientConfig: ccfgSVC("allow"), - Rules: newMatchEverythingRules(), - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyIgnore, + Name: "cache3", + ClientConfig: ccfgSVC("allow"), + Rules: newMatchEverythingRules(), + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyIgnore, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, ExpectCacheMiss: false, @@ -603,11 +637,12 @@ func NewCachedClientTestcases(url *url.URL) []CachedTest { { Name: "uncached: url webhook, path 'allow'", Webhooks: []registrationv1beta1.Webhook{{ - Name: "cache4", - ClientConfig: ccfgURL("allow"), - Rules: newMatchEverythingRules(), - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyIgnore, + Name: "cache4", + ClientConfig: ccfgURL("allow"), + Rules: newMatchEverythingRules(), + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyIgnore, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, ExpectCacheMiss: true, @@ -615,11 +650,12 @@ func NewCachedClientTestcases(url *url.URL) []CachedTest { { Name: "cached: service webhook, path 'allow'", Webhooks: []registrationv1beta1.Webhook{{ - Name: "cache5", - ClientConfig: ccfgURL("allow"), - Rules: newMatchEverythingRules(), - NamespaceSelector: &metav1.LabelSelector{}, - FailurePolicy: &policyIgnore, + Name: "cache5", + ClientConfig: ccfgURL("allow"), + Rules: newMatchEverythingRules(), + NamespaceSelector: &metav1.LabelSelector{}, + FailurePolicy: &policyIgnore, + AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, ExpectCacheMiss: false, diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go index 49255eba06..b5fa1ea3ec 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util/client_config.go @@ -40,3 +40,13 @@ func HookClientConfigForWebhook(w *v1beta1.Webhook) webhook.ClientConfig { } return ret } + +// HasAdmissionReviewVersion check whether a version is accepted by a given webhook. +func HasAdmissionReviewVersion(a string, w *v1beta1.Webhook) bool { + for _, b := range w.AdmissionReviewVersions { + if b == a { + return true + } + } + return false +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go index 2a70e4e64e..7889b4f5da 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go @@ -108,6 +108,12 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Webhook, } } + // Currently dispatcher only supports `v1beta1` AdmissionReview + // TODO: Make the dispatcher capable of sending multiple AdmissionReview versions + if !util.HasAdmissionReviewVersion(v1beta1.SchemeGroupVersion.Version, h) { + return &webhook.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("webhook does not accept v1beta1 AdmissionReviewRequest")} + } + // Make the webhook request request := request.CreateAdmissionReview(attr) client, err := d.cm.HookClient(util.HookClientConfigForWebhook(h))