Merge pull request #5430 from stormqueen1990/fix/treat-empty-ns-as-default-ns

fix: handle empty namespace as default

kustomize/commands/edit/add/configmap.go

@@ -148,7 +148,7 @@ func addConfigMap(
 
 func findOrMakeConfigMapArgs(m *types.Kustomization, name, namespace string) *types.ConfigMapArgs {
 	for i, v := range m.ConfigMapGenerator {
-		if name == v.Name && namespace == v.Namespace {
+		if name == v.Name && util.NamespaceEqual(v.Namespace, namespace) {
 			return &m.ConfigMapGenerator[i]
 		}
 	}

kustomize/commands/edit/add/configmap_test.go

@@ -26,7 +26,7 @@ const (
 
 func TestNewAddConfigMapIsNotNil(t *testing.T) {
 	fSys := filesys.MakeFsInMemory()
-	assert.NotNil(t, newCmdAddConfigMap(
+	require.NotNil(t, newCmdAddConfigMap(
 		fSys,
 		kv.NewLoader(
 			loader.NewFileLoaderAtCwd(fSys),
@@ -41,15 +41,14 @@ func TestMakeConfigMapArgs(t *testing.T) {
 		NamePrefix: "test-name-prefix",
 	}
 
-	if len(kustomization.ConfigMapGenerator) != 0 {
-		t.Fatal("Initial kustomization should not have any configmaps")
-	}
+	require.Len(t, kustomization.ConfigMapGenerator, 0, "Initial kustomization should not have any configmaps")
+
 	args := findOrMakeConfigMapArgs(kustomization, cmName, configMapNamespace)
-	assert.NotNil(t, args)
-	assert.Equal(t, 1, len(kustomization.ConfigMapGenerator))
-	assert.Equal(t, &kustomization.ConfigMapGenerator[len(kustomization.ConfigMapGenerator)-1], args)
-	assert.Equal(t, args, findOrMakeConfigMapArgs(kustomization, cmName, configMapNamespace))
-	assert.Equal(t, 1, len(kustomization.ConfigMapGenerator))
+	require.NotNil(t, args)
+	require.Equal(t, 1, len(kustomization.ConfigMapGenerator))
+	require.Equal(t, &kustomization.ConfigMapGenerator[len(kustomization.ConfigMapGenerator)-1], args)
+	require.Equal(t, args, findOrMakeConfigMapArgs(kustomization, cmName, configMapNamespace))
+	require.Equal(t, 1, len(kustomization.ConfigMapGenerator))
 }
 
 func TestMergeFlagsIntoConfigMapArgs_LiteralSources(t *testing.T) {
@@ -352,3 +351,143 @@ func TestEditAddConfigMapWithFileSource(t *testing.T) {
 		})
 	}
 }
+
+// TestEditAddConfigMapNamespaced tests situations regarding namespacing. For example, it
+// verifies that the empty namespace and the default namespace are treated the
+// same when adding a configmap to a kustomization file.
+func TestEditAddConfigMapNamespaced(t *testing.T) {
+	testCases := []struct {
+		name                string
+		configMapName       string
+		configMapNamespace  string
+		literalSources      []string
+		initialArgs         string
+		expectedResult      []types.ConfigMapArgs
+		expectedSliceLength int
+	}{
+		{
+			name:               "adds new key to configmap when default namespace matches empty",
+			configMapName:      "test-cm",
+			configMapNamespace: "default",
+			literalSources:     []string{"key1=value1"},
+			initialArgs: `---
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+configMapGenerator:
+- literals:
+  - key=value
+  name: test-cm
+`,
+			expectedResult: []types.ConfigMapArgs{
+				{
+					GeneratorArgs: types.GeneratorArgs{
+						Namespace: "",
+						Name:      "test-cm",
+						KvPairSources: types.KvPairSources{
+							LiteralSources: []string{"key=value", "key1=value1"},
+						},
+					},
+				},
+			},
+			expectedSliceLength: 1,
+		},
+		{
+			name:               "adds new key to configmap when empty namespace matches default",
+			configMapName:      "test-cm",
+			configMapNamespace: "",
+			literalSources:     []string{"key1=value1"},
+			initialArgs: `---
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+configMapGenerator:
+- literals:
+  - key=value
+  name: test-cm
+  namespace: default
+`,
+			expectedResult: []types.ConfigMapArgs{
+				{
+					GeneratorArgs: types.GeneratorArgs{
+						Namespace: "default",
+						Name:      "test-cm",
+						KvPairSources: types.KvPairSources{
+							LiteralSources: []string{"key=value", "key1=value1"},
+						},
+					},
+				},
+			},
+			expectedSliceLength: 1,
+		},
+		{
+			name:               "creates a new generator when namespaces don't match",
+			configMapName:      "test-cm",
+			configMapNamespace: "",
+			literalSources:     []string{"key1=value1"},
+			initialArgs: `---
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+configMapGenerator:
+- literals:
+  - key=value
+  name: test-cm
+  namespace: ns1
+`,
+			expectedResult: []types.ConfigMapArgs{
+				{
+					GeneratorArgs: types.GeneratorArgs{
+						Namespace: "ns1",
+						Name:      "test-cm",
+						KvPairSources: types.KvPairSources{
+							LiteralSources: []string{"key=value"},
+						},
+					},
+				},
+				{
+					GeneratorArgs: types.GeneratorArgs{
+						Namespace: "",
+						Name:      "test-cm",
+						KvPairSources: types.KvPairSources{
+							LiteralSources: []string{"key1=value1"},
+						},
+					},
+				},
+			},
+			expectedSliceLength: 2,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			fSys := filesys.MakeEmptyDirInMemory()
+			testutils_test.WriteTestKustomizationWith(fSys, []byte(tc.initialArgs))
+
+			pvd := provider.NewDefaultDepProvider()
+			ldr := kv.NewLoader(loader.NewFileLoaderAtCwd(fSys), pvd.GetFieldValidator())
+
+			args := []string{
+				tc.configMapName,
+				fmt.Sprintf(util.FlagFormat, util.NamespaceFlag, tc.configMapNamespace),
+			}
+
+			for _, source := range tc.literalSources {
+				args = append(args, fmt.Sprintf(util.FlagFormat, util.FromLiteralFlag, source))
+			}
+
+			cmd := newCmdAddConfigMap(fSys, ldr, pvd.GetResourceFactory())
+			cmd.SetArgs(args)
+			require.NoError(t, cmd.Execute())
+
+			_, err := testutils_test.ReadTestKustomization(fSys)
+			require.NoError(t, err)
+
+			mf, err := kustfile.NewKustomizationFile(fSys)
+			require.NoError(t, err)
+
+			kustomization, err := mf.Read()
+			require.NoError(t, err)
+
+			require.Len(t, kustomization.ConfigMapGenerator, tc.expectedSliceLength)
+			require.ElementsMatch(t, tc.expectedResult, kustomization.ConfigMapGenerator)
+		})
+	}
+}

kustomize/commands/edit/add/secret.go

@@ -140,7 +140,7 @@ func addSecret(
 
 func findOrMakeSecretArgs(m *types.Kustomization, name, namespace, secretType string) *types.SecretArgs {
 	for i, v := range m.SecretGenerator {
-		if name == v.Name && namespace == v.Namespace {
+		if name == v.Name && util.NamespaceEqual(v.Namespace, namespace) {
 			return &m.SecretGenerator[i]
 		}
 	}

kustomize/commands/edit/add/secret_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"sigs.k8s.io/kustomize/api/ifc"
 	"sigs.k8s.io/kustomize/api/kv"
 	"sigs.k8s.io/kustomize/api/pkg/loader"
 	"sigs.k8s.io/kustomize/api/provider"
@@ -237,3 +238,150 @@ func TestEditAddSecretWithFileSource(t *testing.T) {
 	require.Equal(t, secretName, newSecretGenerator.Name)
 	require.Contains(t, newSecretGenerator.FileSources, fileSource)
 }
+
+// TestEditAddSecretNamespaced tests situations regarding namespacing. For example, it
+// verifies that the empty namespace and the default namespace are treated the
+// same when adding a configmap to a kustomization file.
+func TestEditAddSecretNamespaced(t *testing.T) {
+	testCases := []struct {
+		name                string
+		secretName          string
+		secretNamespace     string
+		literalSources      []string
+		initialArgs         string
+		expectedResult      []types.SecretArgs
+		expectedSliceLength int
+	}{
+		{
+			name:            "adds new key to secret when default namespace matches empty",
+			secretName:      "test-secret",
+			secretNamespace: "default",
+			literalSources:  []string{"key1=value1"},
+			initialArgs: `---
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+secretGenerator:
+- literals:
+  - key=value
+  name: test-secret
+  type: Opaque
+`,
+			expectedResult: []types.SecretArgs{
+				{
+					GeneratorArgs: types.GeneratorArgs{
+						Namespace: "",
+						Name:      "test-secret",
+						KvPairSources: types.KvPairSources{
+							LiteralSources: []string{"key=value", "key1=value1"},
+						},
+					},
+					Type: ifc.SecretTypeOpaque,
+				},
+			},
+			expectedSliceLength: 1,
+		},
+		{
+			name:            "adds new key to secret when empty namespace matches default",
+			secretName:      "test-secret",
+			secretNamespace: "",
+			literalSources:  []string{"key1=value1"},
+			initialArgs: `---
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+secretGenerator:
+- literals:
+  - key=value
+  name: test-secret
+  namespace: default
+  type: Opaque
+`,
+			expectedResult: []types.SecretArgs{
+				{
+					GeneratorArgs: types.GeneratorArgs{
+						Namespace: "default",
+						Name:      "test-secret",
+						KvPairSources: types.KvPairSources{
+							LiteralSources: []string{"key=value", "key1=value1"},
+						},
+					},
+					Type: ifc.SecretTypeOpaque,
+				},
+			},
+			expectedSliceLength: 1,
+		},
+		{
+			name:            "creates a new generator when namespaces don't match",
+			secretName:      "test-secret",
+			secretNamespace: "",
+			literalSources:  []string{"key1=value1"},
+			initialArgs: `---
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+secretGenerator:
+- literals:
+  - key=value
+  name: test-secret
+  namespace: ns1
+  type: Opaque
+`,
+			expectedResult: []types.SecretArgs{
+				{
+					GeneratorArgs: types.GeneratorArgs{
+						Namespace: "ns1",
+						Name:      "test-secret",
+						KvPairSources: types.KvPairSources{
+							LiteralSources: []string{"key=value"},
+						},
+					},
+					Type: ifc.SecretTypeOpaque,
+				},
+				{
+					GeneratorArgs: types.GeneratorArgs{
+						Namespace: "",
+						Name:      "test-secret",
+						KvPairSources: types.KvPairSources{
+							LiteralSources: []string{"key1=value1"},
+						},
+					},
+					Type: ifc.SecretTypeOpaque,
+				},
+			},
+			expectedSliceLength: 2,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			fSys := filesys.MakeEmptyDirInMemory()
+			testutils_test.WriteTestKustomizationWith(fSys, []byte(tc.initialArgs))
+
+			pvd := provider.NewDefaultDepProvider()
+			ldr := kv.NewLoader(loader.NewFileLoaderAtCwd(fSys), pvd.GetFieldValidator())
+
+			args := []string{
+				tc.secretName,
+				fmt.Sprintf(util.FlagFormat, util.NamespaceFlag, tc.secretNamespace),
+			}
+
+			for _, source := range tc.literalSources {
+				args = append(args, fmt.Sprintf(util.FlagFormat, util.FromLiteralFlag, source))
+			}
+
+			cmd := newCmdAddSecret(fSys, ldr, pvd.GetResourceFactory())
+			cmd.SetArgs(args)
+			require.NoError(t, cmd.Execute())
+
+			_, err := testutils_test.ReadTestKustomization(fSys)
+			require.NoError(t, err)
+
+			mf, err := kustfile.NewKustomizationFile(fSys)
+			require.NoError(t, err)
+
+			kustomization, err := mf.Read()
+			require.NoError(t, err)
+
+			require.Len(t, kustomization.SecretGenerator, tc.expectedSliceLength)
+			require.ElementsMatch(t, tc.expectedResult, kustomization.SecretGenerator)
+		})
+	}
+}

kustomize/commands/internal/util/util.go

@@ -12,6 +12,9 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
+// DefaultNamespace is the default namespace name in Kubernetes.
+const DefaultNamespace = "default"
+
 // GlobPatterns accepts a slice of glob strings and returns the set of
 // matching file paths.
 func GlobPatterns(fSys filesys.FileSystem, patterns []string) ([]string, error) {
@@ -30,7 +33,7 @@ func GlobPatterns(fSys filesys.FileSystem, patterns []string) ([]string, error)
 	return result, nil
 }
 
-// GlobPatterns accepts a slice of glob strings and returns the set of matching file paths.
+// GlobPatternsWithLoader accepts a slice of glob strings and returns the set of matching file paths.
 // If validation is skipped, then it will return the patterns as provided.
 // Otherwise, It will try to load the files from the filesystem.
 // If files are not found in the filesystem, it will try to load from remote.
@@ -109,3 +112,18 @@ func trimQuotes(s string) string {
 	}
 	return s
 }
+
+// NamespaceEqual checks if two namespaces are the same. It considers the empty namespace and the default namespace to
+// be the same. As such, when one namespace is the empty string ('""') and the other namespace is "default", this function
+// will return true.
+func NamespaceEqual(namespace string, otherNamespace string) bool {
+	if "" == namespace {
+		namespace = DefaultNamespace
+	}
+
+	if "" == otherNamespace {
+		otherNamespace = DefaultNamespace
+	}
+
+	return namespace == otherNamespace
+}

kustomize/commands/internal/util/util_test.go

@@ -86,6 +86,58 @@ func TestGlobPatternsWithLoaderRemoteFile(t *testing.T) {
 	require.Equal(t, invalidURL, resources[0], "incorrect resources")
 }
 
+func TestNamespaceEqual(t *testing.T) {
+	testCases := []struct {
+		name       string
+		namespace1 string
+		namespace2 string
+		want       func(require.TestingT, bool, ...interface{})
+	}{
+		{
+			name:       "succeeds when namespaces are the same",
+			namespace1: "ns1",
+			namespace2: "ns1",
+			want:       require.True,
+		},
+		{
+			name:       "succeeds when namespaces are default and empty string",
+			namespace1: "",
+			namespace2: DefaultNamespace,
+			want:       require.True,
+		},
+		{
+			name:       "succeeds when namespaces are empty string and default",
+			namespace1: DefaultNamespace,
+			namespace2: "",
+			want:       require.True,
+		},
+		{
+			name:       "fails when namespaces are not the same",
+			namespace1: "ns1",
+			namespace2: "ns2",
+			want:       require.False,
+		},
+		{
+			name:       "fails when one is empty and other is different from default",
+			namespace1: "",
+			namespace2: "ns1",
+			want:       require.False,
+		},
+		{
+			name:       "fails when one is different from default and other is empty",
+			namespace1: "ns1",
+			namespace2: "",
+			want:       require.False,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			tc.want(t, NamespaceEqual(tc.namespace1, tc.namespace2))
+		})
+	}
+}
+
 type fakeLoader struct {
 	path string
 }