Tighten feature gate interface to split out mutating methods

staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate.go

@@ -51,8 +51,15 @@ var (
 		allAlphaGate: setUnsetAlphaGates,
 	}
 
+	// DefaultMutableFeatureGate is a mutable version of DefaultFeatureGate.
+	// Only top-level commands/options setup and the k8s.io/apiserver/pkg/util/feature/testing package should make use of this.
+	// Tests that need to modify feature gates for the duration of their test should use:
+	//   defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.<FeatureName>, <value>)()
+	DefaultMutableFeatureGate MutableFeatureGate = NewFeatureGate()
+
 	// DefaultFeatureGate is a shared global FeatureGate.
-	DefaultFeatureGate FeatureGate = NewFeatureGate()
+	// Top-level commands/options setup that needs to modify this feature gate should use DefaultMutableFeatureGate.
+	DefaultFeatureGate FeatureGate = DefaultMutableFeatureGate
 )
 
 type FeatureSpec struct {
@@ -72,9 +79,23 @@ const (
 	Deprecated = prerelease("DEPRECATED")
 )
 
-// FeatureGate parses and stores flag gates for known features from
+// FeatureGate indicates whether a given feature is enabled or not
-// a string like feature1=true,feature2=false,...
 type FeatureGate interface {
+	// Enabled returns true if the key is enabled.
+	Enabled(key Feature) bool
+	// KnownFeatures returns a slice of strings describing the FeatureGate's known features.
+	KnownFeatures() []string
+	// DeepCopy returns a deep copy of the FeatureGate object, such that gates can be
+	// set on the copy without mutating the original. This is useful for validating
+	// config against potential feature gate changes before committing those changes.
+	DeepCopy() MutableFeatureGate
+}
+
+// MutableFeatureGate parses and stores flag gates for known features from
+// a string like feature1=true,feature2=false,...
+type MutableFeatureGate interface {
+	FeatureGate
+
 	// AddFlag adds a flag for setting global feature gates to the specified FlagSet.
 	AddFlag(fs *pflag.FlagSet)
 	// Set parses and stores flag gates for known features
@@ -82,16 +103,8 @@ type FeatureGate interface {
 	Set(value string) error
 	// SetFromMap stores flag gates for known features from a map[string]bool or returns an error
 	SetFromMap(m map[string]bool) error
-	// Enabled returns true if the key is enabled.
-	Enabled(key Feature) bool
 	// Add adds features to the featureGate.
 	Add(features map[Feature]FeatureSpec) error
-	// KnownFeatures returns a slice of strings describing the FeatureGate's known features.
-	KnownFeatures() []string
-	// DeepCopy returns a deep copy of the FeatureGate object, such that gates can be
-	// set on the copy without mutating the original. This is useful for validating
-	// config against potential feature gate changes before committing those changes.
-	DeepCopy() FeatureGate
 }
 
 // featureGate implements FeatureGate as well as pflag.Value for flag parsing.
@@ -294,7 +307,7 @@ func (f *featureGate) KnownFeatures() []string {
 // DeepCopy returns a deep copy of the FeatureGate object, such that gates can be
 // set on the copy without mutating the original. This is useful for validating
 // config against potential feature gate changes before committing those changes.
-func (f *featureGate) DeepCopy() FeatureGate {
+func (f *featureGate) DeepCopy() MutableFeatureGate {
 	// Copy existing state.
 	known := map[Feature]FeatureSpec{}
 	for k, v := range f.known.Load().(map[Feature]FeatureSpec) {

staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate_test.go

@@ -148,7 +148,7 @@ func TestFeatureGateOverride(t *testing.T) {
 	const testBetaGate Feature = "TestBeta"
 
 	// Don't parse the flag, assert defaults are used.
-	var f FeatureGate = NewFeatureGate()
+	var f *featureGate = NewFeatureGate()
 	f.Add(map[Feature]FeatureSpec{
 		testAlphaGate: {Default: false, PreRelease: Alpha},
 		testBetaGate:  {Default: false, PreRelease: Beta},
@@ -177,7 +177,7 @@ func TestFeatureGateFlagDefaults(t *testing.T) {
 	const testBetaGate Feature = "TestBeta"
 
 	// Don't parse the flag, assert defaults are used.
-	var f FeatureGate = NewFeatureGate()
+	var f *featureGate = NewFeatureGate()
 	f.Add(map[Feature]FeatureSpec{
 		testAlphaGate: {Default: false, PreRelease: Alpha},
 		testBetaGate:  {Default: true, PreRelease: Beta},
@@ -201,7 +201,7 @@ func TestFeatureGateKnownFeatures(t *testing.T) {
 	)
 
 	// Don't parse the flag, assert defaults are used.
-	var f FeatureGate = NewFeatureGate()
+	var f *featureGate = NewFeatureGate()
 	f.Add(map[Feature]FeatureSpec{
 		testAlphaGate:      {Default: false, PreRelease: Alpha},
 		testBetaGate:       {Default: true, PreRelease: Beta},

staging/src/k8s.io/apiserver/pkg/util/feature/testing/feature_gate_testing.go

@@ -69,16 +69,16 @@ func VerifyFeatureGatesUnchanged(gate feature.FeatureGate, tests func() int) {
 // Example use:
 //
 // defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.<FeatureName>, true)()
-func SetFeatureGateDuringTest(t *testing.T, gate feature.FeatureGate, feature feature.Feature, value bool) func() {
+func SetFeatureGateDuringTest(t *testing.T, gate feature.FeatureGate, f feature.Feature, value bool) func() {
-	originalValue := gate.Enabled(feature)
+	originalValue := gate.Enabled(f)
 
-	if err := gate.Set(fmt.Sprintf("%s=%v", feature, value)); err != nil {
+	if err := gate.(feature.MutableFeatureGate).Set(fmt.Sprintf("%s=%v", f, value)); err != nil {
-		t.Errorf("error setting %s=%v: %v", feature, value, err)
+		t.Errorf("error setting %s=%v: %v", f, value, err)
 	}
 
 	return func() {
-		if err := gate.Set(fmt.Sprintf("%s=%v", feature, originalValue)); err != nil {
+		if err := gate.(feature.MutableFeatureGate).Set(fmt.Sprintf("%s=%v", f, originalValue)); err != nil {
-			t.Errorf("error restoring %s=%v: %v", feature, originalValue, err)
+			t.Errorf("error restoring %s=%v: %v", f, originalValue, err)
 		}
 	}
 }