Merge pull request #3302 from natasha41575/TestGvknChange

tests for gvkn changing
2026-05-22 06:47:00 +00:00 · 2020-12-04 10:29:59 -08:00
parent 709f499b44 724bbe9452
commit c1dca7fdb5
4 changed files with 658 additions and 6 deletions
--- a/api/krusty/gvknpatch_test.go
+++ b/api/krusty/gvknpatch_test.go
@@ -0,0 +1,622 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package krusty_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
+)
+
+// Regression test for https://github.com/kubernetes-sigs/kustomize/issues/3280
+// GVKN shouldn't change with default options
+func TestKeepOriginalGVKN(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("apps/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+
+	th.WriteF("apps/patch.yaml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: new-name
+`)
+
+	th.WriteK("apps", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	m := th.Run("apps", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+`)
+}
+
+// https://github.com/kubernetes-sigs/kustomize/issues/3280
+// These tests document behavior that will change
+func TestChangeName(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("apps/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+
+	th.WriteF("apps/patch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: new-name
+`)
+
+	th.WriteK("apps", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// name should become `new-name`
+	m := th.Run("apps", options)
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+`)
+}
+
+func TestChangeKind(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("apps/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+
+	th.WriteF("apps/patch.yaml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: old-name
+`)
+
+	th.WriteK("apps", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// kind should become `StatefulSet`
+	m := th.Run("apps", options)
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+`)
+}
+
+func TestChangeNameAndKind(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("apps/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+
+	th.WriteF("apps/patch.yaml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: new-name
+`)
+
+	th.WriteK("apps", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// kind should become `StatefulSet`
+	// name should become `new-name`
+	m := th.Run("apps", options)
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+`)
+}
+
+// https://github.com/kubernetes-sigs/kustomize/issues/3280
+// Should be able to refer to a resource with either its
+// original GVKN or its current one
+func TestPatchOriginalName(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("base/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+	th.WriteF("base/patch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: new-name
+`)
+	th.WriteK("base", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	th.WriteK("overlay", `
+resources:
+- ../base
+patchesStrategicMerge:
+- depPatch.yaml
+`)
+	th.WriteF("overlay/depPatch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  replicas: 999
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// name should become `new-name`
+	m := th.Run("overlay", options)
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  replicas: 999
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+`)
+}
+
+func TestPatchNewName(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("base/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+	th.WriteF("base/patch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: new-name
+`)
+	th.WriteK("base", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	th.WriteK("overlay", `
+resources:
+- ../base
+patchesStrategicMerge:
+- depPatch.yaml
+`)
+	th.WriteF("overlay/depPatch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: new-name
+spec:
+  replicas: 999
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// depPatch cannot find target with the name `new-name`
+	// because base/patch.yaml can't yet edit the name
+	assert.Error(t, th.RunWithErr("overlay", options))
+}
+
+func TestPatchOriginalNameAndKind(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("base/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+	th.WriteF("base/patch.yaml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: new-name
+`)
+	th.WriteK("base", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	th.WriteK("overlay", `
+resources:
+- ../base
+patchesStrategicMerge:
+- depPatch.yaml
+`)
+	th.WriteF("overlay/depPatch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  replicas: 999
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// kind should become `StatefulSet`
+	// name should become `new-name`
+	m := th.Run("overlay", options)
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  replicas: 999
+  template:
+    spec:
+      containers:
+      - image: nginx
+        name: nginx
+`)
+}
+
+func TestPatchNewNameAndKind(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("base/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+	th.WriteF("base/patch.yaml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: new-name
+`)
+	th.WriteK("base", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	th.WriteK("overlay", `
+resources:
+- ../base
+patchesStrategicMerge:
+- depPatch.yaml
+`)
+	th.WriteF("overlay/depPatch.yaml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: new-name
+spec:
+  replicas: 999
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// depPatch cannot find target with kind `StatefulSet` and name `new-name`
+	// because base/patch.yaml can't yet edit the kind or name
+	assert.Error(t, th.RunWithErr("overlay", options))
+}
+
+// Use original name, but new kind
+// Should fail, even after #3280 is done, because this ID is invalid
+func TestPatchOriginalNameAndNewKind(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteF("base/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: old-name
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+	th.WriteF("base/patch.yaml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: new-name
+`)
+	th.WriteK("base", `
+resources:
+- deployment.yaml
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+`)
+
+	th.WriteK("overlay", `
+resources:
+- ../base
+patchesStrategicMerge:
+- depPatch.yaml
+`)
+	th.WriteF("overlay/depPatch.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: new-name
+spec:
+  replicas: 999
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// depPatch cannot find target with kind `Deployment` and name `new-name`
+	// because the resource never had this GVKN
+	assert.Error(t, th.RunWithErr("overlay", options))
+}
+
+// Here is a structure of a kustomization of two components, component1
+// and component2, that both use a shared deployment definition, which
+// component2 adjusts by changing the kind via a patch. This test case
+// checks that it does not have a conflicting definition.
+// Currently documents broken behavior.
+//
+//                   root
+//              /            \
+//  component1/overlay  component2/overlay
+//             |              |
+//    component1/base    component2/base
+//              \            /
+//                   base
+//
+// This is the directory layout:
+//
+// ├── component1
+// │   ├── base
+// │   │   └── kustomization.yaml
+// │   └── overlay
+// │       └── kustomization.yaml
+// ├── component2
+// │   ├── base
+// │   │   └── kustomization.yaml
+// │   └── overlay
+// │       └── kustomization.yaml
+// ├── shared
+// │   ├── kustomization.yaml
+// │   └── deployment.yaml
+// ├── kustomization.yaml
+
+func TestBaseReuseNameAndKindConflict(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+
+	th.WriteK("/app/shared", `
+resources:
+  - deployment.yaml
+`)
+	th.WriteF("/app/shared/deployment.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: my-deploy
+spec:
+  template:
+    spec:
+      containers:
+        - name: nginx
+          image: nginx
+`)
+
+	th.WriteK("/app/component1/base", `
+resources:
+  - ../../shared
+`)
+	th.WriteK("/app/component1/overlay", `
+resources:
+  - ../base
+
+namePrefix: overlay-
+`)
+
+	th.WriteK("/app/component2/base", `
+resources:
+  - ../../shared
+
+patches:
+- path: patch.yaml
+  target:
+    kind: Deployment
+    name: my-deploy
+`)
+	th.WriteF("/app/component2/base/patch.yaml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: my-stateful-set
+`)
+	th.WriteK("/app/component2/overlay", `
+resources:
+  - ../base
+
+namePrefix: overlay-
+`)
+
+	th.WriteK("/app", `
+resources:
+  - component1/overlay
+  - component2/overlay
+`)
+
+	options := th.MakeDefaultOptions()
+	options.AllowResourceIdChanges = true
+
+	// Error occurs when app/component2/base tries to load the shared resources
+	// because the kind is not (yet) allowed to change yet
+	// so it loads a second Deployment with the name my-deploy
+	// instead of a StatefulSet as specified by the patch.
+	// Will be fixed by #3280.
+	assert.Error(t, th.RunWithErr("overlay", options))
+}
--- a/api/krusty/options.go
+++ b/api/krusty/options.go
@@ -36,16 +36,21 @@ type Options struct {
 	// When true, use kyaml/ packages to manipulate KRM yaml.
 	// When false, use k8sdeps/ instead (uses k8s.io/api* packages).
 	UseKyaml bool
+
+	// When true, allow name and kind changing via a patch
+	// When false, patch name/kind don't overwrite target name/kind
+	AllowResourceIdChanges bool
 }

 // MakeDefaultOptions returns a default instance of Options.
 func MakeDefaultOptions() *Options {
 	return &Options{
-		DoLegacyResourceSort: false,
-		AddManagedbyLabel:    false,
-		LoadRestrictions:     types.LoadRestrictionsRootOnly,
-		DoPrune:              false,
-		PluginConfig:         konfig.DisabledPluginConfig(),
-		UseKyaml:             false,
+		DoLegacyResourceSort:   false,
+		AddManagedbyLabel:      false,
+		LoadRestrictions:       types.LoadRestrictionsRootOnly,
+		DoPrune:                false,
+		PluginConfig:           konfig.DisabledPluginConfig(),
+		UseKyaml:               false,
+		AllowResourceIdChanges: false,
 	}
 }
--- a/kustomize/internal/commands/build/build.go
+++ b/kustomize/internal/commands/build/build.go
@@ -99,6 +99,7 @@ func NewCmdBuild(out io.Writer) *cobra.Command {
 	addFlagReorderOutput(cmd.Flags())
 	addFlagEnableManagedbyLabel(cmd.Flags())
 	addFlagEnableKyaml(cmd.Flags())
+	addFlagAllowResourceIdChanges(cmd.Flags())

 	return cmd
 }
@@ -137,6 +138,7 @@ func (o *Options) makeOptions() *krusty.Options {
 	}
 	opts.AddManagedbyLabel = isManagedbyLabelEnabled()
 	opts.UseKyaml = flagEnableKyamlValue
+	opts.AllowResourceIdChanges = flagAllowResourceIdChangesValue
 	return opts
 }

--- a/kustomize/internal/commands/build/flagallowresourceidchanges.go
+++ b/kustomize/internal/commands/build/flagallowresourceidchanges.go
@@ -0,0 +1,23 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package build
+
+import (
+	"github.com/spf13/pflag"
+)
+
+const (
+	flagAllowResourceIdChangesName = "allow_id_changes"
+	flagAllowResourceIdChangesHelp = `enable changes to a resourceId`
+)
+
+var (
+	flagAllowResourceIdChangesValue = false
+)
+
+func addFlagAllowResourceIdChanges(set *pflag.FlagSet) {
+	set.BoolVar(
+		&flagAllowResourceIdChangesValue, flagAllowResourceIdChangesName,
+		false, flagAllowResourceIdChangesHelp)
+}