Merge pull request kubernetes#123135 from munnerz/4193-beta-promotion

KEP-4193: promote ServiceAccountTokenJTI, ServiceAccountTokenPodNodeInfo and ServiceAccountTokenNodeBindingValidation to beta
openshift · Mar 2, 2024 · 8845c4c · 8845c4c
2 parents 3e1da21 + 105ec3d
commit 8845c4c
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 11 deletions.
diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go
@@ -690,6 +690,7 @@ const (
 	// owner: @munnerz
 	// kep: http://kep.k8s.io/4193
 	// alpha: v1.29
+	// beta: v1.30
 	//
 	// Controls whether JTIs (UUIDs) are embedded into generated service account tokens, and whether these JTIs are
 	// recorded into the audit log for future requests made by these tokens.
@@ -705,13 +706,15 @@ const (
 	// owner: @munnerz
 	// kep: http://kep.k8s.io/4193
 	// alpha: v1.29
+	// beta: v1.30
 	//
 	// Controls whether the apiserver will validate Node claims in service account tokens.
 	ServiceAccountTokenNodeBindingValidation featuregate.Feature = "ServiceAccountTokenNodeBindingValidation"
 
 	// owner: @munnerz
 	// kep: http://kep.k8s.io/4193
 	// alpha: v1.29
+	// beta: v1.30
 	//
 	// Controls whether the apiserver embeds the node name and uid for the associated node when issuing
 	// service account tokens bound to Pod objects.
@@ -1114,13 +1117,13 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	SeparateTaintEvictionController: {Default: true, PreRelease: featuregate.Beta},
 
-	ServiceAccountTokenJTI: {Default: false, PreRelease: featuregate.Alpha},
+	ServiceAccountTokenJTI: {Default: true, PreRelease: featuregate.Beta},
 
-	ServiceAccountTokenPodNodeInfo: {Default: false, PreRelease: featuregate.Alpha},
+	ServiceAccountTokenPodNodeInfo: {Default: true, PreRelease: featuregate.Beta},
 
 	ServiceAccountTokenNodeBinding: {Default: false, PreRelease: featuregate.Alpha},
 
-	ServiceAccountTokenNodeBindingValidation: {Default: false, PreRelease: featuregate.Alpha},
+	ServiceAccountTokenNodeBindingValidation: {Default: true, PreRelease: featuregate.Beta},
 
 	ServiceNodePortStaticSubrange: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // GA in 1.29; remove in 1.31
 

diff --git a/pkg/kubeapiserver/options/authentication.go b/pkg/kubeapiserver/options/authentication.go
@@ -261,6 +261,11 @@ func (o *BuiltInAuthenticationOptions) Validate() []error {
 		}
 	}
 
+	// verify that if ServiceAccountTokenNodeBinding is enabled, ServiceAccountTokenNodeBindingValidation is also enabled.
+	if utilfeature.DefaultFeatureGate.Enabled(features.ServiceAccountTokenNodeBinding) && !utilfeature.DefaultFeatureGate.Enabled(features.ServiceAccountTokenNodeBindingValidation) {
+		allErrors = append(allErrors, fmt.Errorf("the %q feature gate can only be enabled if the %q feature gate is also enabled", features.ServiceAccountTokenNodeBinding, features.ServiceAccountTokenNodeBindingValidation))
+	}
+
 	if o.WebHook != nil {
 		retryBackoff := o.WebHook.RetryBackoff
 		if retryBackoff != nil && retryBackoff.Steps <= 0 {

diff --git a/pkg/kubeapiserver/options/authentication_test.go b/pkg/kubeapiserver/options/authentication_test.go
@@ -36,19 +36,22 @@ import (
 	"k8s.io/apiserver/pkg/features"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/component-base/featuregate"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
+	kubefeatures "k8s.io/kubernetes/pkg/features"
 	kubeauthenticator "k8s.io/kubernetes/pkg/kubeapiserver/authenticator"
 	"k8s.io/utils/pointer"
 )
 
 func TestAuthenticationValidate(t *testing.T) {
 	testCases := []struct {
-		name                         string
-		testOIDC                     *OIDCAuthenticationOptions
-		testSA                       *ServiceAccountAuthenticationOptions
-		testWebHook                  *WebHookAuthenticationOptions
-		testAuthenticationConfigFile string
-		expectErr                    string
+		name                              string
+		testOIDC                          *OIDCAuthenticationOptions
+		testSA                            *ServiceAccountAuthenticationOptions
+		testWebHook                       *WebHookAuthenticationOptions
+		testAuthenticationConfigFile      string
+		expectErr                         string
+		enabledFeatures, disabledFeatures []featuregate.Feature
 	}{
 		{
 			name: "test when OIDC and ServiceAccounts are nil",
@@ -227,6 +230,12 @@ func TestAuthenticationValidate(t *testing.T) {
 			},
 			expectErr: "authentication-config file and oidc-* flags are mutually exclusive",
 		},
+		{
+			name:             "fails to validate if ServiceAccountTokenNodeBindingValidation is disabled and ServiceAccountTokenNodeBinding is enabled",
+			enabledFeatures:  []featuregate.Feature{kubefeatures.ServiceAccountTokenNodeBinding},
+			disabledFeatures: []featuregate.Feature{kubefeatures.ServiceAccountTokenNodeBindingValidation},
+			expectErr:        "the \"ServiceAccountTokenNodeBinding\" feature gate can only be enabled if the \"ServiceAccountTokenNodeBindingValidation\" feature gate is also enabled",
+		},
 	}
 
 	for _, testcase := range testCases {
@@ -236,7 +245,12 @@ func TestAuthenticationValidate(t *testing.T) {
 			options.ServiceAccounts = testcase.testSA
 			options.WebHook = testcase.testWebHook
 			options.AuthenticationConfigFile = testcase.testAuthenticationConfigFile
-
+			for _, f := range testcase.enabledFeatures {
+				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, f, true)()
+			}
+			for _, f := range testcase.disabledFeatures {
+				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, f, false)()
+			}
 			errs := options.Validate()
 			if len(errs) > 0 && (!strings.Contains(utilerrors.NewAggregate(errs).Error(), testcase.expectErr) || testcase.expectErr == "") {
 				t.Errorf("Got err: %v, Expected err: %s", errs, testcase.expectErr)

diff --git a/test/integration/auth/svcaccttoken_test.go b/test/integration/auth/svcaccttoken_test.go
@@ -235,8 +235,12 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		checkPayload(t, treq.Status.Token, `"test-svcacct"`, "kubernetes.io", "serviceaccount", "name")
 
 		info := doTokenReview(t, cs, treq, false)
+		// we are not testing the credential-id feature, so delete this value from the returned extra info map
 		if info.Extra != nil {
-			t.Fatalf("expected Extra to be nil but got: %#v", info.Extra)
+			delete(info.Extra, apiserverserviceaccount.CredentialIDKey)
+		}
+		if len(info.Extra) > 0 {
+			t.Fatalf("expected Extra to be empty but got: %#v", info.Extra)
 		}
 		delSvcAcct()
 		doTokenReview(t, cs, treq, true)
@@ -304,6 +308,8 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		checkPayload(t, treq.Status.Token, "null", "kubernetes.io", "node")
 
 		info := doTokenReview(t, cs, treq, false)
+		// we are not testing the credential-id feature, so delete this value from the returned extra info map
+		delete(info.Extra, apiserverserviceaccount.CredentialIDKey)
 		if len(info.Extra) != 2 {
 			t.Fatalf("expected Extra have length of 2 but was length %d: %#v", len(info.Extra), info.Extra)
 		}
@@ -398,6 +404,8 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 			}
 
 			info := doTokenReview(t, cs, treq, false)
+			// we are not testing the credential-id feature, so delete this value from the returned extra info map
+			delete(info.Extra, apiserverserviceaccount.CredentialIDKey)
 			if len(info.Extra) != len(expectedExtraValues) {
 				t.Fatalf("expected Extra have length of %d but was length %d: %#v", len(expectedExtraValues), len(info.Extra), info.Extra)
 			}