Merge pull request kubernetes#121485 from ritazh/kmsv2-ga

[KMSv2] promote KMSv2 and KMSv2KDF to GA

Author: k8s-ci-robot

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

@@ -122,13 +122,15 @@ const (
 	// kep: https://kep.k8s.io/3299
 	// alpha: v1.25
 	// beta: v1.27
+	// stable: v1.29
 	//
 	// Enables KMS v2 API for encryption at rest.
 	KMSv2 featuregate.Feature = "KMSv2"
 
 	// owner: @enj
 	// kep: https://kep.k8s.io/3299
 	// beta: v1.28
+	// stable: v1.29
 	//
 	// Enables the use of derived encryption keys with KMS v2.
 	KMSv2KDF featuregate.Feature = "KMSv2KDF"
@@ -279,11 +281,11 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	EfficientWatchResumption: {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 
-	KMSv1: {Default: true, PreRelease: featuregate.Deprecated},
+	KMSv1: {Default: false, PreRelease: featuregate.Deprecated},
 
-	KMSv2: {Default: true, PreRelease: featuregate.Beta},
+	KMSv2: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.31
 
-	KMSv2KDF: {Default: true, PreRelease: featuregate.Beta}, // lock to true in 1.29 once KMSv2 is GA, remove in 1.31
+	KMSv2KDF: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.31
 
 	OpenAPIEnums: {Default: true, PreRelease: featuregate.Beta},
 

staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go

@@ -107,6 +107,26 @@ const (
 
 var codecs serializer.CodecFactory
 
+// this atomic bool allows us to swap enablement of the KMSv2KDF feature in tests
+// as the feature gate is now locked to true starting with v1.29
+// Note: it cannot be set by an end user
+var kdfDisabled atomic.Bool
+
+// this function should only be called in tests to swap enablement of the KMSv2KDF feature
+func SetKDFForTests(b bool) func() {
+	kdfDisabled.Store(!b)
+	return func() {
+		kdfDisabled.Store(false)
+	}
+}
+
+// this function should be used to determine enablement of the KMSv2KDF feature
+// instead of getting it from DefaultFeatureGate as the feature gate is now locked
+// to true starting with v1.29
+func GetKDF() bool {
+	return !kdfDisabled.Load()
+}
+
 func init() {
 	configScheme := runtime.NewScheme()
 	utilruntime.Must(apiserverconfig.AddToScheme(configScheme))
@@ -138,6 +158,7 @@ type kmsv2PluginProbe struct {
 	lastResponse *kmsPluginHealthzResponse
 	l            *sync.Mutex
 	apiServerID  string
+	version      string
 }
 
 type kmsHealthChecker []healthz.HealthChecker
@@ -369,7 +390,7 @@ func (h *kmsv2PluginProbe) rotateDEKOnKeyIDChange(ctx context.Context, statusKey
 	// this gate can only change during tests, but the check is cheap enough to always make
 	// this allows us to easily exercise both modes without restarting the API server
 	// TODO integration test that this dynamically takes effect
-	useSeed := utilfeature.DefaultFeatureGate.Enabled(features.KMSv2KDF)
+	useSeed := GetKDF()
 	stateUseSeed := state.EncryptedObject.EncryptedDEKSourceType == kmstypes.EncryptedDEKSourceType_HKDF_SHA256_XNONCE_AES_GCM_SEED
 
 	// state is valid and status keyID is unchanged from when we generated this DEK/seed so there is no need to rotate it
@@ -454,8 +475,16 @@ func (h *kmsv2PluginProbe) isKMSv2ProviderHealthyAndMaybeRotateDEK(ctx context.C
 	if response.Healthz != "ok" {
 		errs = append(errs, fmt.Errorf("got unexpected healthz status: %s", response.Healthz))
 	}
-	if response.Version != envelopekmsv2.KMSAPIVersion {
-		errs = append(errs, fmt.Errorf("expected KMSv2 API version %s, got %s", envelopekmsv2.KMSAPIVersion, response.Version))
+	if response.Version != envelopekmsv2.KMSAPIVersionv2 && response.Version != envelopekmsv2.KMSAPIVersionv2beta1 {
+		errs = append(errs, fmt.Errorf("expected KMSv2 API version %s, got %s", envelopekmsv2.KMSAPIVersionv2, response.Version))
+	} else {
+		// set version for the first status response
+		if len(h.version) == 0 {
+			h.version = response.Version
+		}
+		if h.version != response.Version {
+			errs = append(errs, fmt.Errorf("KMSv2 API version should not change after the initial status response version %s, got %s", h.version, response.Version))
+		}
 	}
 
 	if errCode, err := envelopekmsv2.ValidateKeyID(response.KeyID); err != nil {

staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go

@@ -187,7 +187,7 @@ func TestLegacyConfig(t *testing.T) {
 }
 
 func TestEncryptionProviderConfigCorrect(t *testing.T) {
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv2, true)()
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv1, true)()
 
 	// Set factory for mock envelope service
 	factory := envelopeServiceFactory
@@ -353,42 +353,33 @@ func TestKMSv1Deprecation(t *testing.T) {
 
 func TestKMSvsEnablement(t *testing.T) {
 	testCases := []struct {
-		name         string
-		kmsv2Enabled bool
-		filePath     string
-		expectedErr  string
+		name        string
+		filePath    string
+		expectedErr string
 	}{
 		{
-			name:         "config with kmsv2 and kmsv1, KMSv2=false",
-			kmsv2Enabled: false,
-			filePath:     "testdata/valid-configs/kms/multiple-providers-kmsv2.yaml",
-			expectedErr:  "KMSv2 feature is not enabled",
-		},
-		{
-			name:         "config with kmsv2 and kmsv1, KMSv2=true",
-			kmsv2Enabled: true,
-			filePath:     "testdata/valid-configs/kms/multiple-providers-kmsv2.yaml",
-			expectedErr:  "",
+			name:        "config with kmsv2 and kmsv1, KMSv2=true, KMSv1=false, should fail when feature is disabled",
+			filePath:    "testdata/valid-configs/kms/multiple-providers-mixed.yaml",
+			expectedErr: "KMSv1 is deprecated and will only receive security updates going forward. Use KMSv2 instead",
 		},
 		{
-			name:         "config with kmsv1, KMSv2=false",
-			kmsv2Enabled: false,
-			filePath:     "testdata/valid-configs/kms/multiple-providers.yaml",
-			expectedErr:  "",
+			name:        "config with kmsv2, KMSv2=true, KMSv1=false",
+			filePath:    "testdata/valid-configs/kms/multiple-providers-kmsv2.yaml",
+			expectedErr: "",
 		},
 	}
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			// Just testing KMSv2 feature flag
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv1, true)()
-
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv2, testCase.kmsv2Enabled)()
+			// only the KMSv2 feature flag is enabled
 			_, err := LoadEncryptionConfig(testContext(t), testCase.filePath, false, "")
 
-			if !strings.Contains(errString(err), testCase.expectedErr) {
+			if len(testCase.expectedErr) > 0 && !strings.Contains(errString(err), testCase.expectedErr) {
 				t.Fatalf("expected error %q, got %q", testCase.expectedErr, errString(err))
 			}
+			if len(testCase.expectedErr) == 0 && err != nil {
+				t.Fatalf("unexpected error %q", errString(err))
+			}
 
 		})
 	}
@@ -400,43 +391,6 @@ func TestKMSvsEnablement(t *testing.T) {
 		config          apiserverconfig.EncryptionConfiguration
 		wantV2Used      bool
 	}{
-		{
-			name:         "with kmsv1 and kmsv2, KMSv2=false",
-			kmsv2Enabled: false,
-			config: apiserverconfig.EncryptionConfiguration{
-				Resources: []apiserverconfig.ResourceConfiguration{
-					{
-						Resources: []string{"secrets"},
-						Providers: []apiserverconfig.ProviderConfiguration{
-							{
-								KMS: &apiserverconfig.KMSConfiguration{
-									Name:       "kms",
-									APIVersion: "v1",
-									Timeout: &metav1.Duration{
-										Duration: 1 * time.Second,
-									},
-									Endpoint:  "unix:///tmp/testprovider.sock",
-									CacheSize: pointer.Int32(1000),
-								},
-							},
-							{
-								KMS: &apiserverconfig.KMSConfiguration{
-									Name:       "another-kms",
-									APIVersion: "v2",
-									Timeout: &metav1.Duration{
-										Duration: 1 * time.Second,
-									},
-									Endpoint:  "unix:///tmp/anothertestprovider.sock",
-									CacheSize: pointer.Int32(1000),
-								},
-							},
-						},
-					},
-				},
-			},
-			expectedErr: "KMSv2 feature is not enabled",
-			wantV2Used:  false,
-		},
 		{
 			name:         "with kmsv1 and kmsv2, KMSv2=true",
 			kmsv2Enabled: true,
@@ -501,7 +455,7 @@ func TestKMSvsEnablement(t *testing.T) {
 }
 
 func TestKMSMaxTimeout(t *testing.T) {
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv2, true)()
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv1, true)()
 
 	testCases := []struct {
 		name            string
@@ -749,7 +703,7 @@ func TestKMSMaxTimeout(t *testing.T) {
 }
 
 func TestKMSPluginHealthz(t *testing.T) {
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv2, true)()
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv1, true)()
 
 	kmsv2Probe := &kmsv2PluginProbe{
 		name:        "foo",
@@ -823,7 +777,7 @@ func TestKMSPluginHealthz(t *testing.T) {
 		},
 		{
 			desc:   "Install multiple healthz with v1 and v2",
-			config: "testdata/valid-configs/kms/multiple-providers-kmsv2.yaml",
+			config: "testdata/valid-configs/kms/multiple-providers-mixed.yaml",
 			want: []healthChecker{
 				kmsv2Probe,
 				&kmsPluginProbe{
@@ -900,6 +854,7 @@ func TestKMSPluginHealthz(t *testing.T) {
 
 // tests for masking rules
 func TestWildcardMasking(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv1, true)()
 
 	testCases := []struct {
 		desc          string
@@ -1308,7 +1263,7 @@ func TestWildcardMasking(t *testing.T) {
 }
 
 func TestWildcardStructure(t *testing.T) {
-
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv1, true)()
 	testCases := []struct {
 		desc                         string
 		expectedResourceTransformers map[string]string
@@ -1752,19 +1707,19 @@ func TestIsKMSv2ProviderHealthyError(t *testing.T) {
 			statusResponse: &kmsservice.StatusResponse{
 				Healthz: "unhealthy",
 			},
-			expectedErr: "got unexpected healthz status: unhealthy, expected KMSv2 API version v2beta1, got , got invalid KMSv2 KeyID ",
+			expectedErr: "got unexpected healthz status: unhealthy, expected KMSv2 API version v2, got , got invalid KMSv2 KeyID ",
 			wantMetrics: `
 			# HELP apiserver_envelope_encryption_invalid_key_id_from_status_total [ALPHA] Number of times an invalid keyID is returned by the Status RPC call split by error.
 			# TYPE apiserver_envelope_encryption_invalid_key_id_from_status_total counter
 			apiserver_envelope_encryption_invalid_key_id_from_status_total{error="empty",provider_name="testplugin"} 1
 			`,
 		},
 		{
-			desc: "version is not v2beta1",
+			desc: "version is not v2",
 			statusResponse: &kmsservice.StatusResponse{
 				Version: "v1beta1",
 			},
-			expectedErr: "got unexpected healthz status: , expected KMSv2 API version v2beta1, got v1beta1, got invalid KMSv2 KeyID ",
+			expectedErr: "got unexpected healthz status: , expected KMSv2 API version v2, got v1beta1, got invalid KMSv2 KeyID ",
 			wantMetrics: `
 			# HELP apiserver_envelope_encryption_invalid_key_id_from_status_total [ALPHA] Number of times an invalid keyID is returned by the Status RPC call split by error.
 			# TYPE apiserver_envelope_encryption_invalid_key_id_from_status_total counter
@@ -1788,7 +1743,7 @@ func TestIsKMSv2ProviderHealthyError(t *testing.T) {
 			desc: "invalid long keyID",
 			statusResponse: &kmsservice.StatusResponse{
 				Healthz: "ok",
-				Version: "v2beta1",
+				Version: "v2",
 				KeyID:   sampleInvalidKeyID,
 			},
 			expectedErr: "got invalid KMSv2 KeyID ",
@@ -1816,6 +1771,52 @@ func TestIsKMSv2ProviderHealthyError(t *testing.T) {
 	}
 }
 
+// test to ensure KMSv2 API version is not changed after the first status response
+func TestKMSv2SameVersionFromStatus(t *testing.T) {
+	probe := &kmsv2PluginProbe{name: "testplugin"}
+	service, _ := newMockEnvelopeKMSv2Service(testContext(t), "unix:///tmp/testprovider.sock", "providerName", 3*time.Second)
+	probe.l = &sync.Mutex{}
+	probe.state.Store(&envelopekmsv2.State{})
+	probe.service = service
+
+	testCases := []struct {
+		desc        string
+		expectedErr string
+		newVersion  string
+	}{
+		{
+			desc:        "version changed",
+			newVersion:  "v2",
+			expectedErr: "KMSv2 API version should not change",
+		},
+		{
+			desc:        "version unchanged",
+			newVersion:  "v2beta1",
+			expectedErr: "",
+		},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.desc, func(t *testing.T) {
+			statusResponse := &kmsservice.StatusResponse{
+				Healthz: "ok",
+				Version: "v2beta1",
+				KeyID:   "1",
+			}
+			if err := probe.isKMSv2ProviderHealthyAndMaybeRotateDEK(testContext(t), statusResponse); err != nil {
+				t.Fatal(err)
+			}
+			statusResponse.Version = tt.newVersion
+			err := probe.isKMSv2ProviderHealthyAndMaybeRotateDEK(testContext(t), statusResponse)
+			if len(tt.expectedErr) > 0 && !strings.Contains(errString(err), tt.expectedErr) {
+				t.Errorf("expected err %q, got %q", tt.expectedErr, errString(err))
+			}
+			if len(tt.expectedErr) == 0 && err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+}
+
 func testContext(t *testing.T) context.Context {
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
@@ -1840,7 +1841,7 @@ func TestComputeEncryptionConfigHash(t *testing.T) {
 }
 
 func Test_kmsv2PluginProbe_rotateDEKOnKeyIDChange(t *testing.T) {
-	defaultUseSeed := utilfeature.DefaultFeatureGate.Enabled(features.KMSv2KDF)
+	defaultUseSeed := GetKDF()
 
 	origNowFunc := envelopekmsv2.NowFunc
 	now := origNowFunc() // freeze time
@@ -2065,7 +2066,7 @@ func Test_kmsv2PluginProbe_rotateDEKOnKeyIDChange(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv2KDF, tt.useSeed)()
+			defer SetKDFForTests(tt.useSeed)()
 
 			var buf bytes.Buffer
 			klog.SetOutput(&buf)

staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/valid-configs/kms/multiple-providers-kmsv2.yaml

@@ -10,6 +10,7 @@ resources:
           endpoint: unix:///tmp/testprovider.sock
           timeout:   15s
       - kms:
+          apiVersion: v2
           name: bar
           endpoint: unix:///tmp/testprovider.sock
           timeout:   15s

staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/valid-configs/kms/multiple-providers-mixed.yaml

@@ -0,0 +1,15 @@
+kind: EncryptionConfiguration
+apiVersion: apiserver.config.k8s.io/v1
+resources:
+  - resources:
+      - secrets
+    providers:
+      - kms:
+          apiVersion: v2
+          name: foo
+          endpoint: unix:///tmp/testprovider.sock
+          timeout:   15s
+      - kms:
+          name: bar
+          endpoint: unix:///tmp/testprovider.sock
+          timeout:   15s

staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go

@@ -263,7 +263,7 @@ func TestParseWatchCacheSizes(t *testing.T) {
 }
 
 func TestKMSHealthzEndpoint(t *testing.T) {
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv2, true)()
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.KMSv1, true)()
 
 	testCases := []struct {
 		name                 string

staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go

@@ -52,8 +52,10 @@ func init() {
 }
 
 const (
-	// KMSAPIVersion is the version of the KMS API.
-	KMSAPIVersion = "v2beta1"
+	// KMSAPIVersionv2 is a version of the KMS API.
+	KMSAPIVersionv2 = "v2"
+	// KMSAPIVersionv2beta1 is a version of the KMS API.
+	KMSAPIVersionv2beta1 = "v2beta1"
 	// annotationsMaxSize is the maximum size of the annotations.
 	annotationsMaxSize = 32 * 1024 // 32 kB
 	// KeyIDMaxSize is the maximum size of the keyID.