From 578388b70bdeaebaf3a7dd1e369e28b459f3e20b Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 13 Aug 2021 11:54:21 -0700 Subject: [PATCH 1/2] Fix registry tests to look at result objects Kubernetes-commit: 42c7e62180446910f41d3a064016c0d7cdebb407 --- pkg/registry/rest/resttest/resttest.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/registry/rest/resttest/resttest.go b/pkg/registry/rest/resttest/resttest.go index b394da620..c271c66ed 100644 --- a/pkg/registry/rest/resttest/resttest.go +++ b/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) } } From cf0112f87e1443945999d8a89c99056aff34741d Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 13 Aug 2021 11:54:00 -0700 Subject: [PATCH 2/2] REST: Document mutable inputs on Create() If one doesn't DeepCopy() on the way into Create, we can end up writing into the original object. This is by design, and should not be a problem EXCEPT for tests. If a test compares the input to this function with the result, but the input was mutated in-situ, it may hide errors, resulting in tests that pass, but shouldn't. Kubernetes-commit: 6dfae64d9bebb2c40680bbd6e8270f69839ab013 --- pkg/registry/generic/registry/store.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/registry/generic/registry/store.go b/pkg/registry/generic/registry/store.go index ec9870a03..d94627834 100644 --- a/pkg/registry/generic/registry/store.go +++ b/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