From 29e89f54e246e2544dc56c0ff04230c3592bd031 Mon Sep 17 00:00:00 2001
From: Alexander Zielenski <zielenski@google.com>
Date: Wed, 22 Mar 2023 11:18:50 -0700
Subject: [PATCH] move check for noop managed field timestamp updates

this check needs to go after any mutations. After the mutating admission chain, rest.BeforeUpdate (which is responsible for reverting updates to immutable timestamp fields, among other things.) is called in the store.Update function. Without moving this check, it will be possible for an object to be written to etcd with only a change to its managed fields timestamp.

Kubernetes-commit: 2b01f63b115e19e8ac9f8ee8e00dde65c5f40290
---
 pkg/endpoints/handlers/patch.go        |  3 ---
 pkg/endpoints/handlers/update.go       |  9 ---------
 pkg/registry/generic/registry/store.go | 10 ++++++++++
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/pkg/endpoints/handlers/patch.go b/pkg/endpoints/handlers/patch.go
index 4f5533f34..f743e7c5d 100644
--- a/pkg/endpoints/handlers/patch.go
+++ b/pkg/endpoints/handlers/patch.go
@@ -654,9 +654,6 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti
 	}
 
 	transformers := []rest.TransformFunc{p.applyPatch, p.applyAdmission, dedupOwnerReferencesTransformer}
-	if scope.FieldManager != nil {
-		transformers = append(transformers, fieldmanager.IgnoreManagedFieldsTimestampsTransformer)
-	}
 
 	wasCreated := false
 	p.updatedObjectInfo = rest.DefaultUpdatedObjectInfo(nil, transformers...)
diff --git a/pkg/endpoints/handlers/update.go b/pkg/endpoints/handlers/update.go
index 630c97cdc..202888639 100644
--- a/pkg/endpoints/handlers/update.go
+++ b/pkg/endpoints/handlers/update.go
@@ -189,15 +189,6 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
 			})
 		}
 
-		// Ignore changes that only affect managed fields
-		// timestamps. FieldManager can't know about changes
-		// like normalized fields, defaulted fields and other
-		// mutations.
-		// Only makes sense when SSA field manager is being used
-		if scope.FieldManager != nil {
-			transformers = append(transformers, fieldmanager.IgnoreManagedFieldsTimestampsTransformer)
-		}
-
 		createAuthorizerAttributes := authorizer.AttributesRecord{
 			User:            userInfo,
 			ResourceRequest: true,
diff --git a/pkg/registry/generic/registry/store.go b/pkg/registry/generic/registry/store.go
index fa23d29d6..55f06f797 100644
--- a/pkg/registry/generic/registry/store.go
+++ b/pkg/registry/generic/registry/store.go
@@ -38,6 +38,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apimachinery/pkg/watch"
+	"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
 	"k8s.io/apiserver/pkg/registry/rest"
@@ -671,6 +672,15 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
 		if err := rest.BeforeUpdate(e.UpdateStrategy, ctx, obj, existing); err != nil {
 			return nil, nil, err
 		}
+
+		// Ignore changes that only affect managed fields timestamps.
+		// FieldManager can't know about changes like normalized fields, defaulted
+		// fields and other mutations.
+		obj, err = fieldmanager.IgnoreManagedFieldsTimestampsTransformer(ctx, obj, existing)
+		if err != nil {
+			return nil, nil, err
+		}
+
 		// at this point we have a fully formed object.  It is time to call the validators that the apiserver
 		// handling chain wants to enforce.
 		if updateValidation != nil {