From 216f89827dffc06e7ffeb083d8c43ac994add4a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 13 Apr 2026 12:40:47 +0300 Subject: [PATCH] Add new KMS foundational mechanism in encryption controller --- .../encryption/controllers/key_controller.go | 6 ++-- .../controllers/key_controller_test.go | 34 +++++++++---------- .../controllers/state_controller_test.go | 8 ++--- .../encryption/encryptionconfig/config.go | 6 ++-- .../encryptionconfig/config_test.go | 2 +- pkg/operator/encryption/secrets/secrets.go | 26 +++++++------- .../encryption/secrets/secrets_test.go | 6 ++-- pkg/operator/encryption/secrets/types.go | 5 +-- pkg/operator/encryption/state/types.go | 4 +-- .../statemachine/transition_test.go | 14 ++++---- pkg/operator/encryption/testing/helpers.go | 14 ++++---- 11 files changed, 63 insertions(+), 62 deletions(-) diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index f5503069e7..d74cf91c7d 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -39,7 +39,7 @@ import ( // greater than the last key's ID (the first key has a key ID of 1). const ( encryptionSecretMigrationInterval = time.Hour * 24 * 7 // one week - defaultKMSEndpoint = "unix:///var/run/kmsplugin/kms-1.sock" + kmsEndpointFormat = "unix:///var/run/kmsplugin/kms-%d.sock" defaultKMSTimeout = 10 * time.Second ) @@ -271,10 +271,10 @@ func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, ExternalReason: externalReason, } if currentMode == state.KMS { - ks.KMSConfiguration = &apiserverv1.KMSConfiguration{ + ks.KMSEncryptionConfig = &apiserverv1.KMSConfiguration{ APIVersion: "v2", Name: fmt.Sprintf("%d", keyID), - Endpoint: defaultKMSEndpoint, + Endpoint: fmt.Sprintf(kmsEndpointFormat, keyID), Timeout: &metav1.Duration{Duration: defaultKMSTimeout}, } } diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index 5baa92d27c..ad4b1279c1 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -351,13 +351,13 @@ func TestKeyController(t *testing.T) { ts.Errorf("expected mode to be KMS, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"]) } - // Verify KMS config annotation exists - kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"] - if kmsConfig == "" { - ts.Error("expected kms-config annotation to be present") + // Verify KMS config is in data field + kmsConfigData := actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"] + if len(kmsConfigData) == 0 { + ts.Error("expected kms-encryption-config data to be present") } - if kmsConfig != `{"apiVersion":"v2","name":"1","endpoint":"unix:///var/run/kmsplugin/kms-1.sock","timeout":"10s"}` { - ts.Errorf("unexpected kms-config: %s", kmsConfig) + if string(kmsConfigData) != `{"apiVersion":"v2","name":"1","endpoint":"unix:///var/run/kmsplugin/kms-1.sock","timeout":"10s"}` { + ts.Errorf("unexpected kms-encryption-config: %s", kmsConfigData) } // Verify internal reason @@ -413,10 +413,10 @@ func TestKeyController(t *testing.T) { ts.Errorf("expected mode to be KMS, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"]) } - // Verify KMS config annotation exists - kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"] - if kmsConfig != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms-1.sock","timeout":"10s"}` { - ts.Errorf("unexpected kms-config: %s", kmsConfig) + // Verify KMS config is in data field + kmsConfigData := actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"] + if string(kmsConfigData) != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms-6.sock","timeout":"10s"}` { + ts.Errorf("unexpected kms-encryption-config: %s", kmsConfigData) } // Verify internal reason is mode changed @@ -488,10 +488,10 @@ func TestKeyController(t *testing.T) { ts.Errorf("expected mode to be KMS, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"]) } - // Verify KMS config annotation exists - kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"] - if kmsConfig != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms-1.sock","timeout":"10s"}` { - ts.Errorf("unexpected kms-config: %s", kmsConfig) + // Verify KMS config is in data field + kmsConfigData := actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"] + if string(kmsConfigData) != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms-6.sock","timeout":"10s"}` { + ts.Errorf("unexpected kms-encryption-config: %s", kmsConfigData) } // Verify internal reason is mode changed @@ -538,9 +538,9 @@ func TestKeyController(t *testing.T) { ts.Errorf("expected mode to be aescbc, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"]) } - // Verify KMS config annotation is removed (not present for AESCBC) - if kmsConfig, exists := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]; exists { - ts.Errorf("expected kms-config annotation to be absent, got: %s", kmsConfig) + // Verify KMS config data is not present for AESCBC + if kmsConfigData, exists := actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"]; exists { + ts.Errorf("expected kms-encryption-config data to be absent, got: %s", kmsConfigData) } // Verify internal reason is mode changed diff --git a/pkg/operator/encryption/controllers/state_controller_test.go b/pkg/operator/encryption/controllers/state_controller_test.go index e9a804954d..29d1f8b205 100644 --- a/pkg/operator/encryption/controllers/state_controller_test.go +++ b/pkg/operator/encryption/controllers/state_controller_test.go @@ -919,7 +919,7 @@ func TestStateController(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -945,7 +945,7 @@ func TestStateController(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -969,7 +969,7 @@ func TestStateController(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -1072,7 +1072,7 @@ func TestStateController(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { diff --git a/pkg/operator/encryption/encryptionconfig/config.go b/pkg/operator/encryption/encryptionconfig/config.go index ccface2ef0..1b63026bc5 100644 --- a/pkg/operator/encryption/encryptionconfig/config.go +++ b/pkg/operator/encryption/encryptionconfig/config.go @@ -209,12 +209,12 @@ func stateToProviders(resource string, desired state.GroupResourceState) []apise }, }) case state.KMS: - if key.KMSConfiguration == nil { - klog.Infof("skipping key %s for %s in KMS mode as its KMSConfiguration is nil", key.Key.Name, resource) + if key.KMSEncryptionConfig == nil { + klog.Infof("skipping key %s for %s in KMS mode as its KMSEncryptionConfig is nil", key.Key.Name, resource) continue // this should never happen } // In order to preserve the uniqueness, we should insert resource name - kmsCopy := key.KMSConfiguration.DeepCopy() + kmsCopy := key.KMSEncryptionConfig.DeepCopy() kmsCopy.Name = createKMSProviderName(key.Key.Name, resource) provider := apiserverconfigv1.ProviderConfiguration{ KMS: kmsCopy, diff --git a/pkg/operator/encryption/encryptionconfig/config_test.go b/pkg/operator/encryption/encryptionconfig/config_test.go index e59852f681..dcdd3fb60f 100644 --- a/pkg/operator/encryption/encryptionconfig/config_test.go +++ b/pkg/operator/encryption/encryptionconfig/config_test.go @@ -687,7 +687,7 @@ func keyToKMSConfiguration(key *corev1.Secret, resource string) *apiserverconfig return &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: fmt.Sprintf("%d_%s", keyID, resource), - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: fmt.Sprintf("unix:///var/run/kmsplugin/kms-%d.sock", keyID), Timeout: &metav1.Duration{ Duration: 10 * time.Second, }, diff --git a/pkg/operator/encryption/secrets/secrets.go b/pkg/operator/encryption/secrets/secrets.go index 54aec75f2e..eccb06b32e 100644 --- a/pkg/operator/encryption/secrets/secrets.go +++ b/pkg/operator/encryption/secrets/secrets.go @@ -58,21 +58,21 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) { key.ExternalReason = v } - if v, ok := s.Annotations[EncryptionSecretKMSConfig]; ok && len(v) > 0 { - kmsConfiguration := &apiserverconfigv1.KMSConfiguration{} - if err := json.Unmarshal([]byte(v), kmsConfiguration); err != nil { - return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid %s annotation: %v", s.Namespace, s.Name, EncryptionSecretKMSConfig, err) - } - key.KMSConfiguration = kmsConfiguration - } - keyMode := state.Mode(s.Annotations[encryptionSecretMode]) switch keyMode { case state.AESCBC, state.AESGCM, state.SecretBox, state.Identity: key.Mode = keyMode case state.KMS: - if key.KMSConfiguration == nil { - return state.KeyState{}, fmt.Errorf("KMSConfiguration can not be nil, when mode is KMS") + if v, ok := s.Data[EncryptionSecretKMSEncryptionConfig]; ok && len(v) > 0 { + kmsConfiguration := &apiserverconfigv1.KMSConfiguration{} + if err := json.Unmarshal(v, kmsConfiguration); err != nil { + return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid %s data: %w", s.Namespace, s.Name, EncryptionSecretKMSEncryptionConfig, err) + } + key.KMSEncryptionConfig = kmsConfiguration + } else { + // encryption.apiserver.operator.openshift.io-kms-encryption-config data field is required for KMS + // encryption mode. + return state.KeyState{}, fmt.Errorf("%s can not be empty, when mode is KMS", EncryptionSecretKMSEncryptionConfig) } key.Mode = keyMode default: @@ -126,12 +126,12 @@ func FromKeyState(component string, ks state.KeyState) (*corev1.Secret, error) { s.Annotations[EncryptionSecretMigratedResources] = string(bs) } - if ks.KMSConfiguration != nil { - ksJSON, err := json.Marshal(ks.KMSConfiguration) + if ks.KMSEncryptionConfig != nil { + kmsEncCfgJSON, err := json.Marshal(ks.KMSEncryptionConfig) if err != nil { return nil, err } - s.Annotations[EncryptionSecretKMSConfig] = string(ksJSON) + s.Data[EncryptionSecretKMSEncryptionConfig] = kmsEncCfgJSON } return s, nil diff --git a/pkg/operator/encryption/secrets/secrets_test.go b/pkg/operator/encryption/secrets/secrets_test.go index 45bb4270ab..9574904390 100644 --- a/pkg/operator/encryption/secrets/secrets_test.go +++ b/pkg/operator/encryption/secrets/secrets_test.go @@ -123,7 +123,7 @@ func TestRoundtrip(t *testing.T) { }, Backed: true, Mode: "KMS", - KMSConfiguration: &v1.KMSConfiguration{ + KMSEncryptionConfig: &v1.KMSConfiguration{ APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", @@ -150,10 +150,10 @@ func TestRoundtrip(t *testing.T) { }, Backed: true, Mode: "KMS", - KMSConfiguration: &v1.KMSConfiguration{ + KMSEncryptionConfig: &v1.KMSConfiguration{ APIVersion: "v2", Name: "2", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", }, }, }, diff --git a/pkg/operator/encryption/secrets/types.go b/pkg/operator/encryption/secrets/types.go index 443c7975e1..ed7ba5e8ba 100644 --- a/pkg/operator/encryption/secrets/types.go +++ b/pkg/operator/encryption/secrets/types.go @@ -50,8 +50,9 @@ const ( // deletion of secrets by enforcing a two phase delete. EncryptionSecretFinalizer = "encryption.apiserver.operator.openshift.io/deletion-protection" - // EncryptionSecretKMSConfig is the annotation that stores the encoded KMS configuration. - EncryptionSecretKMSConfig = "encryption.apiserver.operator.openshift.io/kms-config" + // EncryptionSecretKMSEncryptionConfig is the data field key that stores the serialized KMS + // encryption configuration for KMS mode in the encryption-key secret. + EncryptionSecretKMSEncryptionConfig = "encryption.apiserver.operator.openshift.io-kms-encryption-config" ) // MigratedGroupResources is the data structured stored in the diff --git a/pkg/operator/encryption/state/types.go b/pkg/operator/encryption/state/types.go index 61313d745e..d8b6fe605b 100644 --- a/pkg/operator/encryption/state/types.go +++ b/pkg/operator/encryption/state/types.go @@ -40,8 +40,8 @@ type KeyState struct { InternalReason string // the user via unsupportConfigOverrides.encryption.reason triggered this key. ExternalReason string - // Encoded KMSConfiguration that stores the KMS related fields - KMSConfiguration *apiserverconfigv1.KMSConfiguration + // Encoded KMSEncryptionConfig that stores the KMS related fields + KMSEncryptionConfig *apiserverconfigv1.KMSConfiguration } type MigrationState struct { diff --git a/pkg/operator/encryption/statemachine/transition_test.go b/pkg/operator/encryption/statemachine/transition_test.go index 344941a829..8a12957cf6 100644 --- a/pkg/operator/encryption/statemachine/transition_test.go +++ b/pkg/operator/encryption/statemachine/transition_test.go @@ -1052,7 +1052,7 @@ func TestGetDesiredEncryptionState(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -1079,7 +1079,7 @@ func TestGetDesiredEncryptionState(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -1102,7 +1102,7 @@ func TestGetDesiredEncryptionState(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -1129,7 +1129,7 @@ func TestGetDesiredEncryptionState(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -1158,7 +1158,7 @@ func TestGetDesiredEncryptionState(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -1184,7 +1184,7 @@ func TestGetDesiredEncryptionState(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { @@ -1213,7 +1213,7 @@ func TestGetDesiredEncryptionState(t *testing.T) { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: "2_secrets", - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, { diff --git a/pkg/operator/encryption/testing/helpers.go b/pkg/operator/encryption/testing/helpers.go index 43e344e7e9..d7e171fa98 100644 --- a/pkg/operator/encryption/testing/helpers.go +++ b/pkg/operator/encryption/testing/helpers.go @@ -21,10 +21,10 @@ import ( ) const ( - encryptionSecretKeyDataForTest = "encryption.apiserver.operator.openshift.io-key" - encryptionSecretMigratedTimestampForTest = "encryption.apiserver.operator.openshift.io/migrated-timestamp" - encryptionSecretMigratedResourcesForTest = "encryption.apiserver.operator.openshift.io/migrated-resources" - encryptionSecretKMSConfigForTest = "encryption.apiserver.operator.openshift.io/kms-config" + encryptionSecretKeyDataForTest = "encryption.apiserver.operator.openshift.io-key" + encryptionSecretMigratedTimestampForTest = "encryption.apiserver.operator.openshift.io/migrated-timestamp" + encryptionSecretMigratedResourcesForTest = "encryption.apiserver.operator.openshift.io/migrated-resources" + encryptionSecretKMSEncryptionConfigForTest = "encryption.apiserver.operator.openshift.io-kms-encryption-config" ) func CreateEncryptionKeySecretNoData(targetNS string, grs []schema.GroupResource, keyID uint64) *corev1.Secret { @@ -101,11 +101,11 @@ func CreateEncryptionKeySecretWithKMSConfig(targetNS string, grs []schema.GroupR kmsConfig := &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: fmt.Sprintf("%d", keyID), - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: fmt.Sprintf("unix:///var/run/kmsplugin/kms-%d.sock", keyID), Timeout: &metav1.Duration{Duration: 10 * time.Second}, } kmsConfigJSON, _ := json.Marshal(kmsConfig) - secret.Annotations[encryptionSecretKMSConfigForTest] = string(kmsConfigJSON) + secret.Data[encryptionSecretKMSEncryptionConfigForTest] = kmsConfigJSON return secret } @@ -273,7 +273,7 @@ func createProviderCfg(mode string, resource string, key apiserverconfigv1.Key) KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", Name: fmt.Sprintf("%s_%s", key.Name, resource), - Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Endpoint: fmt.Sprintf("unix:///var/run/kmsplugin/kms-%s.sock", key.Name), Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, }