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.
This commit is contained in:
Alexander Zielenski 2023-03-22 11:18:50 -07:00
parent 3cf9f66e90
commit 2b01f63b11
4 changed files with 60 additions and 12 deletions

View File

@ -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...)

View File

@ -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,

View File

@ -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 {

View File

@ -27,6 +27,7 @@ import (
"time"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"sigs.k8s.io/yaml"
appsv1 "k8s.io/api/apps/v1"
@ -249,6 +250,55 @@ func getRV(obj runtime.Object) (string, error) {
return acc.GetResourceVersion(), nil
}
func TestNoopChangeCreationTime(t *testing.T) {
client, closeFn := setup(t)
defer closeFn()
ssBytes := []byte(`{
"apiVersion": "v1",
"kind": "ConfigMap",
"metadata": {
"name": "myconfig",
"creationTimestamp": null,
"resourceVersion": null
},
"data": {
"key": "value"
}
}`)
obj, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
Namespace("default").
Param("fieldManager", "apply_test").
Resource("configmaps").
Name("myconfig").
Body(ssBytes).
Do(context.TODO()).
Get()
if err != nil {
t.Fatalf("Failed to create object: %v", err)
}
require.NoError(t, err)
// Sleep for one second to make sure that the times of each update operation is different.
time.Sleep(1200 * time.Millisecond)
newObj, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
Namespace("default").
Param("fieldManager", "apply_test").
Resource("configmaps").
Name("myconfig").
Body(ssBytes).
Do(context.TODO()).
Get()
if err != nil {
t.Fatalf("Failed to create object: %v", err)
}
require.NoError(t, err)
require.Equal(t, obj, newObj)
}
// TestNoSemanticUpdateAppleSameResourceVersion makes sure that APPLY requests which makes no semantic changes
// will not change the resource version (no write to etcd is done)
//