Repository: kubernetes/kubernetes (language: go)

Issue:
Name of feature gate `DisableKubeletCSRAdmissionValidation` doesn't match other fixups
This comes from https://kubernetes.slack.com/archives/C5P3FE08M/p1722095887026819?thread_ts=1721949988.922209&cid=C5P3FE08M

We've [added](https://github.com/kubernetes/kubernetes/pull/126015) a feature gate, `DisableKubeletCSRAdmissionValidation`. However, we try to name feature gates after the positive thing they add, even if it's on by default, or even if we're setting if false because the positive thing is being dropped (rare, but it happens).

Typically we don't start feature gate names with `Disable`, as it makes the docs jar:
Example quote 1: _To temporarily revert to the legacy behavior, enable the_ `DisableFoo` _feature gate._
vs
Example quote 2: _You can enable the_ `LegacyFooHandling` _feature gate if you run code that relies on the previous behavior. You should plan to migrate._

and because disabling things isn't really a feature.

---

Some examples of gates you can or once could disable by turning them _off_ include:

- `AllowInsecureBackendProxy`
- `APIServingWithRoutine`
- `StorageNamespaceIndex`
- `LegacyNodeRoleBehavior`
- `LegacyServiceAccountTokenNoAutoGeneration`
- `LegacyServiceAccountTokenTracking`

So, I recommend a rename of the gate before the v1.31 release.

/sig auth architecture
Name of feature gate `DisableKubeletCSRAdmissionValidation` doesn't match other fixups
This comes from https://kubernetes.slack.com/archives/C5P3FE08M/p1722095887026819?thread_ts=1721949988.922209&cid=C5P3FE08M

We've [added](https://github.com/kubernetes/kubernetes/pull/126015) a feature gate, `DisableKubeletCSRAdmissionValidation`. However, we try to name feature gates after the positive thing they add, even if it's on by default, or even if we're setting if false because the positive thing is being dropped (rare, but it happens).

Typically we don't start feature gate names with `Disable`, as it makes the docs jar:
Example quote 1: _To temporarily revert to the legacy behavior, enable the_ `DisableFoo` _feature gate._
vs
Example quote 2: _You can enable the_ `LegacyFooHandling` _feature gate if you run code that relies on the previous behavior. You should plan to migrate._

and because disabling things isn't really a feature.

---

Some examples of gates you can or once could disable by turning them _off_ include:

- `AllowInsecureBackendProxy`
- `APIServingWithRoutine`
- `StorageNamespaceIndex`
- `LegacyNodeRoleBehavior`
- `LegacyServiceAccountTokenNoAutoGeneration`
- `LegacyServiceAccountTokenTracking`

So, I recommend a rename of the gate before the v1.31 release.

/sig auth architecture


Discussion / hints:
@liggitt @micahhausler @kubernetes/sig-auth-misc FYI
Example gate names:
- `LegacyNodeCertificateRequestValidation` (off by default)
- `LegacyKubeletCertificateRequestChecks`  (off by default)
- `StrictNodeCSRAdmissionValidation` (on by default)
- `VerifyCommonNameForKubeletServingCertificates` (on by default)
- `VerifyCommonNameForNodeCertificateRequests` (on by default)

from https://github.com/kubernetes/kubernetes/issues/126410#issuecomment-2254208550
- `AllowInsecureKubeletCertificateSigningRequests` (off by default, deprecated)
/milestone v1.31
@sftim: You must be a member of the [kubernetes/milestone-maintainers](https://github.com/orgs/kubernetes/teams/milestone-maintainers/members) GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Milestone Maintainers Team and have them propose you as an additional delegate for this responsibility.

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/126410#issuecomment-2254205680):

>/milestone v1.31


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
Another possibility:

`AllowInsecureKubeletCertificateSigningRequests`, deprecated, off by default
Hello there!
I am new to kubernetes and I want to contribute in it. Is it something that I can contribute to? 🙂 
This issue is not a good issue for a first-time contributor to work on.
I'm a fan of `AllowInsecureKubeletCertificateSigningRequests`, deprecated, off by default
@micahhausler @enj hoping one of you are doing the PR? :)

/milestone v1.31
@liggitt @micahhausler @kubernetes/sig-auth-misc FYI
Example gate names:
- `LegacyNodeCertificateRequestValidation` (off by default)
- `LegacyKubeletCertificateRequestChecks`  (off by default)
- `StrictNodeCSRAdmissionValidation` (on by default)
- `VerifyCommonNameForKubeletServingCertificates` (on by default)
- `VerifyCommonNameForNodeCertificateRequests` (on by default)

from https://github.com/kubernetes/kubernetes/issues/126410#issuecomment-2254208550
- `AllowInsecureKubeletCertificateSigningRequests` (off by default, deprecated)
/milestone v1.31
@sftim: You must be a member of the [kubernetes/milestone-maintainers](https://github.com/orgs/kubernetes/teams/milestone-maintainers/members) GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Milestone Maintainers Team and have them propose you as an additional delegate for this responsibility.

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/126410#issuecomment-2254205680):

>/milestone v1.31


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
Another possibility:

`AllowInsecureKubeletCertificateSigningRequests`, deprecated, off by default
Hello there!
I am new to kubernetes and I want to contribute in it. Is it something that I can contribute to? 🙂 
This issue is not a good issue for a first-time contributor to work on.
I'm a fan of `AllowInsecureKubeletCertificateSigningRequests`, deprecated, off by default
@micahhausler @enj hoping one of you are doing the PR? :)

/milestone v1.31

Fix patch:
diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go
index 65d0260320aa6..993a7485c5b5d 100644
--- a/pkg/features/kube_features.go
+++ b/pkg/features/kube_features.go
@@ -228,9 +228,10 @@ const (
 	// owner: @micahhausler
 	// Deprecated: v1.31
 	//
-	// Disable Node Admission plugin validation of CSRs for kubelet signers where CN=system:node:$nodeName.
+	// Setting AllowInsecureKubeletCertificateSigningRequests to true disables node admission validation of CSRs
+	// for kubelet signers where CN=system:node:$nodeName.
 	// Remove in v1.33
-	DisableKubeletCSRAdmissionValidation featuregate.Feature = "DisableKubeletCSRAdmissionValidation"
+	AllowInsecureKubeletCertificateSigningRequests featuregate.Feature = "AllowInsecureKubeletCertificateSigningRequests"
 
 	// owner: @HirazawaUi
 	// kep: http://kep.k8s.io/4004
@@ -1326,7 +1327,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	// ...
 	HPAScaleToZero: {Default: false, PreRelease: featuregate.Alpha},
 
-	DisableKubeletCSRAdmissionValidation: {Default: false, PreRelease: featuregate.Deprecated}, // remove in 1.33
+	AllowInsecureKubeletCertificateSigningRequests: {Default: false, PreRelease: featuregate.Deprecated}, // remove in 1.33
 
 	StorageNamespaceIndex: {Default: true, PreRelease: featuregate.Beta},
 
diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go
index cb1aacb35d94d..25e4322e66097 100644
--- a/plugin/pkg/admission/noderestriction/admission.go
+++ b/plugin/pkg/admission/noderestriction/admission.go
@@ -74,9 +74,9 @@ type Plugin struct {
 	podsGetter     corev1lister.PodLister
 	nodesGetter    corev1lister.NodeLister
 
-	expansionRecoveryEnabled              bool
-	dynamicResourceAllocationEnabled      bool
-	kubeletCSRAdmissionValidationDisabled bool
+	expansionRecoveryEnabled                       bool
+	dynamicResourceAllocationEnabled               bool
+	allowInsecureKubeletCertificateSigningRequests bool
 }
 
 var (
@@ -89,7 +89,7 @@ var (
 func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
 	p.expansionRecoveryEnabled = featureGates.Enabled(features.RecoverVolumeExpansionFailure)
 	p.dynamicResourceAllocationEnabled = featureGates.Enabled(features.DynamicResourceAllocation)
-	p.kubeletCSRAdmissionValidationDisabled = featureGates.Enabled(features.DisableKubeletCSRAdmissionValidation)
+	p.allowInsecureKubeletCertificateSigningRequests = featureGates.Enabled(features.AllowInsecureKubeletCertificateSigningRequests)
 }
 
 // SetExternalKubeInformerFactory registers an informer factory into Plugin
@@ -176,7 +176,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
 		return p.admitResourceSlice(nodeName, a)
 
 	case csrResource:
-		if p.kubeletCSRAdmissionValidationDisabled {
+		if p.allowInsecureKubeletCertificateSigningRequests {
 			return nil
 		}
 		return p.admitCSR(nodeName, a)


Test patch:
diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go
index 0ddf9f1057915..3b6e107b2af9e 100644
--- a/plugin/pkg/admission/noderestriction/admission_test.go
+++ b/plugin/pkg/admission/noderestriction/admission_test.go
@@ -1278,7 +1278,7 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			features:   feature.DefaultFeatureGate,
 			setupFunc: func(t *testing.T) {
 				t.Helper()
-				featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.DisableKubeletCSRAdmissionValidation, true)
+				featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.AllowInsecureKubeletCertificateSigningRequests, true)
 			},
 		},
 		{