From f6cec17fe5550ba81149587edda977c8ef0b7e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 29 Jul 2026 13:50:00 +0300 Subject: [PATCH] Add in-place field update for latest KMS mode --- .../encryption/controllers/key_controller.go | 206 ++++++++++++++---- .../controllers/key_controller_test.go | 70 +++++- .../controllers/state_controller.go | 8 +- test/e2e-encryption/encryption_test.go | 64 +++++- 4 files changed, 285 insertions(+), 63 deletions(-) diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index 27220d2c7f..d16bbdf94e 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -11,6 +11,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -181,17 +182,13 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact return err } - currentConfig, desiredEncryptionState, secrets, isProgressingReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) + currentConfig, desiredEncryptionState, encryptionSecrets, isProgressingReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) if err != nil { return err } - if len(isProgressingReason) > 0 { - syncContext.Queue().AddAfter(syncContext.QueueKey(), 2*time.Minute) - return nil - } // avoid intended start of encryption - hasBeenOnBefore := currentConfig != nil || len(secrets) > 0 + hasBeenOnBefore := currentConfig != nil || len(encryptionSecrets) > 0 if currentMode == state.Identity && !hasBeenOnBefore { return nil } @@ -215,12 +212,18 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact } } - var commonReason *string + var ( + commonReason *string + observedLatestKeyID uint64 + ) for gr, grKeys := range desiredEncryptionState { latestKeyID, internalReason, needed, err := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs, desiredProviderCfg) if err != nil { return err } + if latestKeyID > observedLatestKeyID { + observedLatestKeyID = latestKeyID + } if !needed { continue } @@ -239,6 +242,14 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact reasons = append(reasons, fmt.Sprintf("%s-%s", gr.Resource, internalReason)) } if !newKeyRequired { + if state.Mode(apiEncryptionConfiguration.Type) == state.KMS && observedLatestKeyID > 0 { + return c.updateInPlaceFieldsIfChanged(ctx, syncContext, apiEncryptionConfiguration, observedLatestKeyID, encryptionSecrets) + } + return nil + } + // We must not create a new key if there is active progress + if len(isProgressingReason) > 0 { + syncContext.Queue().AddAfter(syncContext.QueueKey(), 2*time.Minute) return nil } if commonReason != nil && len(*commonReason) > 0 && len(reasons) > 1 { @@ -306,45 +317,147 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur Plugin: apiServerEncryption.KMS, } - if secretName, expectedKeys, err := desiredProviderCfg.referencedSecretName(); err != nil { + secretData, err := c.fetchReferencedSecretData(ctx, desiredProviderCfg) + if err != nil { return nil, err - } else if len(secretName) > 0 { - refSecret, err := c.secretClient.Secrets(openshiftConfigNS).Get(ctx, secretName, metav1.GetOptions{}) - if err != nil { - return nil, fmt.Errorf("failed to get secret %s in %s: %w", secretName, openshiftConfigNS, err) - } - for _, key := range expectedKeys { - v, ok := refSecret.Data[key] - if !ok { - return nil, fmt.Errorf("secret %s in %s is missing required key %q", secretName, openshiftConfigNS, key) - } - if err := ks.KMS.PluginSecretData.Set(secretName, key, v); err != nil { - return nil, err - } - } - } - - if cmName, expectedKeys, err := desiredProviderCfg.referencedConfigMapName(); err != nil { + } + ks.KMS.PluginSecretData = secretData + + configMapData, err := c.fetchReferencedConfigMapData(ctx, desiredProviderCfg) + if err != nil { return nil, err - } else if len(cmName) > 0 { - refCM, err := c.configMapClient.ConfigMaps(openshiftConfigNS).Get(ctx, cmName, metav1.GetOptions{}) - if err != nil { - return nil, fmt.Errorf("failed to get configmap %s in %s: %w", cmName, openshiftConfigNS, err) - } - for _, key := range expectedKeys { - v, ok := refCM.Data[key] - if !ok { - return nil, fmt.Errorf("configmap %s in %s is missing required key %q", cmName, openshiftConfigNS, key) - } - if err := ks.KMS.PluginConfigMapData.Set(cmName, key, []byte(v)); err != nil { - return nil, err - } - } } + ks.KMS.PluginConfigMapData = configMapData } return secrets.FromKeyState(c.instanceName, ks) } +func (c *keyController) fetchReferencedSecretData(ctx context.Context, providerCfg kmsProviderConfig) (state.KMSReferenceData, error) { + var secretData state.KMSReferenceData + secretName, expectedKeys, err := providerCfg.referencedSecretName() + if err != nil { + return secretData, err + } + if len(secretName) == 0 { + return secretData, nil + } + refSecret, err := c.secretClient.Secrets(openshiftConfigNS).Get(ctx, secretName, metav1.GetOptions{}) + if err != nil { + return secretData, fmt.Errorf("failed to get secret %s in %s: %w", secretName, openshiftConfigNS, err) + } + for _, key := range expectedKeys { + v, ok := refSecret.Data[key] + if !ok { + return secretData, fmt.Errorf("secret %s in %s is missing required key %q", secretName, openshiftConfigNS, key) + } + if err := secretData.Set(secretName, key, v); err != nil { + return secretData, err + } + } + return secretData, nil +} + +func (c *keyController) fetchReferencedConfigMapData(ctx context.Context, providerCfg kmsProviderConfig) (state.KMSReferenceData, error) { + var configMapData state.KMSReferenceData + cmName, expectedKeys, err := providerCfg.referencedConfigMapName() + if err != nil { + return configMapData, err + } + if len(cmName) == 0 { + return configMapData, nil + } + refCM, err := c.configMapClient.ConfigMaps(openshiftConfigNS).Get(ctx, cmName, metav1.GetOptions{}) + if err != nil { + return configMapData, fmt.Errorf("failed to get configmap %s in %s: %w", cmName, openshiftConfigNS, err) + } + for _, key := range expectedKeys { + v, ok := refCM.Data[key] + if !ok { + return configMapData, fmt.Errorf("configmap %s in %s is missing required key %q", cmName, openshiftConfigNS, key) + } + if err := configMapData.Set(cmName, key, []byte(v)); err != nil { + return configMapData, err + } + } + return configMapData, nil +} + +// updateInPlaceFieldsIfChanged updates the latest KMS key secret's plugin config, +// credential data, and configmap data when non-migration fields have changed. +func (c *keyController) updateInPlaceFieldsIfChanged(ctx context.Context, syncContext factory.SyncContext, apiServerEncryption configv1.APIServerEncryption, latestKeyID uint64, keySecrets []*corev1.Secret) error { + for _, rawSecret := range keySecrets { + ks, err := secrets.ToKeyState(rawSecret) + if err != nil { + klog.Warningf("skipping invalid key secret %s: %v", rawSecret.Name, err) + continue + } + if ks.Mode != state.KMS || !ks.HasKMSPlugin() { + continue + } + + keyID, ok := state.NameToKeyID(ks.Key.Name) + if !ok { + klog.Warningf("skipping key secret %s with unparseable key name %q", rawSecret.Name, ks.Key.Name) + continue + } + if keyID != latestKeyID { + continue + } + + storedProviderCfg, err := newKMSProviderConfig(ks.KMS.Plugin) + if err != nil { + return err + } + + currentSecretData, err := c.fetchReferencedSecretData(ctx, storedProviderCfg) + if err != nil { + return err + } + currentConfigMapData, err := c.fetchReferencedConfigMapData(ctx, storedProviderCfg) + if err != nil { + return err + } + + pluginChanged := !equality.Semantic.DeepEqual(apiServerEncryption.KMS, ks.KMS.Plugin) + secretDataChanged := !equality.Semantic.DeepEqual(currentSecretData.FlatEntries(), ks.KMS.PluginSecretData.FlatEntries()) + configMapDataChanged := !equality.Semantic.DeepEqual(currentConfigMapData.FlatEntries(), ks.KMS.PluginConfigMapData.FlatEntries()) + + if !pluginChanged && !secretDataChanged && !configMapDataChanged { + return nil + } + + if pluginChanged { + ks.KMS.Plugin = apiServerEncryption.KMS + } + if secretDataChanged { + ks.KMS.PluginSecretData = currentSecretData + } + if configMapDataChanged { + ks.KMS.PluginConfigMapData = currentConfigMapData + } + + desired, err := secrets.FromKeyState(c.instanceName, ks) + if err != nil { + return fmt.Errorf("failed to build desired key secret for %s: %v", rawSecret.Name, err) + } + + rawSecret.Data = desired.Data + + _, updateErr := c.secretClient.Secrets(rawSecret.Namespace).Update(ctx, rawSecret, metav1.UpdateOptions{}) + if errors.IsConflict(updateErr) { + klog.V(4).Infof("conflict updating key secret %s, will retry on next sync", rawSecret.Name) + return nil + } + if updateErr != nil { + return updateErr + } + syncContext.Recorder().Eventf("EncryptionKeyKMSConfigUpdated", "Updated KMS config on key secret %q in-place", rawSecret.Name) + return nil + } + + return nil +} + func (c *keyController) getCurrentModeReasonAndEncryptionConfig(ctx context.Context) (state.Mode, string, configv1.APIServerEncryption, error) { apiServer, err := c.apiServerClient.Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { @@ -375,8 +488,15 @@ func (c *keyController) getCurrentModeReasonAndEncryptionConfig(ctx context.Cont } } -// needsNewKey checks whether a new key must be created for the given resource. If true, it also returns the latest -// used key ID and a reason string. +// needsNewKey checks whether a new key must be created for the given resource. +// It returns: +// - latestKeyID: the parsed key ID of the most recent key when the key name +// is parseable and the key is backed by a secret. Zero when no usable +// key exists (no keys, invalid key name, too many backed keys, identity +// mode, or errors). +// - reason: a human-readable string describing why a new key is needed. +// - needed: whether a new key should be created. +// - err: non-nil on unrecoverable errors. func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, externalReason string, encryptedGRs []schema.GroupResource, desiredProviderCfg kmsProviderConfig) (uint64, string, bool, error) { // we always need to have some encryption keys unless we are turned off if len(grKeys.ReadKeys) == 0 { @@ -407,7 +527,7 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern // we have not migrated the latest key, do nothing until that is complete if allMigrated, _, _ := state.MigratedFor(encryptedGRs, latestKey); !allMigrated { - return 0, "", false, nil + return latestKeyID, "", false, nil } // if the most recent secret was encrypted in a mode different than the current mode, we need to generate a new key @@ -438,7 +558,7 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern // For KMS mode, we don't do time-based rotation. KMS keys are rotated // externally by the KMS provider. Moreover, we don't trigger new key when external reason is changed. // Because it would lead to duplicate providers which is not allowed. - return 0, "", false, nil + return latestKeyID, "", false, nil } // if the most recent secret has a different external reason than the current reason, we need to generate a new key diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index 76e911c2be..1bdcb6a157 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -421,10 +421,12 @@ func TestKeyController(t *testing.T) { initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), + encryptiontesting.CreateVaultCABundleConfigMap("vault-ca-bundle", "test-ca-cert"), }, apiServerObjects: []runtime.Object{apiServerWithKMS}, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "update:secrets:openshift-config-managed", "create:events:kms"}, }, { @@ -502,11 +504,13 @@ func TestKeyController(t *testing.T) { encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateExpiredMigratedEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5), encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 6), + encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), + encryptiontesting.CreateVaultCABundleConfigMap("vault-ca-bundle", "test-ca-cert"), }, apiServerObjects: []runtime.Object{apiServerWithKMS}, targetNamespace: "kms", // Should be no-op because KMS keys don't have time-based rotation - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "update:secrets:openshift-config-managed", "create:events:kms"}, }, { name: "no-op when latest KMS key is not migrated yet", @@ -516,11 +520,13 @@ func TestKeyController(t *testing.T) { initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 3), + encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), + encryptiontesting.CreateVaultCABundleConfigMap("vault-ca-bundle", "test-ca-cert"), }, apiServerObjects: []runtime.Object{apiServerWithKMS}, targetNamespace: "kms", - // Should be no-op because migration hasn't completed yet - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + // Migration hasn't completed but in-place updates still run + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "update:secrets:openshift-config-managed", "create:events:kms"}, }, { @@ -771,13 +777,15 @@ func TestKeyController(t *testing.T) { }, { - name: "no-op when only KMSPluginImage changes (non-migration field)", + name: "in-place update when only KMSPluginImage changes (non-migration field)", targetGRs: []schema.GroupResource{ {Group: "", Resource: "secrets"}, }, initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateMigratedEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, time.Now()), + encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), + encryptiontesting.CreateVaultCABundleConfigMap("vault-ca-bundle", "test-ca-cert"), }, apiServerObjects: []runtime.Object{func() runtime.Object { s := simpleAPIServer.DeepCopy() @@ -787,17 +795,19 @@ func TestKeyController(t *testing.T) { return s }()}, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "update:secrets:openshift-config-managed", "create:events:kms"}, }, { - name: "no-op when only Authentication changes (non-migration field)", + name: "in-place update when only Authentication changes (non-migration field)", targetGRs: []schema.GroupResource{ {Group: "", Resource: "secrets"}, }, initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateMigratedEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, time.Now()), + encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), + encryptiontesting.CreateVaultCABundleConfigMap("vault-ca-bundle", "test-ca-cert"), }, apiServerObjects: []runtime.Object{func() runtime.Object { s := simpleAPIServer.DeepCopy() @@ -807,17 +817,19 @@ func TestKeyController(t *testing.T) { return s }()}, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "update:secrets:openshift-config-managed", "create:events:kms"}, }, { - name: "no-op when only TLS changes (non-migration field)", + name: "in-place update when only TLS changes (non-migration field)", targetGRs: []schema.GroupResource{ {Group: "", Resource: "secrets"}, }, initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateMigratedEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, time.Now()), + encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), + encryptiontesting.CreateVaultCABundleConfigMap("vault-ca-bundle", "test-ca-cert"), }, apiServerObjects: []runtime.Object{func() runtime.Object { s := simpleAPIServer.DeepCopy() @@ -829,7 +841,45 @@ func TestKeyController(t *testing.T) { return s }()}, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "update:secrets:openshift-config-managed", "create:events:kms"}, + }, + + { + name: "in-place credential update propagates rotated AppRole secret data to key secret", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "rotated-role-id", "rotated-secret-id"), + encryptiontesting.CreateVaultCABundleConfigMap("vault-ca-bundle", "test-ca-cert"), + }, + apiServerObjects: []runtime.Object{apiServerWithKMS}, + targetNamespace: "kms", + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "update:secrets:openshift-config-managed", "create:events:kms"}, + }, + + { + name: "no update when KMS key secrets already have matching credentials and config", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1) + s.Data["encryption.apiserver.operator.openshift.io-kms-plugin-secret-vault-approle-secret_role-id"] = []byte("test-role-id") + s.Data["encryption.apiserver.operator.openshift.io-kms-plugin-secret-vault-approle-secret_secret-id"] = []byte("test-secret-id") + s.Data["encryption.apiserver.operator.openshift.io-kms-plugin-configmap-vault-ca-bundle_ca-bundle.crt"] = []byte("test-ca-cert") + return s + }(), + encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), + encryptiontesting.CreateVaultCABundleConfigMap("vault-ca-bundle", "test-ca-cert"), + }, + apiServerObjects: []runtime.Object{apiServerWithKMS}, + targetNamespace: "kms", + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config"}, }, } diff --git a/pkg/operator/encryption/controllers/state_controller.go b/pkg/operator/encryption/controllers/state_controller.go index 85b224e19c..57118f7a42 100644 --- a/pkg/operator/encryption/controllers/state_controller.go +++ b/pkg/operator/encryption/controllers/state_controller.go @@ -23,8 +23,6 @@ import ( operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers" ) -const stateWorkKey = "key" - // stateController is responsible for creating a single secret in // openshift-config-managed with the name destName. This single secret // contains the complete EncryptionConfiguration that is consumed by the API @@ -127,14 +125,10 @@ type eventWithReason struct { } func (c *stateController) generateAndApplyCurrentEncryptionConfigSecret(ctx context.Context, queue workqueue.RateLimitingInterface, recorder events.Recorder, encryptedGRs []schema.GroupResource) error { - currentConfig, desiredEncryptionState, encryptionSecrets, transitioningReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) + currentConfig, desiredEncryptionState, encryptionSecrets, _, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) if err != nil { return err } - if len(transitioningReason) > 0 { - queue.AddAfter(stateWorkKey, 2*time.Minute) - return nil - } if currentConfig == nil && len(encryptionSecrets) == 0 { // we depend on the key controller to create the first key to bootstrap encryption. diff --git a/test/e2e-encryption/encryption_test.go b/test/e2e-encryption/encryption_test.go index d1a127eb23..ea10481393 100644 --- a/test/e2e-encryption/encryption_test.go +++ b/test/e2e-encryption/encryption_test.go @@ -267,6 +267,24 @@ func TestEncryptionIntegration(tt *testing.T) { require.NoError(t, err) } + waitForKeyData := func(key string, dataKey string, expectedContains string) { + t.Helper() + err := wait.PollUntilContextTimeout(ctx, time.Millisecond*100, wait.ForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + s, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-key-%s-%s", component, key), metav1.GetOptions{}) + if errors.IsNotFound(err) { + return false, nil + } + require.NoError(t, err) + + data, ok := s.Data[dataKey] + if !ok { + return false, nil + } + return strings.Contains(string(data), expectedContains), nil + }) + require.NoError(t, err) + } + verifyKMSPlugins := func() { t.Helper() encryptionConfigSecret, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-config-%s", component), metav1.GetOptions{}) @@ -659,11 +677,51 @@ func TestEncryptionIntegration(tt *testing.T) { require.Equal(t, "transit/keys/test-transit-key", pluginConfig13.Vault.VaultKeyPath) t.Logf("KMS non-migration change: only KMSPluginImage changes (no new key expected)") - _, err = fakeApiServerClient.Patch(ctx, "cluster", types.MergePatchType, []byte(`{"spec":{"encryption":{"type":"KMS","kms":{"type":"Vault","vault":{"kmsPluginImage":"registry.example.com/kms-plugin@sha256:0000000000000000000000000000000000000000000000000000000000000000","vaultAddress":"https://vault-new.example.com","authentication":{"type":"AppRole","appRole":{"secret":{"name":"vault-approle-secret"}}},"vaultKeyPath":"transit/keys/test-transit-key"}}}}}`), metav1.PatchOptions{}) + newImage := "registry.example.com/kms-plugin@sha256:0000000000000000000000000000000000000000000000000000000000000000" + _, err = fakeApiServerClient.Patch(ctx, "cluster", types.MergePatchType, []byte(fmt.Sprintf(`{"spec":{"encryption":{"type":"KMS","kms":{"type":"Vault","vault":{"kmsPluginImage":"%s","vaultAddress":"https://vault-new.example.com","tls":{"caBundle":{"name":"vault-ca-bundle"}},"authentication":{"type":"AppRole","appRole":{"secret":{"name":"vault-approle-secret"}}},"vaultKeyPath":"transit/keys/test-transit-key"}}}}}`, newImage)), metav1.PatchOptions{}) require.NoError(t, err) - time.Sleep(5 * time.Second) + waitForKeyData("13", "encryption.apiserver.operator.openshift.io-kms-plugin-config", newImage) + waitForKeys(12) + waitForConfigs( + fmt.Sprintf("kubeapiservers.operator.openshift.io=kms:%s,kms:%s,identity;kubeschedulers.operator.openshift.io=kms:%s,kms:%s,identity", kms13, kms12, kms13Sched, kms12Sched), + ) + + t.Logf("Rotate AppRole credentials and verify propagation to all KMS key secrets") + _, err = kubeClient.CoreV1().Secrets("openshift-config").Update(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "vault-approle-secret", Namespace: "openshift-config"}, + Data: map[string][]byte{ + "role-id": []byte("rotated-role-id"), + "secret-id": []byte("rotated-secret-id"), + }, + Type: corev1.SecretTypeOpaque, + }, metav1.UpdateOptions{}) + require.NoError(t, err) + // Only the latest key (13) gets in-place updates + waitForKeyData("13", "encryption.apiserver.operator.openshift.io-kms-plugin-secret-vault-approle-secret_role-id", "rotated-role-id") + expectedCfg := fmt.Sprintf("kubeapiservers.operator.openshift.io=kms:%s,kms:%s,identity;kubeschedulers.operator.openshift.io=kms:%s,kms:%s,identity", kms13, kms12, kms13Sched, kms12Sched) + waitForConfigs(expectedCfg) + + t.Logf("Verify encryption-config secret has rotated credentials for latest key") + encConfigSecret, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-config-%s", component), metav1.GetOptions{}) + require.NoError(t, err) + require.Contains(t, string(encConfigSecret.Data[encryptiondata.FormatKMSSecretDataKey("vault-approle-secret_role-id", "13")]), "rotated-role-id") + + t.Logf("Rotate CA bundle and verify propagation to latest KMS key secret") + _, err = kubeClient.CoreV1().ConfigMaps("openshift-config").Update(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "vault-ca-bundle", Namespace: "openshift-config"}, + Data: map[string]string{ + "ca-bundle.crt": "rotated-ca-cert", + }, + }, metav1.UpdateOptions{}) + require.NoError(t, err) + waitForKeyData("13", "encryption.apiserver.operator.openshift.io-kms-plugin-configmap-vault-ca-bundle_ca-bundle.crt", "rotated-ca-cert") + waitForConfigs(expectedCfg) + + t.Logf("Verify encryption-config secret has rotated CA bundle for latest key") + encConfigSecret, err = kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-config-%s", component), metav1.GetOptions{}) + require.NoError(t, err) + require.Contains(t, string(encConfigSecret.Data[encryptiondata.FormatKMSConfigMapDataKey("vault-ca-bundle_ca-bundle.crt", "13")]), "rotated-ca-cert") waitForKeys(12) - waitForConditionStatus("Encrypted", operatorv1.ConditionTrue) t.Logf("Delete the encryption-config while in KMS mode") _, err = kubeClient.CoreV1().Secrets("openshift-config-managed").Patch(ctx, fmt.Sprintf("encryption-config-%s", component), types.JSONPatchType, []byte(`[{"op":"remove","path":"/metadata/finalizers"}]`), metav1.PatchOptions{})