Server-Side Apply: Status Wiping/Reset Fields

Adds and implements ResetFieldsProvder interface in order to ensure that
the fieldmanager no longer owns fields that get reset before the object
is persisted.

Co-authored-by: Kevin Wiesmueller <kwiesmul@redhat.com>
Co-authored-by: Kevin Delgado <kevindelgado@google.com>

go.mod

@@ -137,6 +137,7 @@ require (
 	k8s.io/sample-apiserver v0.0.0
 	k8s.io/system-validators v1.4.0
 	k8s.io/utils v0.0.0-20201110183641-67b214c5f920
+	sigs.k8s.io/structured-merge-diff/v4 v4.0.3
 	sigs.k8s.io/yaml v1.2.0
 )
 

hack/make-rules/test-cmd.sh

@@ -130,6 +130,10 @@ EOF
 # Creates a node object with name 127.0.0.1. This is required because we do not
 # run kubelet.
 #
+# An arbitrary annotation is needed to ensure field managers are saved on the
+# object. Without it, we would be creating an empty object and because status
+# and name get wiped, there were be no field managers tracking any fields.
+#
 # Exports:
 #   SUPPORTED_RESOURCES(Array of all resources supported by the apiserver).
 function create_node() {
@@ -138,7 +142,10 @@ function create_node() {
   "kind": "Node",
   "apiVersion": "v1",
   "metadata": {
-    "name": "127.0.0.1"
+    "name": "127.0.0.1",
+    "annotations": {
+      "save-managers": "true"
+    }
   },
   "status": {
     "capacity": {

pkg/registry/apiserverinternal/storageversion/storage/storage.go

@@ -29,6 +29,7 @@ import (
 	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
 	printerstorage "k8s.io/kubernetes/pkg/printers/storage"
 	strategy "k8s.io/kubernetes/pkg/registry/apiserverinternal/storageversion"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 // REST implements a RESTStorage for storage version against etcd
@@ -46,17 +47,19 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) {
 		},
 		DefaultQualifiedResource: apiserverinternal.Resource("storageversions"),
 
-		CreateStrategy: strategy.Strategy,
-		UpdateStrategy: strategy.Strategy,
-		DeleteStrategy: strategy.Strategy,
-		TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)},
+		CreateStrategy:      strategy.Strategy,
+		UpdateStrategy:      strategy.Strategy,
+		DeleteStrategy:      strategy.Strategy,
+		ResetFieldsStrategy: strategy.Strategy,
+		TableConvertor:      printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)},
 	}
 	options := &generic.StoreOptions{RESTOptions: optsGetter}
 	if err := store.CompleteWithOptions(options); err != nil {
 		return nil, nil, err
 	}
 	statusStore := *store
 	statusStore.UpdateStrategy = strategy.StatusStrategy
+	statusStore.ResetFieldsStrategy = strategy.StatusStrategy
 	return &REST{store}, &StatusREST{store: &statusStore}, nil
 }
 
@@ -81,3 +84,8 @@ func (r *StatusREST) Update(ctx context.Context, name string, objInfo rest.Updat
 	// subresources should never allow create on update.
 	return r.store.Update(ctx, name, objInfo, createValidation, updateValidation, false, options)
 }
+
+// GetResetFields implements rest.ResetFieldsStrategy
+func (r *StatusREST) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	return r.store.GetResetFields()
+}

pkg/registry/apiserverinternal/storageversion/strategy.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/apis/apiserverinternal"
 	"k8s.io/kubernetes/pkg/apis/apiserverinternal/validation"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 // storageVersionStrategy implements verification logic for StorageVersion.
@@ -42,6 +43,18 @@ func (storageVersionStrategy) NamespaceScoped() bool {
 	return false
 }
 
+// GetResetFields returns the set of fields that get reset by the strategy
+// and should not be modified by the user.
+func (storageVersionStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	fields := map[fieldpath.APIVersion]*fieldpath.Set{
+		"internal.apiserver.k8s.io/v1alpha1": fieldpath.NewSet(
+			fieldpath.MakePathOrDie("status"),
+		),
+	}
+
+	return fields
+}
+
 // PrepareForCreate clears the status of an StorageVersion before creation.
 func (storageVersionStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	sv := obj.(*apiserverinternal.StorageVersion)
@@ -90,6 +103,19 @@ type storageVersionStatusStrategy struct {
 // StatusStrategy is the default logic invoked when updating object status.
 var StatusStrategy = storageVersionStatusStrategy{Strategy}
 
+// GetResetFields returns the set of fields that get reset by the strategy
+// and should not be modified by the user.
+func (storageVersionStatusStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	fields := map[fieldpath.APIVersion]*fieldpath.Set{
+		"internal.apiserver.k8s.io/v1alpha1": fieldpath.NewSet(
+			fieldpath.MakePathOrDie("metadata"),
+			fieldpath.MakePathOrDie("spec"),
+		),
+	}
+
+	return fields
+}
+
 func (storageVersionStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	newSV := obj.(*apiserverinternal.StorageVersion)
 	oldSV := old.(*apiserverinternal.StorageVersion)

pkg/registry/apps/daemonset/storage/storage.go

@@ -29,6 +29,7 @@ import (
 	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
 	printerstorage "k8s.io/kubernetes/pkg/printers/storage"
 	"k8s.io/kubernetes/pkg/registry/apps/daemonset"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 // REST implements a RESTStorage for DaemonSets
@@ -44,9 +45,10 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) {
 		NewListFunc:              func() runtime.Object { return &apps.DaemonSetList{} },
 		DefaultQualifiedResource: apps.Resource("daemonsets"),
 
-		CreateStrategy: daemonset.Strategy,
-		UpdateStrategy: daemonset.Strategy,
-		DeleteStrategy: daemonset.Strategy,
+		CreateStrategy:      daemonset.Strategy,
+		UpdateStrategy:      daemonset.Strategy,
+		DeleteStrategy:      daemonset.Strategy,
+		ResetFieldsStrategy: daemonset.Strategy,
 
 		TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)},
 	}
@@ -57,6 +59,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) {
 
 	statusStore := *store
 	statusStore.UpdateStrategy = daemonset.StatusStrategy
+	statusStore.ResetFieldsStrategy = daemonset.StatusStrategy
 
 	return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore}, nil
 }
@@ -103,3 +106,8 @@ func (r *StatusREST) Update(ctx context.Context, name string, objInfo rest.Updat
 	// subresources should never allow create on update.
 	return r.store.Update(ctx, name, objInfo, createValidation, updateValidation, false, options)
 }
+
+// GetResetFields implements rest.ResetFieldsStrategy
+func (r *StatusREST) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	return r.store.GetResetFields()
+}

pkg/registry/apps/daemonset/strategy.go

@@ -36,6 +36,7 @@ import (
 	"k8s.io/kubernetes/pkg/apis/apps"
 	"k8s.io/kubernetes/pkg/apis/apps/validation"
 	"k8s.io/kubernetes/pkg/features"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 // daemonSetStrategy implements verification logic for daemon sets.
@@ -68,6 +69,18 @@ func (daemonSetStrategy) NamespaceScoped() bool {
 	return true
 }
 
+// GetResetFields returns the set of fields that get reset by the strategy
+// and should not be modified by the user.
+func (daemonSetStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	fields := map[fieldpath.APIVersion]*fieldpath.Set{
+		"apps/v1": fieldpath.NewSet(
+			fieldpath.MakePathOrDie("status"),
+		),
+	}
+
+	return fields
+}
+
 // PrepareForCreate clears the status of a daemon set before creation.
 func (daemonSetStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	daemonSet := obj.(*apps.DaemonSet)
@@ -202,6 +215,16 @@ type daemonSetStatusStrategy struct {
 // StatusStrategy is the default logic invoked when updating object status.
 var StatusStrategy = daemonSetStatusStrategy{Strategy}
 
+// GetResetFields returns the set of fields that get reset by the strategy
+// and should not be modified by the user.
+func (daemonSetStatusStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	return map[fieldpath.APIVersion]*fieldpath.Set{
+		"apps/v1": fieldpath.NewSet(
+			fieldpath.MakePathOrDie("spec"),
+		),
+	}
+}
+
 func (daemonSetStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	newDaemonSet := obj.(*apps.DaemonSet)
 	oldDaemonSet := old.(*apps.DaemonSet)

pkg/registry/apps/deployment/storage/storage.go

@@ -43,6 +43,7 @@ import (
 	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
 	printerstorage "k8s.io/kubernetes/pkg/printers/storage"
 	"k8s.io/kubernetes/pkg/registry/apps/deployment"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 // DeploymentStorage includes dummy storage for Deployments and for Scale subresource.
@@ -81,9 +82,10 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Rollbac
 		NewListFunc:              func() runtime.Object { return &apps.DeploymentList{} },
 		DefaultQualifiedResource: apps.Resource("deployments"),
 
-		CreateStrategy: deployment.Strategy,
-		UpdateStrategy: deployment.Strategy,
-		DeleteStrategy: deployment.Strategy,
+		CreateStrategy:      deployment.Strategy,
+		UpdateStrategy:      deployment.Strategy,
+		DeleteStrategy:      deployment.Strategy,
+		ResetFieldsStrategy: deployment.Strategy,
 
 		TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)},
 	}
@@ -94,6 +96,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, *Rollbac
 
 	statusStore := *store
 	statusStore.UpdateStrategy = deployment.StatusStrategy
+	statusStore.ResetFieldsStrategy = deployment.StatusStrategy
 	return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore}, &RollbackREST{store: store}, nil
 }
 
@@ -141,6 +144,11 @@ func (r *StatusREST) Update(ctx context.Context, name string, objInfo rest.Updat
 	return r.store.Update(ctx, name, objInfo, createValidation, updateValidation, false, options)
 }
 
+// GetResetFields implements rest.ResetFieldsStrategy
+func (r *StatusREST) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	return r.store.GetResetFields()
+}
+
 // RollbackREST implements the REST endpoint for initiating the rollback of a deployment
 type RollbackREST struct {
 	store *genericregistry.Store

pkg/registry/apps/deployment/strategy.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/kubernetes/pkg/api/pod"
 	"k8s.io/kubernetes/pkg/apis/apps"
 	"k8s.io/kubernetes/pkg/apis/apps/validation"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 // deploymentStrategy implements behavior for Deployments.
@@ -67,6 +68,18 @@ func (deploymentStrategy) NamespaceScoped() bool {
 	return true
 }
 
+// GetResetFields returns the set of fields that get reset by the strategy
+// and should not be modified by the user.
+func (deploymentStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	fields := map[fieldpath.APIVersion]*fieldpath.Set{
+		"apps/v1": fieldpath.NewSet(
+			fieldpath.MakePathOrDie("status"),
+		),
+	}
+
+	return fields
+}
+
 // PrepareForCreate clears fields that are not allowed to be set by end users on creation.
 func (deploymentStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	deployment := obj.(*apps.Deployment)
@@ -147,6 +160,17 @@ type deploymentStatusStrategy struct {
 // StatusStrategy is the default logic invoked when updating object status.
 var StatusStrategy = deploymentStatusStrategy{Strategy}
 
+// GetResetFields returns the set of fields that get reset by the strategy
+// and should not be modified by the user.
+func (deploymentStatusStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	return map[fieldpath.APIVersion]*fieldpath.Set{
+		"apps/v1": fieldpath.NewSet(
+			fieldpath.MakePathOrDie("spec"),
+			fieldpath.MakePathOrDie("metadata", "labels"),
+		),
+	}
+}
+
 // PrepareForUpdate clears fields that are not allowed to be set by end users on update of status
 func (deploymentStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	newDeployment := obj.(*apps.Deployment)

pkg/registry/apps/replicaset/storage/storage.go

@@ -40,6 +40,7 @@ import (
 	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
 	printerstorage "k8s.io/kubernetes/pkg/printers/storage"
 	"k8s.io/kubernetes/pkg/registry/apps/replicaset"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 // ReplicaSetStorage includes dummy storage for ReplicaSets and for Scale subresource.
@@ -77,9 +78,10 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) {
 		PredicateFunc:            replicaset.MatchReplicaSet,
 		DefaultQualifiedResource: apps.Resource("replicasets"),
 
-		CreateStrategy: replicaset.Strategy,
-		UpdateStrategy: replicaset.Strategy,
-		DeleteStrategy: replicaset.Strategy,
+		CreateStrategy:      replicaset.Strategy,
+		UpdateStrategy:      replicaset.Strategy,
+		DeleteStrategy:      replicaset.Strategy,
+		ResetFieldsStrategy: replicaset.Strategy,
 
 		TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)},
 	}
@@ -90,6 +92,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) {
 
 	statusStore := *store
 	statusStore.UpdateStrategy = replicaset.StatusStrategy
+	statusStore.ResetFieldsStrategy = replicaset.StatusStrategy
 
 	return &REST{store, []string{"all"}}, &StatusREST{store: &statusStore}, nil
 }
@@ -138,6 +141,11 @@ func (r *StatusREST) Update(ctx context.Context, name string, objInfo rest.Updat
 	return r.store.Update(ctx, name, objInfo, createValidation, updateValidation, false, options)
 }
 
+// GetResetFields implements rest.ResetFieldsStrategy
+func (r *StatusREST) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	return r.store.GetResetFields()
+}
+
 // ScaleREST implements a Scale for ReplicaSet.
 type ScaleREST struct {
 	store *genericregistry.Store

pkg/registry/apps/replicaset/strategy.go

@@ -41,6 +41,7 @@ import (
 	"k8s.io/kubernetes/pkg/api/pod"
 	"k8s.io/kubernetes/pkg/apis/apps"
 	"k8s.io/kubernetes/pkg/apis/apps/validation"
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
 // rsStrategy implements verification logic for ReplicaSets.
@@ -73,6 +74,18 @@ func (rsStrategy) NamespaceScoped() bool {
 	return true
 }
 
+// GetResetFields returns the set of fields that get reset by the strategy
+// and should not be modified by the user.
+func (rsStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	fields := map[fieldpath.APIVersion]*fieldpath.Set{
+		"apps/v1": fieldpath.NewSet(
+			fieldpath.MakePathOrDie("status"),
+		),
+	}
+
+	return fields
+}
+
 // PrepareForCreate clears the status of a ReplicaSet before creation.
 func (rsStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	rs := obj.(*apps.ReplicaSet)
@@ -189,6 +202,16 @@ type rsStatusStrategy struct {
 // StatusStrategy is the default logic invoked when updating object status.
 var StatusStrategy = rsStatusStrategy{Strategy}
 
+// GetResetFields returns the set of fields that get reset by the strategy
+// and should not be modified by the user.
+func (rsStatusStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
+	return map[fieldpath.APIVersion]*fieldpath.Set{
+		"apps/v1": fieldpath.NewSet(
+			fieldpath.MakePathOrDie("spec"),
+		),
+	}
+}
+
 func (rsStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	newRS := obj.(*apps.ReplicaSet)
 	oldRS := old.(*apps.ReplicaSet)