K8s: Improve key generation and parsing (#90014)

pkg/apiserver/go.mod

@@ -4,6 +4,7 @@ go 1.21.10
 
 require (
 	github.com/bwmarrin/snowflake v0.3.0
+	github.com/google/go-cmp v0.6.0
 	github.com/grafana/grafana/pkg/apimachinery v0.0.0-20240701135906-559738ce6ae1
 	github.com/prometheus/client_golang v1.19.0
 	github.com/stretchr/testify v1.9.0
@@ -11,7 +12,9 @@ require (
 	k8s.io/apimachinery v0.29.3
 	k8s.io/apiserver v0.29.2
 	k8s.io/client-go v0.29.3
+	k8s.io/component-base v0.29.2
 	k8s.io/klog/v2 v2.120.1
+	k8s.io/utils v0.0.0-20230726121419-3b25d923346b
 )
 
 require (
@@ -34,7 +37,6 @@ require (
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/btree v1.1.2 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
-	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/pprof v0.0.0-20240416155748-26353dc0451f // indirect
 	github.com/google/uuid v1.6.0 // indirect
@@ -84,9 +86,7 @@ require (
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/api v0.29.3 // indirect
-	k8s.io/component-base v0.29.2 // indirect
 	k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
-	k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.28.0 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect

pkg/apiserver/registry/generic/key.go

@@ -12,63 +12,75 @@ import (
 )
 
 type Key struct {
-	Group     string
+	Group     string `json:"group,omitempty"`
-	Resource  string
+	Resource  string `json:"resource"`
-	Namespace string
+	Namespace string `json:"namespace,omitempty"`
-	Name      string
+	Name      string `json:"name,omitempty"`
 }
 
-func ParseKey(key string) (*Key, error) {
+// ParseKey parses a key string into a Key.
-	// /<group>/<resource>[/namespaces/<namespace>][/<name>]
+// Format: [/group/<group>]/resource/<resource>[/namespace/<namespace>][/name/<name>]
-	parts := strings.Split(key, "/")
+func ParseKey(raw string) (*Key, error) {
-	if len(parts) < 3 {
+	parts := strings.Split(raw, "/")
-		return nil, fmt.Errorf("invalid key (expecting at least 2 parts): %s", key)
+	key := &Key{}
+
+	// Skip the first empty string
+	if parts[0] == "" {
+		parts = parts[1:]
 	}
 
-	if parts[0] != "" {
+	for i := 0; i < len(parts); i += 2 {
-		return nil, fmt.Errorf("invalid key (expecting leading slash): %s", key)
+		k := parts[i]
+		if i+1 >= len(parts) {
+			return nil, fmt.Errorf("invalid key: %s", raw)
+		}
+		v := parts[i+1]
+		switch k {
+		case "group":
+			key.Group = v
+		case "resource":
+			key.Resource = v
+		case "namespace":
+			key.Namespace = v
+		case "name":
+			key.Name = v
+		default:
+			return nil, fmt.Errorf("invalid key name: %s", key)
+		}
 	}
 
-	k := &Key{
+	if len(key.Resource) == 0 {
-		Group:    parts[1],
+		return nil, fmt.Errorf("missing resource: %s", raw)
-		Resource: parts[2],
 	}
 
-	if len(parts) == 3 {
+	return key, nil
-		return k, nil
-	}
-
-	if parts[3] != "namespaces" {
-		k.Name = parts[3]
-		return k, nil
-	}
-
-	if len(parts) < 5 {
-		return nil, fmt.Errorf("invalid key (expecting namespace after 'namespaces'): %s", key)
-	}
-
-	k.Namespace = parts[4]
-
-	if len(parts) == 5 {
-		return k, nil
-	}
-
-	k.Name = parts[5]
-
-	return k, nil
 }
 
+// String returns the string representation of the Key.
 func (k *Key) String() string {
-	s := "/" + k.Group + "/" + k.Resource
+	var builder strings.Builder
+
+	if len(k.Group) > 0 {
+		builder.WriteString("/group/")
+		builder.WriteString(k.Group)
+	}
+	if len(k.Resource) > 0 {
+		builder.WriteString("/resource/")
+		builder.WriteString(k.Resource)
+	}
 	if len(k.Namespace) > 0 {
-		s += "/namespaces/" + k.Namespace
+		builder.WriteString("/namespace/")
+		builder.WriteString(k.Namespace)
 	}
 	if len(k.Name) > 0 {
-		s += "/" + k.Name
+		builder.WriteString("/name/")
+		builder.WriteString(k.Name)
 	}
-	return s
+
+	return builder.String()
 }
 
+// IsEqual returns true if the keys are equal.
 func (k *Key) IsEqual(other *Key) bool {
 	return k.Group == other.Group &&
 		k.Resource == other.Resource &&

pkg/apiserver/registry/generic/key_test.go

@@ -0,0 +1,120 @@
+package generic
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestParseKey(t *testing.T) {
+	tests := []struct {
+		name     string
+		raw      string
+		expected *Key
+		wantErr  bool
+	}{
+		{
+			name:     "All keys",
+			raw:      "/group/test-group/resource/test-resource/namespace/test-namespace/name/test-name",
+			expected: &Key{Group: "test-group", Resource: "test-resource", Namespace: "test-namespace", Name: "test-name"},
+			wantErr:  false,
+		},
+		{
+			name:     "Missing group",
+			raw:      "/resource/test-resource/namespace/test-namespace/name/test-name",
+			expected: &Key{Group: "", Resource: "test-resource", Namespace: "test-namespace", Name: "test-name"},
+			wantErr:  false,
+		},
+		{
+			name:     "Missing namespace",
+			raw:      "/group/test-group/resource/test-resource/name/test-name",
+			expected: &Key{Group: "test-group", Resource: "test-resource", Namespace: "", Name: "test-name"},
+			wantErr:  false,
+		},
+		{
+			name:     "Missing name",
+			raw:      "/group/test-group/resource/test-resource/namespace/test-namespace",
+			expected: &Key{Group: "test-group", Resource: "test-resource", Namespace: "test-namespace", Name: ""},
+			wantErr:  false,
+		},
+		{
+			name:     "Missing resource",
+			raw:      "/group/test-group/namespace/test-namespace/name/test-name",
+			expected: nil,
+			wantErr:  true,
+		},
+		{
+			name:     "Empty string",
+			raw:      "",
+			expected: nil,
+			wantErr:  true,
+		},
+		{
+			name:     "Invalid key",
+			raw:      "/",
+			expected: nil,
+			wantErr:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ParseKey(tt.raw)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ParseKey() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.expected) {
+				t.Errorf("ParseKey() = %v, expected %v", got, tt.expected)
+			}
+		})
+	}
+}
+
+func BenchmarkKey_String(b *testing.B) {
+	key := &Key{Group: "test-group", Resource: "test-resource", Namespace: "test-namespace", Name: "test-name"}
+	for i := 0; i < b.N; i++ {
+		_ = key.String()
+	}
+}
+func TestKey_String(t *testing.T) {
+	tests := []struct {
+		name     string
+		key      *Key
+		expected string
+	}{
+		{
+			name:     "All fields",
+			key:      &Key{Group: "test-group", Resource: "test-resource", Namespace: "test-namespace", Name: "test-name"},
+			expected: "/group/test-group/resource/test-resource/namespace/test-namespace/name/test-name",
+		},
+		{
+			name:     "Missing group",
+			key:      &Key{Resource: "test-resource", Namespace: "test-namespace", Name: "test-name"},
+			expected: "/resource/test-resource/namespace/test-namespace/name/test-name",
+		},
+		{
+			name:     "Missing namespace",
+			key:      &Key{Group: "test-group", Resource: "test-resource", Name: "test-name"},
+			expected: "/group/test-group/resource/test-resource/name/test-name",
+		},
+		{
+			name:     "Missing name",
+			key:      &Key{Group: "test-group", Resource: "test-resource", Namespace: "test-namespace"},
+			expected: "/group/test-group/resource/test-resource/namespace/test-namespace",
+		},
+		{
+			name:     "Missing resource",
+			key:      &Key{Group: "test-group", Namespace: "test-namespace", Name: "test-name"},
+			expected: "/group/test-group/namespace/test-namespace/name/test-name",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.key.String()
+			if got != tt.expected {
+				t.Errorf("Key.String() = %s, expected %s", got, tt.expected)
+			}
+		})
+	}
+}

pkg/services/apiserver/storage/entity/utils_test.go

@@ -86,7 +86,7 @@ func TestResourceToEntity(t *testing.T) {
 					},
 				},
 			},
-			expectedKey:          "/playlist.grafana.app/playlists/namespaces/default/test-name",
+			expectedKey:          "/group/playlist.grafana.app/resource/playlists/namespace/default/name/test-name",
 			expectedGroupVersion: "v0alpha1",
 			expectedName:         "test-name",
 			expectedNamespace:    "default",
@@ -157,7 +157,7 @@ func TestEntityToResource(t *testing.T) {
 	}{
 		{
 			entity: &entityStore.Entity{
-				Key:             "/playlist.grafana.app/playlists/namespaces/default/test-uid",
+				Key:             "/group/playlist.grafana.app/resource/playlists/namespaces/default/name/test-uid",
 				GroupVersion:    "v0alpha1",
 				Name:            "test-uid",
 				Title:           "A playlist",

pkg/services/store/entity/sqlstash/testdata/grpc-req-create.json

@@ -6,7 +6,7 @@
         "namespace": "default",
         "name": "adnj1llchbbi8a",
         "group_version": "v0alpha1",
-        "key": "/playlist.grafana.app/playlists/namespaces/default/adnj1llchbbi8a",
+        "key": "/group/playlist.grafana.app/resource/playlists/namespaces/default/name/adnj1llchbbi8a",
         "meta": "eyJtZXRhZGF0YSI6eyJuYW1lIjoiYWRuajFsbGNoYmJpOGEiLCJuYW1lc3BhY2UiOiJkZWZhdWx0IiwidWlkIjoiYjAxOTljNjAtNWYzYS00MWJlLTliYTYtN2E1MmYxZGU4M2ZmIiwiY3JlYXRpb25UaW1lc3RhbXAiOiIyMDI0LTA2LTAyVDAzOjI4OjE3WiIsImFubm90YXRpb25zIjp7ImdyYWZhbmEuYXBwL29yaWdpbktleSI6IjIiLCJncmFmYW5hLmFwcC9vcmlnaW5OYW1lIjoiU1FMIiwiZ3JhZmFuYS5hcHAvb3JpZ2luVGltZXN0YW1wIjoiMjAyNC0wNi0wMlQwMzoyODoxN1oiLCJncmFmYW5hLmFwcC91cGRhdGVkVGltZXN0YW1wIjoiMjAyNC0wNi0wMlQwMzoyODoxN1oifX19",
         "body": "eyJraW5kIjoiUGxheWxpc3QiLCJhcGlWZXJzaW9uIjoicGxheWxpc3QuZ3JhZmFuYS5hcHAvdjBhbHBoYTEiLCJtZXRhZGF0YSI6eyJuYW1lIjoiYWRuajFsbGNoYmJpOGEiLCJuYW1lc3BhY2UiOiJkZWZhdWx0IiwidWlkIjoiYjAxOTljNjAtNWYzYS00MWJlLTliYTYtN2E1MmYxZGU4M2ZmIiwiY3JlYXRpb25UaW1lc3RhbXAiOiIyMDI0LTA2LTAyVDAzOjI4OjE3WiIsImFubm90YXRpb25zIjp7ImdyYWZhbmEuYXBwL29yaWdpbktleSI6IjIiLCJncmFmYW5hLmFwcC9vcmlnaW5OYW1lIjoiU1FMIiwiZ3JhZmFuYS5hcHAvb3JpZ2luVGltZXN0YW1wIjoiMjAyNC0wNi0wMlQwMzoyODoxN1oiLCJncmFmYW5hLmFwcC91cGRhdGVkVGltZXN0YW1wIjoiMjAyNC0wNi0wMlQwMzoyODoxN1oifX0sInNwZWMiOnsidGl0bGUiOiJ0ZXN0IHBsYXlsaXN0IiwiaW50ZXJ2YWwiOiI1bSIsIml0ZW1zIjpbeyJ0eXBlIjoiZGFzaGJvYXJkX2J5X3VpZCIsInZhbHVlIjoiY2RuaXY1M2dtZDR3MGUifV19fQo=",
         "title": "test playlist",

pkg/services/store/entity/sqlstash/testdata/grpc-req-delete.json

@@ -1,3 +1,3 @@
 {
-    "key": "/playlist.grafana.app/playlists/namespaces/default/sdfsdfsdf"
+    "key": "/group/playlist.grafana.app/resource/playlists/namespaces/default/name/sdfsdfsdf"
 }

pkg/services/store/entity/sqlstash/testdata/grpc-req-update.json

@@ -7,7 +7,7 @@
         "namespace": "default",
         "name": "sdfsdfsdf",
         "group_version": "v0alpha1",
-        "key": "/playlist.grafana.app/playlists/namespaces/default/sdfsdfsdf",
+        "key": "/group/playlist.grafana.app/resource/playlists/namespaces/default/name/sdfsdfsdf",
         "meta": "eyJtZXRhZGF0YSI6eyJuYW1lIjoic2Rmc2Rmc2RmIiwibmFtZXNwYWNlIjoiZGVmYXVsdCIsInVpZCI6IjNjNzY5YjJlLWFhYTctNDZmNi1hYjgzLWUwMzgwNTBhNmE3NSIsInJlc291cmNlVmVyc2lvbiI6IjEiLCJjcmVhdGlvblRpbWVzdGFtcCI6IjIwMjQtMDYtMDJUMDM6NDk6MjlaIiwibWFuYWdlZEZpZWxkcyI6W3sibWFuYWdlciI6Ik1vemlsbGEiLCJvcGVyYXRpb24iOiJVcGRhdGUiLCJhcGlWZXJzaW9uIjoicGxheWxpc3QuZ3JhZmFuYS5hcHAvdjBhbHBoYTEiLCJ0aW1lIjoiMjAyNC0wNi0wMlQwMzo1Mzo1NVoiLCJmaWVsZHNUeXBlIjoiRmllbGRzVjEiLCJmaWVsZHNWMSI6eyJmOnNwZWMiOnsiZjppbnRlcnZhbCI6e30sImY6aXRlbXMiOnt9LCJmOnRpdGxlIjp7fX19fV19fQ==",
         "body": "eyJraW5kIjoiUGxheWxpc3QiLCJhcGlWZXJzaW9uIjoicGxheWxpc3QuZ3JhZmFuYS5hcHAvdjBhbHBoYTEiLCJtZXRhZGF0YSI6eyJuYW1lIjoic2Rmc2Rmc2RmIiwibmFtZXNwYWNlIjoiZGVmYXVsdCIsInVpZCI6IjNjNzY5YjJlLWFhYTctNDZmNi1hYjgzLWUwMzgwNTBhNmE3NSIsInJlc291cmNlVmVyc2lvbiI6IjEiLCJjcmVhdGlvblRpbWVzdGFtcCI6IjIwMjQtMDYtMDJUMDM6NDk6MjlaIiwibWFuYWdlZEZpZWxkcyI6W3sibWFuYWdlciI6Ik1vemlsbGEiLCJvcGVyYXRpb24iOiJVcGRhdGUiLCJhcGlWZXJzaW9uIjoicGxheWxpc3QuZ3JhZmFuYS5hcHAvdjBhbHBoYTEiLCJ0aW1lIjoiMjAyNC0wNi0wMlQwMzo1Mzo1NVoiLCJmaWVsZHNUeXBlIjoiRmllbGRzVjEiLCJmaWVsZHNWMSI6eyJmOnNwZWMiOnsiZjppbnRlcnZhbCI6e30sImY6aXRlbXMiOnt9LCJmOnRpdGxlIjp7fX19fV19LCJzcGVjIjp7InRpdGxlIjoieHpjdnp4Y3Zxd2Vxd2UiLCJpbnRlcnZhbCI6IjVtIiwiaXRlbXMiOlt7InR5cGUiOiJkYXNoYm9hcmRfYnlfdWlkIiwidmFsdWUiOiJjZG5pdjUzZ21kNHcwZSJ9XX19Cg==",
         "title": "xzcvzxcvqweqwe",

pkg/services/store/entity/sqlstash/testdata/grpc-res-entity.json

@@ -6,7 +6,7 @@
     "namespace": "default",
     "name": "sdfsdfsdf",
     "group_version": "v0alpha1",
-    "key": "/playlist.grafana.app/playlists/namespaces/default/sdfsdfsdf",
+    "key": "/group/playlist.grafana.app/resource/playlists/namespace/default/name/sdfsdfsdf",
     "meta": "eyJtZXRhZGF0YSI6eyJuYW1lIjoic2Rmc2Rmc2RmIiwibmFtZXNwYWNlIjoiZGVmYXVsdCIsInVpZCI6IjAyZmVhOGVlLTk2ZDYtNGIzMy04ZGI5LTU5MmI0NzU4NTM4NSIsImNyZWF0aW9uVGltZXN0YW1wIjoiMjAyNC0wNi0wNFQxNToxODozNFoiLCJtYW5hZ2VkRmllbGRzIjpbeyJtYW5hZ2VyIjoiTW96aWxsYSIsIm9wZXJhdGlvbiI6IlVwZGF0ZSIsImFwaVZlcnNpb24iOiJwbGF5bGlzdC5ncmFmYW5hLmFwcC92MGFscGhhMSIsInRpbWUiOiIyMDI0LTA2LTA0VDE1OjE4OjM0WiIsImZpZWxkc1R5cGUiOiJGaWVsZHNWMSIsImZpZWxkc1YxIjp7ImY6c3BlYyI6eyJmOmludGVydmFsIjp7fSwiZjppdGVtcyI6e30sImY6dGl0bGUiOnt9fX19XX19",
     "body": "eyJraW5kIjoiUGxheWxpc3QiLCJhcGlWZXJzaW9uIjoicGxheWxpc3QuZ3JhZmFuYS5hcHAvdjBhbHBoYTEiLCJtZXRhZGF0YSI6eyJuYW1lIjoic2Rmc2Rmc2RmIiwibmFtZXNwYWNlIjoiZGVmYXVsdCIsInVpZCI6IjAyZmVhOGVlLTk2ZDYtNGIzMy04ZGI5LTU5MmI0NzU4NTM4NSIsImNyZWF0aW9uVGltZXN0YW1wIjoiMjAyNC0wNi0wNFQxNToxODozNFoiLCJtYW5hZ2VkRmllbGRzIjpbeyJtYW5hZ2VyIjoiTW96aWxsYSIsIm9wZXJhdGlvbiI6IlVwZGF0ZSIsImFwaVZlcnNpb24iOiJwbGF5bGlzdC5ncmFmYW5hLmFwcC92MGFscGhhMSIsInRpbWUiOiIyMDI0LTA2LTA0VDE1OjE4OjM0WiIsImZpZWxkc1R5cGUiOiJGaWVsZHNWMSIsImZpZWxkc1YxIjp7ImY6c3BlYyI6eyJmOmludGVydmFsIjp7fSwiZjppdGVtcyI6e30sImY6dGl0bGUiOnt9fX19XX0sInNwZWMiOnsidGl0bGUiOiJ4emN2enhjdiIsImludGVydmFsIjoiNW0iLCJpdGVtcyI6W3sidHlwZSI6ImRhc2hib2FyZF9ieV91aWQiLCJ2YWx1ZSI6ImNkbml2NTNnbWQ0dzBlIn1dfX0K",
     "title": "xzcvzxcv",