improve gvk and resid strings for error messages

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.

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) {

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 {

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",
+		"[noKind].[noVer].[noGrp]/nm.[noNs]",
 	},
 	{
 		ResId{
 			Gvk: Gvk{},
 		},
-		"~G_~V_~K|~X|~N",
-	},
-	{
-		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) {