From a5760dee812d280e4de203fccf58214051d0d62a Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Sat, 12 Oct 2019 09:59:14 -0400 Subject: [PATCH] Add support for --runtime-config=api/beta=false, --feature-gates=AllBeta=false Allow disabling all beta features and APIs --- cmd/kube-apiserver/app/options/options.go | 2 +- .../pkg/server/options/api_enablement.go | 17 ++- .../pkg/server/resourceconfig/helpers.go | 49 +++++-- .../pkg/server/resourceconfig/helpers_test.go | 15 +++ .../pkg/server/storage/resource_config.go | 18 +++ .../featuregate/feature_gate.go | 18 +++ .../featuregate/feature_gate_test.go | 122 +++++++++++++++--- 7 files changed, 205 insertions(+), 36 deletions(-) diff --git a/cmd/kube-apiserver/app/options/options.go b/cmd/kube-apiserver/app/options/options.go index c88511ea716..e64604758db 100644 --- a/cmd/kube-apiserver/app/options/options.go +++ b/cmd/kube-apiserver/app/options/options.go @@ -142,7 +142,7 @@ func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) { s.Authentication.AddFlags(fss.FlagSet("authentication")) s.Authorization.AddFlags(fss.FlagSet("authorization")) s.CloudProvider.AddFlags(fss.FlagSet("cloud provider")) - s.APIEnablement.AddFlags(fss.FlagSet("api enablement")) + s.APIEnablement.AddFlags(fss.FlagSet("API enablement")) s.EgressSelector.AddFlags(fss.FlagSet("egress selector")) s.Admission.AddFlags(fss.FlagSet("admission")) 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 65f143bf0d7..794e89dedb6 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 @@ -43,11 +43,14 @@ func NewAPIEnablementOptions() *APIEnablementOptions { // AddFlags adds flags for a specific APIServer to the specified FlagSet func (s *APIEnablementOptions) AddFlags(fs *pflag.FlagSet) { fs.Var(&s.RuntimeConfig, "runtime-config", ""+ - "A set of key=value pairs that describe runtime configuration that may be passed "+ - "to apiserver. / (or for the core group) key can be used to "+ - "turn on/off specific api versions. api/all is special key to control all api versions, "+ - "be careful setting it false, unless you know what you do. api/legacy is deprecated, "+ - "we will remove it in the future, so stop using it.") + "A set of key=value pairs that enable or disable built-in APIs. Supported options are:\n"+ + "v1=true|false for the core API group\n"+ + "/=true|false for a specific API group and version (e.g. apps/v1=true)\n"+ + "api/all=true|false controls all API versions\n"+ + "api/ga=true|false controls all API versions of the form v[0-9]+\n"+ + "api/beta=true|false controls all API versions of the form v[0-9]+beta[0-9]+\n"+ + "api/alpha=true|false controls all API versions of the form v[0-9]+alpha[0-9]+\n"+ + "api/legacy is deprecated, and will be removed in a future version") } // Validate validates RuntimeConfig with a list of registries. @@ -61,9 +64,9 @@ func (s *APIEnablementOptions) Validate(registries ...GroupRegisty) []error { } errors := []error{} - if s.RuntimeConfig["api/all"] == "false" && len(s.RuntimeConfig) == 1 { + 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 api/all=false")) + return append(errors, fmt.Errorf("invalid key with only %v=false", resourceconfig.APIAll)) } groups, err := resourceconfig.ParseGroups(s.RuntimeConfig) 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 09047cfe978..85e5e1c46d9 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go +++ b/staging/src/k8s.io/apiserver/pkg/server/resourceconfig/helpers.go @@ -18,6 +18,7 @@ package resourceconfig import ( "fmt" + "regexp" "strconv" "strings" @@ -51,6 +52,33 @@ func MergeResourceEncodingConfigs( return resourceEncodingConfig } +// Recognized values for the --runtime-config parameter to enable/disable groups of APIs +const ( + APIAll = "api/all" + APIGA = "api/ga" + APIBeta = "api/beta" + APIAlpha = "api/alpha" +) + +var ( + gaPattern = regexp.MustCompile(`^v\d+$`) + betaPattern = regexp.MustCompile(`^v\d+beta\d+$`) + alphaPattern = regexp.MustCompile(`^v\d+alpha\d+$`) + + matchers = 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]+ + APIGA: func(gv schema.GroupVersion) bool { return gaPattern.MatchString(gv.Version) }, + // allows users to address all beta api versions + APIBeta: func(gv schema.GroupVersion) bool { return betaPattern.MatchString(gv.Version) }, + // allows users to address all alpha api versions + APIAlpha: func(gv schema.GroupVersion) bool { return alphaPattern.MatchString(gv.Version) }, + } + + matcherOrder = []string{APIAll, APIGA, APIBeta, APIAlpha} +) + // 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. @@ -62,14 +90,15 @@ func MergeAPIResourceConfigs( resourceConfig := defaultAPIResourceConfig overrides := resourceConfigOverrides - // "api/all=false" allows users to selectively enable specific api versions. - allAPIFlagValue, ok := overrides["api/all"] - if ok { - if allAPIFlagValue == "false" { - // Disable all group versions. - resourceConfig.DisableAll() - } else if allAPIFlagValue == "true" { - resourceConfig.EnableAll() + for _, flag := range matcherOrder { + if value, ok := overrides[flag]; ok { + if value == "false" { + resourceConfig.DisableMatchingVersions(matchers[flag]) + } else if value == "true" { + resourceConfig.EnableMatchingVersions(matchers[flag]) + } else { + return nil, fmt.Errorf("invalid value %v=%v", flag, value) + } } } @@ -78,7 +107,7 @@ func MergeAPIResourceConfigs( // Iterate through all group/version overrides specified in runtimeConfig. for key := range overrides { // Have already handled them above. Can skip them here. - if key == "api/all" { + if _, ok := matchers[key]; ok { continue } @@ -153,7 +182,7 @@ func getRuntimeConfigValue(overrides cliflag.ConfigurationMap, apiKey string, de func ParseGroups(resourceConfig cliflag.ConfigurationMap) ([]string, error) { groups := []string{} for key := range resourceConfig { - if key == "api/all" { + if _, ok := matchers[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 9f905941bfe..f78550d5251 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 @@ -195,6 +195,21 @@ func TestParseRuntimeConfig(t *testing.T) { }, err: false, // no error for backwards compatibility }, + { + // disable all beta resources + runtimeConfig: map[string]string{ + "api/beta": "false", + }, + defaultResourceConfig: func() *serverstore.ResourceConfig { + return newFakeAPIResourceConfigSource() + }, + expectedAPIConfig: func() *serverstore.ResourceConfig { + config := newFakeAPIResourceConfigSource() + config.DisableVersions(extensionsapiv1beta1.SchemeGroupVersion) + return config + }, + err: false, // no error for backwards compatibility + }, } for index, test := range testCases { t.Log(scheme.PrioritizedVersionsAllGroups()) 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 eb892b5a5c0..0f2d45c9e78 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 @@ -52,6 +52,24 @@ func (o *ResourceConfig) EnableAll() { } } +// 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 { + if matcher(k) { + o.GroupVersionConfigs[k] = false + } + } +} + +// EnableMatchingVersions enables all group/versions for which the matcher function returns true. It does not modify individual resource enablement/disablement. +func (o *ResourceConfig) EnableMatchingVersions(matcher func(gv schema.GroupVersion) bool) { + for k := range o.GroupVersionConfigs { + if matcher(k) { + o.GroupVersionConfigs[k] = true + } + } +} + // DisableVersions disables the versions entirely. func (o *ResourceConfig) DisableVersions(versions ...schema.GroupVersion) { for _, version := range versions { diff --git a/staging/src/k8s.io/component-base/featuregate/feature_gate.go b/staging/src/k8s.io/component-base/featuregate/feature_gate.go index 7a847044631..50e17622539 100644 --- a/staging/src/k8s.io/component-base/featuregate/feature_gate.go +++ b/staging/src/k8s.io/component-base/featuregate/feature_gate.go @@ -40,17 +40,25 @@ const ( // AllAlpha=false,NewFeature=true will result in newFeature=true // AllAlpha=true,NewFeature=false will result in newFeature=false allAlphaGate Feature = "AllAlpha" + + // allBetaGate is a global toggle for beta features. Per-feature key + // values override the default set by allBetaGate. Examples: + // AllBeta=false,NewFeature=true will result in NewFeature=true + // AllBeta=true,NewFeature=false will result in NewFeature=false + allBetaGate Feature = "AllBeta" ) var ( // The generic features. defaultFeatures = map[Feature]FeatureSpec{ allAlphaGate: {Default: false, PreRelease: Alpha}, + allBetaGate: {Default: false, PreRelease: Beta}, } // Special handling for a few gates. specialFeatures = map[Feature]func(known map[Feature]FeatureSpec, enabled map[Feature]bool, val bool){ allAlphaGate: setUnsetAlphaGates, + allBetaGate: setUnsetBetaGates, } ) @@ -129,6 +137,16 @@ func setUnsetAlphaGates(known map[Feature]FeatureSpec, enabled map[Feature]bool, } } +func setUnsetBetaGates(known map[Feature]FeatureSpec, enabled map[Feature]bool, val bool) { + for k, v := range known { + if v.PreRelease == Beta { + if _, found := enabled[k]; !found { + enabled[k] = val + } + } + } +} + // Set, String, and Type implement pflag.Value var _ pflag.Value = &featureGate{} diff --git a/staging/src/k8s.io/component-base/featuregate/feature_gate_test.go b/staging/src/k8s.io/component-base/featuregate/feature_gate_test.go index 18c4e3547a6..610b271dc38 100644 --- a/staging/src/k8s.io/component-base/featuregate/feature_gate_test.go +++ b/staging/src/k8s.io/component-base/featuregate/feature_gate_test.go @@ -39,6 +39,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "", expect: map[Feature]bool{ allAlphaGate: false, + allBetaGate: false, testAlphaGate: false, testBetaGate: false, }, @@ -47,6 +48,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "fooBarBaz=true", expect: map[Feature]bool{ allAlphaGate: false, + allBetaGate: false, testAlphaGate: false, testBetaGate: false, }, @@ -56,6 +58,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "AllAlpha=false", expect: map[Feature]bool{ allAlphaGate: false, + allBetaGate: false, testAlphaGate: false, testBetaGate: false, }, @@ -64,6 +67,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "AllAlpha=true", expect: map[Feature]bool{ allAlphaGate: true, + allBetaGate: false, testAlphaGate: true, testBetaGate: false, }, @@ -72,6 +76,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "AllAlpha=banana", expect: map[Feature]bool{ allAlphaGate: false, + allBetaGate: false, testAlphaGate: false, testBetaGate: false, }, @@ -81,6 +86,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "AllAlpha=false,TestAlpha=true", expect: map[Feature]bool{ allAlphaGate: false, + allBetaGate: false, testAlphaGate: true, testBetaGate: false, }, @@ -89,6 +95,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "TestAlpha=true,AllAlpha=false", expect: map[Feature]bool{ allAlphaGate: false, + allBetaGate: false, testAlphaGate: true, testBetaGate: false, }, @@ -97,6 +104,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "AllAlpha=true,TestAlpha=false", expect: map[Feature]bool{ allAlphaGate: true, + allBetaGate: false, testAlphaGate: false, testBetaGate: false, }, @@ -105,6 +113,7 @@ func TestFeatureGateFlag(t *testing.T) { arg: "TestAlpha=false,AllAlpha=true", expect: map[Feature]bool{ allAlphaGate: true, + allBetaGate: false, testAlphaGate: false, testBetaGate: false, }, @@ -113,33 +122,110 @@ func TestFeatureGateFlag(t *testing.T) { arg: "TestBeta=true,AllAlpha=false", expect: map[Feature]bool{ allAlphaGate: false, + allBetaGate: false, testAlphaGate: false, testBetaGate: true, }, }, + + { + arg: "AllBeta=false", + expect: map[Feature]bool{ + allAlphaGate: false, + allBetaGate: false, + testAlphaGate: false, + testBetaGate: false, + }, + }, + { + arg: "AllBeta=true", + expect: map[Feature]bool{ + allAlphaGate: false, + allBetaGate: true, + testAlphaGate: false, + testBetaGate: true, + }, + }, + { + arg: "AllBeta=banana", + expect: map[Feature]bool{ + allAlphaGate: false, + allBetaGate: false, + testAlphaGate: false, + testBetaGate: false, + }, + parseError: "invalid value of AllBeta", + }, + { + arg: "AllBeta=false,TestBeta=true", + expect: map[Feature]bool{ + allAlphaGate: false, + allBetaGate: false, + testAlphaGate: false, + testBetaGate: true, + }, + }, + { + arg: "TestBeta=true,AllBeta=false", + expect: map[Feature]bool{ + allAlphaGate: false, + allBetaGate: false, + testAlphaGate: false, + testBetaGate: true, + }, + }, + { + arg: "AllBeta=true,TestBeta=false", + expect: map[Feature]bool{ + allAlphaGate: false, + allBetaGate: true, + testAlphaGate: false, + testBetaGate: false, + }, + }, + { + arg: "TestBeta=false,AllBeta=true", + expect: map[Feature]bool{ + allAlphaGate: false, + allBetaGate: true, + testAlphaGate: false, + testBetaGate: false, + }, + }, + { + arg: "TestAlpha=true,AllBeta=false", + expect: map[Feature]bool{ + allAlphaGate: false, + allBetaGate: false, + testAlphaGate: true, + testBetaGate: false, + }, + }, } for i, test := range tests { - fs := pflag.NewFlagSet("testfeaturegateflag", pflag.ContinueOnError) - f := NewFeatureGate() - f.Add(map[Feature]FeatureSpec{ - testAlphaGate: {Default: false, PreRelease: Alpha}, - testBetaGate: {Default: false, PreRelease: Beta}, - }) - f.AddFlag(fs) + t.Run(test.arg, func(t *testing.T) { + fs := pflag.NewFlagSet("testfeaturegateflag", pflag.ContinueOnError) + f := NewFeatureGate() + f.Add(map[Feature]FeatureSpec{ + testAlphaGate: {Default: false, PreRelease: Alpha}, + testBetaGate: {Default: false, PreRelease: Beta}, + }) + f.AddFlag(fs) - err := fs.Parse([]string{fmt.Sprintf("--%s=%s", flagName, test.arg)}) - if test.parseError != "" { - if !strings.Contains(err.Error(), test.parseError) { - t.Errorf("%d: Parse() Expected %v, Got %v", i, test.parseError, err) + err := fs.Parse([]string{fmt.Sprintf("--%s=%s", flagName, test.arg)}) + if test.parseError != "" { + if !strings.Contains(err.Error(), test.parseError) { + t.Errorf("%d: Parse() Expected %v, Got %v", i, test.parseError, err) + } + } else if err != nil { + t.Errorf("%d: Parse() Expected nil, Got %v", i, err) } - } else if err != nil { - t.Errorf("%d: Parse() Expected nil, Got %v", i, err) - } - for k, v := range test.expect { - if actual := f.enabled.Load().(map[Feature]bool)[k]; actual != v { - t.Errorf("%d: expected %s=%v, Got %v", i, k, v, actual) + for k, v := range test.expect { + if actual := f.enabled.Load().(map[Feature]bool)[k]; actual != v { + t.Errorf("%d: expected %s=%v, Got %v", i, k, v, actual) + } } - } + }) } }