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 b9a12007dd8..c5e36a587b7 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 @@ -51,6 +51,7 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/apis/example:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/example/v1:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/example2/v1:go_default_library", + "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/github.com/stretchr/testify/require: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 0bda44562a2..048afb3569f 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 @@ -102,6 +102,13 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta return &webhookerrors.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook response was absent")} } + for k, v := range response.Response.AuditAnnotations { + key := h.Name + "/" + k + if err := attr.AddAnnotation(key, v); err != nil { + glog.Warningf("Failed to set admission audit annotation %s to %s for mutating webhook %s: %v", key, v, h.Name, err) + } + } + if !response.Response.Allowed { return webhookerrors.ToStatusErr(h.Name, response.Response.Result) } 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 165a8c89472..e41f84aee43 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 @@ -22,6 +22,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/api/admission/v1beta1" @@ -102,6 +103,16 @@ func TestAdmit(t *testing.T) { if _, isStatusErr := err.(*errors.StatusError); err != nil && !isStatusErr { t.Errorf("%s: expected a StatusError, got %T", tt.Name, err) } + fakeAttr, ok := attr.(*webhooktesting.FakeAttributes) + if !ok { + t.Errorf("Unexpected error, failed to convert attr to webhooktesting.FakeAttributes") + continue + } + if len(tt.ExpectAnnotations) == 0 { + assert.Empty(t, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + } else { + assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + } } } 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 5fa385680ed..b6ea69480c3 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 @@ -18,6 +18,7 @@ package testing import ( "net/url" + "sync" registrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" corev1 "k8s.io/api/core/v1" @@ -95,7 +96,37 @@ func newAttributesRecord(object metav1.Object, oldObject metav1.Object, kind sch UID: "webhook-test", } - return admission.NewAttributesRecord(object.(runtime.Object), oldObject.(runtime.Object), kind, namespace, name, gvr, subResource, admission.Update, dryRun, &userInfo) + return &FakeAttributes{ + Attributes: admission.NewAttributesRecord(object.(runtime.Object), oldObject.(runtime.Object), kind, namespace, name, gvr, subResource, admission.Update, dryRun, &userInfo), + } +} + +// FakeAttributes decorate admission.Attributes. It's used to trace the added annotations. +type FakeAttributes struct { + admission.Attributes + annotations map[string]string + mutex sync.Mutex +} + +// AddAnnotation adds an annotation key value pair to FakeAttributes +func (f *FakeAttributes) AddAnnotation(k, v string) error { + f.mutex.Lock() + defer f.mutex.Unlock() + if err := f.Attributes.AddAnnotation(k, v); err != nil { + return err + } + if f.annotations == nil { + f.annotations = make(map[string]string) + } + f.annotations[k] = v + return nil +} + +// GetAnnotations reads annotations from FakeAttributes +func (f *FakeAttributes) GetAnnotations() map[string]string { + f.mutex.Lock() + defer f.mutex.Unlock() + return f.annotations } // NewAttribute returns static admission Attributes for testing. @@ -145,15 +176,16 @@ func (c urlConfigGenerator) ccfgURL(urlPath string) registrationv1beta1.WebhookC // Test is a webhook test case. type Test struct { - Name string - Webhooks []registrationv1beta1.Webhook - Path string - IsCRD bool - IsDryRun bool - AdditionalLabels map[string]string - ExpectLabels map[string]string - ExpectAllow bool - ErrorContains string + Name string + Webhooks []registrationv1beta1.Webhook + Path string + IsCRD bool + IsDryRun bool + AdditionalLabels map[string]string + ExpectLabels map[string]string + ExpectAllow bool + ErrorContains string + ExpectAnnotations map[string]string } // NewNonMutatingTestCases returns test cases with a given base url. @@ -181,12 +213,13 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match & allow", Webhooks: []registrationv1beta1.Webhook{{ - Name: "allow", + Name: "allow.example.com", ClientConfig: ccfgSVC("allow"), Rules: matchEverythingRules, NamespaceSelector: &metav1.LabelSelector{}, }}, - ExpectAllow: true, + ExpectAllow: true, + ExpectAnnotations: map[string]string{"allow.example.com/key1": "value1"}, }, { Name: "match & disallow", @@ -312,12 +345,13 @@ func NewNonMutatingTestCases(url *url.URL) []Test { { Name: "match & allow (url)", Webhooks: []registrationv1beta1.Webhook{{ - Name: "allow", + Name: "allow.example.com", ClientConfig: ccfgURL("allow"), Rules: matchEverythingRules, NamespaceSelector: &metav1.LabelSelector{}, }}, - ExpectAllow: true, + ExpectAllow: true, + ExpectAnnotations: map[string]string{"allow.example.com/key1": "value1"}, }, { Name: "match & disallow (url)", @@ -374,6 +408,16 @@ func NewNonMutatingTestCases(url *url.URL) []Test { IsDryRun: true, ErrorContains: "does not support dry run", }, + { + Name: "illegal annotation format", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "invalidAnnotation", + ClientConfig: ccfgURL("invalidAnnotation"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + ExpectAllow: true, + }, // No need to test everything with the url case, since only the // connection is different. } @@ -387,14 +431,15 @@ func NewMutatingTestCases(url *url.URL) []Test { { Name: "match & remove label", Webhooks: []registrationv1beta1.Webhook{{ - Name: "removeLabel", + Name: "removelabel.example.com", ClientConfig: ccfgSVC("removeLabel"), Rules: matchEverythingRules, NamespaceSelector: &metav1.LabelSelector{}, }}, - ExpectAllow: true, - AdditionalLabels: map[string]string{"remove": "me"}, - ExpectLabels: map[string]string{"pod.name": "my-pod"}, + 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"}, }, { Name: "match & add label", @@ -422,15 +467,16 @@ func NewMutatingTestCases(url *url.URL) []Test { { Name: "match CRD & remove label", Webhooks: []registrationv1beta1.Webhook{{ - Name: "removeLabel", + Name: "removelabel.example.com", 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"}, + 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"}, }, { Name: "match & invalid mutation", 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 a8bb1ac823c..0af53633532 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 @@ -83,6 +83,9 @@ func webhookHandler(w http.ResponseWriter, r *http.Request) { json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{ Response: &v1beta1.AdmissionResponse{ Allowed: true, + AuditAnnotations: map[string]string{ + "key1": "value1", + }, }, }) case "/removeLabel": @@ -93,6 +96,9 @@ func webhookHandler(w http.ResponseWriter, r *http.Request) { Allowed: true, PatchType: &pt, Patch: []byte(`[{"op": "remove", "path": "/metadata/labels/remove"}]`), + AuditAnnotations: map[string]string{ + "key1": "value1", + }, }, }) case "/addLabel": @@ -118,6 +124,16 @@ func webhookHandler(w http.ResponseWriter, r *http.Request) { case "/nilResponse": w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{}) + case "/invalidAnnotation": + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{ + Response: &v1beta1.AdmissionResponse{ + Allowed: true, + AuditAnnotations: map[string]string{ + "invalid*key": "value1", + }, + }, + }) default: http.NotFound(w, r) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD index 60227ff8371..c66ef201728 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD @@ -36,6 +36,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing:go_default_library", + "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/github.com/stretchr/testify/require:go_default_library", ], ) 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 28cfb23b035..89884e8e1c3 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 @@ -23,15 +23,15 @@ import ( "time" "github.com/golang/glog" - "k8s.io/apiserver/pkg/admission/plugin/webhook/config" - "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/api/admissionregistration/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" + "k8s.io/apiserver/pkg/admission/plugin/webhook/config" webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors" + "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" "k8s.io/apiserver/pkg/admission/plugin/webhook/request" ) @@ -116,6 +116,12 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Webhook, if response.Response == nil { return &webhookerrors.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook response was absent")} } + for k, v := range response.Response.AuditAnnotations { + key := h.Name + "/" + k + if err := attr.AddAnnotation(key, v); err != nil { + glog.Warningf("Failed to set admission audit annotation %s to %s for validating webhook %s: %v", key, v, h.Name, err) + } + } if response.Response.Allowed { return nil } 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 60d7d9c3684..3dff3864dbc 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 @@ -21,6 +21,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/api/admission/v1beta1" @@ -72,7 +73,8 @@ func TestValidate(t *testing.T) { continue } - err = wh.Validate(webhooktesting.NewAttribute(ns, nil, tt.IsDryRun)) + attr := webhooktesting.NewAttribute(ns, nil, tt.IsDryRun) + err = wh.Validate(attr) if tt.ExpectAllow != (err == nil) { t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err) } @@ -85,6 +87,16 @@ func TestValidate(t *testing.T) { if _, isStatusErr := err.(*errors.StatusError); err != nil && !isStatusErr { t.Errorf("%s: expected a StatusError, got %T", tt.Name, err) } + fakeAttr, ok := attr.(*webhooktesting.FakeAttributes) + if !ok { + t.Errorf("Unexpected error, failed to convert attr to webhooktesting.FakeAttributes") + continue + } + if len(tt.ExpectAnnotations) == 0 { + assert.Empty(t, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + } else { + assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + } } }