diff --git a/pkg/admission/plugin/webhook/mutating/dispatcher_test.go b/pkg/admission/plugin/webhook/mutating/dispatcher_test.go new file mode 100644 index 000000000..d64dd15f3 --- /dev/null +++ b/pkg/admission/plugin/webhook/mutating/dispatcher_test.go @@ -0,0 +1,133 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mutating + +import ( + "encoding/json" + "reflect" + "testing" + + jsonpatch "github.com/evanphx/json-patch" + "github.com/stretchr/testify/assert" +) + +func TestMutationAnnotationValue(t *testing.T) { + tcs := []struct { + config string + webhook string + mutated bool + expected string + }{ + { + config: "test-config", + webhook: "test-webhook", + mutated: true, + expected: `{"configuration":"test-config","webhook":"test-webhook","mutated":true}`, + }, + { + config: "test-config", + webhook: "test-webhook", + mutated: false, + expected: `{"configuration":"test-config","webhook":"test-webhook","mutated":false}`, + }, + } + + for _, tc := range tcs { + actual, err := mutationAnnotationValue(tc.config, tc.webhook, tc.mutated) + assert.NoError(t, err, "unexpected error") + if actual != tc.expected { + t.Errorf("composed mutation annotation value doesn't match, want: %s, got: %s", tc.expected, actual) + } + } +} + +func TestJSONPatchAnnotationValue(t *testing.T) { + tcs := []struct { + name string + config string + webhook string + patch []byte + expected string + }{ + { + name: "valid patch annotation", + config: "test-config", + webhook: "test-webhook", + patch: []byte(`[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`), + expected: `{"configuration":"test-config","webhook":"test-webhook","patch":[{"op":"add","path":"/metadata/labels/a","value":"true"}],"patchType":"JSONPatch"}`, + }, + { + name: "empty configuration", + config: "", + webhook: "test-webhook", + patch: []byte(`[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`), + expected: `{"configuration":"","webhook":"test-webhook","patch":[{"op":"add","path":"/metadata/labels/a","value":"true"}],"patchType":"JSONPatch"}`, + }, + { + name: "empty webhook", + config: "test-config", + webhook: "", + patch: []byte(`[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`), + expected: `{"configuration":"test-config","webhook":"","patch":[{"op":"add","path":"/metadata/labels/a","value":"true"}],"patchType":"JSONPatch"}`, + }, + { + name: "valid JSON patch empty operation", + config: "test-config", + webhook: "test-webhook", + patch: []byte("[{}]"), + expected: `{"configuration":"test-config","webhook":"test-webhook","patch":[{}],"patchType":"JSONPatch"}`, + }, + { + name: "empty slice patch", + config: "test-config", + webhook: "test-webhook", + patch: []byte("[]"), + expected: `{"configuration":"test-config","webhook":"test-webhook","patch":[],"patchType":"JSONPatch"}`, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + jsonPatch, err := jsonpatch.DecodePatch(tc.patch) + assert.NoError(t, err, "unexpected error decode patch") + actual, err := jsonPatchAnnotationValue(tc.config, tc.webhook, jsonPatch) + assert.NoError(t, err, "unexpected error getting json patch annotation") + if actual != tc.expected { + t.Errorf("composed patch annotation value doesn't match, want: %s, got: %s", tc.expected, actual) + } + + var p map[string]interface{} + if err := json.Unmarshal([]byte(actual), &p); err != nil { + t.Errorf("unexpected error unmarshaling patch annotation: %v", err) + } + if p["configuration"] != tc.config { + t.Errorf("unmarshaled configuration doesn't match, want: %s, got: %v", tc.config, p["configuration"]) + } + if p["webhook"] != tc.webhook { + t.Errorf("unmarshaled webhook doesn't match, want: %s, got: %v", tc.webhook, p["webhook"]) + } + var expectedPatch interface{} + err = json.Unmarshal(tc.patch, &expectedPatch) + if err != nil { + t.Errorf("unexpected error unmarshaling patch: %v, %v", tc.patch, err) + } + if !reflect.DeepEqual(expectedPatch, p["patch"]) { + t.Errorf("unmarshaled patch doesn't match, want: %v, got: %v", expectedPatch, p["patch"]) + } + }) + } +} diff --git a/pkg/admission/plugin/webhook/mutating/plugin_test.go b/pkg/admission/plugin/webhook/mutating/plugin_test.go index 428bad9ab..c036c9fd8 100644 --- a/pkg/admission/plugin/webhook/mutating/plugin_test.go +++ b/pkg/admission/plugin/webhook/mutating/plugin_test.go @@ -48,8 +48,8 @@ func TestAdmit(t *testing.T) { stopCh := make(chan struct{}) defer close(stopCh) - testCases := append(webhooktesting.NewMutatingTestCases(serverURL), - webhooktesting.ConvertToMutatingTestCases(webhooktesting.NewNonMutatingTestCases(serverURL))...) + testCases := append(webhooktesting.NewMutatingTestCases(serverURL, "test-webhooks"), + webhooktesting.ConvertToMutatingTestCases(webhooktesting.NewNonMutatingTestCases(serverURL), "test-webhooks")...) for _, tt := range testCases { t.Run(tt.Name, func(t *testing.T) { diff --git a/pkg/admission/plugin/webhook/testing/testcase.go b/pkg/admission/plugin/webhook/testing/testcase.go index ee728a535..cbcee41db 100644 --- a/pkg/admission/plugin/webhook/testing/testcase.go +++ b/pkg/admission/plugin/webhook/testing/testcase.go @@ -17,8 +17,11 @@ limitations under the License. package testing import ( + "fmt" "net/http" "net/url" + "reflect" + "strings" "sync" registrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" @@ -29,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/plugin/webhook/testcerts" + auditinternal "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" @@ -136,6 +140,11 @@ type FakeAttributes struct { // AddAnnotation adds an annotation key value pair to FakeAttributes func (f *FakeAttributes) AddAnnotation(k, v string) error { + return f.AddAnnotationWithLevel(k, v, auditinternal.LevelMetadata) +} + +// AddAnnotationWithLevel adds an annotation key value pair to FakeAttributes +func (f *FakeAttributes) AddAnnotationWithLevel(k, v string, _ auditinternal.Level) error { f.mutex.Lock() defer f.mutex.Unlock() if err := f.Attributes.AddAnnotation(k, v); err != nil { @@ -149,7 +158,7 @@ func (f *FakeAttributes) AddAnnotation(k, v string) error { } // GetAnnotations reads annotations from FakeAttributes -func (f *FakeAttributes) GetAnnotations() map[string]string { +func (f *FakeAttributes) GetAnnotations(level auditinternal.Level) map[string]string { f.mutex.Lock() defer f.mutex.Unlock() return f.annotations @@ -233,9 +242,26 @@ type MutatingTest struct { } // ConvertToMutatingTestCases converts a validating test case to a mutating one for test purposes. -func ConvertToMutatingTestCases(tests []ValidatingTest) []MutatingTest { +func ConvertToMutatingTestCases(tests []ValidatingTest, configurationName string) []MutatingTest { r := make([]MutatingTest, len(tests)) for i, t := range tests { + for idx, hook := range t.Webhooks { + if t.ExpectAnnotations == nil { + t.ExpectAnnotations = map[string]string{} + } + // Add expected annotation if the converted webhook is intended to match + if reflect.DeepEqual(hook.NamespaceSelector, &metav1.LabelSelector{}) && + reflect.DeepEqual(hook.ObjectSelector, &metav1.LabelSelector{}) && + reflect.DeepEqual(hook.Rules, matchEverythingRules) { + key := fmt.Sprintf("mutation.webhook.admission.k8s.io/round_0_index_%d", idx) + value := mutationAnnotationValue(configurationName, hook.Name, false) + t.ExpectAnnotations[key] = value + } + // Break if the converted webhook is intended to fail close + if strings.Contains(hook.Name, "internalErr") && (hook.FailurePolicy == nil || *hook.FailurePolicy == registrationv1beta1.Fail) { + break + } + } r[i] = MutatingTest{t.Name, ConvertToMutatingWebhooks(t.Webhooks), t.Path, t.IsCRD, t.IsDryRun, t.AdditionalLabels, t.ExpectLabels, t.ExpectAllow, t.ErrorContains, t.ExpectAnnotations, t.ExpectStatusCode, t.ExpectReinvokeWebhooks} } return r @@ -631,10 +657,18 @@ func NewNonMutatingTestCases(url *url.URL) []ValidatingTest { } } +func mutationAnnotationValue(configuration, webhook string, mutated bool) string { + return fmt.Sprintf(`{"configuration":"%s","webhook":"%s","mutated":%t}`, configuration, webhook, mutated) +} + +func patchAnnotationValue(configuration, webhook string, patch string) string { + return strings.Replace(fmt.Sprintf(`{"configuration": "%s", "webhook": "%s", "patch": %s, "patchType": "JSONPatch"}`, configuration, webhook, patch), " ", "", -1) +} + // NewMutatingTestCases returns test cases with a given base url. // All test cases in NewMutatingTestCases have Patch set in // AdmissionResponse. The test cases are only used by both MutatingAdmissionWebhook. -func NewMutatingTestCases(url *url.URL) []MutatingTest { +func NewMutatingTestCases(url *url.URL, configurationName string) []MutatingTest { return []MutatingTest{ { Name: "match & remove label", @@ -646,10 +680,14 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { ObjectSelector: &metav1.LabelSelector{}, AdmissionReviewVersions: []string{"v1beta1"}, }}, - ExpectAllow: true, - AdditionalLabels: map[string]string{"remove": "me"}, - ExpectLabels: map[string]string{"pod.name": "my-pod"}, - ExpectAnnotations: map[string]string{"removelabel.example.com/key1": "value1"}, + ExpectAllow: true, + AdditionalLabels: map[string]string{"remove": "me"}, + ExpectLabels: map[string]string{"pod.name": "my-pod"}, + ExpectAnnotations: map[string]string{ + "removelabel.example.com/key1": "value1", + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "removelabel.example.com", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "removelabel.example.com", `[{"op": "remove", "path": "/metadata/labels/remove"}]`), + }, }, { Name: "match & add label", @@ -663,6 +701,10 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { }}, ExpectAllow: true, ExpectLabels: map[string]string{"pod.name": "my-pod", "added": "test"}, + ExpectAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "addLabel", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "addLabel", `[{"op": "add", "path": "/metadata/labels/added", "value": "test"}]`), + }, }, { Name: "match CRD & add label", @@ -677,6 +719,10 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { IsCRD: true, ExpectAllow: true, ExpectLabels: map[string]string{"crd.name": "my-test-crd", "added": "test"}, + ExpectAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "addLabel", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "addLabel", `[{"op": "add", "path": "/metadata/labels/added", "value": "test"}]`), + }, }, { Name: "match CRD & remove label", @@ -688,11 +734,15 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { ObjectSelector: &metav1.LabelSelector{}, AdmissionReviewVersions: []string{"v1beta1"}, }}, - IsCRD: true, - ExpectAllow: true, - AdditionalLabels: map[string]string{"remove": "me"}, - ExpectLabels: map[string]string{"crd.name": "my-test-crd"}, - ExpectAnnotations: map[string]string{"removelabel.example.com/key1": "value1"}, + IsCRD: true, + ExpectAllow: true, + AdditionalLabels: map[string]string{"remove": "me"}, + ExpectLabels: map[string]string{"crd.name": "my-test-crd"}, + ExpectAnnotations: map[string]string{ + "removelabel.example.com/key1": "value1", + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "removelabel.example.com", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "removelabel.example.com", `[{"op": "remove", "path": "/metadata/labels/remove"}]`), + }, }, { Name: "match & invalid mutation", @@ -706,6 +756,9 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { }}, ExpectStatusCode: http.StatusInternalServerError, ErrorContains: "invalid character", + ExpectAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "invalidMutation", false), + }, }, { Name: "match & remove label dry run unsupported", @@ -721,6 +774,9 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { IsDryRun: true, ExpectStatusCode: http.StatusBadRequest, ErrorContains: "does not support dry run", + ExpectAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "removeLabel", false), + }, }, { Name: "first webhook remove labels, second webhook shouldn't be called", @@ -747,10 +803,14 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { Rules: matchEverythingRules, AdmissionReviewVersions: []string{"v1beta1"}, }}, - ExpectAllow: true, - AdditionalLabels: map[string]string{"remove": "me"}, - ExpectLabels: map[string]string{"pod.name": "my-pod"}, - ExpectAnnotations: map[string]string{"removelabel.example.com/key1": "value1"}, + ExpectAllow: true, + AdditionalLabels: map[string]string{"remove": "me"}, + ExpectLabels: map[string]string{"pod.name": "my-pod"}, + ExpectAnnotations: map[string]string{ + "removelabel.example.com/key1": "value1", + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "removelabel.example.com", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "removelabel.example.com", `[{"op": "remove", "path": "/metadata/labels/remove"}]`), + }, }, { Name: "first webhook remove labels from CRD, second webhook shouldn't be called", @@ -777,11 +837,15 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { Rules: matchEverythingRules, AdmissionReviewVersions: []string{"v1beta1"}, }}, - IsCRD: true, - ExpectAllow: true, - AdditionalLabels: map[string]string{"remove": "me"}, - ExpectLabels: map[string]string{"crd.name": "my-test-crd"}, - ExpectAnnotations: map[string]string{"removelabel.example.com/key1": "value1"}, + IsCRD: true, + ExpectAllow: true, + AdditionalLabels: map[string]string{"remove": "me"}, + ExpectLabels: map[string]string{"crd.name": "my-test-crd"}, + ExpectAnnotations: map[string]string{ + "removelabel.example.com/key1": "value1", + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "removelabel.example.com", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "removelabel.example.com", `[{"op": "remove", "path": "/metadata/labels/remove"}]`), + }, }, // No need to test everything with the url case, since only the // connection is different. @@ -807,6 +871,12 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { AdditionalLabels: map[string]string{"remove": "me"}, ExpectAllow: true, ExpectReinvokeWebhooks: map[string]bool{"addLabel": true}, + ExpectAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "addLabel", true), + "mutation.webhook.admission.k8s.io/round_0_index_1": mutationAnnotationValue(configurationName, "removeLabel", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "addLabel", `[{"op": "add", "path": "/metadata/labels/added", "value": "test"}]`), + "patch.webhook.admission.k8s.io/round_0_index_1": patchAnnotationValue(configurationName, "removeLabel", `[{"op": "remove", "path": "/metadata/labels/remove"}]`), + }, }, { Name: "match & never reinvoke policy", @@ -821,6 +891,10 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { }}, ExpectAllow: true, ExpectReinvokeWebhooks: map[string]bool{"addLabel": false}, + ExpectAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "addLabel", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "addLabel", `[{"op": "add", "path": "/metadata/labels/added", "value": "test"}]`), + }, }, { Name: "match & never reinvoke policy (by default)", @@ -834,6 +908,10 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { }}, ExpectAllow: true, ExpectReinvokeWebhooks: map[string]bool{"addLabel": false}, + ExpectAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "addLabel", true), + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue(configurationName, "addLabel", `[{"op": "add", "path": "/metadata/labels/added", "value": "test"}]`), + }, }, { Name: "match & no reinvoke", @@ -846,6 +924,9 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest { AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectAllow: true, + ExpectAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue(configurationName, "noop", false), + }, }, } }