Merge pull request kubernetes#71211 from jsafrane/meta-status-reset

Refactor status PrepareForUpdate into standalone method
iiq374 · Nov 29, 2018 · 4372d62 · 4372d62
2 parents 05fe21f + 787611a
commit 4372d62
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 10 deletions.
diff --git a/pkg/registry/storage/volumeattachment/BUILD b/pkg/registry/storage/volumeattachment/BUILD
@@ -13,6 +13,7 @@ go_library(
         "//pkg/apis/storage:go_default_library",
         "//pkg/apis/storage/validation:go_default_library",
         "//staging/src/k8s.io/api/storage/v1beta1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",

diff --git a/pkg/registry/storage/volumeattachment/strategy.go b/pkg/registry/storage/volumeattachment/strategy.go
@@ -20,6 +20,7 @@ import (
 	"context"
 
 	storageapiv1beta1 "k8s.io/api/storage/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -134,14 +135,5 @@ func (volumeAttachmentStatusStrategy) PrepareForUpdate(ctx context.Context, obj,
 	oldVolumeAttachment := old.(*storage.VolumeAttachment)
 
 	newVolumeAttachment.Spec = oldVolumeAttachment.Spec
-
-	oldMeta := oldVolumeAttachment.ObjectMeta
-	newMeta := &newVolumeAttachment.ObjectMeta
-	newMeta.SetDeletionTimestamp(oldMeta.GetDeletionTimestamp())
-	newMeta.SetGeneration(oldMeta.GetGeneration())
-	newMeta.SetSelfLink(oldMeta.GetSelfLink())
-	newMeta.SetLabels(oldMeta.GetLabels())
-	newMeta.SetAnnotations(oldMeta.GetAnnotations())
-	newMeta.SetFinalizers(oldMeta.GetFinalizers())
-	newMeta.SetOwnerReferences(oldMeta.GetOwnerReferences())
+	metav1.ResetObjectMetaForStatus(&newVolumeAttachment.ObjectMeta, &oldVolumeAttachment.ObjectMeta)
 }
diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/BUILD b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/BUILD
@@ -25,6 +25,9 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
+        "//vendor/github.com/google/gofuzz:go_default_library",
         "//vendor/sigs.k8s.io/yaml:go_default_library",
     ],
 )

diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers.go
@@ -232,3 +232,15 @@ func HasObjectMetaSystemFieldValues(meta Object) bool {
 	return !meta.GetCreationTimestamp().Time.IsZero() ||
 		len(meta.GetUID()) != 0
 }
+
+// ResetObjectMetaForStatus forces the meta fields for a status update to match the meta fields
+// for a pre-existing object. This is opt-in for new objects with Status subresource.
+func ResetObjectMetaForStatus(meta, existingMeta Object) {
+	meta.SetDeletionTimestamp(existingMeta.GetDeletionTimestamp())
+	meta.SetGeneration(existingMeta.GetGeneration())
+	meta.SetSelfLink(existingMeta.GetSelfLink())
+	meta.SetLabels(existingMeta.GetLabels())
+	meta.SetAnnotations(existingMeta.GetAnnotations())
+	meta.SetFinalizers(existingMeta.GetFinalizers())
+	meta.SetOwnerReferences(existingMeta.GetOwnerReferences())
+}
diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/helpers_test.go
@@ -22,7 +22,11 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/gofuzz"
+
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/diff"
 )
 
 func TestLabelSelectorAsSelector(t *testing.T) {
@@ -159,3 +163,36 @@ func TestLabelSelectorAsMap(t *testing.T) {
 		}
 	}
 }
+
+func TestResetObjectMetaForStatus(t *testing.T) {
+	meta := &ObjectMeta{}
+	existingMeta := &ObjectMeta{}
+
+	// fuzz the existingMeta to set every field, no nils
+	f := fuzz.New().NilChance(0).NumElements(1, 1)
+	f.Fuzz(existingMeta)
+	ResetObjectMetaForStatus(meta, existingMeta)
+
+	// not all fields are stomped during the reset.  These fields should not have been set. False
+	// set them all to their zero values.  Before you add anything to this list, consider whether or not
+	// you're enforcing immutability (those are fine) and whether /status should be able to update
+	// these values (these are usually not fine).
+
+	// generateName doesn't do anything after create
+	existingMeta.SetGenerateName("")
+	// resourceVersion is enforced in validation and used during the storage update
+	existingMeta.SetResourceVersion("")
+	// fields made immutable in validation
+	existingMeta.SetUID(types.UID(""))
+	existingMeta.SetName("")
+	existingMeta.SetNamespace("")
+	existingMeta.SetClusterName("")
+	existingMeta.SetCreationTimestamp(Time{})
+	existingMeta.SetDeletionTimestamp(nil)
+	existingMeta.SetDeletionGracePeriodSeconds(nil)
+	existingMeta.SetInitializers(nil)
+
+	if !reflect.DeepEqual(meta, existingMeta) {
+		t.Error(diff.ObjectDiff(meta, existingMeta))
+	}
+}