From 392bb8e77723f3b53b065e06ca5014facc439481 Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Wed, 12 Nov 2025 10:52:17 +0200 Subject: [PATCH] Refactor API group validation and error reporting Updated API group validation logic to improve error reporting. The Validate method now provides more descriptive error messages when unknown API groups are encountered. Unit test cases are updated to match the new error messages. Co-authored-by: Jordan Liggitt --- .../pkg/server/options/api_enablement.go | 38 ++++++++++--------- .../pkg/server/options/api_enablement_test.go | 12 ++++-- .../pkg/server/resourceconfig/helpers.go | 19 ++++++++-- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement.go b/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement.go index 44a5ddcc5e4..576126ccb9c 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement.go @@ -22,6 +22,7 @@ import ( "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/server/resourceconfig" serverstore "k8s.io/apiserver/pkg/server/storage" @@ -70,20 +71,22 @@ func (s *APIEnablementOptions) Validate(registries ...GroupRegistry) []error { errors := []error{} if s.RuntimeConfig[resourceconfig.APIAll] == "false" && len(s.RuntimeConfig) == 1 { // Do not allow only set api/all=false, in such case apiserver startup has no meaning. - return append(errors, fmt.Errorf("invalid key with only %v=false", resourceconfig.APIAll)) + return append(errors, fmt.Errorf("invalid runtime-config with only %v=false", resourceconfig.APIAll)) } - groups, err := resourceconfig.ParseGroups(s.RuntimeConfig) + groupVersions, err := resourceconfig.ParseGroupVersions(s.RuntimeConfig) if err != nil { return append(errors, err) } - for _, registry := range registries { - // filter out known groups - groups = unknownGroups(groups, registry) + unknownGroupVersions := sets.New[string]() + for _, groupVersion := range groupVersions { + if !isGroupRegistered(groupVersion.Group, registries) { + unknownGroupVersions.Insert(groupVersion.String()) + } } - if len(groups) != 0 { - errors = append(errors, fmt.Errorf("unknown api groups %s", strings.Join(groups, ","))) + if len(unknownGroupVersions) != 0 { + errors = append(errors, fmt.Errorf("unknown api groups %s", strings.Join(sets.List(unknownGroupVersions), ","))) } return errors @@ -117,18 +120,17 @@ func (s *APIEnablementOptions) ApplyTo(c *server.Config, defaultResourceConfig * return err } -func unknownGroups(groups []string, registry GroupRegistry) []string { - unknownGroups := []string{} - for _, group := range groups { - if !registry.IsGroupRegistered(group) { - unknownGroups = append(unknownGroups, group) - } - } - return unknownGroups -} - // GroupRegistry provides a method to check whether given group is registered. type GroupRegistry interface { - // IsRegistered returns true if given group is registered. + // IsGroupRegistered returns true if given group is registered. IsGroupRegistered(group string) bool } + +func isGroupRegistered(group string, registries []GroupRegistry) bool { + for _, registry := range registries { + if registry.IsGroupRegistered(group) { + return true + } + } + return false +} diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement_test.go index a14319e5373..0ac267d7554 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/api_enablement_test.go @@ -40,9 +40,9 @@ func TestAPIEnablementOptionsValidate(t *testing.T) { name: "test when options is nil", }, { - name: "test when invalid key with only api/all=false", + name: "test when invalid runtime-config with only api/all=false", runtimeConfig: cliflag.ConfigurationMap{"api/all": "false"}, - expectErr: "invalid key with only api/all=false", + expectErr: "invalid runtime-config with only api/all=false", }, { name: "test when ConfigurationMap key is invalid", @@ -51,13 +51,17 @@ func TestAPIEnablementOptionsValidate(t *testing.T) { }, { name: "test when unknown api groups", - runtimeConfig: cliflag.ConfigurationMap{"api/v1": "true"}, - expectErr: "unknown api groups", + runtimeConfig: cliflag.ConfigurationMap{"api/v1": "true", "api/v1beta2": "true"}, + expectErr: "unknown api groups api/v1,api/v1beta2", }, { name: "test when valid api groups", runtimeConfig: cliflag.ConfigurationMap{"apiregistration.k8s.io/v1beta1": "true"}, }, + { + name: "test when invalid api groups", + runtimeConfig: cliflag.ConfigurationMap{"apiregistration.k8s.io/v1beta1": "true"}, + }, } testGroupRegistry := fakeGroupRegistry{} diff --git a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go index c546112ec9c..d99d8b8b2a8 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go +++ b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go @@ -221,24 +221,37 @@ func getRuntimeConfigValue(overrides cliflag.ConfigurationMap, apiKey string, de // ParseGroups takes in resourceConfig and returns parsed groups. func ParseGroups(resourceConfig cliflag.ConfigurationMap) ([]string, error) { + groupVersions, err := ParseGroupVersions(resourceConfig) + if err != nil { + return nil, err + } groups := []string{} + for _, gv := range groupVersions { + groups = append(groups, gv.Group) + } + return groups, nil +} + +// ParseGroupVersions takes in resourceConfig and returns parsed group versions. +func ParseGroupVersions(resourceConfig cliflag.ConfigurationMap) ([]schema.GroupVersion, error) { + groupVersions := []schema.GroupVersion{} for key := range resourceConfig { if _, ok := groupVersionMatchers[key]; ok { continue } tokens := strings.Split(key, "/") if len(tokens) != 2 && len(tokens) != 3 { - return groups, fmt.Errorf("runtime-config invalid key %s", key) + return nil, fmt.Errorf("runtime-config invalid key %s", key) } groupVersionString := tokens[0] + "/" + tokens[1] groupVersion, err := schema.ParseGroupVersion(groupVersionString) if err != nil { return nil, fmt.Errorf("runtime-config invalid key %s", key) } - groups = append(groups, groupVersion.Group) + groupVersions = append(groupVersions, groupVersion) } - return groups, nil + return groupVersions, nil } // EmulationForwardCompatibleResourceConfig creates a new ResourceConfig that besides all the enabled resources in resourceConfig,