From adb76219198235bb069bb67e924455dceaaba7c4 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 6 Apr 2022 12:34:42 -0400 Subject: [PATCH 1/2] Revert "remove unwanted values returned from dry-run" This reverts commit 60c1d58d02c7374645c00281dda3fd656264e1c5. --- .../pkg/registry/generic/registry/store.go | 5 --- .../apiserver/pkg/util/dryrun/dryrun.go | 34 ------------------- test/integration/dryrun/dryrun_test.go | 14 ++------ 3 files changed, 3 insertions(+), 50 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index 6a090ae7b60..2d9a7599e88 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -439,11 +439,6 @@ func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation if e.Decorator != nil { e.Decorator(out) } - if dryrun.IsDryRun(options.DryRun) { - if err := dryrun.ResetMetadata(obj, out); err != nil { - return nil, err - } - } return out, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/util/dryrun/dryrun.go b/staging/src/k8s.io/apiserver/pkg/util/dryrun/dryrun.go index f94542b65da..3e28c293453 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/dryrun/dryrun.go +++ b/staging/src/k8s.io/apiserver/pkg/util/dryrun/dryrun.go @@ -16,41 +16,7 @@ limitations under the License. package dryrun -import ( - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/runtime" -) - // IsDryRun returns true if the DryRun flag is an actual dry-run. func IsDryRun(flag []string) bool { return len(flag) > 0 } - -// ResetMetadata resets metadata fields that are not allowed to be set by dry-run. -func ResetMetadata(originalObj, newObj runtime.Object) error { - originalObjMeta, err := meta.Accessor(originalObj) - if err != nil { - return errors.NewInternalError(err) - } - newObjMeta, err := meta.Accessor(newObj) - if err != nil { - return errors.NewInternalError(err) - } - // If a resource is created with dry-run enabled where generateName is set, the - // store will set the name to the generated name. We need to reset the name and restore - // the generateName metadata fields in order for the returned object to match the intent - // of the original template. - if originalObjMeta.GetGenerateName() != "" { - newObjMeta.SetName("") - } - newObjMeta.SetGenerateName(originalObjMeta.GetGenerateName()) - // If UID is set in the dry-run output then that output cannot be used to create a resource. Reset - // the UID to allow the output to be used to create resources. - newObjMeta.SetUID("") - // If the resourceVersion is set in the dry-run output then that output cannot be used to create - // a resource. Reset the resourceVersion to allow the output to be used to create resources. - newObjMeta.SetResourceVersion("") - - return nil -} diff --git a/test/integration/dryrun/dryrun_test.go b/test/integration/dryrun/dryrun_test.go index 6f002c791aa..95897b90e70 100644 --- a/test/integration/dryrun/dryrun_test.go +++ b/test/integration/dryrun/dryrun_test.go @@ -18,7 +18,6 @@ package dryrun import ( "context" - "strings" "testing" v1 "k8s.io/api/core/v1" @@ -48,11 +47,7 @@ var kindAllowList = sets.NewString() const testNamespace = "dryrunnamespace" func DryRunCreateWithGenerateNameTest(t *testing.T, rsc dynamic.ResourceInterface, obj *unstructured.Unstructured, gvResource schema.GroupVersionResource) { - // special resources with dots in the name cannot use generateName - if strings.Contains(obj.GetName(), ".") { - return - } - // Create a new object with generateName to ensure we don't taint the original object + // Create a new object with generateName gnObj := obj.DeepCopy() gnObj.SetGenerateName(obj.GetName() + "-") gnObj.SetName("") @@ -79,11 +74,8 @@ func DryRunCreateTest(t *testing.T, rsc dynamic.ResourceInterface, obj *unstruct t.Fatalf("created object's name should be an empty string if using GenerateName: %v", createdObj) } - // we won't have a generated name here, so we won't check for this case - if obj.GetGenerateName() == "" { - if _, err := rsc.Get(context.TODO(), obj.GetName(), metav1.GetOptions{}); !apierrors.IsNotFound(err) { - t.Fatalf("object shouldn't exist: %v, %v", obj, err) - } + if _, err := rsc.Get(context.TODO(), obj.GetName(), metav1.GetOptions{}); !apierrors.IsNotFound(err) { + t.Fatalf("object shouldn't exist: %v", err) } } From e570744ca99e39d210855d7d7fe50f63fdc5470f Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 6 Apr 2022 12:34:49 -0400 Subject: [PATCH 2/2] Revert "add test to dry-run for unwanted generated values" This reverts commit 48994c1518982143de81ccec555f6b947f8d8d8f. --- test/integration/dryrun/dryrun_test.go | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/test/integration/dryrun/dryrun_test.go b/test/integration/dryrun/dryrun_test.go index 95897b90e70..aa6138a7892 100644 --- a/test/integration/dryrun/dryrun_test.go +++ b/test/integration/dryrun/dryrun_test.go @@ -46,33 +46,16 @@ var kindAllowList = sets.NewString() // namespace used for all tests, do not change this const testNamespace = "dryrunnamespace" -func DryRunCreateWithGenerateNameTest(t *testing.T, rsc dynamic.ResourceInterface, obj *unstructured.Unstructured, gvResource schema.GroupVersionResource) { - // Create a new object with generateName - gnObj := obj.DeepCopy() - gnObj.SetGenerateName(obj.GetName() + "-") - gnObj.SetName("") - DryRunCreateTest(t, rsc, gnObj, gvResource) -} - func DryRunCreateTest(t *testing.T, rsc dynamic.ResourceInterface, obj *unstructured.Unstructured, gvResource schema.GroupVersionResource) { createdObj, err := rsc.Create(context.TODO(), obj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}) if err != nil { - t.Fatalf("failed to dry-run create stub for %s: %#v: %v", gvResource, err, obj) + t.Fatalf("failed to dry-run create stub for %s: %#v", gvResource, err) } if obj.GroupVersionKind() != createdObj.GroupVersionKind() { t.Fatalf("created object doesn't have the same gvk as original object: got %v, expected %v", createdObj.GroupVersionKind(), obj.GroupVersionKind()) } - if createdObj.GetUID() != "" { - t.Fatalf("created object shouldn't have a uid: %v", createdObj) - } - if createdObj.GetResourceVersion() != "" { - t.Fatalf("created object shouldn't have a resource version: %v", createdObj) - } - if obj.GetGenerateName() != "" && createdObj.GetName() != "" { - t.Fatalf("created object's name should be an empty string if using GenerateName: %v", createdObj) - } if _, err := rsc.Get(context.TODO(), obj.GetName(), metav1.GetOptions{}); !apierrors.IsNotFound(err) { t.Fatalf("object shouldn't exist: %v", err) @@ -299,7 +282,6 @@ func TestDryRun(t *testing.T) { name := obj.GetName() DryRunCreateTest(t, rsc, obj, gvResource) - DryRunCreateWithGenerateNameTest(t, rsc, obj, gvResource) if _, err := rsc.Create(context.TODO(), obj, metav1.CreateOptions{}); err != nil { t.Fatalf("failed to create stub for %s: %#v", gvResource, err)