From e378fd2bae0fec4756a8e755395193337d13caa2 Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 9 Feb 2022 13:05:12 -0500 Subject: [PATCH 1/2] update the --runtime-config handling to ensure that user preferences always take priority over hardcoded preferences --- .../pkg/server/resourceconfig/helpers.go | 108 +++++--- .../pkg/server/resourceconfig/helpers_test.go | 240 ++++++++++++++++-- .../pkg/server/storage/resource_config.go | 14 + 3 files changed, 308 insertions(+), 54 deletions(-) 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 bfcce54b8d5..1049ff85e89 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go +++ b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go @@ -26,7 +26,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" serverstore "k8s.io/apiserver/pkg/server/storage" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/klog/v2" ) // GroupVersionRegistry provides access to registered group versions. @@ -65,7 +64,7 @@ var ( betaPattern = regexp.MustCompile(`^v\d+beta\d+$`) alphaPattern = regexp.MustCompile(`^v\d+alpha\d+$`) - matchers = map[string]func(gv schema.GroupVersion) bool{ + groupVersionMatchers = map[string]func(gv schema.GroupVersion) bool{ // allows users to address all api versions APIAll: func(gv schema.GroupVersion) bool { return true }, // allows users to address all api versions in the form v[0-9]+ @@ -76,9 +75,28 @@ var ( APIAlpha: func(gv schema.GroupVersion) bool { return alphaPattern.MatchString(gv.Version) }, } - matcherOrder = []string{APIAll, APIGA, APIBeta, APIAlpha} + groupVersionMatchersOrder = []string{APIAll, APIGA, APIBeta, APIAlpha} + + groupVersionResourceMatchers = map[string]func(gvr schema.GroupVersionResource) bool{ + // allows users to address all api versions + APIAll: func(gvr schema.GroupVersionResource) bool { return true }, + // allows users to address all api versions in the form v[0-9]+ + APIGA: func(gvr schema.GroupVersionResource) bool { return gaPattern.MatchString(gvr.Version) }, + // allows users to address all beta api versions + APIBeta: func(gvr schema.GroupVersionResource) bool { return betaPattern.MatchString(gvr.Version) }, + // allows users to address all alpha api versions + APIAlpha: func(gvr schema.GroupVersionResource) bool { return alphaPattern.MatchString(gvr.Version) }, + } + + groupVersionResourceMatchersOrder = []string{APIAll, APIGA, APIBeta, APIAlpha} ) +func resourceMatcherForVersion(gv schema.GroupVersion) func(gvr schema.GroupVersionResource) bool { + return func(gvr schema.GroupVersionResource) bool { + return gv == gvr.GroupVersion() + } +} + // MergeAPIResourceConfigs merges the given defaultAPIResourceConfig with the given resourceConfigOverrides. // Exclude the groups not registered in registry, and check if version is // not registered in group, then it will fail. @@ -90,29 +108,44 @@ func MergeAPIResourceConfigs( resourceConfig := defaultAPIResourceConfig overrides := resourceConfigOverrides - for _, flag := range matcherOrder { + for _, flag := range groupVersionMatchersOrder { if value, ok := overrides[flag]; ok { if value == "false" { - resourceConfig.DisableMatchingVersions(matchers[flag]) + resourceConfig.DisableMatchingVersions(groupVersionMatchers[flag]) } else if value == "true" { - resourceConfig.EnableMatchingVersions(matchers[flag]) + resourceConfig.EnableMatchingVersions(groupVersionMatchers[flag]) } else { return nil, fmt.Errorf("invalid value %v=%v", flag, value) } + // remove individual resource preferences that were hardcoded into the default. The override trumps those settings. + resourceConfig.RemoveMatchingResourcePreferences(groupVersionResourceMatchers[flag]) } } + type versionEnablementPreference struct { + key string + enabled bool + groupVersion schema.GroupVersion + } + type resourceEnablementPreference struct { + key string + enabled bool + groupVersionResource schema.GroupVersionResource + } + versionPreferences := []versionEnablementPreference{} + resourcePreferences := []resourceEnablementPreference{} + // "={true|false} allows users to enable/disable API. // This takes preference over api/all, if specified. // Iterate through all group/version overrides specified in runtimeConfig. for key := range overrides { // Have already handled them above. Can skip them here. - if _, ok := matchers[key]; ok { + if _, ok := groupVersionMatchers[key]; ok { continue } tokens := strings.Split(key, "/") - if len(tokens) < 2 { + if len(tokens) < 2 || len(tokens) > 3 { continue } groupVersionString := tokens[0] + "/" + tokens[1] @@ -121,13 +154,6 @@ func MergeAPIResourceConfigs( return nil, fmt.Errorf("invalid key %s", key) } - // individual resource enablement/disablement is only supported in the extensions/v1beta1 API group for legacy reasons. - // all other API groups are expected to contain coherent sets of resources that are enabled/disabled together. - if len(tokens) > 2 && (groupVersion != schema.GroupVersion{Group: "extensions", Version: "v1beta1"}) { - klog.Warningf("ignoring invalid key %s, individual resource enablement/disablement is not supported in %s, and will prevent starting in future releases", key, groupVersion.String()) - continue - } - // Exclude group not registered into the registry. if !registry.IsGroupRegistered(groupVersion.Group) { continue @@ -141,22 +167,46 @@ func MergeAPIResourceConfigs( if err != nil { return nil, err } - if enabled { - // enable the groupVersion for "group/version=true" and "group/version/resource=true" - resourceConfig.EnableVersions(groupVersion) - } else if len(tokens) == 2 { - // disable the groupVersion only for "group/version=false", not "group/version/resource=false" - resourceConfig.DisableVersions(groupVersion) - } - if len(tokens) < 3 { - continue + switch len(tokens) { + case 2: + versionPreferences = append(versionPreferences, versionEnablementPreference{ + key: key, + enabled: enabled, + groupVersion: groupVersion, + }) + case 3: + if strings.ToLower(tokens[2]) != tokens[2] { + return nil, fmt.Errorf("invalid key %v: group/version/resource and resource is always lowercase plural, not %q", key, tokens[2]) + } + resourcePreferences = append(resourcePreferences, resourceEnablementPreference{ + key: key, + enabled: enabled, + groupVersionResource: groupVersion.WithResource(tokens[2]), + }) } - groupVersionResource := groupVersion.WithResource(tokens[2]) - if enabled { - resourceConfig.EnableResources(groupVersionResource) + } + + // apply version preferences first, so that we can remove the hardcoded resource preferences that are being overridden + for _, versionPreference := range versionPreferences { + // if a user has expressed a preference about a version, that preference takes priority over the hardcoded resources + resourceConfig.RemoveMatchingResourcePreferences(resourceMatcherForVersion(versionPreference.groupVersion)) + if versionPreference.enabled { + // enable the groupVersion for "group/version=true" and "group/version/resource=true" + resourceConfig.EnableVersions(versionPreference.groupVersion) + } else { - resourceConfig.DisableResources(groupVersionResource) + // disable the groupVersion only for "group/version=false", not "group/version/resource=false" + resourceConfig.DisableVersions(versionPreference.groupVersion) + } + } + + // apply resource preferences last, so they have the highest priority + for _, resourcePreference := range resourcePreferences { + if resourcePreference.enabled { + resourceConfig.EnableResources(resourcePreference.groupVersionResource) + } else { + resourceConfig.DisableResources(resourcePreference.groupVersionResource) } } @@ -182,7 +232,7 @@ func getRuntimeConfigValue(overrides cliflag.ConfigurationMap, apiKey string, de func ParseGroups(resourceConfig cliflag.ConfigurationMap) ([]string, error) { groups := []string{} for key := range resourceConfig { - if _, ok := matchers[key]; ok { + if _, ok := groupVersionMatchers[key]; ok { continue } tokens := strings.Split(key, "/") diff --git a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers_test.go b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers_test.go index f78550d5251..9bd97e30ca5 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers_test.go @@ -20,6 +20,10 @@ import ( "reflect" "testing" + appsv1 "k8s.io/api/apps/v1" + + "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/stretchr/testify/require" apiv1 "k8s.io/api/core/v1" @@ -32,13 +36,28 @@ func TestParseRuntimeConfig(t *testing.T) { scheme := newFakeScheme(t) apiv1GroupVersion := apiv1.SchemeGroupVersion testCases := []struct { + name string runtimeConfig map[string]string defaultResourceConfig func() *serverstore.ResourceConfig expectedAPIConfig func() *serverstore.ResourceConfig err bool }{ { - // everything default value. + name: "using-kind", + runtimeConfig: map[string]string{ + "apps/v1/Deployment": "false", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedEnabledAPIs: defaultFakeEnabledResources(), + err: true, + }, + { + name: "everything-default-value", runtimeConfig: map[string]string{}, defaultResourceConfig: func() *serverstore.ResourceConfig { return newFakeAPIResourceConfigSource() @@ -49,7 +68,7 @@ func TestParseRuntimeConfig(t *testing.T) { err: false, }, { - // no runtimeConfig override. + name: "no-runtimeConfig-override", runtimeConfig: map[string]string{}, defaultResourceConfig: func() *serverstore.ResourceConfig { config := newFakeAPIResourceConfigSource() @@ -64,7 +83,7 @@ func TestParseRuntimeConfig(t *testing.T) { err: false, }, { - // version enabled by runtimeConfig override. + name: "version-enabled-by-runtimeConfig-override", runtimeConfig: map[string]string{ "apps/v1": "", }, @@ -79,7 +98,7 @@ func TestParseRuntimeConfig(t *testing.T) { err: false, }, { - // Disable v1. + name: "disable-v1", runtimeConfig: map[string]string{ "/v1": "false", }, @@ -94,7 +113,7 @@ func TestParseRuntimeConfig(t *testing.T) { err: false, }, { - // invalid runtime config + name: "invalid-runtime-config", runtimeConfig: map[string]string{ "invalidgroup/version": "false", }, @@ -107,7 +126,7 @@ func TestParseRuntimeConfig(t *testing.T) { err: false, }, { - // enable all + name: "enable-all", runtimeConfig: map[string]string{ "api/all": "true", }, @@ -117,12 +136,14 @@ func TestParseRuntimeConfig(t *testing.T) { expectedAPIConfig: func() *serverstore.ResourceConfig { config := newFakeAPIResourceConfigSource() config.EnableVersions(scheme.PrioritizedVersionsAllGroups()...) + // disabling groups of APIs removes the individual resource preferences from the default + config.RemoveMatchingResourcePreferences(matchAllExplicitResourcesForFake) return config }, err: false, }, { - // only enable v1 + name: "only-enable-v1", runtimeConfig: map[string]string{ "api/all": "false", "/v1": "true", @@ -132,13 +153,16 @@ func TestParseRuntimeConfig(t *testing.T) { }, expectedAPIConfig: func() *serverstore.ResourceConfig { config := newFakeAPIResourceConfigSource() + config.DisableVersions(appsv1.SchemeGroupVersion) config.DisableVersions(extensionsapiv1beta1.SchemeGroupVersion) + // disabling groups of APIs removes the individual resource preferences from the default + config.RemoveMatchingResourcePreferences(matchAllExplicitResourcesForFake) return config }, err: false, }, { - // enable specific extensions resources + name: "enable-specific-extensions-resources", runtimeConfig: map[string]string{ "extensions/v1beta1/deployments": "true", }, @@ -153,7 +177,7 @@ func TestParseRuntimeConfig(t *testing.T) { err: false, }, { - // disable specific extensions resources + name: "disable-specific-extensions-resources", runtimeConfig: map[string]string{ "extensions/v1beta1/ingresses": "false", }, @@ -168,7 +192,7 @@ func TestParseRuntimeConfig(t *testing.T) { err: false, }, { - // disable all extensions resources + name: "disable-all-extensions-resources", runtimeConfig: map[string]string{ "extensions/v1beta1": "false", }, @@ -178,12 +202,14 @@ func TestParseRuntimeConfig(t *testing.T) { expectedAPIConfig: func() *serverstore.ResourceConfig { config := newFakeAPIResourceConfigSource() config.DisableVersions(extensionsapiv1beta1.SchemeGroupVersion) + // disabling groups of APIs removes the individual resource preferences from the default + config.RemoveMatchingResourcePreferences(matchAllExplicitResourcesForFake) return config }, err: false, }, { - // disable a non-extensions resource + name: "disable-a-no-extensions-resources", runtimeConfig: map[string]string{ "apps/v1/deployments": "false", }, @@ -191,12 +217,14 @@ func TestParseRuntimeConfig(t *testing.T) { return newFakeAPIResourceConfigSource() }, expectedAPIConfig: func() *serverstore.ResourceConfig { - return newFakeAPIResourceConfigSource() + config := newFakeAPIResourceConfigSource() + config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) + return config }, err: false, // no error for backwards compatibility }, { - // disable all beta resources + name: "disable-all-beta-resources", runtimeConfig: map[string]string{ "api/beta": "false", }, @@ -206,24 +234,172 @@ func TestParseRuntimeConfig(t *testing.T) { expectedAPIConfig: func() *serverstore.ResourceConfig { config := newFakeAPIResourceConfigSource() config.DisableVersions(extensionsapiv1beta1.SchemeGroupVersion) + // disabling groups of APIs removes the individual resource preferences from the default + config.RemoveMatchingResourcePreferences(matchAllExplicitResourcesForFake) + return config + }, + err: false, // no error for backwards compatibility + }, + { + name: "user-explicit-disable-resource-over-user-version-enable", + runtimeConfig: map[string]string{ + "apps/v1": "true", + "apps/v1/deployments": "false", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) + return config + }, + err: false, // no error for backwards compatibility + }, + { + name: "user-explicit-enable-resource-over-user-version-disable", + runtimeConfig: map[string]string{ + "apps/v1": "false", + "apps/v1/deployments": "true", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.DisableVersions(appsv1.SchemeGroupVersion) + config.EnableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) + return config + }, + err: false, // no error for backwards compatibility + }, + { + name: "user-explicit-disable-resource-over-user-stability-enable", + runtimeConfig: map[string]string{ + "api/ga": "true", + "apps/v1/deployments": "false", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) + return config + }, + err: false, // no error for backwards compatibility + }, + { + name: "user-explicit-enable-resource-over-user-stability-disable", + runtimeConfig: map[string]string{ + "api/ga": "false", + "apps/v1/deployments": "true", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.DisableVersions(apiv1.SchemeGroupVersion) + config.DisableVersions(appsv1.SchemeGroupVersion) + config.EnableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) + return config + }, + err: false, // no error for backwards compatibility + }, + { + name: "user-explicit-disable-resource-over-user-version-enable-over-user-stability-disable", + runtimeConfig: map[string]string{ + "api/ga": "false", + "apps/v1": "true", + "apps/v1/deployments": "false", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.DisableVersions(apiv1.SchemeGroupVersion) + config.EnableVersions(appsv1.SchemeGroupVersion) + config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) + return config + }, + err: false, // no error for backwards compatibility + }, + { + name: "user-explicit-enable-resource-over-user-version-disable-over-user-stability-disable", + runtimeConfig: map[string]string{ + "api/ga": "false", + "apps/v1": "false", + "apps/v1/deployments": "true", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.DisableVersions(apiv1.SchemeGroupVersion) + config.DisableVersions(appsv1.SchemeGroupVersion) + config.EnableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) + return config + }, + err: false, // no error for backwards compatibility + }, + { + name: "user-explicit-disable-resource-over-user-version-enable-over-user-stability-enable", + runtimeConfig: map[string]string{ + "api/ga": "true", + "apps/v1": "true", + "apps/v1/deployments": "false", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.EnableVersions(appsv1.SchemeGroupVersion) + config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) + return config + }, + err: false, // no error for backwards compatibility + }, + { + name: "user-explicit-enable-resource-over-user-version-disable-over-user-stability-enable", + runtimeConfig: map[string]string{ + "api/ga": "true", + "apps/v1": "false", + "apps/v1/deployments": "true", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.DisableVersions(appsv1.SchemeGroupVersion) + config.EnableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, err: false, // no error for backwards compatibility }, } - for index, test := range testCases { - t.Log(scheme.PrioritizedVersionsAllGroups()) - actualDisablers, err := MergeAPIResourceConfigs(test.defaultResourceConfig(), test.runtimeConfig, scheme) - if err == nil && test.err { - t.Fatalf("expected error for test case: %v", index) - } else if err != nil && !test.err { - t.Fatalf("unexpected error: %s, for test: %v", err, test) - } + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + t.Log(scheme.PrioritizedVersionsAllGroups()) + actualDisablers, err := MergeAPIResourceConfigs(test.defaultResourceConfig(), test.runtimeConfig, scheme) + if err == nil && test.err { + t.Fatalf("expected error") + } else if err != nil && !test.err { + t.Fatalf("unexpected error: %s, for test: %v", err, test) + } + if err != nil { + return + } - expectedConfig := test.expectedAPIConfig() - if err == nil && !reflect.DeepEqual(actualDisablers, expectedConfig) { - t.Fatalf("%v: unexpected apiResourceDisablers. Actual: %v\n expected: %v", test.runtimeConfig, actualDisablers, expectedConfig) - } + expectedConfig := test.expectedAPIConfig() + if !reflect.DeepEqual(actualDisablers, expectedConfig) { + t.Fatalf("%v: unexpected apiResourceDisablers. Actual: %v\n expected: %v", test.runtimeConfig, actualDisablers, expectedConfig) + } + }) } } @@ -232,6 +408,7 @@ func newFakeAPIResourceConfigSource() *serverstore.ResourceConfig { // NOTE: GroupVersions listed here will be enabled by default. Don't put alpha versions in the list. ret.EnableVersions( apiv1.SchemeGroupVersion, + appsv1.SchemeGroupVersion, extensionsapiv1beta1.SchemeGroupVersion, ) ret.EnableResources( @@ -246,9 +423,22 @@ func newFakeAPIResourceConfigSource() *serverstore.ResourceConfig { return ret } +func matchAllExplicitResourcesForFake(gvr schema.GroupVersionResource) bool { + switch gvr { + case extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"), + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"), + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"), + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): + return true + } + return false + +} + func newFakeScheme(t *testing.T) *runtime.Scheme { ret := runtime.NewScheme() require.NoError(t, apiv1.AddToScheme(ret)) + require.NoError(t, appsv1.AddToScheme(ret)) require.NoError(t, extensionsapiv1beta1.AddToScheme(ret)) require.NoError(t, ret.SetVersionPriority(apiv1.SchemeGroupVersion)) diff --git a/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config.go b/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config.go index bd85ac230b2..cba20ee779b 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config.go @@ -70,6 +70,20 @@ func (o *ResourceConfig) EnableMatchingVersions(matcher func(gv schema.GroupVers } } +// RemoveMatchingResourcePreferences removes individual resource preferences that match. This is useful when an override of a version or level enablement should +// override the previously individual preferences. +func (o *ResourceConfig) RemoveMatchingResourcePreferences(matcher func(gvr schema.GroupVersionResource) bool) { + keysToRemove := []schema.GroupVersionResource{} + for k := range o.ResourceConfigs { + if matcher(k) { + keysToRemove = append(keysToRemove, k) + } + } + for _, k := range keysToRemove { + delete(o.ResourceConfigs, k) + } +} + // DisableVersions disables the versions entirely. func (o *ResourceConfig) DisableVersions(versions ...schema.GroupVersion) { for _, version := range versions { From 41b2662bac7a80d41dfbab13d72c9e1557c0f613 Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 9 Feb 2022 15:44:47 -0500 Subject: [PATCH 2/2] update resourceconfig to have per-resource preferences take priority --- cmd/kube-apiserver/app/aggregator.go | 2 +- pkg/controlplane/instance.go | 2 +- .../pkg/server/resourceconfig/helpers.go | 7 +- .../pkg/server/resourceconfig/helpers_test.go | 189 +++++++++++++++++- .../pkg/server/storage/resource_config.go | 37 ++-- .../server/storage/resource_config_test.go | 96 ++++----- .../pkg/server/storage/storage_factory.go | 2 +- 7 files changed, 235 insertions(+), 100 deletions(-) diff --git a/cmd/kube-apiserver/app/aggregator.go b/cmd/kube-apiserver/app/aggregator.go index 581d236c857..b719a4547a8 100644 --- a/cmd/kube-apiserver/app/aggregator.go +++ b/cmd/kube-apiserver/app/aggregator.go @@ -146,7 +146,7 @@ func createAggregatorServer(aggregatorConfig *aggregatorapiserver.Config, delega // let the CRD controller process the initial set of CRDs before starting the autoregistration controller. // this prevents the autoregistration controller's initial sync from deleting APIServices for CRDs that still exist. // we only need to do this if CRDs are enabled on this server. We can't use discovery because we are the source for discovery. - if aggregatorConfig.GenericConfig.MergedResourceConfig.AnyVersionForGroupEnabled("apiextensions.k8s.io") { + if aggregatorConfig.GenericConfig.MergedResourceConfig.AnyResourceForGroupEnabled("apiextensions.k8s.io") { crdRegistrationController.WaitForInitialSync() } autoRegistrationController.Run(5, context.StopCh) diff --git a/pkg/controlplane/instance.go b/pkg/controlplane/instance.go index 424cd0ce54b..60369c2316f 100644 --- a/pkg/controlplane/instance.go +++ b/pkg/controlplane/instance.go @@ -563,7 +563,7 @@ func (m *Instance) InstallAPIs(apiResourceConfigSource serverstorage.APIResource for _, restStorageBuilder := range restStorageProviders { groupName := restStorageBuilder.GroupName() - if !apiResourceConfigSource.AnyVersionForGroupEnabled(groupName) { + if !apiResourceConfigSource.AnyResourceForGroupEnabled(groupName) { klog.V(1).Infof("Skipping disabled API group %q.", groupName) continue } 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 1049ff85e89..f0e30701c9b 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go +++ b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go @@ -87,8 +87,6 @@ var ( // allows users to address all alpha api versions APIAlpha: func(gvr schema.GroupVersionResource) bool { return alphaPattern.MatchString(gvr.Version) }, } - - groupVersionResourceMatchersOrder = []string{APIAll, APIGA, APIBeta, APIAlpha} ) func resourceMatcherForVersion(gv schema.GroupVersion) func(gvr schema.GroupVersionResource) bool { @@ -192,11 +190,11 @@ func MergeAPIResourceConfigs( // if a user has expressed a preference about a version, that preference takes priority over the hardcoded resources resourceConfig.RemoveMatchingResourcePreferences(resourceMatcherForVersion(versionPreference.groupVersion)) if versionPreference.enabled { - // enable the groupVersion for "group/version=true" and "group/version/resource=true" + // enable the groupVersion for "group/version=true" resourceConfig.EnableVersions(versionPreference.groupVersion) } else { - // disable the groupVersion only for "group/version=false", not "group/version/resource=false" + // disable the groupVersion only for "group/version=false" resourceConfig.DisableVersions(versionPreference.groupVersion) } } @@ -204,6 +202,7 @@ func MergeAPIResourceConfigs( // apply resource preferences last, so they have the highest priority for _, resourcePreference := range resourcePreferences { if resourcePreference.enabled { + // enable the resource for "group/version/resource=true" resourceConfig.EnableResources(resourcePreference.groupVersionResource) } else { resourceConfig.DisableResources(resourcePreference.groupVersionResource) diff --git a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers_test.go b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers_test.go index 9bd97e30ca5..cd27e16f566 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers_test.go @@ -20,15 +20,13 @@ import ( "reflect" "testing" - appsv1 "k8s.io/api/apps/v1" - - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" apiv1 "k8s.io/api/core/v1" extensionsapiv1beta1 "k8s.io/api/extensions/v1beta1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" serverstore "k8s.io/apiserver/pkg/server/storage" ) @@ -40,6 +38,7 @@ func TestParseRuntimeConfig(t *testing.T) { runtimeConfig map[string]string defaultResourceConfig func() *serverstore.ResourceConfig expectedAPIConfig func() *serverstore.ResourceConfig + expectedEnabledAPIs map[schema.GroupVersionResource]bool err bool }{ { @@ -65,7 +64,8 @@ func TestParseRuntimeConfig(t *testing.T) { expectedAPIConfig: func() *serverstore.ResourceConfig { return newFakeAPIResourceConfigSource() }, - err: false, + expectedEnabledAPIs: defaultFakeEnabledResources(), + err: false, }, { name: "no-runtimeConfig-override", @@ -80,7 +80,8 @@ func TestParseRuntimeConfig(t *testing.T) { config.DisableVersions(extensionsapiv1beta1.SchemeGroupVersion) return config }, - err: false, + expectedEnabledAPIs: defaultFakeEnabledResources(), + err: false, }, { name: "version-enabled-by-runtimeConfig-override", @@ -95,7 +96,8 @@ func TestParseRuntimeConfig(t *testing.T) { config := newFakeAPIResourceConfigSource() return config }, - err: false, + expectedEnabledAPIs: defaultFakeEnabledResources(), + err: false, }, { name: "disable-v1", @@ -110,6 +112,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.DisableVersions(apiv1GroupVersion) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): false, + }, err: false, }, { @@ -123,7 +133,8 @@ func TestParseRuntimeConfig(t *testing.T) { expectedAPIConfig: func() *serverstore.ResourceConfig { return newFakeAPIResourceConfigSource() }, - err: false, + expectedEnabledAPIs: defaultFakeEnabledResources(), + err: false, }, { name: "enable-all", @@ -140,6 +151,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.RemoveMatchingResourcePreferences(matchAllExplicitResourcesForFake) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): true, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, }, { @@ -159,6 +178,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.RemoveMatchingResourcePreferences(matchAllExplicitResourcesForFake) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): false, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, }, { @@ -174,7 +201,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.EnableResources(extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments")) return config }, - err: false, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, }, { name: "disable-specific-extensions-resources", @@ -189,7 +223,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.DisableResources(extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses")) return config }, - err: false, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, }, { name: "disable-all-extensions-resources", @@ -206,7 +247,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.RemoveMatchingResourcePreferences(matchAllExplicitResourcesForFake) return config }, - err: false, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, }, { name: "disable-a-no-extensions-resources", @@ -221,6 +269,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): false, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, // no error for backwards compatibility }, { @@ -238,6 +294,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.RemoveMatchingResourcePreferences(matchAllExplicitResourcesForFake) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, // no error for backwards compatibility }, { @@ -254,6 +318,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): false, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, // no error for backwards compatibility }, { @@ -271,6 +343,15 @@ func TestParseRuntimeConfig(t *testing.T) { config.EnableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + appsv1.SchemeGroupVersion.WithResource("other"): false, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, // no error for backwards compatibility }, { @@ -287,6 +368,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): false, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, // no error for backwards compatibility }, { @@ -305,6 +394,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.EnableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): false, + }, err: false, // no error for backwards compatibility }, { @@ -324,6 +421,15 @@ func TestParseRuntimeConfig(t *testing.T) { config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): false, + appsv1.SchemeGroupVersion.WithResource("other"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): false, + }, err: false, // no error for backwards compatibility }, { @@ -343,6 +449,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.EnableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): false, + }, err: false, // no error for backwards compatibility }, { @@ -361,6 +475,14 @@ func TestParseRuntimeConfig(t *testing.T) { config.DisableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): false, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, // no error for backwards compatibility }, { @@ -379,6 +501,15 @@ func TestParseRuntimeConfig(t *testing.T) { config.EnableResources(appsv1.SchemeGroupVersion.WithResource("deployments")) return config }, + expectedEnabledAPIs: map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + appsv1.SchemeGroupVersion.WithResource("other"): false, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + }, err: false, // no error for backwards compatibility }, } @@ -399,6 +530,20 @@ func TestParseRuntimeConfig(t *testing.T) { if !reflect.DeepEqual(actualDisablers, expectedConfig) { t.Fatalf("%v: unexpected apiResourceDisablers. Actual: %v\n expected: %v", test.runtimeConfig, actualDisablers, expectedConfig) } + + for _, resourceToCheck := range apiResourcesToCheck() { + actual := actualDisablers.ResourceEnabled(resourceToCheck) + expected := test.expectedEnabledAPIs[resourceToCheck] + if actual != expected { + t.Errorf("for %v, actual=%v, expected=%v", resourceToCheck, actual, expected) + } + } + for resourceToCheck, expected := range test.expectedEnabledAPIs { + actual := actualDisablers.ResourceEnabled(resourceToCheck) + if actual != expected { + t.Errorf("for %v, actual=%v, expected=%v", resourceToCheck, actual, expected) + } + } }) } } @@ -432,7 +577,29 @@ func matchAllExplicitResourcesForFake(gvr schema.GroupVersionResource) bool { return true } return false +} +// apiResourcesToCheck are the apis we use in this set of unit tests. They will be check for enable/disable status +func apiResourcesToCheck() []schema.GroupVersionResource { + return []schema.GroupVersionResource{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"), + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"), + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"), + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"), + appsv1.SchemeGroupVersion.WithResource("deployments"), + apiv1.SchemeGroupVersion.WithResource("pods"), + } +} + +func defaultFakeEnabledResources() map[schema.GroupVersionResource]bool { + return map[schema.GroupVersionResource]bool{ + extensionsapiv1beta1.SchemeGroupVersion.WithResource("ingresses"): true, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("deployments"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("replicasets"): false, + extensionsapiv1beta1.SchemeGroupVersion.WithResource("daemonsets"): false, + appsv1.SchemeGroupVersion.WithResource("deployments"): true, + apiv1.SchemeGroupVersion.WithResource("pods"): true, + } } func newFakeScheme(t *testing.T) *runtime.Scheme { diff --git a/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config.go b/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config.go index cba20ee779b..78771148d4a 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config.go @@ -24,7 +24,7 @@ import ( type APIResourceConfigSource interface { VersionEnabled(version schema.GroupVersion) bool ResourceEnabled(resource schema.GroupVersionResource) bool - AnyVersionForGroupEnabled(group string) bool + AnyResourceForGroupEnabled(group string) bool } var _ APIResourceConfigSource = &ResourceConfig{} @@ -38,20 +38,6 @@ func NewResourceConfig() *ResourceConfig { return &ResourceConfig{GroupVersionConfigs: map[schema.GroupVersion]bool{}, ResourceConfigs: map[schema.GroupVersionResource]bool{}} } -// DisableAll disables all group/versions. It does not modify individual resource enablement/disablement. -func (o *ResourceConfig) DisableAll() { - for k := range o.GroupVersionConfigs { - o.GroupVersionConfigs[k] = false - } -} - -// EnableAll enables all group/versions. It does not modify individual resource enablement/disablement. -func (o *ResourceConfig) EnableAll() { - for k := range o.GroupVersionConfigs { - o.GroupVersionConfigs[k] = true - } -} - // DisableMatchingVersions disables all group/versions for which the matcher function returns true. It does not modify individual resource enablement/disablement. func (o *ResourceConfig) DisableMatchingVersions(matcher func(gv schema.GroupVersion) bool) { for k := range o.GroupVersionConfigs { @@ -97,6 +83,7 @@ func (o *ResourceConfig) EnableVersions(versions ...schema.GroupVersion) { } } +// TODO this must be removed and we enable/disable individual resources. func (o *ResourceConfig) VersionEnabled(version schema.GroupVersion) bool { enabled, _ := o.GroupVersionConfigs[version] return enabled @@ -115,17 +102,20 @@ func (o *ResourceConfig) EnableResources(resources ...schema.GroupVersionResourc } func (o *ResourceConfig) ResourceEnabled(resource schema.GroupVersionResource) bool { + // if a resource is explicitly set, that takes priority over the preference of the version. + resourceEnabled, explicitlySet := o.ResourceConfigs[resource] + if explicitlySet { + return resourceEnabled + } + if !o.VersionEnabled(resource.GroupVersion()) { return false } - resourceEnabled, explicitlySet := o.ResourceConfigs[resource] - if !explicitlySet { - return true - } - return resourceEnabled + // they are enabled by default. + return true } -func (o *ResourceConfig) AnyVersionForGroupEnabled(group string) bool { +func (o *ResourceConfig) AnyResourceForGroupEnabled(group string) bool { for version := range o.GroupVersionConfigs { if version.Group == group { if o.VersionEnabled(version) { @@ -133,6 +123,11 @@ func (o *ResourceConfig) AnyVersionForGroupEnabled(group string) bool { } } } + for resource := range o.ResourceConfigs { + if resource.Group == group && o.ResourceEnabled(resource) { + return true + } + } return false } diff --git a/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config_test.go b/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config_test.go index 25a6fb0dc52..431179115bd 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/storage/resource_config_test.go @@ -65,16 +65,19 @@ func TestDisabledResource(t *testing.T) { config.EnableResources(g1v1rEnabled, g1v2rEnabled, g2v1rEnabled) config.DisableResources(g1v1rDisabled, g1v2rDisabled, g2v1rDisabled) - // all resources under g1v1 are disabled because the group-version is disabled + // all resources not explicitly enabled under g1v1 are disabled because the group-version is disabled if config.ResourceEnabled(g1v1rUnspecified) { t.Errorf("expected disabled for %v, from %v", g1v1rUnspecified, config) } - if config.ResourceEnabled(g1v1rEnabled) { - t.Errorf("expected disabled for %v, from %v", g1v1rEnabled, config) + if !config.ResourceEnabled(g1v1rEnabled) { + t.Errorf("expected enabled for %v, from %v", g1v1rEnabled, config) } if config.ResourceEnabled(g1v1rDisabled) { t.Errorf("expected disabled for %v, from %v", g1v1rDisabled, config) } + if config.ResourceEnabled(g1v1rUnspecified) { + t.Errorf("expected disabled for %v, from %v", g1v1rUnspecified, config) + } // explicitly disabled resources in enabled group-versions are disabled if config.ResourceEnabled(g1v2rDisabled) { @@ -97,61 +100,6 @@ func TestDisabledResource(t *testing.T) { if !config.ResourceEnabled(g2v1rEnabled) { t.Errorf("expected enabled for %v, from %v", g2v1rEnabled, config) } - - // DisableAll() only disables to the group/version level for compatibility - // corresponds to --runtime-config=api/all=false - config.DisableAll() - if config.ResourceEnabled(g1v1rEnabled) { - t.Errorf("expected disabled for %v, from %v", g1v1rEnabled, config) - } - if config.ResourceEnabled(g1v2rEnabled) { - t.Errorf("expected disabled for %v, from %v", g1v2rEnabled, config) - } - if config.ResourceEnabled(g2v1rEnabled) { - t.Errorf("expected disabled for %v, from %v", g2v1rEnabled, config) - } - - // DisableAll() only disables to the group/version level for compatibility - // corresponds to --runtime-config=api/all=false,g1/v1=true - config.DisableAll() - config.EnableVersions(g1v1) - if !config.ResourceEnabled(g1v1rEnabled) { - t.Errorf("expected enabled for %v, from %v", g1v1rEnabled, config) - } - - // EnableAll() only enables to the group/version level for compatibility - config.EnableAll() - - // all unspecified or enabled resources under all groups now enabled - if !config.ResourceEnabled(g1v1rUnspecified) { - t.Errorf("expected enabled for %v, from %v", g1v1rUnspecified, config) - } - if !config.ResourceEnabled(g1v1rEnabled) { - t.Errorf("expected enabled for %v, from %v", g1v1rEnabled, config) - } - if !config.ResourceEnabled(g1v2rUnspecified) { - t.Errorf("expected enabled for %v, from %v", g1v2rUnspecified, config) - } - if !config.ResourceEnabled(g1v2rEnabled) { - t.Errorf("expected enabled for %v, from %v", g1v2rEnabled, config) - } - if !config.ResourceEnabled(g2v1rUnspecified) { - t.Errorf("expected enabled for %v, from %v", g2v1rUnspecified, config) - } - if !config.ResourceEnabled(g2v1rEnabled) { - t.Errorf("expected enabled for %v, from %v", g2v1rEnabled, config) - } - - // previously disabled resources are still disabled - if config.ResourceEnabled(g1v1rDisabled) { - t.Errorf("expected disabled for %v, from %v", g1v1rDisabled, config) - } - if config.ResourceEnabled(g1v2rDisabled) { - t.Errorf("expected disabled for %v, from %v", g1v2rDisabled, config) - } - if config.ResourceEnabled(g2v1rDisabled) { - t.Errorf("expected disabled for %v, from %v", g2v1rDisabled, config) - } } func TestAnyVersionForGroupEnabled(t *testing.T) { @@ -192,13 +140,39 @@ func TestAnyVersionForGroupEnabled(t *testing.T) { }, testGroup: "one", + expectedResult: true, + }, + { + name: "present, and one resource enabled", + creator: func() APIResourceConfigSource { + ret := NewResourceConfig() + ret.DisableVersions(schema.GroupVersion{Group: "one", Version: "version1"}) + ret.EnableResources(schema.GroupVersionResource{Group: "one", Version: "version2", Resource: "foo"}) + return ret + }, + testGroup: "one", + + expectedResult: true, + }, + { + name: "present, and one resource under disabled version enabled", + creator: func() APIResourceConfigSource { + ret := NewResourceConfig() + ret.DisableVersions(schema.GroupVersion{Group: "one", Version: "version1"}) + ret.EnableResources(schema.GroupVersionResource{Group: "one", Version: "version1", Resource: "foo"}) + return ret + }, + testGroup: "one", + expectedResult: true, }, } for _, tc := range tests { - if e, a := tc.expectedResult, tc.creator().AnyVersionForGroupEnabled(tc.testGroup); e != a { - t.Errorf("%s: expected %v, got %v", tc.name, e, a) - } + t.Run(tc.name, func(t *testing.T) { + if e, a := tc.expectedResult, tc.creator().AnyResourceForGroupEnabled(tc.testGroup); e != a { + t.Errorf("expected %v, got %v", e, a) + } + }) } } diff --git a/staging/src/k8s.io/apiserver/pkg/server/storage/storage_factory.go b/staging/src/k8s.io/apiserver/pkg/server/storage/storage_factory.go index 3b8c71de1fa..dfe8d4104a7 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/storage/storage_factory.go +++ b/staging/src/k8s.io/apiserver/pkg/server/storage/storage_factory.go @@ -240,7 +240,7 @@ func getAllResourcesAlias(resource schema.GroupResource) schema.GroupResource { func (s *DefaultStorageFactory) getStorageGroupResource(groupResource schema.GroupResource) schema.GroupResource { for _, potentialStorageResource := range s.Overrides[groupResource].cohabitatingResources { - if s.APIResourceConfigSource.AnyVersionForGroupEnabled(potentialStorageResource.Group) { + if s.APIResourceConfigSource.AnyResourceForGroupEnabled(potentialStorageResource.Group) { return potentialStorageResource } }