Merge pull request kubernetes#77824 from roycaihw/webhook-trace

mutating webhook: audit log mutation existence and patch

Author: k8s-ci-robot

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
 }
 

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 ")
 	}
 

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
 	}

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.

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",
     ],
 )

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
+}