diff --git a/pkg/controller/registry/resolver/cache/operators.go b/pkg/controller/registry/resolver/cache/operators.go index f636cc8c9e..84ede77343 100644 --- a/pkg/controller/registry/resolver/cache/operators.go +++ b/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) diff --git a/pkg/controller/registry/resolver/cache/operators_test.go b/pkg/controller/registry/resolver/cache/operators_test.go index 61f8a8bf3a..91aab9d3e3 100644 --- a/pkg/controller/registry/resolver/cache/operators_test.go +++ b/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: "", }, }