diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD index 96cf3fcba0..460ecab2ae 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD @@ -15,6 +15,7 @@ go_library( "//vendor/k8s.io/api/admission/v1beta1:go_default_library", "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", 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 d10f1935cd..adfe9f37f3 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 @@ -29,6 +29,7 @@ import ( admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/api/admissionregistration/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" @@ -113,11 +114,24 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta if err != nil { return apierrors.NewInternalError(err) } + + var newVersionedObject runtime.Object + if _, ok := attr.VersionedObject.(*unstructured.Unstructured); ok { + // Custom Resources don't have corresponding Go struct's. + // They are represented as Unstructured. + newVersionedObject = &unstructured.Unstructured{} + } else { + newVersionedObject, err = a.plugin.scheme.New(attr.GetKind()) + if err != nil { + return apierrors.NewInternalError(err) + } + } // TODO: if we have multiple mutating webhooks, we can remember the json // instead of encoding and decoding for each one. - if _, _, err := a.plugin.jsonSerializer.Decode(patchedJS, nil, attr.VersionedObject); err != nil { + if _, _, err := a.plugin.jsonSerializer.Decode(patchedJS, nil, newVersionedObject); err != nil { return apierrors.NewInternalError(err) } + attr.VersionedObject = newVersionedObject a.plugin.scheme.Default(attr.VersionedObject) return nil } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go index 82c4e39727..b60a62f3bd 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go @@ -18,13 +18,16 @@ package mutating import ( "net/url" + "reflect" "strings" "testing" "k8s.io/api/admission/v1beta1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/admission" webhooktesting "k8s.io/apiserver/pkg/admission/plugin/webhook/testing" ) @@ -69,10 +72,22 @@ func TestAdmit(t *testing.T) { continue } - err = wh.Admit(webhooktesting.NewAttribute(ns)) + var attr admission.Attributes + if tt.IsCRD { + attr = webhooktesting.NewAttributeUnstructured(ns, tt.AdditionalLabels) + } else { + attr = webhooktesting.NewAttribute(ns, tt.AdditionalLabels) + } + + err = wh.Admit(attr) if tt.ExpectAllow != (err == nil) { t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err) } + if tt.ExpectLabels != nil { + if !reflect.DeepEqual(tt.ExpectLabels, attr.GetObject().(metav1.Object).GetLabels()) { + t.Errorf("%s: expected labels '%v', but got '%v'", tt.Name, tt.ExpectLabels, attr.GetObject().(metav1.Object).GetLabels()) + } + } // ErrWebhookRejected is not an error for our purposes if tt.ErrorContains != "" { if err == nil || !strings.Contains(err.Error(), tt.ErrorContains) { @@ -127,7 +142,7 @@ func TestAdmitCachedClient(t *testing.T) { continue } - err = wh.Admit(webhooktesting.NewAttribute(ns)) + err = wh.Admit(webhooktesting.NewAttribute(ns, nil)) if tt.ExpectAllow != (err == nil) { t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/BUILD index 46650326fe..fcb3ce0967 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/BUILD @@ -15,7 +15,9 @@ go_library( "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/config:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/testcerts:go_default_library", 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 eb74a58f24..96d696788a 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 @@ -22,7 +22,9 @@ import ( registrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/plugin/webhook/testcerts" "k8s.io/apiserver/pkg/authentication/user" @@ -74,36 +76,57 @@ func NewFakeDataSource(name string, webhooks []registrationv1beta1.Webhook, muta return client, informerFactory } -// NewAttribute returns static admission Attributes for testing. -func NewAttribute(namespace string) admission.Attributes { - // Set up a test object for the call - kind := corev1.SchemeGroupVersion.WithKind("Pod") - name := "my-pod" - object := corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "pod.name": name, - }, - Name: name, - Namespace: namespace, - }, - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Pod", - }, +func newAttributesRecord(object metav1.Object, oldObject metav1.Object, kind schema.GroupVersionKind, namespace string, name string, resource string, labels map[string]string) admission.Attributes { + object.SetName(name) + object.SetNamespace(namespace) + objectLabels := map[string]string{resource + ".name": name} + for k, v := range labels { + objectLabels[k] = v } - oldObject := corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, - } - operation := admission.Update - resource := corev1.Resource("pods").WithVersion("v1") + object.SetLabels(objectLabels) + + oldObject.SetName(name) + oldObject.SetNamespace(namespace) + + gvr := kind.GroupVersion().WithResource(resource) subResource := "" userInfo := user.DefaultInfo{ Name: "webhook-test", UID: "webhook-test", } - return admission.NewAttributesRecord(&object, &oldObject, kind, namespace, name, resource, subResource, operation, &userInfo) + return admission.NewAttributesRecord(object.(runtime.Object), oldObject.(runtime.Object), kind, namespace, name, gvr, subResource, admission.Update, &userInfo) +} + +// NewAttribute returns static admission Attributes for testing. +func NewAttribute(namespace string, labels map[string]string) admission.Attributes { + // Set up a test object for the call + object := corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Pod", + }, + } + oldObject := corev1.Pod{} + kind := corev1.SchemeGroupVersion.WithKind("Pod") + name := "my-pod" + + return newAttributesRecord(&object, &oldObject, kind, namespace, name, "pod", labels) +} + +// NewAttributeUnstructured returns static admission Attributes for testing with custom resources. +func NewAttributeUnstructured(namespace string, labels map[string]string) admission.Attributes { + // Set up a test object for the call + object := unstructured.Unstructured{} + object.SetKind("TestCRD") + object.SetAPIVersion("custom.resource/v1") + oldObject := unstructured.Unstructured{} + oldObject.SetKind("TestCRD") + oldObject.SetAPIVersion("custom.resource/v1") + kind := object.GroupVersionKind() + name := "my-test-crd" + + return newAttributesRecord(&object, &oldObject, kind, namespace, name, "crd", labels) } type urlConfigGenerator struct { @@ -122,11 +145,14 @@ func (c urlConfigGenerator) ccfgURL(urlPath string) registrationv1beta1.WebhookC // Test is a webhook test case. type Test struct { - Name string - Webhooks []registrationv1beta1.Webhook - Path string - ExpectAllow bool - ErrorContains string + Name string + Webhooks []registrationv1beta1.Webhook + Path string + IsCRD bool + AdditionalLabels map[string]string + ExpectLabels map[string]string + ExpectAllow bool + ErrorContains string } // NewTestCases returns test cases with a given base url. @@ -158,6 +184,64 @@ func NewTestCases(url *url.URL) []Test { }}, ExpectAllow: true, }, + { + Name: "match & remove label", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "removeLabel", + ClientConfig: ccfgSVC("removeLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + ExpectAllow: true, + AdditionalLabels: map[string]string{"remove": "me"}, + ExpectLabels: map[string]string{"pod.name": "my-pod"}, + }, + { + Name: "match & add label", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "addLabel", + ClientConfig: ccfgSVC("addLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + ExpectAllow: true, + ExpectLabels: map[string]string{"pod.name": "my-pod", "added": "test"}, + }, + { + Name: "match CRD & add label", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "addLabel", + ClientConfig: ccfgSVC("addLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + IsCRD: true, + ExpectAllow: true, + ExpectLabels: map[string]string{"crd.name": "my-test-crd", "added": "test"}, + }, + { + Name: "match CRD & remove label", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "removeLabel", + ClientConfig: ccfgSVC("removeLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + IsCRD: true, + ExpectAllow: true, + AdditionalLabels: map[string]string{"remove": "me"}, + ExpectLabels: map[string]string{"crd.name": "my-test-crd"}, + }, + { + Name: "match & invalid mutation", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "invalidMutation", + ClientConfig: ccfgSVC("invalidMutation"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + ErrorContains: "invalid character", + }, { Name: "match & disallow", Webhooks: []registrationv1beta1.Webhook{{ diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go index 1736b46731..a8bb1ac823 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go @@ -85,6 +85,36 @@ func webhookHandler(w http.ResponseWriter, r *http.Request) { Allowed: true, }, }) + case "/removeLabel": + w.Header().Set("Content-Type", "application/json") + pt := v1beta1.PatchTypeJSONPatch + json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{ + Response: &v1beta1.AdmissionResponse{ + Allowed: true, + PatchType: &pt, + Patch: []byte(`[{"op": "remove", "path": "/metadata/labels/remove"}]`), + }, + }) + case "/addLabel": + w.Header().Set("Content-Type", "application/json") + pt := v1beta1.PatchTypeJSONPatch + json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{ + Response: &v1beta1.AdmissionResponse{ + Allowed: true, + PatchType: &pt, + Patch: []byte(`[{"op": "add", "path": "/metadata/labels/added", "value": "test"}]`), + }, + }) + case "/invalidMutation": + w.Header().Set("Content-Type", "application/json") + pt := v1beta1.PatchTypeJSONPatch + json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{ + Response: &v1beta1.AdmissionResponse{ + Allowed: true, + PatchType: &pt, + Patch: []byte(`[{"op": "add", "CORRUPTED_KEY":}]`), + }, + }) case "/nilResponse": w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{}) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go index 65832d0a23..1cc031c577 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go @@ -75,7 +75,7 @@ func TestValidate(t *testing.T) { continue } - err = wh.Validate(webhooktesting.NewAttribute(ns)) + err = wh.Validate(webhooktesting.NewAttribute(ns, nil)) if tt.ExpectAllow != (err == nil) { t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err) } @@ -133,7 +133,7 @@ func TestValidateCachedClient(t *testing.T) { continue } - err = wh.Validate(webhooktesting.NewAttribute(ns)) + err = wh.Validate(webhooktesting.NewAttribute(ns, nil)) if tt.ExpectAllow != (err == nil) { t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err) }