drop managed fields from audit entries

drop the managed fields of the objects from the audit entries when we
are logging request and response bodies.

staging/src/k8s.io/apiserver/pkg/audit/request.go

@@ -18,6 +18,7 @@ package audit
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"net/http"
 	"reflect"
@@ -111,7 +112,8 @@ func LogImpersonatedUser(ae *auditinternal.Event, user user.Info) {
 
 // LogRequestObject fills in the request object into an audit event. The passed runtime.Object
 // will be converted to the given gv.
-func LogRequestObject(ae *auditinternal.Event, obj runtime.Object, objGV schema.GroupVersion, gvr schema.GroupVersionResource, subresource string, s runtime.NegotiatedSerializer) {
+func LogRequestObject(ctx context.Context, obj runtime.Object, objGV schema.GroupVersion, gvr schema.GroupVersionResource, subresource string, s runtime.NegotiatedSerializer) {
+	ae := AuditEventFrom(ctx)
 	if ae == nil || ae.Level.Less(auditinternal.LevelMetadata) {
 		return
 	}
@@ -151,6 +153,16 @@ func LogRequestObject(ae *auditinternal.Event, obj runtime.Object, objGV schema.
 		return
 	}
 
+	if shouldOmitManagedFields(ctx) {
+		copy, ok, err := copyWithoutManagedFields(obj)
+		if err != nil {
+			klog.Warningf("error while dropping managed fields from the request for %q error: %v", reflect.TypeOf(obj).Name(), err)
+		}
+		if ok {
+			obj = copy
+		}
+	}
+
 	// TODO(audit): hook into the serializer to avoid double conversion
 	var err error
 	ae.RequestObject, err = encodeObject(obj, objGV, s)
@@ -162,7 +174,8 @@ func LogRequestObject(ae *auditinternal.Event, obj runtime.Object, objGV schema.
 }
 
 // LogRequestPatch fills in the given patch as the request object into an audit event.
-func LogRequestPatch(ae *auditinternal.Event, patch []byte) {
+func LogRequestPatch(ctx context.Context, patch []byte) {
+	ae := AuditEventFrom(ctx)
 	if ae == nil || ae.Level.Less(auditinternal.LevelRequest) {
 		return
 	}
@@ -175,7 +188,8 @@ func LogRequestPatch(ae *auditinternal.Event, patch []byte) {
 
 // LogResponseObject fills in the response object into an audit event. The passed runtime.Object
 // will be converted to the given gv.
-func LogResponseObject(ae *auditinternal.Event, obj runtime.Object, gv schema.GroupVersion, s runtime.NegotiatedSerializer) {
+func LogResponseObject(ctx context.Context, obj runtime.Object, gv schema.GroupVersion, s runtime.NegotiatedSerializer) {
+	ae := AuditEventFrom(ctx)
 	if ae == nil || ae.Level.Less(auditinternal.LevelMetadata) {
 		return
 	}
@@ -191,6 +205,17 @@ func LogResponseObject(ae *auditinternal.Event, obj runtime.Object, gv schema.Gr
 	if ae.Level.Less(auditinternal.LevelRequestResponse) {
 		return
 	}
+
+	if shouldOmitManagedFields(ctx) {
+		copy, ok, err := copyWithoutManagedFields(obj)
+		if err != nil {
+			klog.Warningf("error while dropping managed fields from the response for %q error: %v", reflect.TypeOf(obj).Name(), err)
+		}
+		if ok {
+			obj = copy
+		}
+	}
+
 	// TODO(audit): hook into the serializer to avoid double conversion
 	var err error
 	ae.ResponseObject, err = encodeObject(obj, gv, s)
@@ -242,3 +267,72 @@ func maybeTruncateUserAgent(req *http.Request) string {
 
 	return ua
 }
+
+// copyWithoutManagedFields will make a deep copy of the specified object and
+// will discard the managed fields from the copy.
+// The specified object is expected to be a meta.Object or a "list".
+// The specified object obj is treated as readonly and hence not mutated.
+// On return, an error is set if the function runs into any error while
+// removing the managed fields, the boolean value is true if the copy has
+// been made successfully, otherwise false.
+func copyWithoutManagedFields(obj runtime.Object) (runtime.Object, bool, error) {
+	isAccessor := true
+	if _, err := meta.Accessor(obj); err != nil {
+		isAccessor = false
+	}
+	isList := meta.IsListType(obj)
+	_, isTable := obj.(*metav1.Table)
+	if !isAccessor && !isList && !isTable {
+		return nil, false, nil
+	}
+
+	// TODO a deep copy isn't really needed here, figure out how we can reliably
+	//  use shallow copy here to omit the manageFields.
+	copy := obj.DeepCopyObject()
+
+	if isAccessor {
+		if err := removeManagedFields(copy); err != nil {
+			return nil, false, err
+		}
+	}
+
+	if isList {
+		if err := meta.EachListItem(copy, removeManagedFields); err != nil {
+			return nil, false, err
+		}
+	}
+
+	if isTable {
+		table := copy.(*metav1.Table)
+		for i := range table.Rows {
+			rowObj := table.Rows[i].Object
+			if err := removeManagedFields(rowObj.Object); err != nil {
+				return nil, false, err
+			}
+		}
+	}
+
+	return copy, true, nil
+}
+
+func removeManagedFields(obj runtime.Object) error {
+	if obj == nil {
+		return nil
+	}
+	accessor, err := meta.Accessor(obj)
+	if err != nil {
+		return err
+	}
+	accessor.SetManagedFields(nil)
+	return nil
+}
+
+func shouldOmitManagedFields(ctx context.Context) bool {
+	if auditContext := AuditContextFrom(ctx); auditContext != nil {
+		return auditContext.RequestAuditConfig.OmitManagedFields
+	}
+
+	// If we can't decide, return false to maintain current behavior which is
+	// to retain the manage fields in the audit.
+	return false
+}

staging/src/k8s.io/apiserver/pkg/audit/request_test.go

@@ -18,11 +18,17 @@ package audit
 
 import (
 	"net/http"
+	"reflect"
 	"testing"
+	"time"
 
-	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
-
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	auditinternal "k8s.io/apiserver/pkg/apis/audit"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestLogAnnotation(t *testing.T) {
@@ -54,3 +60,184 @@ func TestMaybeTruncateUserAgent(t *testing.T) {
 	req.Header.Set("User-Agent", ua)
 	assert.NotEqual(t, ua, maybeTruncateUserAgent(req))
 }
+
+func TestCopyWithoutManagedFields(t *testing.T) {
+	tests := []struct {
+		name     string
+		object   runtime.Object
+		expected runtime.Object
+		ok       bool
+		err      error
+	}{
+		{
+			name:   "object specified is not a meta.Accessor or a list or a table",
+			object: &metav1.Status{},
+		},
+		{
+			name: "object specified is a meta.Accessor and has managed fields",
+			object: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "bar",
+					ManagedFields: []metav1.ManagedFieldsEntry{
+						{Manager: "a", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+						{Manager: "b", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+					},
+				},
+			},
+			expected: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "bar",
+				},
+			},
+			ok: true,
+		},
+		{
+			name: "object specified is a list and its items have managed fields",
+			object: &corev1.PodList{
+				Items: []corev1.Pod{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "foo",
+							Namespace: "ns1",
+							ManagedFields: []metav1.ManagedFieldsEntry{
+								{Manager: "a", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+								{Manager: "b", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+							},
+						},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "bar",
+							Namespace: "ns2",
+							ManagedFields: []metav1.ManagedFieldsEntry{
+								{Manager: "c", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+								{Manager: "d", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+							},
+						},
+					},
+				},
+			},
+			expected: &corev1.PodList{
+				Items: []corev1.Pod{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "foo",
+							Namespace: "ns1",
+						},
+					},
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "bar",
+							Namespace: "ns2",
+						},
+					},
+				},
+			},
+			ok: true,
+		},
+		{
+			name: "object specified is a Table and objects in its rows have managed fields",
+			object: &metav1.Table{
+				Rows: []metav1.TableRow{
+					{
+						Object: runtime.RawExtension{
+							Object: &corev1.Pod{
+								ObjectMeta: metav1.ObjectMeta{
+									Name:      "foo",
+									Namespace: "ns1",
+									ManagedFields: []metav1.ManagedFieldsEntry{
+										{Manager: "a", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+										{Manager: "b", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+									},
+								},
+							},
+						},
+					},
+					{
+						Object: runtime.RawExtension{
+							Object: &corev1.Pod{
+								ObjectMeta: metav1.ObjectMeta{
+									Name:      "bar",
+									Namespace: "ns2",
+									ManagedFields: []metav1.ManagedFieldsEntry{
+										{Manager: "c", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+										{Manager: "d", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{Time: time.Now()}},
+									},
+								},
+							},
+						},
+					},
+					// add an empty row to make sure we don't panic
+					{
+						Object: runtime.RawExtension{},
+					},
+				},
+			},
+			expected: &metav1.Table{
+				Rows: []metav1.TableRow{
+					{
+						Object: runtime.RawExtension{
+							Object: &corev1.Pod{
+								ObjectMeta: metav1.ObjectMeta{
+									Name:      "foo",
+									Namespace: "ns1",
+								},
+							},
+						},
+					},
+					{
+						Object: runtime.RawExtension{
+							Object: &corev1.Pod{
+								ObjectMeta: metav1.ObjectMeta{
+									Name:      "bar",
+									Namespace: "ns2",
+								},
+							},
+						},
+					},
+					{
+						Object: runtime.RawExtension{},
+					},
+				},
+			},
+			ok: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			original := test.object.DeepCopyObject()
+			objectGot, ok, err := copyWithoutManagedFields(test.object)
+
+			if test.err != err {
+				t.Errorf("expected error: %v, but got: %v", test.err, err)
+			}
+			if test.ok != ok {
+				t.Errorf("expected ok: %t, but got: %t", test.ok, ok)
+			}
+
+			switch {
+			case test.expected == nil:
+				if objectGot != nil {
+					t.Errorf("expected the returned object to be nil, but got %#v", objectGot)
+				}
+			default:
+				// verify that a deep copy of the specified object is made before mutating it.
+				if expected, actual := reflect.ValueOf(test.object), reflect.ValueOf(objectGot); expected.Pointer() == actual.Pointer() {
+					t.Error("expected the returned object to be a deep copy of the input object")
+				}
+
+				if !cmp.Equal(test.expected, objectGot) {
+					t.Errorf("expected and actual do not match, diff: %s", cmp.Diff(test.expected, objectGot))
+				}
+			}
+
+			// we always expect the original object to be unchanged.
+			if !cmp.Equal(original, test.object) {
+				t.Errorf("the original object has mutated, diff: %s", cmp.Diff(original, test.object))
+			}
+		})
+	}
+}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go

@@ -143,7 +143,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
 
 		ae := audit.AuditEventFrom(ctx)
 		admit = admission.WithAudit(admit, ae)
-		audit.LogRequestObject(ae, obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
+		audit.LogRequestObject(req.Context(), obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
 
 		userInfo, _ := request.UserFrom(ctx)
 

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go

@@ -103,9 +103,8 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc
 				}
 				trace.Step("Decoded delete options")
 
-				ae := audit.AuditEventFrom(ctx)
 				objGV := gvk.GroupVersion()
-				audit.LogRequestObject(ae, obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
+				audit.LogRequestObject(req.Context(), obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
 				trace.Step("Recorded the audit event")
 			} else {
 				if err := metainternalversionscheme.ParameterCodec.DecodeParameters(req.URL.Query(), scope.MetaGroupVersion, options); err != nil {
@@ -250,9 +249,8 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc
 					return
 				}
 
-				ae := audit.AuditEventFrom(ctx)
 				objGV := gvk.GroupVersion()
-				audit.LogRequestObject(ae, obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
+				audit.LogRequestObject(req.Context(), obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
 			} else {
 				if err := metainternalversionscheme.ParameterCodec.DecodeParameters(req.URL.Query(), scope.MetaGroupVersion, options); err != nil {
 					err = errors.NewBadRequest(err.Error())

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go

@@ -126,7 +126,7 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac
 		ae := audit.AuditEventFrom(ctx)
 		admit = admission.WithAudit(admit, ae)
 
-		audit.LogRequestPatch(ae, patchBytes)
+		audit.LogRequestPatch(req.Context(), patchBytes)
 		trace.Step("Recorded the audit event")
 
 		baseContentType := runtime.ContentTypeJSON

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go

@@ -267,9 +267,7 @@ func WriteObjectNegotiated(s runtime.NegotiatedSerializer, restrictions negotiat
 		return
 	}
 
-	if ae := audit.AuditEventFrom(req.Context()); ae != nil {
+	audit.LogResponseObject(req.Context(), object, gv, s)
-		audit.LogResponseObject(ae, object, gv, s)
-	}
 
 	encoder := s.EncoderForVersion(serializer.Serializer, gv)
 	SerializeObject(serializer.MediaType, encoder, w, req, statusCode, object)

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go

@@ -119,7 +119,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
 		trace.Step("Conversion done")
 
 		ae := audit.AuditEventFrom(ctx)
-		audit.LogRequestObject(ae, obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
+		audit.LogRequestObject(req.Context(), obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
 		admit = admission.WithAudit(admit, ae)
 
 		if err := checkName(obj, name, namespace, scope.Namer); err != nil {