resolver/cache: validate APIKey before GVK string serialization

pkg/controller/registry/resolver/cache/operators.go

@@ -37,7 +37,6 @@ func (s APISet) PopAPIKey() *opregistry.APIKey {
 
 func GVKStringToProvidedAPISet(gvksStr string) APISet {
 	set := make(APISet)
-	// TODO: Should we make gvk strings lowercase to avoid issues with user set gvks?
 	gvks := strings.Split(strings.Replace(gvksStr, " ", "", -1), ",")
 	for _, gvkStr := range gvks {
 		gvk, _ := schema.ParseKindArg(gvkStr)
@@ -49,12 +48,22 @@ func GVKStringToProvidedAPISet(gvksStr string) APISet {
 	return set
 }
 
+// Group is intentionally excluded — an empty Group is valid for core Kubernetes APIs.
+func isValidAPIKey(key opregistry.APIKey) bool {
+	return key.Kind != "" && key.Version != ""
+}
+
 func APIKeyToGVKString(key opregistry.APIKey) string {
-	// TODO: Add better validation of GVK
+	if !isValidAPIKey(key) {
+		return ""
+	}
 	return strings.Join([]string{key.Kind, key.Version, key.Group}, ".")
 }
 
 func APIKeyToGVKHash(key opregistry.APIKey) (string, error) {
+	if !isValidAPIKey(key) {
+		return "", fmt.Errorf("invalid APIKey: Kind and Version must be non-empty")
+	}
 	hash := fnv.New64a()
 	if _, err := hash.Write([]byte(APIKeyToGVKString(key))); err != nil {
 		return "", err
@@ -63,12 +72,11 @@ func APIKeyToGVKHash(key opregistry.APIKey) (string, error) {
 }
 
 func (s APISet) String() string {
-	gvkStrs := make([]string, len(s))
-	i := 0
+	gvkStrs := make([]string, 0, len(s))
 	for api := range s {
-		// TODO: Only add valid GVK strings
-		gvkStrs[i] = APIKeyToGVKString(api)
-		i++
+		if str := APIKeyToGVKString(api); str != "" {
+			gvkStrs = append(gvkStrs, str)
+		}
 	}
 	sort.Strings(gvkStrs)
 

pkg/controller/registry/resolver/cache/operators_test.go

@@ -81,18 +81,23 @@ func TestAPIKeyToGVKString(t *testing.T) {
 		{
 			name: "EmptyAPIKey",
 			in:   opregistry.APIKey{},
-			want: "..",
+			want: "",
 		},
 		{
-			name: "BadAPIKey",
+			name: "BadAPIKey/MissingKind",
 			in:   opregistry.APIKey{Group: "birds. ", Version: "-"},
-			want: ".-.birds. ",
+			want: "",
 		},
 		{
 			name: "GoodAPIKey",
 			in:   opregistry.APIKey{Group: "birds.com", Version: "v1alpha1", Kind: "Goose"},
 			want: "Goose.v1alpha1.birds.com",
 		},
+		{
+			name: "CoreAPIKey/EmptyGroup",
+			in:   opregistry.APIKey{Version: "v1", Kind: "Pod"},
+			want: "Pod.v1.",
+		},
 	}
 
 	for _, tt := range tests {
@@ -102,6 +107,43 @@ func TestAPIKeyToGVKString(t *testing.T) {
 	}
 }
 
+func TestAPIKeyToGVKHash(t *testing.T) {
+	tests := []struct {
+		name    string
+		in      opregistry.APIKey
+		wantErr bool
+	}{
+		{
+			name:    "EmptyAPIKey/Error",
+			in:      opregistry.APIKey{},
+			wantErr: true,
+		},
+		{
+			name:    "MissingKind/Error",
+			in:      opregistry.APIKey{Group: "birds.com", Version: "v1alpha1"},
+			wantErr: true,
+		},
+		{
+			name:    "GoodAPIKey/NoError",
+			in:      opregistry.APIKey{Group: "birds.com", Version: "v1alpha1", Kind: "Goose"},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			hash, err := APIKeyToGVKHash(tt.in)
+			if tt.wantErr {
+				require.Error(t, err)
+				require.Empty(t, hash)
+			} else {
+				require.NoError(t, err)
+				require.NotEmpty(t, hash)
+			}
+		})
+	}
+}
+
 func TestAPISetString(t *testing.T) {
 	tests := []struct {
 		name string
@@ -135,7 +177,15 @@ func TestAPISetString(t *testing.T) {
 				opregistry.APIKey{Group: "birds.com", Version: "v1alpha1", Kind: "Goose"}: {},
 				opregistry.APIKey{Group: "birds.com", Version: "v1alpha1", Kind: "Egret"}: {},
 			},
-			want: ".v1alpha1.birds.com,Egret.v1alpha1.birds.com,Goose.v1alpha1.birds.com",
+			want: "Egret.v1alpha1.birds.com,Goose.v1alpha1.birds.com",
+		},
+		{
+			name: "AllInvalidKeys/EmptyOutput",
+			in: APISet{
+				opregistry.APIKey{}: {},
+				opregistry.APIKey{Group: "birds.com", Version: "v1alpha1"}: {},
+			},
+			want: "",
 		},
 	}