improve gvk and resid strings for error messages

api/filters/replacement/replacement.go

@@ -172,7 +172,7 @@ func getReplacement(nodes []*yaml.RNode, r *types.Replacement) (*yaml.RNode, err
 		return nil, err
 	}
 	if rn.IsNilOrEmpty() {
-		return nil, fmt.Errorf("fieldPath `%s` is missing for replacement source %s", r.Source.FieldPath, r.Source)
+		return nil, fmt.Errorf("fieldPath `%s` is missing for replacement source %s", r.Source.FieldPath, r.Source.ResId)
 	}
 
 	return getRefinedValue(r.Source.Options, rn)

api/filters/replacement/replacement_test.go

@@ -269,7 +269,7 @@ spec:
   - select:
       kind: Deployment
 `,
-			expectedErr: "multiple matches for selector ~G_~V_Deployment|~X|~N",
+			expectedErr: "multiple matches for selector {kind: Deployment}",
 		},
 		"replacement has no source": {
 			input: `apiVersion: v1
@@ -1514,9 +1514,9 @@ kind: Deployment
 metadata:
   name: pre-deploy
   annotations: 
-    internal.config.kubernetes.io/previousNames: deploy,deploy
-    internal.config.kubernetes.io/previousKinds: CronJob,Deployment
-    internal.config.kubernetes.io/previousNamespaces: default,default
+    internal.config.kubernetes.io/previousNames: deploy;deploy
+    internal.config.kubernetes.io/previousKinds: CronJob;Deployment
+    internal.config.kubernetes.io/previousNamespaces: default;default
 spec:
   template:
     spec:
@@ -1543,9 +1543,9 @@ kind: Deployment
 metadata:
   name: pre-deploy
   annotations:
-    internal.config.kubernetes.io/previousNames: deploy,deploy
-    internal.config.kubernetes.io/previousKinds: CronJob,Deployment
-    internal.config.kubernetes.io/previousNamespaces: default,default
+    internal.config.kubernetes.io/previousNames: deploy;deploy
+    internal.config.kubernetes.io/previousKinds: CronJob;Deployment
+    internal.config.kubernetes.io/previousNamespaces: default;default
 spec:
   template:
     spec:
@@ -1586,7 +1586,7 @@ data:
     options:
       create: true
 `,
-			expectedErr: "fieldPath `data.httpPort` is missing for replacement source ~G_~V_ConfigMap|~X|ports-from:data.httpPort",
+			expectedErr: "fieldPath `data.httpPort` is missing for replacement source {kind: ConfigMap, name: ports-from}",
 		},
 		"annotationSelector": {
 			input: `apiVersion: v1

api/internal/accumulator/namereferencetransformer_test.go

@@ -521,7 +521,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) {
 					},
 				}).ResMap(),
 			expectedErr: `updating name reference in 'rules/resourceNames' field of ` +
-				`'rbac.authorization.k8s.io_v1_ClusterRole|~X|cr'` +
+				`'{group: rbac.authorization.k8s.io, version: v1, kind: ClusterRole, name: cr}'` +
 				`: considering field 'rules/resourceNames' of object
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole

api/internal/utils/makeResIds.go

@@ -8,6 +8,8 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
+const Separator = ";"
+
 // MakeResIds returns all of an RNode's current and previous Ids
 func MakeResIds(n *yaml.RNode) ([]resid.ResId, error) {
 	var result []resid.ResId
@@ -36,9 +38,9 @@ func PrevIds(n *yaml.RNode) ([]resid.ResId, error) {
 	if _, ok := annotations[BuildAnnotationPreviousNames]; !ok {
 		return nil, nil
 	}
-	names := strings.Split(annotations[BuildAnnotationPreviousNames], ",")
-	ns := strings.Split(annotations[BuildAnnotationPreviousNamespaces], ",")
-	kinds := strings.Split(annotations[BuildAnnotationPreviousKinds], ",")
+	names := strings.Split(annotations[BuildAnnotationPreviousNames], Separator)
+	ns := strings.Split(annotations[BuildAnnotationPreviousNamespaces], Separator)
+	kinds := strings.Split(annotations[BuildAnnotationPreviousKinds], Separator)
 	// This should never happen
 	if len(names) != len(ns) || len(names) != len(kinds) {
 		return nil, fmt.Errorf(

api/krusty/complexcomposition_test.go

@@ -368,7 +368,7 @@ resources:
 		t.Fatalf("Expected resource accumulation error")
 	}
 	if !strings.Contains(
-		err.Error(), "already registered id: apps_v1_StatefulSet|~X|my-sts") {
+		err.Error(), "already registered id: {group: apps, version: v1, kind: StatefulSet, name: my-sts}") {
 		t.Fatalf("Unexpected err: %v", err)
 	}
 }
@@ -459,7 +459,7 @@ resources:
 		t.Fatalf("Expected resource accumulation error")
 	}
 	if !strings.Contains(
-		err.Error(), "already registered id: apps_v1_StatefulSet|~X|my-sts") {
+		err.Error(), "already registered id: {group: apps, version: v1, kind: StatefulSet, name: my-sts}") {
 		t.Fatalf("Unexpected err: %v", err)
 	}
 }

api/krusty/component_test.go

@@ -589,7 +589,7 @@ components:
 - ../comp-b`),
 			},
 			runPath:       "prod",
-			expectedError: "may not add resource with an already registered id: ~G_v1_Deployment|~X|proxy",
+			expectedError: "may not add resource with an already registered id: {version: v1, kind: Deployment, name: proxy}",
 		},
 		"components-cannot-add-the-same-base": {
 			input: []FileGen{writeTestBase,
@@ -608,7 +608,7 @@ components:
 - ../comp-b`),
 			},
 			runPath:       "prod",
-			expectedError: "may not add resource with an already registered id: ~G_v1_Deployment|~X|storefront",
+			expectedError: "may not add resource with an already registered id: {version: v1, kind: Deployment, name: storefront}",
 		},
 		"components-cannot-add-bases-containing-the-same-resource": {
 			input: []FileGen{writeTestBase,
@@ -639,7 +639,7 @@ components:
 - ../comp-b`),
 			},
 			runPath:       "prod",
-			expectedError: "may not add resource with an already registered id: ~G_v1_Deployment|~X|proxy",
+			expectedError: "may not add resource with an already registered id: {version: v1, kind: Deployment, name: proxy}",
 		},
 	}
 

api/krusty/diamondcomposition_test.go

@@ -141,7 +141,7 @@ resources:
 		t.Fatalf("Expected resource accumulation error")
 	}
 	if !strings.Contains(
-		err.Error(), "already registered id: apps_v1_Deployment|~X|my-deployment") {
+		err.Error(), "already registered id: {group: apps, version: v1, kind: Deployment, name: my-deployment}") {
 		t.Fatalf("Unexpected err: %v", err)
 	}
 }

api/krusty/repeatbase_test.go

@@ -128,7 +128,8 @@ spec:
 
 	err := th.RunWithErr("mango", th.MakeDefaultOptions())
 	if !strings.Contains(
-		err.Error(), "multiple matches for Id apps_v1_Deployment|~X|banana; failed to find unique target for patch") {
+		err.Error(), "multiple matches for Id {group: apps, version: v1, kind: Deployment, name: banana};"+
+			" failed to find unique target for patch {group: apps, version: v1, kind: Deployment, name: banana}") {
 		t.Fatalf("Unexpected err: %v", err)
 	}
 

api/resmap/reswrangler.go

@@ -215,7 +215,7 @@ func (m *resWrangler) GetById(
 	if err != nil {
 		return nil, fmt.Errorf(
 			"%s; failed to find unique target for patch %s",
-			err.Error(), id.GvknString())
+			err.Error(), id.String())
 	}
 	return r, nil
 }

api/resmap/reswrangler_test.go

@@ -356,9 +356,9 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) {
 			"metadata": map[string]interface{}{
 				"name": "new-bob",
 				"annotations": map[string]interface{}{
-					"internal.config.kubernetes.io/previousKinds":      "ConfigMap,ConfigMap",
-					"internal.config.kubernetes.io/previousNames":      "bob,bob2",
-					"internal.config.kubernetes.io/previousNamespaces": "default,default",
+					"internal.config.kubernetes.io/previousKinds":      "ConfigMap;ConfigMap",
+					"internal.config.kubernetes.io/previousNames":      "bob;bob2",
+					"internal.config.kubernetes.io/previousNamespaces": "default;default",
 				},
 			},
 		})

api/resource/resource.go

@@ -194,7 +194,7 @@ func (r *Resource) appendCsvAnnotation(name, value string) {
 	}
 	annotations := r.GetAnnotations()
 	if existing, ok := annotations[name]; ok {
-		annotations[name] = existing + "," + value
+		annotations[name] = existing + utils.Separator + value
 	} else {
 		annotations[name] = value
 	}
@@ -218,7 +218,7 @@ func (r *Resource) getCsvAnnotation(name string) []string {
 	if _, ok := annotations[name]; !ok {
 		return nil
 	}
-	return strings.Split(annotations[name], ",")
+	return strings.Split(annotations[name], utils.Separator)
 }
 
 // PrefixesSuffixesEquals is conceptually doing the same task

api/resource/resource_test.go

@@ -996,9 +996,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    internal.config.kubernetes.io/previousKinds: Secret,Secret
-    internal.config.kubernetes.io/previousNames: oldName,oldName2
-    internal.config.kubernetes.io/previousNamespaces: default,default
+    internal.config.kubernetes.io/previousKinds: Secret;Secret
+    internal.config.kubernetes.io/previousNames: oldName;oldName2
+    internal.config.kubernetes.io/previousNamespaces: default;default
   name: newName
 `,
 		},
@@ -1020,9 +1020,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    internal.config.kubernetes.io/previousKinds: Secret,Secret
-    internal.config.kubernetes.io/previousNames: oldName,oldName2
-    internal.config.kubernetes.io/previousNamespaces: default,oldNamespace
+    internal.config.kubernetes.io/previousKinds: Secret;Secret
+    internal.config.kubernetes.io/previousNames: oldName;oldName2
+    internal.config.kubernetes.io/previousNamespaces: default;oldNamespace
   name: newName
   namespace: newNamespace
 `,
@@ -1089,9 +1089,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    internal.config.kubernetes.io/previousKinds: Secret,Secret
-    internal.config.kubernetes.io/previousNames: oldName,oldName2
-    internal.config.kubernetes.io/previousNamespaces: default,oldNamespace
+    internal.config.kubernetes.io/previousKinds: Secret;Secret
+    internal.config.kubernetes.io/previousNames: oldName;oldName2
+    internal.config.kubernetes.io/previousNamespaces: default;oldNamespace
   name: newName
   namespace: newNamespace
 `,
@@ -1367,6 +1367,7 @@ spec:
 		t.Fatalf("expected '%s', got '%s'", expected, actual)
 	}
 }
+
 func TestSetGvk(t *testing.T) {
 	r, err := factory.FromBytes([]byte(`
 apiVersion: v1
@@ -1377,7 +1378,7 @@ spec:
   numReplicas: 1
 `))
 	assert.NoError(t, err)
-	r.SetGvk(resid.GvkFromString("grp_ver_knd"))
+	r.SetGvk(resid.GvkFromString("group: grp, version: ver, kind: knd"))
 	gvk := r.GetGvk()
 	if expected, actual := "grp", gvk.Group; expected != actual {
 		t.Fatalf("expected '%s', got '%s'", expected, actual)
@@ -1390,6 +1391,7 @@ spec:
 	}
 }
 
+// nolint
 func TestRefBy(t *testing.T) {
 	r, err := factory.FromBytes([]byte(`
 apiVersion: v1
@@ -1400,30 +1402,31 @@ spec:
   numReplicas: 1
 `))
 	assert.NoError(t, err)
-	r.AppendRefBy(resid.FromString("gr1_ver1_knd1|ns1|name1"))
+	r.AppendRefBy(resid.FromString("{group: gr1, version: ver1, kind: knd1, namespace: ns1, name: name1}"))
 	assert.Equal(t, r.RNode.MustString(), `apiVersion: v1
 kind: Deployment
 metadata:
   name: clown
   annotations:
-    internal.config.kubernetes.io/refBy: gr1_ver1_knd1|ns1|name1
+    internal.config.kubernetes.io/refBy: '{group: gr1, version: ver1, kind: knd1, name: name1, namespace: ns1}'
 spec:
   numReplicas: 1
 `)
-	assert.Equal(t, r.GetRefBy(), []resid.ResId{resid.FromString("gr1_ver1_knd1|ns1|name1")})
+	assert.Equal(t, r.GetRefBy(), []resid.ResId{resid.FromString(
+		"{group: gr1, version: ver1, kind: knd1, namespace: ns1, name: name1}")})
 
-	r.AppendRefBy(resid.FromString("gr2_ver2_knd2|ns2|name2"))
+	r.AppendRefBy(resid.FromString("{group: gr2, version: ver2, kind: knd2, namespace: ns2, name: name2}"))
 	assert.Equal(t, r.RNode.MustString(), `apiVersion: v1
 kind: Deployment
 metadata:
   name: clown
   annotations:
-    internal.config.kubernetes.io/refBy: gr1_ver1_knd1|ns1|name1,gr2_ver2_knd2|ns2|name2
+    internal.config.kubernetes.io/refBy: '{group: gr1, version: ver1, kind: knd1, name: name1, namespace: ns1};{group: gr2, version: ver2, kind: knd2, name: name2, namespace: ns2}'
 spec:
   numReplicas: 1
 `)
 	assert.Equal(t, r.GetRefBy(), []resid.ResId{
-		resid.FromString("gr1_ver1_knd1|ns1|name1"),
-		resid.FromString("gr2_ver2_knd2|ns2|name2"),
+		resid.FromString("{group: gr1, version: ver1, kind: knd1, namespace: ns1, name: name1}"),
+		resid.FromString("{group: gr2, version: ver2, kind: knd2, namespace: ns2, name: name2}"),
 	})
 }

kustomize/commands/build/writer.go

@@ -58,6 +58,9 @@ func (w Writer) write(path, fName string, res *resource.Resource) error {
 }
 
 func fileName(res *resource.Resource) string {
-	return strings.ToLower(res.GetGvk().StringWoEmptyField()) +
-		"_" + strings.ToLower(res.GetName()) + ".yaml"
+	gvk := res.GetGvk()
+	return strings.ToLower(
+		strings.Trim(
+			strings.Join(
+				[]string{gvk.Group, gvk.Version, gvk.Kind, res.GetName()}, "_")+".yaml", "_"))
 }