diff --git a/staging/src/k8s.io/apiserver/pkg/admission/attributes.go b/staging/src/k8s.io/apiserver/pkg/admission/attributes.go index beea941fc30..1d291f6b22e 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/attributes.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/attributes.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation" + auditinternal "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/authentication/user" ) @@ -42,12 +43,17 @@ type attributesRecord struct { // other elements are always accessed in single goroutine. // But ValidatingAdmissionWebhook add annotations concurrently. - annotations map[string]string + annotations map[string]annotation annotationsLock sync.RWMutex reinvocationContext ReinvocationContext } +type annotation struct { + level auditinternal.Level + value string +} + func NewAttributesRecord(object runtime.Object, oldObject runtime.Object, kind schema.GroupVersionKind, namespace, name string, resource schema.GroupVersionResource, subresource string, operation Operation, operationOptions runtime.Object, dryRun bool, userInfo user.Info) Attributes { return &attributesRecord{ kind: kind, @@ -111,7 +117,7 @@ func (record *attributesRecord) GetUserInfo() user.Info { // getAnnotations implements privateAnnotationsGetter.It's a private method used // by WithAudit decorator. -func (record *attributesRecord) getAnnotations() map[string]string { +func (record *attributesRecord) getAnnotations(maxLevel auditinternal.Level) map[string]string { record.annotationsLock.RLock() defer record.annotationsLock.RUnlock() @@ -120,26 +126,36 @@ func (record *attributesRecord) getAnnotations() map[string]string { } cp := make(map[string]string, len(record.annotations)) for key, value := range record.annotations { - cp[key] = value + if value.level.Less(maxLevel) || value.level == maxLevel { + cp[key] = value.value + } } return cp } +// AddAnnotation adds an annotation to attributesRecord with Metadata audit level func (record *attributesRecord) AddAnnotation(key, value string) error { + return record.AddAnnotationWithLevel(key, value, auditinternal.LevelMetadata) +} + +func (record *attributesRecord) AddAnnotationWithLevel(key, value string, level auditinternal.Level) error { if err := checkKeyFormat(key); err != nil { return err } - + if level.Less(auditinternal.LevelMetadata) { + return fmt.Errorf("admission annotations are not allowed to be set at audit level lower than Metadata, key: %q, level: %s", key, level) + } record.annotationsLock.Lock() defer record.annotationsLock.Unlock() if record.annotations == nil { - record.annotations = make(map[string]string) + record.annotations = make(map[string]annotation) } - if v, ok := record.annotations[key]; ok && v != value { - return fmt.Errorf("admission annotations are not allowd to be overwritten, key:%q, old value: %q, new value:%q", key, record.annotations[key], value) + annotation := annotation{level: level, value: value} + if v, ok := record.annotations[key]; ok && v != annotation { + return fmt.Errorf("admission annotations are not allowd to be overwritten, key:%q, old value: %v, new value: %v", key, record.annotations[key], annotation) } - record.annotations[key] = value + record.annotations[key] = annotation return nil } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/attributes_test.go b/staging/src/k8s.io/apiserver/pkg/admission/attributes_test.go index d54780d9998..d4f886e30f9 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/attributes_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/attributes_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + auditinternal "k8s.io/apiserver/pkg/apis/audit" ) func TestAddAnnotation(t *testing.T) { @@ -28,13 +29,13 @@ func TestAddAnnotation(t *testing.T) { // test AddAnnotation attr.AddAnnotation("podsecuritypolicy.admission.k8s.io/validate-policy", "privileged") attr.AddAnnotation("podsecuritypolicy.admission.k8s.io/admit-policy", "privileged") - annotations := attr.getAnnotations() + annotations := attr.getAnnotations(auditinternal.LevelMetadata) assert.Equal(t, annotations["podsecuritypolicy.admission.k8s.io/validate-policy"], "privileged") // test overwrite assert.Error(t, attr.AddAnnotation("podsecuritypolicy.admission.k8s.io/validate-policy", "privileged-overwrite"), "admission annotations should not be allowd to be overwritten") - annotations = attr.getAnnotations() + annotations = attr.getAnnotations(auditinternal.LevelMetadata) assert.Equal(t, annotations["podsecuritypolicy.admission.k8s.io/validate-policy"], "privileged", "admission annotations should not be overwritten") // test invalid plugin names @@ -47,7 +48,7 @@ func TestAddAnnotation(t *testing.T) { for name, invalidKey := range testCases { err := attr.AddAnnotation(invalidKey, "value-foo") assert.Error(t, err) - annotations = attr.getAnnotations() + annotations = attr.getAnnotations(auditinternal.LevelMetadata) assert.Equal(t, annotations[invalidKey], "", name+": invalid pluginName is not allowed ") } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/audit.go b/staging/src/k8s.io/apiserver/pkg/admission/audit.go index 6762f53dbf0..d1e103cfc62 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/audit.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/audit.go @@ -85,11 +85,18 @@ func ensureAnnotationGetter(a Attributes) error { } func (handler auditHandler) logAnnotations(a Attributes) { + if handler.ae == nil { + return + } switch a := a.(type) { case privateAnnotationsGetter: - audit.LogAnnotations(handler.ae, a.getAnnotations()) + for key, value := range a.getAnnotations(handler.ae.Level) { + audit.LogAnnotation(handler.ae, key, value) + } case AnnotationsGetter: - audit.LogAnnotations(handler.ae, a.GetAnnotations()) + for key, value := range a.GetAnnotations(handler.ae.Level) { + audit.LogAnnotation(handler.ae, key, value) + } default: // this will never happen, because we have already checked it in ensureAnnotationGetter } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go b/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go index a3e3cbdbe2a..8882680b2c5 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + auditinternal "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/authentication/user" ) @@ -62,8 +63,15 @@ type Attributes interface { // "podsecuritypolicy" is the name of the plugin, "admission.k8s.io" is the name of the organization, "admit-policy" is the key name. // An error is returned if the format of key is invalid. When trying to overwrite annotation with a new value, an error is returned. // Both ValidationInterface and MutationInterface are allowed to add Annotations. + // By default, an annotation gets logged into audit event if the request's audit level is greater or + // equal to Metadata. AddAnnotation(key, value string) error + // AddAnnotationWithLevel sets annotation according to key-value pair with additional intended audit level. + // An Annotation gets logged into audit event if the request's audit level is greater or equal to the + // intended audit level. + AddAnnotationWithLevel(key, value string, level auditinternal.Level) error + // GetReinvocationContext tracks the admission request information relevant to the re-invocation policy. GetReinvocationContext() ReinvocationContext } @@ -86,13 +94,13 @@ type ObjectInterfaces interface { // privateAnnotationsGetter is a private interface which allows users to get annotations from Attributes. type privateAnnotationsGetter interface { - getAnnotations() map[string]string + getAnnotations(maxLevel auditinternal.Level) map[string]string } // AnnotationsGetter allows users to get annotations from Attributes. An alternate Attribute should implement // this interface. type AnnotationsGetter interface { - GetAnnotations() map[string]string + GetAnnotations(maxLevel auditinternal.Level) map[string]string } // ReinvocationContext provides access to the admission related state required to implement the re-invocation policy. 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 c2abdf51bd4..366b917332b 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 @@ -29,6 +29,7 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/util:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/audit:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library", "//vendor/github.com/evanphx/json-patch:go_default_library", "//vendor/k8s.io/klog:go_default_library", @@ -38,13 +39,18 @@ go_library( go_test( name = "go_default_test", - srcs = ["plugin_test.go"], + srcs = [ + "dispatcher_test.go", + "plugin_test.go", + ], embed = [":go_default_library"], deps = [ "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/audit:go_default_library", + "//vendor/github.com/evanphx/json-patch:go_default_library", "//vendor/github.com/stretchr/testify/assert: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 8c75fdcf2f7..83cba52e9cc 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 @@ -41,10 +41,23 @@ import ( "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" webhookrequest "k8s.io/apiserver/pkg/admission/plugin/webhook/request" "k8s.io/apiserver/pkg/admission/plugin/webhook/util" + auditinternal "k8s.io/apiserver/pkg/apis/audit" webhookutil "k8s.io/apiserver/pkg/util/webhook" utiltrace "k8s.io/utils/trace" ) +const ( + // PatchAuditAnnotationPrefix is a prefix for persisting webhook patch in audit annotation. + // Audit handler decides whether annotation with this prefix should be logged based on audit level. + // Since mutating webhook patches the request body, audit level must be greater or equal to Request + // for the annotation to be logged + PatchAuditAnnotationPrefix = "patch.webhook.admission.k8s.io/" + // MutationAuditAnnotationPrefix is a prefix for presisting webhook mutation existence in audit annotation. + MutationAuditAnnotationPrefix = "mutation.webhook.admission.k8s.io/" +) + +var encodingjson = json.CaseSensitiveJsonIterator() + type mutatingDispatcher struct { cm *webhookutil.ClientManager plugin *Plugin @@ -77,7 +90,7 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib webhookReinvokeCtx.SetLastWebhookInvocationOutput(attr.GetObject()) }() var versionedAttr *generic.VersionedAttributes - for _, hook := range hooks { + for i, hook := range hooks { attrForCheck := attr if versionedAttr != nil { attrForCheck = versionedAttr @@ -116,8 +129,11 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib } t := time.Now() - - changed, err := a.callAttrMutatingHook(ctx, hook, invocation, versionedAttr, o) + round := 0 + if reinvokeCtx.IsReinvoke() { + round = 1 + } + changed, err := a.callAttrMutatingHook(ctx, hook, invocation, versionedAttr, o, round, i) admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, versionedAttr.Attributes, "admit", hook.Name) if changed { // Patch had changed the object. Prepare to reinvoke all previous webhooks that are eligible for re-invocation. @@ -162,7 +178,11 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib // note that callAttrMutatingHook updates attr -func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta1.MutatingWebhook, invocation *generic.WebhookInvocation, attr *generic.VersionedAttributes, o admission.ObjectInterfaces) (bool, error) { +func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta1.MutatingWebhook, invocation *generic.WebhookInvocation, attr *generic.VersionedAttributes, o admission.ObjectInterfaces, round, idx int) (bool, error) { + configurationName := invocation.Webhook.GetConfigurationName() + annotator := newWebhookAnnotator(attr, round, idx, h.Name, configurationName) + changed := false + defer func() { annotator.addMutationAnnotation(changed) }() if attr.Attributes.IsDryRun() { if h.SideEffects == nil { return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook SideEffects is nil")} @@ -182,7 +202,7 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } trace := utiltrace.New("Call mutating webhook", - utiltrace.Field{"configuration", invocation.Webhook.GetConfigurationName()}, + utiltrace.Field{"configuration", configurationName}, utiltrace.Field{"webhook", h.Name}, utiltrace.Field{"resource", attr.GetResource()}, utiltrace.Field{"subresource", attr.GetSubresource()}, @@ -240,6 +260,7 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta if err != nil { return false, apierrors.NewInternalError(err) } + if len(patchObj) == 0 { return false, nil } @@ -284,10 +305,103 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta return false, apierrors.NewInternalError(err) } - changed := !apiequality.Semantic.DeepEqual(attr.VersionedObject, newVersionedObject) + changed = !apiequality.Semantic.DeepEqual(attr.VersionedObject, newVersionedObject) trace.Step("Patch applied") + annotator.addPatchAnnotation(patchObj, result.PatchType) attr.Dirty = true attr.VersionedObject = newVersionedObject o.GetObjectDefaulter().Default(attr.VersionedObject) return changed, nil } + +type webhookAnnotator struct { + attr *generic.VersionedAttributes + patchAnnotationKey string + mutationAnnotationKey string + webhook string + configuration string +} + +func newWebhookAnnotator(attr *generic.VersionedAttributes, round, idx int, webhook, configuration string) *webhookAnnotator { + return &webhookAnnotator{ + attr: attr, + patchAnnotationKey: fmt.Sprintf("%sround_%d_index_%d", PatchAuditAnnotationPrefix, round, idx), + mutationAnnotationKey: fmt.Sprintf("%sround_%d_index_%d", MutationAuditAnnotationPrefix, round, idx), + webhook: webhook, + configuration: configuration, + } +} + +func (w *webhookAnnotator) addMutationAnnotation(mutated bool) { + if w.attr == nil || w.attr.Attributes == nil { + return + } + value, err := mutationAnnotationValue(w.configuration, w.webhook, mutated) + if err != nil { + klog.Warningf("unexpected error composing mutating webhook annotation: %v", err) + return + } + if err := w.attr.Attributes.AddAnnotation(w.mutationAnnotationKey, value); err != nil { + klog.Warningf("failed to set mutation annotation for mutating webhook key %s to %s: %v", w.mutationAnnotationKey, value, err) + } +} + +func (w *webhookAnnotator) addPatchAnnotation(patch interface{}, patchType admissionv1.PatchType) { + if w.attr == nil || w.attr.Attributes == nil { + return + } + var value string + var err error + switch patchType { + case admissionv1.PatchTypeJSONPatch: + value, err = jsonPatchAnnotationValue(w.configuration, w.webhook, patch) + if err != nil { + klog.Warningf("unexpected error composing mutating webhook JSON patch annotation: %v", err) + return + } + default: + klog.Warningf("unsupported patch type for mutating webhook annotation: %v", patchType) + return + } + if err := w.attr.Attributes.AddAnnotationWithLevel(w.patchAnnotationKey, value, auditinternal.LevelRequest); err != nil { + // NOTE: we don't log actual patch in kube-apiserver log to avoid potentially + // leaking information + klog.Warningf("failed to set patch annotation for mutating webhook key %s; confugiration name: %s, webhook name: %s", w.patchAnnotationKey, w.configuration, w.webhook) + } +} + +// MutationAuditAnnotation logs if a webhook invocation mutated the request object +type MutationAuditAnnotation struct { + Configuration string `json:"configuration"` + Webhook string `json:"webhook"` + Mutated bool `json:"mutated"` +} + +// PatchAuditAnnotation logs a patch from a mutating webhook +type PatchAuditAnnotation struct { + Configuration string `json:"configuration"` + Webhook string `json:"webhook"` + Patch interface{} `json:"patch,omitempty"` + PatchType string `json:"patchType,omitempty"` +} + +func mutationAnnotationValue(configuration, webhook string, mutated bool) (string, error) { + m := MutationAuditAnnotation{ + Configuration: configuration, + Webhook: webhook, + Mutated: mutated, + } + bytes, err := encodingjson.Marshal(m) + return string(bytes), err +} + +func jsonPatchAnnotationValue(configuration, webhook string, patch interface{}) (string, error) { + p := PatchAuditAnnotation{ + Configuration: configuration, + Webhook: webhook, + Patch: patch, + PatchType: string(admissionv1.PatchTypeJSONPatch), + } + bytes, err := encodingjson.Marshal(p) + return string(bytes), err +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher_test.go new file mode 100644 index 00000000000..d64dd15f327 --- /dev/null +++ b/staging/src/k8s.io/apiserver/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/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 a24087360ea..c036c9fd882 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 @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/admission" webhooktesting "k8s.io/apiserver/pkg/admission/plugin/webhook/testing" + auditinternal "k8s.io/apiserver/pkg/apis/audit" ) // TestAdmit tests that MutatingWebhook#Admit works as expected @@ -47,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) { @@ -109,9 +110,9 @@ func TestAdmit(t *testing.T) { return } if len(tt.ExpectAnnotations) == 0 { - assert.Empty(t, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + assert.Empty(t, fakeAttr.GetAnnotations(auditinternal.LevelMetadata), tt.Name+": annotations not set as expected.") } else { - assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(auditinternal.LevelMetadata), tt.Name+": annotations not set as expected.") } reinvocationCtx := fakeAttr.Attributes.GetReinvocationContext() reinvocationCtx.SetIsReinvoke() 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 33e4f2e35fd..c55234a7333 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 @@ -21,6 +21,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testcerts:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/audit:go_default_library", "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library", "//staging/src/k8s.io/client-go/informers: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 ee728a535ba..cbcee41db3e 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 @@ -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), + }, }, } } 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 e99e9cac4c3..7c75e50e5ee 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( deps = [ "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/audit:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", ], ) 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 0734600f7e6..f09c88e3865 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 @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" webhooktesting "k8s.io/apiserver/pkg/admission/plugin/webhook/testing" + auditinternal "k8s.io/apiserver/pkg/apis/audit" ) // TestValidate tests that ValidatingWebhook#Validate works as expected @@ -87,9 +88,9 @@ func TestValidate(t *testing.T) { continue } if len(tt.ExpectAnnotations) == 0 { - assert.Empty(t, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + assert.Empty(t, fakeAttr.GetAnnotations(auditinternal.LevelMetadata), tt.Name+": annotations not set as expected.") } else { - assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(auditinternal.LevelMetadata), tt.Name+": annotations not set as expected.") } } } diff --git a/staging/src/k8s.io/apiserver/pkg/audit/request.go b/staging/src/k8s.io/apiserver/pkg/audit/request.go index 9cbbb90dada..7099e9622cd 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/request.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/request.go @@ -230,16 +230,6 @@ func LogAnnotation(ae *auditinternal.Event, key, value string) { ae.Annotations[key] = value } -// LogAnnotations fills in the Annotations according to the annotations map. -func LogAnnotations(ae *auditinternal.Event, annotations map[string]string) { - if ae == nil || ae.Level.Less(auditinternal.LevelMetadata) { - return - } - for key, value := range annotations { - LogAnnotation(ae, key, value) - } -} - // truncate User-Agent if too long, otherwise return it directly. func maybeTruncateUserAgent(req *http.Request) string { ua := req.UserAgent() diff --git a/test/integration/apiserver/admissionwebhook/BUILD b/test/integration/apiserver/admissionwebhook/BUILD index c7ce021645d..d7a18714f19 100644 --- a/test/integration/apiserver/admissionwebhook/BUILD +++ b/test/integration/apiserver/admissionwebhook/BUILD @@ -35,12 +35,15 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/audit:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/audit/v1:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", "//staging/src/k8s.io/client-go/util/retry:go_default_library", "//test/integration/etcd:go_default_library", "//test/integration/framework:go_default_library", + "//test/utils:go_default_library", ], ) diff --git a/test/integration/apiserver/admissionwebhook/reinvocation_test.go b/test/integration/apiserver/admissionwebhook/reinvocation_test.go index 8876411dbf6..f57a778e1a4 100644 --- a/test/integration/apiserver/admissionwebhook/reinvocation_test.go +++ b/test/integration/apiserver/admissionwebhook/reinvocation_test.go @@ -24,6 +24,7 @@ import ( "io/ioutil" "net/http" "net/http/httptest" + "os" "reflect" "strings" "sync" @@ -39,14 +40,26 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" + auditinternal "k8s.io/apiserver/pkg/apis/audit" + auditv1 "k8s.io/apiserver/pkg/apis/audit/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" "k8s.io/kubernetes/test/integration/framework" + "k8s.io/kubernetes/test/utils" ) const ( testReinvocationClientUsername = "webhook-reinvocation-integration-client" + auditPolicy = ` +apiVersion: audit.k8s.io/v1 +kind: Policy +rules: + - level: Request + resources: + - group: "" # core + resources: ["pods"] +` ) // TestWebhookReinvocationPolicyWithWatchCache ensures that the admission webhook reinvocation policy is applied correctly with the watch cache enabled. @@ -59,6 +72,14 @@ func TestWebhookReinvocationPolicyWithoutWatchCache(t *testing.T) { testWebhookReinvocationPolicy(t, false) } +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) +} + // testWebhookReinvocationPolicy ensures that the admission webhook reinvocation policy is applied correctly. func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { reinvokeNever := registrationv1beta1.NeverReinvocationPolicy @@ -71,13 +92,15 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { } testCases := []struct { - name string - initialPriorityClass string - webhooks []testWebhook - expectLabels map[string]string - expectInvocations map[string]int - expectError bool - errorContains string + name string + initialPriorityClass string + webhooks []testWebhook + expectLabels map[string]string + expectInvocations map[string]int + expectError bool + errorContains string + expectAuditMutationAnnotations map[string]string + expectAuditPatchAnnotations map[string]string }{ { // in-tree (mutation), webhook (no mutation), no reinvocation required name: "no reinvocation for in-tree only mutation", @@ -86,6 +109,9 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { {path: "/noop", policy: &reinvokeIfNeeded}, }, expectInvocations: map[string]int{"/noop": 1}, + expectAuditMutationAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue("admission.integration.test-0", "admission.integration.test.0.noop", false), + }, }, { // in-tree (mutation), webhook (mutation), reinvoke in-tree (no-mutation), no webhook reinvocation required name: "no webhook reinvocation for webhook when no in-tree reinvocation mutations", @@ -94,6 +120,12 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { {path: "/addlabel", policy: &reinvokeIfNeeded}, }, expectInvocations: map[string]int{"/addlabel": 1}, + expectAuditPatchAnnotations: map[string]string{ + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue("admission.integration.test-1", "admission.integration.test.0.addlabel", `[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`), + }, + expectAuditMutationAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue("admission.integration.test-1", "admission.integration.test.0.addlabel", true), + }, }, { // in-tree (mutation), webhook (mutation), reinvoke in-tree (mutation), webhook (no-mutation), both reinvoked name: "webhook is reinvoked after in-tree reinvocation", @@ -103,6 +135,13 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { {path: "/setpriority", policy: &reinvokeIfNeeded}, // trigger in-tree reinvoke mutation }, expectInvocations: map[string]int{"/setpriority": 2}, + expectAuditPatchAnnotations: map[string]string{ + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue("admission.integration.test-2", "admission.integration.test.0.setpriority", `[{"op": "add", "path": "/spec/priorityClassName", "value": "high-priority"},{"op": "remove", "path": "/spec/priority"}]`), + }, + expectAuditMutationAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue("admission.integration.test-2", "admission.integration.test.0.setpriority", true), + "mutation.webhook.admission.k8s.io/round_1_index_0": mutationAnnotationValue("admission.integration.test-2", "admission.integration.test.0.setpriority", false), + }, }, { // in-tree (mutation), webhook A (mutation), webhook B (mutation), reinvoke in-tree (no-mutation), reinvoke webhook A (no-mutation), no reinvocation of webhook B required name: "no reinvocation of webhook B when in-tree or prior webhook mutations", @@ -113,6 +152,15 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { }, expectLabels: map[string]string{"x": "true", "a": "true", "b": "true"}, expectInvocations: map[string]int{"/addlabel": 2, "/conditionaladdlabel": 1}, + expectAuditPatchAnnotations: map[string]string{ + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue("admission.integration.test-3", "admission.integration.test.0.addlabel", `[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`), + "patch.webhook.admission.k8s.io/round_0_index_1": patchAnnotationValue("admission.integration.test-3", "admission.integration.test.1.conditionaladdlabel", `[{"op": "add", "path": "/metadata/labels/b", "value": "true"}]`), + }, + expectAuditMutationAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue("admission.integration.test-3", "admission.integration.test.0.addlabel", true), + "mutation.webhook.admission.k8s.io/round_0_index_1": mutationAnnotationValue("admission.integration.test-3", "admission.integration.test.1.conditionaladdlabel", true), + "mutation.webhook.admission.k8s.io/round_1_index_0": mutationAnnotationValue("admission.integration.test-3", "admission.integration.test.0.addlabel", false), + }, }, { // in-tree (mutation), webhook A (mutation), webhook B (mutation), reinvoke in-tree (no-mutation), reinvoke webhook A (mutation), reinvoke webhook B (mutation), both webhooks reinvoked name: "all webhooks reinvoked when any webhook reinvocation causes mutation", @@ -123,6 +171,18 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { }, expectLabels: map[string]string{"x": "true", "fight": "false"}, expectInvocations: map[string]int{"/settrue": 2, "/setfalse": 2}, + expectAuditPatchAnnotations: map[string]string{ + "patch.webhook.admission.k8s.io/round_0_index_0": patchAnnotationValue("admission.integration.test-4", "admission.integration.test.0.settrue", `[{"op": "replace", "path": "/metadata/labels/fight", "value": "true"}]`), + "patch.webhook.admission.k8s.io/round_0_index_1": patchAnnotationValue("admission.integration.test-4", "admission.integration.test.1.setfalse", `[{"op": "replace", "path": "/metadata/labels/fight", "value": "false"}]`), + "patch.webhook.admission.k8s.io/round_1_index_0": patchAnnotationValue("admission.integration.test-4", "admission.integration.test.0.settrue", `[{"op": "replace", "path": "/metadata/labels/fight", "value": "true"}]`), + "patch.webhook.admission.k8s.io/round_1_index_1": patchAnnotationValue("admission.integration.test-4", "admission.integration.test.1.setfalse", `[{"op": "replace", "path": "/metadata/labels/fight", "value": "false"}]`), + }, + expectAuditMutationAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue("admission.integration.test-4", "admission.integration.test.0.settrue", true), + "mutation.webhook.admission.k8s.io/round_0_index_1": mutationAnnotationValue("admission.integration.test-4", "admission.integration.test.1.setfalse", true), + "mutation.webhook.admission.k8s.io/round_1_index_0": mutationAnnotationValue("admission.integration.test-4", "admission.integration.test.0.settrue", true), + "mutation.webhook.admission.k8s.io/round_1_index_1": mutationAnnotationValue("admission.integration.test-4", "admission.integration.test.1.setfalse", true), + }, }, { // in-tree (mutation), webhook A is SKIPPED due to objectSelector not matching, webhook B (mutation), reinvoke in-tree (no-mutation), webhook A is SKIPPED even though the labels match now, because it's not called in the first round. No reinvocation of webhook B required name: "no reinvocation of webhook B when in-tree or prior webhook mutations", @@ -133,6 +193,12 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { }, expectLabels: map[string]string{"x": "true", "a": "true"}, expectInvocations: map[string]int{"/addlabel": 1, "/conditionaladdlabel": 0}, + expectAuditPatchAnnotations: map[string]string{ + "patch.webhook.admission.k8s.io/round_0_index_1": patchAnnotationValue("admission.integration.test-5", "admission.integration.test.1.addlabel", `[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`), + }, + expectAuditMutationAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_1": mutationAnnotationValue("admission.integration.test-5", "admission.integration.test.1.addlabel", true), + }, }, { name: "invalid priority class set by webhook should result in error from in-tree priority plugin", @@ -152,6 +218,13 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { }, expectLabels: map[string]string{"x": "true", "a": "true"}, expectInvocations: map[string]int{"/conditionaladdlabel": 1, "/addlabel": 1}, + expectAuditPatchAnnotations: map[string]string{ + "patch.webhook.admission.k8s.io/round_0_index_1": patchAnnotationValue("admission.integration.test-7", "admission.integration.test.1.addlabel", `[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`), + }, + expectAuditMutationAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue("admission.integration.test-7", "admission.integration.test.0.conditionaladdlabel", false), + "mutation.webhook.admission.k8s.io/round_0_index_1": mutationAnnotationValue("admission.integration.test-7", "admission.integration.test.1.addlabel", true), + }, }, { name: "'reinvoke never' (by default) policy respected", @@ -161,6 +234,13 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { }, expectLabels: map[string]string{"x": "true", "a": "true"}, expectInvocations: map[string]int{"/conditionaladdlabel": 1, "/addlabel": 1}, + expectAuditPatchAnnotations: map[string]string{ + "patch.webhook.admission.k8s.io/round_0_index_1": patchAnnotationValue("admission.integration.test-8", "admission.integration.test.1.addlabel", `[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`), + }, + expectAuditMutationAnnotations: map[string]string{ + "mutation.webhook.admission.k8s.io/round_0_index_0": mutationAnnotationValue("admission.integration.test-8", "admission.integration.test.0.conditionaladdlabel", false), + "mutation.webhook.admission.k8s.io/round_0_index_1": mutationAnnotationValue("admission.integration.test-8", "admission.integration.test.1.addlabel", true), + }, }, } @@ -183,9 +263,33 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { webhookServer.StartTLS() defer webhookServer.Close() + // prepare audit policy file + policyFile, err := ioutil.TempFile("", "audit-policy.yaml") + if err != nil { + t.Fatalf("Failed to create audit policy file: %v", err) + } + defer os.Remove(policyFile.Name()) + if _, err := policyFile.Write([]byte(auditPolicy)); err != nil { + t.Fatalf("Failed to write audit policy file: %v", err) + } + if err := policyFile.Close(); err != nil { + t.Fatalf("Failed to close audit policy file: %v", err) + } + + // prepare audit log file + logFile, err := ioutil.TempFile("", "audit.log") + if err != nil { + t.Fatalf("Failed to create audit log file: %v", err) + } + defer os.Remove(logFile.Name()) + s := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{ "--disable-admission-plugins=ServiceAccount", fmt.Sprintf("--watch-cache=%v", watchCache), + "--audit-policy-file", policyFile.Name(), + "--audit-log-version", "audit.k8s.io/v1", + "--audit-log-mode", "blocking", + "--audit-log-path", logFile.Name(), }, framework.SharedEtcd()) defer s.TearDownFn() @@ -320,6 +424,25 @@ func testWebhookReinvocationPolicy(t *testing.T, watchCache bool) { } } } + + stream, err := os.OpenFile(logFile.Name(), os.O_RDWR, 0600) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + defer stream.Close() + missing, err := utils.CheckAuditLines(stream, expectedAuditEvents(tt.expectAuditMutationAnnotations, tt.expectAuditPatchAnnotations, ns), auditv1.SchemeGroupVersion) + if err != nil { + t.Errorf("unexpected error checking audit lines: %v", err) + } + if len(missing.MissingEvents) > 0 { + t.Errorf("failed to get expected events -- missing: %s", missing) + } + if err := stream.Truncate(0); err != nil { + t.Errorf("unexpected error truncate file: %v", err) + } + if _, err := stream.Seek(0, 0); err != nil { + t.Errorf("unexpected error reset offset: %v", err) + } }) } } @@ -455,6 +578,28 @@ func newReinvokeWebhookHandler(recorder *invocationRecorder) http.Handler { }) } +func expectedAuditEvents(webhookMutationAnnotations, webhookPatchAnnotations map[string]string, namespace string) []utils.AuditEvent { + return []utils.AuditEvent{ + { + Level: auditinternal.LevelRequest, + Stage: auditinternal.StageResponseComplete, + RequestURI: fmt.Sprintf("/api/v1/namespaces/%s/pods", namespace), + Verb: "create", + Code: 201, + User: "system:apiserver", + ImpersonatedUser: testReinvocationClientUsername, + ImpersonatedGroups: "system:authenticated,system:masters", + Resource: "pods", + Namespace: namespace, + AuthorizeDecision: "allow", + RequestObject: true, + ResponseObject: false, + AdmissionWebhookMutationAnnotations: webhookMutationAnnotations, + AdmissionWebhookPatchAnnotations: webhookPatchAnnotations, + }, + } +} + var reinvocationMarkerFixture = &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", diff --git a/test/integration/master/BUILD b/test/integration/master/BUILD index 3a83d1fb7e3..413b4b46b4e 100644 --- a/test/integration/master/BUILD +++ b/test/integration/master/BUILD @@ -27,6 +27,8 @@ go_test( "//cmd/kube-apiserver/app/options:go_default_library", "//cmd/kube-apiserver/app/testing:go_default_library", "//pkg/master:go_default_library", + "//staging/src/k8s.io/api/admission/v1beta1:go_default_library", + "//staging/src/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//staging/src/k8s.io/api/apps/v1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/networking/v1:go_default_library", @@ -39,6 +41,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/audit:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/audit/v1:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1:go_default_library", diff --git a/test/integration/master/audit_test.go b/test/integration/master/audit_test.go index ae12c72d2d0..332171a17f1 100644 --- a/test/integration/master/audit_test.go +++ b/test/integration/master/audit_test.go @@ -20,23 +20,36 @@ import ( "encoding/json" "fmt" "io/ioutil" + "net/http" "os" "strings" "testing" + "time" + "k8s.io/api/admission/v1beta1" + admissionv1beta1 "k8s.io/api/admissionregistration/v1beta1" apiv1 "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/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating" auditinternal "k8s.io/apiserver/pkg/apis/audit" auditv1 "k8s.io/apiserver/pkg/apis/audit/v1" auditv1beta1 "k8s.io/apiserver/pkg/apis/audit/v1beta1" "k8s.io/client-go/kubernetes" + clientset "k8s.io/client-go/kubernetes" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" "k8s.io/kubernetes/test/integration/framework" "k8s.io/kubernetes/test/utils" - "github.com/evanphx/json-patch" + jsonpatch "github.com/evanphx/json-patch" +) + +const ( + testWebhookConfigurationName = "auditmutation.integration.test" + testWebhookName = "auditmutation.integration.test" ) var ( @@ -44,7 +57,7 @@ var ( apiVersion: {version} kind: Policy rules: - - level: RequestResponse + - level: {level} resources: - group: "" # core resources: ["configmaps"] @@ -163,20 +176,59 @@ rules: // TestAudit ensures that both v1beta1 and v1 version audit api could work. func TestAudit(t *testing.T) { + tcs := []struct { + auditLevel auditinternal.Level + enableMutatingWebhook bool + }{ + { + auditLevel: auditinternal.LevelRequestResponse, + enableMutatingWebhook: false, + }, + { + auditLevel: auditinternal.LevelMetadata, + enableMutatingWebhook: true, + }, + { + auditLevel: auditinternal.LevelRequest, + enableMutatingWebhook: true, + }, + { + auditLevel: auditinternal.LevelRequestResponse, + enableMutatingWebhook: true, + }, + } for version := range versions { - testAudit(t, version) + for _, tc := range tcs { + t.Run(fmt.Sprintf("%s.%s.%t", version, tc.auditLevel, tc.enableMutatingWebhook), func(t *testing.T) { + testAudit(t, version, tc.auditLevel, tc.enableMutatingWebhook) + }) + } } } -func testAudit(t *testing.T, version string) { +func testAudit(t *testing.T, version string, level auditinternal.Level, enableMutatingWebhook bool) { + var url string + var err error + closeFunc := func() {} + if enableMutatingWebhook { + webhookMux := http.NewServeMux() + webhookMux.Handle("/mutation", utils.AdmissionWebhookHandler(t, admitFunc)) + url, closeFunc, err = utils.NewAdmissionWebhookServer(webhookMux) + } + defer closeFunc() + if err != nil { + t.Fatalf("%v", err) + } + // prepare audit policy file - auditPolicy := []byte(strings.Replace(auditPolicyPattern, "{version}", version, 1)) + auditPolicy := strings.Replace(auditPolicyPattern, "{version}", version, 1) + auditPolicy = strings.Replace(auditPolicy, "{level}", string(level), 1) policyFile, err := ioutil.TempFile("", "audit-policy.yaml") if err != nil { t.Fatalf("Failed to create audit policy file: %v", err) } defer os.Remove(policyFile.Name()) - if _, err := policyFile.Write(auditPolicy); err != nil { + if _, err := policyFile.Write([]byte(auditPolicy)); err != nil { t.Fatalf("Failed to write audit policy file: %v", err) } if err := policyFile.Close(); err != nil { @@ -205,21 +257,92 @@ func testAudit(t *testing.T, version string) { t.Fatalf("Unexpected error: %v", err) } - // perform configmap operations - configMapOperations(t, kubeclient) + if enableMutatingWebhook { + if err := createV1beta1MutationWebhook(kubeclient, url+"/mutation"); err != nil { + t.Fatal(err) + } + } - // check for corresponding audit logs - stream, err := os.Open(logFile.Name()) - if err != nil { - t.Fatalf("Unexpected error: %v", err) + var lastMissingReport string + if err := wait.Poll(500*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) { + // perform configmap operations + configMapOperations(t, kubeclient) + + // check for corresponding audit logs + stream, err := os.Open(logFile.Name()) + if err != nil { + return false, fmt.Errorf("unexpected error: %v", err) + } + defer stream.Close() + missingReport, err := utils.CheckAuditLines(stream, getExpectedEvents(level, enableMutatingWebhook), versions[version]) + if err != nil { + return false, fmt.Errorf("unexpected error: %v", err) + } + if len(missingReport.MissingEvents) > 0 { + lastMissingReport = missingReport.String() + return false, nil + } + return true, nil + }); err != nil { + t.Fatalf("failed to get expected events -- missingReport: %s, error: %v", lastMissingReport, err) } - defer stream.Close() - missingReport, err := utils.CheckAuditLines(stream, expectedEvents, versions[version]) - if err != nil { - t.Fatalf("Unexpected error: %v", err) +} + +func getExpectedEvents(level auditinternal.Level, enableMutatingWebhook bool) []utils.AuditEvent { + if !enableMutatingWebhook { + return expectedEvents } - if len(missingReport.MissingEvents) > 0 { - t.Errorf(missingReport.String()) + + var webhookMutationAnnotations, webhookPatchAnnotations map[string]string + var requestObject, responseObject bool + if level.GreaterOrEqual(auditinternal.LevelMetadata) { + // expect mutation existence annotation + webhookMutationAnnotations = map[string]string{} + webhookMutationAnnotations[mutating.MutationAuditAnnotationPrefix+"round_0_index_0"] = fmt.Sprintf(`{"configuration":"%s","webhook":"%s","mutated":%t}`, testWebhookConfigurationName, testWebhookName, true) + } + if level.GreaterOrEqual(auditinternal.LevelRequest) { + // expect actual patch annotation + webhookPatchAnnotations = map[string]string{} + webhookPatchAnnotations[mutating.PatchAuditAnnotationPrefix+"round_0_index_0"] = strings.Replace(fmt.Sprintf(`{"configuration": "%s", "webhook": "%s", "patch": %s, "patchType": "JSONPatch"}`, testWebhookConfigurationName, testWebhookName, `[{"op":"add","path":"/data","value":{"test":"dummy"}}]`), " ", "", -1) + // expect request object in audit log + requestObject = true + } + if level.GreaterOrEqual(auditinternal.LevelRequestResponse) { + // expect response obect in audit log + responseObject = true + } + return []utils.AuditEvent{ + { + // expect CREATE audit event with webhook in effect + Level: level, + Stage: auditinternal.StageResponseComplete, + RequestURI: fmt.Sprintf("/api/v1/namespaces/%s/configmaps", namespace), + Verb: "create", + Code: 201, + User: auditTestUser, + Resource: "configmaps", + Namespace: namespace, + AuthorizeDecision: "allow", + RequestObject: requestObject, + ResponseObject: responseObject, + AdmissionWebhookMutationAnnotations: webhookMutationAnnotations, + AdmissionWebhookPatchAnnotations: webhookPatchAnnotations, + }, { + // expect UPDATE audit event with webhook in effect + Level: level, + Stage: auditinternal.StageResponseComplete, + RequestURI: fmt.Sprintf("/api/v1/namespaces/%s/configmaps/audit-configmap", namespace), + Verb: "update", + Code: 200, + User: auditTestUser, + Resource: "configmaps", + Namespace: namespace, + AuthorizeDecision: "allow", + RequestObject: requestObject, + ResponseObject: responseObject, + AdmissionWebhookMutationAnnotations: webhookMutationAnnotations, + AdmissionWebhookPatchAnnotations: webhookPatchAnnotations, + }, } } @@ -269,3 +392,56 @@ func expectNoError(t *testing.T, err error, msg string) { t.Fatalf("%s: %v", msg, err) } } + +func admitFunc(review *v1beta1.AdmissionReview) error { + gvk := schema.GroupVersionKind{Group: "admission.k8s.io", Version: "v1beta1", Kind: "AdmissionReview"} + if review.GetObjectKind().GroupVersionKind() != gvk { + return fmt.Errorf("invalid admission review kind: %#v", review.GetObjectKind().GroupVersionKind()) + } + if len(review.Request.Object.Raw) > 0 { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(review.Request.Object.Raw, u); err != nil { + return fmt.Errorf("failed to deserialize object: %s with error: %v", string(review.Request.Object.Raw), err) + } + review.Request.Object.Object = u + } + if len(review.Request.OldObject.Raw) > 0 { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(review.Request.OldObject.Raw, u); err != nil { + return fmt.Errorf("failed to deserialize object: %s with error: %v", string(review.Request.OldObject.Raw), err) + } + review.Request.OldObject.Object = u + } + + review.Response = &v1beta1.AdmissionResponse{ + Allowed: true, + UID: review.Request.UID, + Result: &metav1.Status{Message: "admitted"}, + } + review.Response.Patch = []byte(`[{"op":"add","path":"/data","value":{"test":"dummy"}}]`) + jsonPatch := v1beta1.PatchTypeJSONPatch + review.Response.PatchType = &jsonPatch + return nil +} + +func createV1beta1MutationWebhook(client clientset.Interface, endpoint string) error { + fail := admissionv1beta1.Fail + // Attaching Mutation webhook to API server + _, err := client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Create(&admissionv1beta1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: testWebhookConfigurationName}, + Webhooks: []admissionv1beta1.MutatingWebhook{{ + Name: testWebhookName, + ClientConfig: admissionv1beta1.WebhookClientConfig{ + URL: &endpoint, + CABundle: utils.LocalhostCert, + }, + Rules: []admissionv1beta1.RuleWithOperations{{ + Operations: []admissionv1beta1.OperationType{admissionv1beta1.Create, admissionv1beta1.Update}, + Rule: admissionv1beta1.Rule{APIGroups: []string{"*"}, APIVersions: []string{"*"}, Resources: []string{"*/*"}}, + }}, + FailurePolicy: &fail, + AdmissionReviewVersions: []string{"v1beta1"}, + }}, + }) + return err +} diff --git a/test/utils/BUILD b/test/utils/BUILD index 2112cebd46c..cd04e9f89a2 100644 --- a/test/utils/BUILD +++ b/test/utils/BUILD @@ -8,6 +8,7 @@ load( go_library( name = "go_default_library", srcs = [ + "admission_webhook.go", "audit.go", "audit_dynamic.go", "conditions.go", @@ -33,6 +34,7 @@ go_library( "//pkg/apis/extensions:go_default_library", "//pkg/controller/deployment/util:go_default_library", "//pkg/util/labels:go_default_library", + "//staging/src/k8s.io/api/admission/v1beta1:go_default_library", "//staging/src/k8s.io/api/apps/v1:go_default_library", "//staging/src/k8s.io/api/auditregistration/v1alpha1:go_default_library", "//staging/src/k8s.io/api/batch/v1:go_default_library", @@ -54,6 +56,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/audit:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/audit/v1:go_default_library", "//staging/src/k8s.io/apiserver/pkg/audit:go_default_library", diff --git a/test/utils/admission_webhook.go b/test/utils/admission_webhook.go new file mode 100644 index 00000000000..c26556e9781 --- /dev/null +++ b/test/utils/admission_webhook.go @@ -0,0 +1,111 @@ +/* +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 utils + +import ( + "crypto/tls" + "crypto/x509" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + "k8s.io/api/admission/v1beta1" +) + +// NewAdmissionWebhookServer sets up a webhook server with TLS enabled, returns URL and Close function +// for the server +func NewAdmissionWebhookServer(handler http.Handler) (string, func(), error) { + // set up webhook server + roots := x509.NewCertPool() + if !roots.AppendCertsFromPEM(LocalhostCert) { + return "", nil, fmt.Errorf("Failed to append Cert from PEM") + } + cert, err := tls.X509KeyPair(LocalhostCert, LocalhostKey) + if err != nil { + return "", nil, fmt.Errorf("Failed to build cert with error: %+v", err) + } + webhookServer := httptest.NewUnstartedServer(handler) + webhookServer.TLS = &tls.Config{ + RootCAs: roots, + Certificates: []tls.Certificate{cert}, + } + webhookServer.StartTLS() + return webhookServer.URL, webhookServer.Close, nil +} + +// AdmissionWebhookHandler creates a HandlerFunc that decodes/encodes AdmissionReview and performs +// given admit function +func AdmissionWebhookHandler(t *testing.T, admit func(*v1beta1.AdmissionReview) error) http.HandlerFunc { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + data, err := ioutil.ReadAll(r.Body) + if err != nil { + t.Error(err) + return + } + if contentType := r.Header.Get("Content-Type"); contentType != "application/json" { + t.Errorf("contentType=%s, expect application/json", contentType) + return + } + + review := v1beta1.AdmissionReview{} + if err := json.Unmarshal(data, &review); err != nil { + t.Errorf("Fail to deserialize object: %s with error: %v", string(data), err) + http.Error(w, err.Error(), 400) + return + } + + if err := admit(&review); err != nil { + t.Errorf("%v", err) + http.Error(w, err.Error(), 400) + return + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(review); err != nil { + t.Errorf("Marshal of response failed with error: %v", err) + } + }) +} + +// LocalhostCert was generated from crypto/tls/generate_cert.go with the following command: +// go run generate_cert.go --rsa-bits 512 --host 127.0.0.1,::1,example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h +var LocalhostCert = []byte(`-----BEGIN CERTIFICATE----- +MIIBjzCCATmgAwIBAgIRAKpi2WmTcFrVjxrl5n5YDUEwDQYJKoZIhvcNAQELBQAw +EjEQMA4GA1UEChMHQWNtZSBDbzAgFw03MDAxMDEwMDAwMDBaGA8yMDg0MDEyOTE2 +MDAwMFowEjEQMA4GA1UEChMHQWNtZSBDbzBcMA0GCSqGSIb3DQEBAQUAA0sAMEgC +QQC9fEbRszP3t14Gr4oahV7zFObBI4TfA5i7YnlMXeLinb7MnvT4bkfOJzE6zktn +59zP7UiHs3l4YOuqrjiwM413AgMBAAGjaDBmMA4GA1UdDwEB/wQEAwICpDATBgNV +HSUEDDAKBggrBgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MC4GA1UdEQQnMCWCC2V4 +YW1wbGUuY29thwR/AAABhxAAAAAAAAAAAAAAAAAAAAABMA0GCSqGSIb3DQEBCwUA +A0EAUsVE6KMnza/ZbodLlyeMzdo7EM/5nb5ywyOxgIOCf0OOLHsPS9ueGLQX9HEG +//yjTXuhNcUugExIjM/AIwAZPQ== +-----END CERTIFICATE-----`) + +// LocalhostKey is the private key for LocalhostCert. +var LocalhostKey = []byte(`-----BEGIN RSA PRIVATE KEY----- +MIIBOwIBAAJBAL18RtGzM/e3XgavihqFXvMU5sEjhN8DmLtieUxd4uKdvsye9Phu +R84nMTrOS2fn3M/tSIezeXhg66quOLAzjXcCAwEAAQJBAKcRxH9wuglYLBdI/0OT +BLzfWPZCEw1vZmMR2FF1Fm8nkNOVDPleeVGTWoOEcYYlQbpTmkGSxJ6ya+hqRi6x +goECIQDx3+X49fwpL6B5qpJIJMyZBSCuMhH4B7JevhGGFENi3wIhAMiNJN5Q3UkL +IuSvv03kaPR5XVQ99/UeEetUgGvBcABpAiBJSBzVITIVCGkGc7d+RCf49KTCIklv +bGWObufAR8Ni4QIgWpILjW8dkGg8GOUZ0zaNA6Nvt6TIv2UWGJ4v5PoV98kCIQDx +rIiZs5QbKdycsv9gQJzwQAogC8o04X3Zz3dsoX+h4A== +-----END RSA PRIVATE KEY-----`) diff --git a/test/utils/audit.go b/test/utils/audit.go index 3a9feea9c1e..3bb9c5b4322 100644 --- a/test/utils/audit.go +++ b/test/utils/audit.go @@ -20,12 +20,14 @@ import ( "bufio" "fmt" "io" + "reflect" "sort" "strings" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating" auditinternal "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/audit" ) @@ -46,6 +48,11 @@ type AuditEvent struct { RequestObject bool ResponseObject bool AuthorizeDecision string + + // The Check functions in this package takes ownerships of these maps. You should + // not reference these maps after calling the Check functions. + AdmissionWebhookMutationAnnotations map[string]string + AdmissionWebhookPatchAnnotations map[string]string } // MissingEventsReport provides an analysis if any events are missing @@ -71,7 +78,7 @@ func (m *MissingEventsReport) String() string { // CheckAuditLines searches the audit log for the expected audit lines. func CheckAuditLines(stream io.Reader, expected []AuditEvent, version schema.GroupVersion) (missingReport *MissingEventsReport, err error) { - expectations := buildEventExpectations(expected) + expectations := newAuditEventTracker(expected) scanner := bufio.NewScanner(stream) @@ -98,24 +105,20 @@ func CheckAuditLines(stream io.Reader, expected []AuditEvent, version schema.Gro return missingReport, err } - // If the event was expected, mark it as found. - if _, found := expectations[event]; found { - expectations[event] = true - } + expectations.Mark(event) } if err := scanner.Err(); err != nil { return missingReport, err } - missingEvents := findMissing(expectations) - missingReport.MissingEvents = missingEvents + missingReport.MissingEvents = expectations.Missing() missingReport.NumEventsChecked = i return missingReport, nil } // CheckAuditList searches an audit event list for the expected audit events. func CheckAuditList(el auditinternal.EventList, expected []AuditEvent) (missing []AuditEvent, err error) { - expectations := buildEventExpectations(expected) + expectations := newAuditEventTracker(expected) for _, e := range el.Items { event, err := testEventFromInternal(&e) @@ -123,20 +126,16 @@ func CheckAuditList(el auditinternal.EventList, expected []AuditEvent) (missing return expected, err } - // If the event was expected, mark it as found. - if _, found := expectations[event]; found { - expectations[event] = true - } + expectations.Mark(event) } - missing = findMissing(expectations) - return missing, nil + return expectations.Missing(), nil } // CheckForDuplicates checks a list for duplicate events func CheckForDuplicates(el auditinternal.EventList) (auditinternal.EventList, error) { - // eventMap holds a map of audit events with just a nil value - eventMap := map[AuditEvent]*bool{} + // existingEvents holds a slice of audit events that have been seen + existingEvents := []AuditEvent{} duplicates := auditinternal.EventList{} var err error for _, e := range el.Items { @@ -145,25 +144,18 @@ func CheckForDuplicates(el auditinternal.EventList) (auditinternal.EventList, er return duplicates, err } event.ID = e.AuditID - if _, ok := eventMap[event]; ok { - duplicates.Items = append(duplicates.Items, e) - err = fmt.Errorf("failed duplicate check") - continue + for _, existing := range existingEvents { + if reflect.DeepEqual(existing, event) { + duplicates.Items = append(duplicates.Items, e) + err = fmt.Errorf("failed duplicate check") + continue + } } - eventMap[event] = nil + existingEvents = append(existingEvents, event) } return duplicates, err } -// buildEventExpectations creates a bool map out of a list of audit events -func buildEventExpectations(expected []AuditEvent) map[AuditEvent]bool { - expectations := map[AuditEvent]bool{} - for _, event := range expected { - expectations[event] = false - } - return expectations -} - // testEventFromInternal takes an internal audit event and returns a test event func testEventFromInternal(e *auditinternal.Event) (AuditEvent, error) { event := AuditEvent{ @@ -192,15 +184,58 @@ func testEventFromInternal(e *auditinternal.Event) (AuditEvent, error) { event.ImpersonatedGroups = strings.Join(e.ImpersonatedUser.Groups, ",") } event.AuthorizeDecision = e.Annotations["authorization.k8s.io/decision"] + for k, v := range e.Annotations { + if strings.HasPrefix(k, mutating.PatchAuditAnnotationPrefix) { + if event.AdmissionWebhookPatchAnnotations == nil { + event.AdmissionWebhookPatchAnnotations = map[string]string{} + } + event.AdmissionWebhookPatchAnnotations[k] = v + } else if strings.HasPrefix(k, mutating.MutationAuditAnnotationPrefix) { + if event.AdmissionWebhookMutationAnnotations == nil { + event.AdmissionWebhookMutationAnnotations = map[string]string{} + } + event.AdmissionWebhookMutationAnnotations[k] = v + } + } return event, nil } -// findMissing checks for false values in the expectations map and returns them as a list -func findMissing(expectations map[AuditEvent]bool) []AuditEvent { +// auditEvent is a private wrapper on top of AuditEvent used by auditEventTracker +type auditEvent struct { + event AuditEvent + found bool +} + +// auditEventTracker keeps track of AuditEvent expectations and marks matching events as found +type auditEventTracker struct { + events []*auditEvent +} + +// newAuditEventTracker creates a tracker that tracks whether expect events are found +func newAuditEventTracker(expected []AuditEvent) *auditEventTracker { + expectations := &auditEventTracker{events: []*auditEvent{}} + for _, event := range expected { + // we copy the references to the maps in event + expectations.events = append(expectations.events, &auditEvent{event: event, found: false}) + } + return expectations +} + +// Mark marks the given event as found if it's expected +func (t *auditEventTracker) Mark(event AuditEvent) { + for _, e := range t.events { + if reflect.DeepEqual(e.event, event) { + e.found = true + } + } +} + +// Missing reports events that are expected but not found +func (t *auditEventTracker) Missing() []AuditEvent { var missing []AuditEvent - for event, found := range expectations { - if !found { - missing = append(missing, event) + for _, e := range t.events { + if !e.found { + missing = append(missing, e.event) } } return missing