diff --git a/pkg/registry/storage/volumeattachment/BUILD b/pkg/registry/storage/volumeattachment/BUILD index 7cb2a4bb38d..6156e767673 100644 --- 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 index 6d37f695c44..1b3a56c51aa 100644 --- 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 index 267bccc4aa9..e05c8d4eb0b 100644 --- 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 index d845d7b0fff..604129ea101 100644 --- 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 index fa42493002d..656e53af22c 100644 --- 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)) + } +}