diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index 0fec344973..bff8bba411 100644 --- a/api/filters/replacement/replacement.go +++ b/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) diff --git a/api/filters/replacement/replacement_test.go b/api/filters/replacement/replacement_test.go index bae167c577..b06b074f4a 100644 --- a/api/filters/replacement/replacement_test.go +++ b/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 Deployment.[noVer].[noGrp]/[noName].[noNs]", }, "replacement has no source": { input: `apiVersion: v1 @@ -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 ConfigMap.[noVer].[noGrp]/ports-from.[noNs]", }, "annotationSelector": { input: `apiVersion: v1 diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index 31ec435362..9c2e9605dc 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/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'` + + `'ClusterRole.v1.rbac.authorization.k8s.io/cr.[noNs]'` + `: considering field 'rules/resourceNames' of object apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/api/krusty/complexcomposition_test.go b/api/krusty/complexcomposition_test.go index 12855a4042..7c3dfc39fd 100644 --- a/api/krusty/complexcomposition_test.go +++ b/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: StatefulSet.v1.apps/my-sts.[noNs]") { 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: StatefulSet.v1.apps/my-sts.[noNs]") { t.Fatalf("Unexpected err: %v", err) } } diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index 8629c2f12d..a76a802844 100644 --- a/api/krusty/component_test.go +++ b/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: Deployment.v1.[noGrp]/proxy.[noNs]", }, "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: Deployment.v1.[noGrp]/storefront.[noNs]", }, "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: Deployment.v1.[noGrp]/proxy.[noNs]", }, } diff --git a/api/krusty/diamondcomposition_test.go b/api/krusty/diamondcomposition_test.go index 430e62563d..401afb1a99 100644 --- a/api/krusty/diamondcomposition_test.go +++ b/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: Deployment.v1.apps/my-deployment.[noNs]") { t.Fatalf("Unexpected err: %v", err) } } diff --git a/api/krusty/repeatbase_test.go b/api/krusty/repeatbase_test.go index c68a9dccd0..438b7af287 100644 --- a/api/krusty/repeatbase_test.go +++ b/api/krusty/repeatbase_test.go @@ -128,7 +128,7 @@ 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 Deployment.v1.apps/banana.[noNs]; failed to find unique target for patch Deployment.v1.apps/banana.[noNs]") { t.Fatalf("Unexpected err: %v", err) } diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index 31bfe1fee9..291efb5bb7 100644 --- a/api/resmap/reswrangler.go +++ b/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 } diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index de6c495e0c..89668c2038 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -1377,7 +1377,7 @@ spec: numReplicas: 1 `)) assert.NoError(t, err) - r.SetGvk(resid.GvkFromString("grp_ver_knd")) + r.SetGvk(resid.GvkFromString("knd.ver.grp")) gvk := r.GetGvk() if expected, actual := "grp", gvk.Group; expected != actual { t.Fatalf("expected '%s', got '%s'", expected, actual) @@ -1400,30 +1400,30 @@ spec: numReplicas: 1 `)) assert.NoError(t, err) - r.AppendRefBy(resid.FromString("gr1_ver1_knd1|ns1|name1")) + r.AppendRefBy(resid.FromString("knd1.ver1.gr1/name1.ns1")) 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: knd1.ver1.gr1/name1.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("knd1.ver1.gr1/name1.ns1")}) - r.AppendRefBy(resid.FromString("gr2_ver2_knd2|ns2|name2")) + r.AppendRefBy(resid.FromString("knd2.ver2.gr2/name2.ns2")) 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: knd1.ver1.gr1/name1.ns1,knd2.ver2.gr2/name2.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("knd1.ver1.gr1/name1.ns1"), + resid.FromString("knd2.ver2.gr2/name2.ns2"), }) } diff --git a/kyaml/resid/gvk.go b/kyaml/resid/gvk.go index 21dad78d33..6279b739cc 100644 --- a/kyaml/resid/gvk.go +++ b/kyaml/resid/gvk.go @@ -49,7 +49,7 @@ func ParseGroupVersion(apiVersion string) (group, version string) { // GvkFromString makes a Gvk from the output of Gvk.String(). func GvkFromString(s string) Gvk { values := strings.Split(s, fieldSep) - if len(values) != 3 { + if len(values) < 3 { // ...then the string didn't come from Gvk.String(). return Gvk{ Group: noGroup, @@ -57,27 +57,27 @@ func GvkFromString(s string) Gvk { Kind: noKind, } } - g := values[0] - if g == noGroup { - g = "" + k := values[0] + if k == noKind { + k = "" } v := values[1] if v == noVersion { v = "" } - k := values[2] - if k == noKind { - k = "" + g := strings.Join(values[2:], fieldSep) + if g == noGroup { + g = "" } return NewGvk(g, v, k) } // Values that are brief but meaningful in logs. const ( - noGroup = "~G" - noVersion = "~V" - noKind = "~K" - fieldSep = "_" + noGroup = "[noGrp]" + noVersion = "[noVer]" + noKind = "[noKind]" + fieldSep = "." ) // String returns a string representation of the GVK. @@ -94,7 +94,7 @@ func (x Gvk) String() string { if k == "" { k = noKind } - return strings.Join([]string{g, v, k}, fieldSep) + return strings.Join([]string{k, v, g}, fieldSep) } // ApiVersion returns the combination of Group and Version @@ -109,7 +109,8 @@ func (x Gvk) ApiVersion() string { } // StringWoEmptyField returns a string representation of the GVK. Non-exist -// fields will be omitted. +// fields will be omitted. This is called when generating a filename for the +// resource. func (x Gvk) StringWoEmptyField() string { var s []string if x.Group != "" { @@ -121,7 +122,7 @@ func (x Gvk) StringWoEmptyField() string { if x.Kind != "" { s = append(s, x.Kind) } - return strings.Join(s, fieldSep) + return strings.Join(s, "_") } // Equals returns true if the Gvk's have equal fields. diff --git a/kyaml/resid/gvk_test.go b/kyaml/resid/gvk_test.go index cb3b9faddd..fde617e8c6 100644 --- a/kyaml/resid/gvk_test.go +++ b/kyaml/resid/gvk_test.go @@ -47,8 +47,8 @@ var lessThanTests = []struct { Gvk{Group: "a", Version: "c", Kind: "ClusterRole"}}, {Gvk{Group: "a", Version: "c", Kind: "Namespace"}, Gvk{Group: "a", Version: "b", Kind: "ClusterRole"}}, - {Gvk{Group: "a", Version: "d", Kind: "Namespace"}, - Gvk{Group: "b", Version: "c", Kind: "Namespace"}}, + {Gvk{Group: "b", Version: "c", Kind: "Namespace"}, + Gvk{Group: "a", Version: "d", Kind: "Namespace"}}, {Gvk{Group: "a", Version: "b", Kind: orderFirst[len(orderFirst)-1]}, Gvk{Group: "a", Version: "b", Kind: orderLast[0]}}, {Gvk{Group: "a", Version: "b", Kind: orderFirst[len(orderFirst)-1]}, @@ -87,14 +87,16 @@ var stringTests = []struct { s string r string }{ - {Gvk{}, "~G_~V_~K", ""}, - {Gvk{Kind: "k"}, "~G_~V_k", "k"}, - {Gvk{Version: "v"}, "~G_v_~K", "v"}, - {Gvk{Version: "v", Kind: "k"}, "~G_v_k", "v_k"}, - {Gvk{Group: "g"}, "g_~V_~K", "g"}, - {Gvk{Group: "g", Kind: "k"}, "g_~V_k", "g_k"}, - {Gvk{Group: "g", Version: "v"}, "g_v_~K", "g_v"}, - {Gvk{Group: "g", Version: "v", Kind: "k"}, "g_v_k", "g_v_k"}, + {Gvk{}, "[noKind].[noVer].[noGrp]", ""}, + {Gvk{Kind: "k"}, "k.[noVer].[noGrp]", "k"}, + {Gvk{Version: "v"}, "[noKind].v.[noGrp]", "v"}, + {Gvk{Version: "v", Kind: "k"}, "k.v.[noGrp]", "v_k"}, + {Gvk{Group: "g"}, "[noKind].[noVer].g", "g"}, + {Gvk{Group: "g", Kind: "k"}, "k.[noVer].g", "g_k"}, + {Gvk{Group: "g", Version: "v"}, "[noKind].v.g", "g_v"}, + {Gvk{Group: "g", Version: "v", Kind: "k"}, "k.v.g", "g_v_k"}, + {Gvk{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole", isClusterScoped: true}, + "ClusterRole.v1.rbac.authorization.k8s.io", "rbac.authorization.k8s.io_v1_ClusterRole"}, } func TestString(t *testing.T) { diff --git a/kyaml/resid/resid.go b/kyaml/resid/resid.go index 9a51c3716e..a729cb78dd 100644 --- a/kyaml/resid/resid.go +++ b/kyaml/resid/resid.go @@ -37,9 +37,9 @@ func NewResIdKindOnly(k string, n string) ResId { } const ( - noNamespace = "~X" - noName = "~N" - separator = "|" + noNamespace = "[noNs]" + noName = "[noName]" + separator = "/" TotallyNotANamespace = "_non_namespaceable_" DefaultNamespace = "default" ) @@ -55,33 +55,31 @@ func (id ResId) String() string { nm = noName } return strings.Join( - []string{id.Gvk.String(), ns, nm}, separator) + []string{id.Gvk.String(), strings.Join([]string{nm, ns}, fieldSep)}, separator) } func FromString(s string) ResId { values := strings.Split(s, separator) - g := GvkFromString(values[0]) + gvk := GvkFromString(values[0]) - ns := values[1] + values = strings.Split(values[1], fieldSep) + last := len(values)-1 + + ns := values[last] if ns == noNamespace { ns = "" } - nm := values[2] + nm := strings.Join(values[:last], fieldSep) if nm == noName { nm = "" } return ResId{ - Gvk: g, + Gvk: gvk, Namespace: ns, Name: nm, } } -// GvknString of ResId based on GVK and name -func (id ResId) GvknString() string { - return id.Gvk.String() + separator + id.Name -} - // GvknEquals returns true if the other id matches // Group/Version/Kind/name. func (id ResId) GvknEquals(o ResId) bool { diff --git a/kyaml/resid/resid_test.go b/kyaml/resid/resid_test.go index 0ad979ce88..041608f7cd 100644 --- a/kyaml/resid/resid_test.go +++ b/kyaml/resid/resid_test.go @@ -17,7 +17,7 @@ var resIdStringTests = []struct { Gvk: Gvk{Group: "g", Version: "v", Kind: "k"}, Name: "nm", }, - "g_v_k|ns|nm", + "k.v.g/nm.ns", }, { ResId{ @@ -25,7 +25,7 @@ var resIdStringTests = []struct { Gvk: Gvk{Version: "v", Kind: "k"}, Name: "nm", }, - "~G_v_k|ns|nm", + "k.v.[noGrp]/nm.ns", }, { ResId{ @@ -33,7 +33,7 @@ var resIdStringTests = []struct { Gvk: Gvk{Kind: "k"}, Name: "nm", }, - "~G_~V_k|ns|nm", + "k.[noVer].[noGrp]/nm.ns", }, { ResId{ @@ -41,38 +41,26 @@ var resIdStringTests = []struct { Gvk: Gvk{}, Name: "nm", }, - "~G_~V_~K|ns|nm", + "[noKind].[noVer].[noGrp]/nm.ns", }, { ResId{ Gvk: Gvk{}, Name: "nm", }, - "~G_~V_~K|~X|nm", - }, - { - ResId{ - Gvk: Gvk{}, - Name: "nm", - }, - "~G_~V_~K|~X|nm", - }, - { - ResId{ - Gvk: Gvk{}, - }, - "~G_~V_~K|~X|~N", + "[noKind].[noVer].[noGrp]/nm.[noNs]", }, { ResId{ Gvk: Gvk{}, }, - "~G_~V_~K|~X|~N", + "[noKind].[noVer].[noGrp]/[noName].[noNs]", }, { ResId{}, - "~G_~V_~K|~X|~N", + "[noKind].[noVer].[noGrp]/[noName].[noNs]", }, + } func TestResIdString(t *testing.T) { @@ -83,84 +71,7 @@ func TestResIdString(t *testing.T) { } } -var gvknStringTests = []struct { - x ResId - s string -}{ - { - ResId{ - Namespace: "ns", - Gvk: Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - "g_v_k|nm", - }, - { - ResId{ - Namespace: "ns", - Gvk: Gvk{Version: "v", Kind: "k"}, - Name: "nm", - }, - "~G_v_k|nm", - }, - { - ResId{ - Namespace: "ns", - Gvk: Gvk{Kind: "k"}, - Name: "nm", - }, - "~G_~V_k|nm", - }, - { - ResId{ - Namespace: "ns", - Gvk: Gvk{}, - Name: "nm", - }, - "~G_~V_~K|nm", - }, - { - ResId{ - Gvk: Gvk{}, - Name: "nm", - }, - "~G_~V_~K|nm", - }, - { - ResId{ - Gvk: Gvk{}, - Name: "nm", - }, - "~G_~V_~K|nm", - }, - { - ResId{ - Gvk: Gvk{}, - }, - "~G_~V_~K|", - }, - { - ResId{ - Gvk: Gvk{}, - }, - "~G_~V_~K|", - }, - { - ResId{}, - "~G_~V_~K|", - }, -} - -func TestGvknString(t *testing.T) { - for _, hey := range gvknStringTests { - if hey.x.GvknString() != hey.s { - t.Fatalf("Actual: %s, Expected: '%s'", hey.x.GvknString(), hey.s) - } - } -} - func TestResIdEquals(t *testing.T) { - var GvknEqualsTest = []struct { id1 ResId id2 ResId @@ -358,6 +269,24 @@ var ids = []ResId{ { Gvk: Gvk{}, }, + { + Gvk: Gvk{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + isClusterScoped: true, + }, + Name: "nm", + }, + { + Gvk: Gvk{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + isClusterScoped: true, + }, + Name: "my.name", + }, } func TestResIdIsSelected(t *testing.T) { diff --git a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer_test.go b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer_test.go index 0a7e0a291b..63488febfe 100644 --- a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer_test.go +++ b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer_test.go @@ -218,7 +218,7 @@ spec: t.Fatalf("No match should return an error") } if err.Error() != - "resource with name service does not match a config with the following GVK [~G_~V_Deployment]" { + "resource with name service does not match a config with the following GVK [Deployment.[noVer].[noGrp]]" { t.Fatalf("Unexpected error: %v", err) } }