Merge pull request #129813 from yongruilin/golangci-featuregate-add

feat: add a lint rule to prevent Add unversioned featuregate

hack/golangci-hints.yaml

@@ -46,6 +46,12 @@ issues:
       text: should not be used because managedFields was removed
       path: _test.go$
 
+    # Adding unversioned feature gates is allowed in tests
+    - linters:
+        - forbidigo
+      text: should not use MutableFeatureGate.Add, use AddVersioned instead
+      path: _test.go$
+
     # The Kubernetes naming convention for conversion functions uses underscores
     # and intentionally deviates from normal Go conventions to make those function
     # names more readable. Same for SetDefaults_*.
@@ -197,6 +203,10 @@ linters-settings: # please keep this alphabetized
     - p: \.Extract
       pkg: ^k8s\.io/client-go/applyconfigurations/
       msg: should not be used because managedFields was removed
+    - p: \.Add$
+      pkg: ^k8s\.io/component-base/featuregate$
+      type: ^MutableFeatureGate$
+      msg: should not use MutableFeatureGate.Add, use AddVersioned instead
     - p: ^gomega\.BeTrue$
       pkg: ^github.com/onsi/gomega$
       msg: "it does not produce a good failure message - use BeTrueBecause with an explicit printf-style failure message instead, or plain Go: if ... { ginkgo.Fail(...) }"

hack/golangci-strict.yaml

@@ -46,6 +46,12 @@ issues:
       text: should not be used because managedFields was removed
       path: _test.go$
 
+    # Adding unversioned feature gates is allowed in tests
+    - linters:
+        - forbidigo
+      text: should not use MutableFeatureGate.Add, use AddVersioned instead
+      path: _test.go$
+
     # The Kubernetes naming convention for conversion functions uses underscores
     # and intentionally deviates from normal Go conventions to make those function
     # names more readable. Same for SetDefaults_*.
@@ -243,6 +249,10 @@ linters-settings: # please keep this alphabetized
     - p: \.Extract
       pkg: ^k8s\.io/client-go/applyconfigurations/
       msg: should not be used because managedFields was removed
+    - p: \.Add$
+      pkg: ^k8s\.io/component-base/featuregate$
+      type: ^MutableFeatureGate$
+      msg: should not use MutableFeatureGate.Add, use AddVersioned instead
   revive:
     # Only these rules are enabled.
     rules:

hack/golangci.yaml

@@ -46,6 +46,12 @@ issues:
       text: should not be used because managedFields was removed
       path: _test.go$
 
+    # Adding unversioned feature gates is allowed in tests
+    - linters:
+        - forbidigo
+      text: should not use MutableFeatureGate.Add, use AddVersioned instead
+      path: _test.go$
+
     # TODO(oscr) Remove these excluded directories and fix findings. Due to large amount of findings in different components
     # with different owners it's hard to fix everything in a single pr. This will therefore be done in multiple prs.
     - path: (pkg/volume/*|test/*|azure/*|pkg/cmd/wait*|request/bearertoken/*|metrics/*|filters/*)
@@ -245,6 +251,10 @@ linters-settings: # please keep this alphabetized
     - p: \.Extract
       pkg: ^k8s\.io/client-go/applyconfigurations/
       msg: should not be used because managedFields was removed
+    - p: \.Add$
+      pkg: ^k8s\.io/component-base/featuregate$
+      type: ^MutableFeatureGate$
+      msg: should not use MutableFeatureGate.Add, use AddVersioned instead
   gocritic:
     enabled-checks:
       - equalFold

hack/golangci.yaml.in

@@ -46,6 +46,12 @@ issues:
       text: should not be used because managedFields was removed
       path: _test.go$
 
+    # Adding unversioned feature gates is allowed in tests
+    - linters:
+        - forbidigo
+      text: should not use MutableFeatureGate.Add, use AddVersioned instead
+      path: _test.go$
+
     {{- if .Base}}
 
     # TODO(oscr) Remove these excluded directories and fix findings. Due to large amount of findings in different components
@@ -200,6 +206,10 @@ linters-settings: # please keep this alphabetized
     - p: \.Extract
       pkg: ^k8s\.io/client-go/applyconfigurations/
       msg: should not be used because managedFields was removed
+    - p: \.Add$
+      pkg: ^k8s\.io/component-base/featuregate$
+      type: ^MutableFeatureGate$
+      msg: should not use MutableFeatureGate.Add, use AddVersioned instead
     {{- if .Hints}}
     - p: ^gomega\.BeTrue$
       pkg: ^github.com/onsi/gomega$

pkg/features/client_adapter.go

@@ -65,7 +65,7 @@ func (a *clientAdapter) Add(in map[clientfeatures.Feature]clientfeatures.Feature
 		}
 		out[featuregate.Feature(name)] = converted
 	}
-	return a.mfg.Add(out)
+	return a.mfg.Add(out) //nolint:forbidigo // No need to support versioned feature gates in client adapter
 }
 
 // Set implements the unexported interface that client-go feature gate testing expects for

pkg/features/kube_features.go

@@ -865,7 +865,7 @@ const (
 )
 
 func init() {
-	runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultKubernetesFeatureGates))
+	runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultKubernetesFeatureGates)) //nolint:forbidigo // TODO(https://github.com/kubernetes/enhancements/tree/master/keps/sig-architecture/4330-compatibility-versions): Remove this once we complete the migration to versioned feature gates
 	runtime.Must(utilfeature.DefaultMutableFeatureGate.AddVersioned(defaultVersionedKubernetesFeatureGates))
 	runtime.Must(zpagesfeatures.AddFeatureGates(utilfeature.DefaultMutableFeatureGate))
 

staging/src/k8s.io/apiserver/pkg/features/kube_features.go

@@ -246,7 +246,7 @@ const (
 )
 
 func init() {
-	runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultKubernetesFeatureGates))
+	runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultKubernetesFeatureGates)) //nolint:forbidigo // TODO(https://github.com/kubernetes/enhancements/tree/master/keps/sig-architecture/4330-compatibility-versions): Remove this once we complete the migration to versioned feature gates
 	runtime.Must(utilfeature.DefaultMutableFeatureGate.AddVersioned(defaultVersionedKubernetesFeatureGates))
 }
 

staging/src/k8s.io/component-base/logs/api/v1/kube_features.go

@@ -68,5 +68,5 @@ func featureGates() map[featuregate.Feature]featuregate.FeatureSpec {
 
 // AddFeatureGates adds all feature gates used by this package.
 func AddFeatureGates(mutableFeatureGate featuregate.MutableFeatureGate) error {
-	return mutableFeatureGate.Add(featureGates())
+	return mutableFeatureGate.Add(featureGates()) //nolint:forbidigo // The logging feature gates are by design unversioned (perpetual alpha/beta).
 }

test/e2e/framework/internal/unittests/bugs/features/features.go

@@ -29,7 +29,7 @@ const (
 )
 
 func init() {
-	runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(testFeatureGates))
+	runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(testFeatureGates)) //nolint:forbidigo // Part of a unit test.
 }
 
 var testFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{

test/e2e/framework/internal/unittests/features/kube_features.go

@@ -27,7 +27,7 @@ const (
 )
 
 func init() {
-	runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultKubernetesFeatureGates))
+	runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultKubernetesFeatureGates)) //nolint:forbidigo // Part of a unit test.
 }
 
 var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{