diff --git a/pkg/admission/plugin/webhook/mutating/dispatcher.go b/pkg/admission/plugin/webhook/mutating/dispatcher.go index a2b367495..441e64f83 100644 --- a/pkg/admission/plugin/webhook/mutating/dispatcher.go +++ b/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/pkg/admission/plugin/webhook/testing/testcase.go b/pkg/admission/plugin/webhook/testing/testcase.go index a09fc8da3..7a8241174 100644 --- a/pkg/admission/plugin/webhook/testing/testcase.go +++ b/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/pkg/admission/plugin/webhook/util/client_config.go b/pkg/admission/plugin/webhook/util/client_config.go index 49255eba0..b5fa1ea3e 100644 --- a/pkg/admission/plugin/webhook/util/client_config.go +++ b/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/pkg/admission/plugin/webhook/validating/dispatcher.go b/pkg/admission/plugin/webhook/validating/dispatcher.go index 2a70e4e64..7889b4f5d 100644 --- a/pkg/admission/plugin/webhook/validating/dispatcher.go +++ b/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))