Merge pull request #103545 from thockin/rest-create-deepcopy

REST: Document mutable inputs on Create() and fix tests

pkg/registry/core/service/storage/rest.go

@@ -189,6 +189,9 @@ func (rs *REST) Watch(ctx context.Context, options *metainternalversion.ListOpti
 }
 
 func (rs *REST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
+	// DeepCopy to prevent writes here propagating back to tests.
+	obj = obj.DeepCopyObject()
+
 	service := obj.(*api.Service)
 
 	// bag of clusterIPs allocated in the process of creation

pkg/registry/core/service/storage/rest_test.go

@@ -993,7 +993,7 @@ func TestServiceRegistryDeleteDryRun(t *testing.T) {
 		t.Errorf("expected NodePort to be allocated")
 	}
 
-	isValidClusterIPFields(t, storage, svc, svc)
+	isValidClusterIPFields(t, storage, svc, createdSvc)
 
 	_, _, err = storage.Delete(ctx, svc.Name, rest.ValidateAllObjectFunc, &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}})
 	if err != nil {
@@ -1371,7 +1371,7 @@ func TestServiceRegistryIPReallocation(t *testing.T) {
 		t.Errorf("Unexpected error deleting service: %v", err)
 	}
 
-	svc2 := svctest.MakeService("bar", svctest.SetClusterIPs(svc1.Spec.ClusterIP))
+	svc2 := svctest.MakeService("bar", svctest.SetClusterIPs(createdSvc1.Spec.ClusterIP))
 	ctx = genericapirequest.NewDefaultContext()
 	obj, err = storage.Create(ctx, svc2, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
 	if err != nil {
@@ -3585,6 +3585,8 @@ func TestDefaultingValidation(t *testing.T) {
 // validates that the service created, updated by REST
 // has correct ClusterIPs related fields
 func isValidClusterIPFields(t *testing.T, storage *REST, pre *api.Service, post *api.Service) {
+	t.Helper()
+
 	// valid for gate off/on scenarios
 	// ClusterIP
 	if len(post.Spec.ClusterIP) == 0 {

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go

@@ -360,6 +360,9 @@ func (e *Store) ListPredicate(ctx context.Context, p storage.SelectionPredicate,
 func finishNothing(context.Context, bool) {}
 
 // Create inserts a new item according to the unique key from the object.
+// Note that registries may mutate the input object (e.g. in the strategy
+// hooks).  Tests which call this might want to call DeepCopy if they expect to
+// be able to examine the input and output objects for differences.
 func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
 	var finishCreate FinishFunc = finishNothing
 

staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go

@@ -381,7 +381,8 @@ func (t *Tester) testCreateGeneratesName(valid runtime.Object) {
 		t.Fatalf("Unexpected error: %v", err)
 	}
 	defer t.delete(t.TestContext(), created)
-	if objectMeta.GetName() == "test-" || !strings.HasPrefix(objectMeta.GetName(), "test-") {
+	createdMeta := t.getObjectMetaOrFail(created)
+	if createdMeta.GetName() == "test-" || !strings.HasPrefix(createdMeta.GetName(), "test-") {
 		t.Errorf("unexpected name: %#v", valid)
 	}
 }
@@ -399,7 +400,8 @@ func (t *Tester) testCreateHasMetadata(valid runtime.Object) {
 		t.Fatalf("Unexpected object from result: %#v", obj)
 	}
 	defer t.delete(t.TestContext(), obj)
-	if !metav1.HasObjectMetaSystemFieldValues(objectMeta) {
+	createdMeta := t.getObjectMetaOrFail(obj)
+	if !metav1.HasObjectMetaSystemFieldValues(createdMeta) {
 		t.Errorf("storage did not populate object meta field values")
 	}
 }
@@ -501,7 +503,8 @@ func (t *Tester) testCreateResetsUserData(valid runtime.Object, opts metav1.Crea
 		t.Fatalf("Unexpected object from result: %#v", obj)
 	}
 	defer t.delete(t.TestContext(), obj)
-	if objectMeta.GetUID() == "bad-uid" || objectMeta.GetCreationTimestamp() == now {
+	createdMeta := t.getObjectMetaOrFail(obj)
+	if createdMeta.GetUID() == "bad-uid" || createdMeta.GetCreationTimestamp() == now {
 		t.Errorf("ObjectMeta did not reset basic fields: %#v", objectMeta)
 	}
 }