diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index d87a1228bf..338c576993 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" @@ -28,6 +29,7 @@ import ( "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/encryption/crypto" + "github.com/openshift/library-go/pkg/operator/encryption/encoding" "github.com/openshift/library-go/pkg/operator/encryption/secrets" "github.com/openshift/library-go/pkg/operator/encryption/state" "github.com/openshift/library-go/pkg/operator/encryption/statemachine" @@ -174,6 +176,16 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact return err } + // Apply in-place KMS plugin config updates (e.g. image, TLS) to the latest key + // secret regardless of convergence. This unblocks stuck revisions and propagates + // operational fixes like CVE image updates. Changes to migration-triggering fields + // (transit-key, vault-address, transit-mount) are skipped via sameProviderInstance. + if currentMode == state.KMS { + if err := c.maybeUpdateKMSPluginConfigInPlace(ctx, syncContext, apiEncryptionConfiguration); err != nil { + return err + } + } + currentConfig, desiredEncryptionState, secrets, isProgressingReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) if err != nil { return err @@ -277,6 +289,79 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c return nil // we made this key earlier } +// maybeUpdateKMSPluginConfigInPlace updates the latest key secret's KMS plugin +// config when only in-place-safe fields changed (image, TLS, authentication). +func (c *keyController) maybeUpdateKMSPluginConfigInPlace(ctx context.Context, syncContext factory.SyncContext, apiServerEncryption configv1.APIServerEncryption) error { + keySecrets, err := secrets.ListKeySecrets(ctx, c.secretClient, c.encryptionSecretSelector) + if err != nil { + return err + } + if len(keySecrets) == 0 { + return nil + } + // Sort by key ID descending so [0] is the newest. + // Parse the newest secret directly — fail fast if it is malformed + // instead of silently falling back to an older key. + sort.Slice(keySecrets, func(i, j int) bool { + iKeyID, _ := state.NameToKeyID(keySecrets[i].Name) + jKeyID, _ := state.NameToKeyID(keySecrets[j].Name) + return iKeyID > jKeyID + }) + storedKey, err := secrets.ToKeyState(keySecrets[0]) + if err != nil { + return fmt.Errorf("latest key secret %s is invalid: %w", keySecrets[0].Name, err) + } + + if storedKey.Mode != state.KMS { + return nil + } + if !storedKey.HasKMSPlugin() { + return fmt.Errorf("latest KMS key %s is missing plugin config", storedKey.Key.Name) + } + + desiredProviderCfg, err := newKMSProviderConfig(apiServerEncryption.KMS) + if err != nil { + return err + } + storedProviderCfg, err := newKMSProviderConfig(storedKey.KMS.Plugin) + if err != nil { + return err + } + if equality.Semantic.DeepEqual(desiredProviderCfg.sourceConfig(), storedProviderCfg.sourceConfig()) { + return nil + } + + sameInstance, err := desiredProviderCfg.sameProviderInstance(storedKey.KMS.Plugin) + if err != nil { + return err + } + if !sameInstance { + return nil + } + + keySecret, err := secrets.FromKeyState(c.instanceName, storedKey) + if err != nil { + return err + } + s, err := c.secretClient.Secrets(keySecret.Namespace).Get(ctx, keySecret.Name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get key secret %s/%s: %v", keySecret.Namespace, keySecret.Name, err) + } + pluginData, err := encoding.EncodeKMSPluginConfig(apiServerEncryption.KMS) + if err != nil { + return fmt.Errorf("failed to encode KMS plugin config: %v", err) + } + s.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] = pluginData + _, updateErr := c.secretClient.Secrets(s.Namespace).Update(ctx, s, metav1.UpdateOptions{}) + if errors.IsConflict(updateErr) { + return nil + } + if updateErr == nil { + syncContext.Recorder().Eventf("EncryptionKeyKMSPluginConfigUpdated", "Updated KMS plugin config on key secret %q in-place", s.Name) + } + return updateErr +} + func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, currentMode state.Mode, apiServerEncryption configv1.APIServerEncryption, desiredProviderCfg kmsProviderConfig, internalReason, externalReason string) (*corev1.Secret, error) { bs := crypto.ModeToNewKeyFunc[currentMode]() ks := state.KeyState{ diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index 394abb54a8..ac6b021b52 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -338,7 +338,7 @@ func TestKeyController(t *testing.T) { {Group: "", Resource: "secrets"}, }, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"}, initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), @@ -424,7 +424,7 @@ func TestKeyController(t *testing.T) { }, apiServerObjects: []runtime.Object{apiServerWithKMS}, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, { @@ -440,7 +440,7 @@ func TestKeyController(t *testing.T) { }, 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", "create:secrets:openshift-config-managed", "create:events:kms"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"}, validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { wasSecretValidated := false for _, action := range actions { @@ -506,7 +506,7 @@ func TestKeyController(t *testing.T) { 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:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, { name: "no-op when latest KMS key is not migrated yet", @@ -520,7 +520,7 @@ func TestKeyController(t *testing.T) { 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"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, { @@ -536,7 +536,7 @@ func TestKeyController(t *testing.T) { }, 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", "create:secrets:openshift-config-managed", "create:events:kms"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"}, validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { wasSecretValidated := false for _, action := range actions { @@ -600,7 +600,7 @@ func TestKeyController(t *testing.T) { {Group: "", Resource: "secrets"}, }, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config"}, initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), }, @@ -629,7 +629,7 @@ func TestKeyController(t *testing.T) { {Group: "", Resource: "secrets"}, }, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config"}, initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), &corev1.Secret{ @@ -659,7 +659,7 @@ func TestKeyController(t *testing.T) { {Group: "", Resource: "secrets"}, }, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config"}, initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), @@ -689,7 +689,7 @@ func TestKeyController(t *testing.T) { {Group: "", Resource: "secrets"}, }, targetNamespace: "kms", - expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config"}, + expectedActions: []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config"}, initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"), @@ -787,7 +787,7 @@ 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:secrets:openshift-config-managed", "get:secrets:openshift-config-managed", "update:secrets:openshift-config-managed", "create:events:kms", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, { @@ -807,7 +807,7 @@ 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:secrets:openshift-config-managed", "get:secrets:openshift-config-managed", "update:secrets:openshift-config-managed", "create:events:kms", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, { @@ -829,7 +829,213 @@ 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:secrets:openshift-config-managed", "get:secrets:openshift-config-managed", "update:secrets:openshift-config-managed", "create:events:kms", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + }, + + // In-place KMS plugin config update tests + { + name: "KMS in-place plugin config update during non-convergence", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: []runtime.Object{ + // No pods — triggers non-convergence + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + }, + apiServerObjects: []runtime.Object{func() *configv1.APIServer { + s := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + s.Spec.Encryption = configv1.APIServerEncryption{Type: "KMS", KMS: updated} + return s + }()}, + targetNamespace: "kms", + expectedActions: []string{ + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "update:secrets:openshift-config-managed", + "create:events:kms", + "list:pods:kms", + }, + validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { + for _, action := range actions { + if action.Matches("update", "secrets") { + updateAction := action.(clientgotesting.UpdateAction) + updatedSecret := updateAction.GetObject().(*corev1.Secret) + pluginData := updatedSecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + expectedData, _ := encoding.EncodeKMSPluginConfig(updated) + if string(pluginData) != string(expectedData) { + ts.Errorf("plugin config not updated: got %s", pluginData) + } + encData := updatedSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"] + original := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", nil, 1) + if string(encData) != string(original.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"]) { + ts.Error("KMS encryption config should not change during in-place update") + } + return + } + } + ts.Error("expected an update action for the key secret") + }, + }, + { + name: "KMS in-place plugin config update during convergence", + 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), + }, + apiServerObjects: []runtime.Object{func() *configv1.APIServer { + s := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + s.Spec.Encryption = configv1.APIServerEncryption{Type: "KMS", KMS: updated} + return s + }()}, + targetNamespace: "kms", + expectedActions: []string{ + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "update:secrets:openshift-config-managed", + "create:events:kms", + "list:pods:kms", + "get:secrets:kms", + "list:secrets:openshift-config-managed", + }, + validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { + for _, action := range actions { + if action.Matches("create", "secrets") { + ts.Error("should not create a new key for in-place update") + } + if action.Matches("update", "secrets") { + updateAction := action.(clientgotesting.UpdateAction) + updatedSecret := updateAction.GetObject().(*corev1.Secret) + pluginData := updatedSecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + expectedData, _ := encoding.EncodeKMSPluginConfig(updated) + if string(pluginData) != string(expectedData) { + ts.Errorf("plugin config not updated: got %s", pluginData) + } + encData := updatedSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"] + original := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", nil, 1) + if string(encData) != string(original.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"]) { + ts.Error("KMS encryption config should not change during in-place update") + } + } + } + }, + }, + { + name: "KMS migration-triggering change skips in-place during non-convergence", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: []runtime.Object{ + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + }, + apiServerObjects: []runtime.Object{func() *configv1.APIServer { + s := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.TransitKey = "different-transit-key" + s.Spec.Encryption = configv1.APIServerEncryption{Type: "KMS", KMS: updated} + return s + }()}, + targetNamespace: "kms", + expectedActions: []string{ + "list:secrets:openshift-config-managed", + "list:pods:kms", + }, + }, + { + name: "KMS no-op when plugin config matches during non-convergence", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: []runtime.Object{ + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + }, + apiServerObjects: []runtime.Object{func() *configv1.APIServer { + s := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + s.Spec.Encryption = configv1.APIServerEncryption{Type: "KMS", KMS: encryptiontesting.DefaultKMSPluginConfig} + return s + }()}, + targetNamespace: "kms", + expectedActions: []string{ + "list:secrets:openshift-config-managed", + "list:pods:kms", + }, + }, + { + name: "skips in-place when latest key is not KMS", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: []runtime.Object{ + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", nil, 2, []byte("61def964fb967f5d7c44a2af8dab6865"), "aescbc"), + }, + apiServerObjects: []runtime.Object{func() *configv1.APIServer { + s := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + s.Spec.Encryption = configv1.APIServerEncryption{Type: "KMS", KMS: updated} + return s + }()}, + targetNamespace: "kms", + expectedActions: []string{ + "list:secrets:openshift-config-managed", + "list:pods:kms", + }, + }, + { + name: "KMS in-place update only modifies latest key", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: []runtime.Object{ + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", nil, 2), + }, + apiServerObjects: []runtime.Object{func() *configv1.APIServer { + s := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + s.Spec.Encryption = configv1.APIServerEncryption{Type: "KMS", KMS: updated} + return s + }()}, + targetNamespace: "kms", + expectedActions: []string{ + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "update:secrets:openshift-config-managed", + "create:events:kms", + "list:pods:kms", + }, + validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { + for _, action := range actions { + if action.Matches("update", "secrets") { + updateAction := action.(clientgotesting.UpdateAction) + updatedSecret := updateAction.GetObject().(*corev1.Secret) + if updatedSecret.Name != "encryption-key-kms-2" { + ts.Errorf("expected key 2 to be updated, got %s", updatedSecret.Name) + } + pluginData := updatedSecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + expectedData, _ := encoding.EncodeKMSPluginConfig(updated) + if string(pluginData) != string(expectedData) { + ts.Errorf("plugin config not updated on latest key") + } + return + } + } + ts.Error("expected an update action for the latest key secret") + }, }, } @@ -983,7 +1189,7 @@ func TestKMSMigrationTriggeredFields(t *testing.T) { err = target.Sync(context.TODO(), factory.NewSyncContext("test", eventRecorder)) require.NoError(t, err) - expectedActions := []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"} + expectedActions := []string{"list:secrets:openshift-config-managed", "list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "get:configmaps:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"} require.NoError(t, encryptiontesting.ValidateActionsVerbs(fakeKubeClient.Actions(), expectedActions)) for _, action := range fakeKubeClient.Actions() { diff --git a/pkg/operator/encryption/controllers/state_controller.go b/pkg/operator/encryption/controllers/state_controller.go index a0b7f1f677..4ef4c5982e 100644 --- a/pkg/operator/encryption/controllers/state_controller.go +++ b/pkg/operator/encryption/controllers/state_controller.go @@ -5,6 +5,8 @@ import ( "fmt" "time" + "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" @@ -16,6 +18,7 @@ import ( configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/encryption/encryptiondata" + "github.com/openshift/library-go/pkg/operator/encryption/secrets" "github.com/openshift/library-go/pkg/operator/encryption/state" "github.com/openshift/library-go/pkg/operator/encryption/statemachine" "github.com/openshift/library-go/pkg/operator/events" @@ -132,6 +135,11 @@ func (c *stateController) generateAndApplyCurrentEncryptionConfigSecret(ctx cont return err } if len(transitioningReason) > 0 { + // Even when not converged, propagate in-place KMS plugin config changes + // so the revision controller can create a new revision with corrected sidecar config. + if err := c.maybeUpdateKMSDataInEncryptionConfigSecret(ctx, recorder); err != nil { + return err + } queue.AddAfter(stateWorkKey, 2*time.Minute) return nil } @@ -163,6 +171,84 @@ func (c *stateController) generateAndApplyCurrentEncryptionConfigSecret(ctx cont return nil } +// maybeUpdateKMSDataInEncryptionConfigSecret propagates in-place KMS plugin +// config changes to the encryption-config secret during non-convergence. It enriches +// the existing state with current key secrets (picking up updated plugin configs from +// key_controller), guards that the EncryptionConfiguration is unchanged (no key +// promotion or structural changes), and applies only kms-plugin-config updates. +func (c *stateController) maybeUpdateKMSDataInEncryptionConfigSecret(ctx context.Context, recorder events.Recorder) error { + keySecrets, err := secrets.ListKeySecrets(ctx, c.secretClient, c.encryptionSecretSelector) + if err != nil { + return err + } + if len(keySecrets) == 0 { + return nil + } + name := fmt.Sprintf("%s-%s", encryptiondata.EncryptionConfSecretName, c.instanceName) + namespace := "openshift-config-managed" + // Read the unrevisioned encryption-config from openshift-config-managed (not the + // revisioned copy in the target namespace) because during non-convergence the deployer + // cannot return a single converged revision. Updating this source secret triggers + // resource-sync → revision → rollout to unblock the stuck revision. + existingSecret, err := c.secretClient.Secrets(namespace).Get(ctx, name, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + return nil + } + if err != nil { + return err + } + + existingConfig, err := encryptiondata.FromSecret(existingSecret) + if err != nil { + return err + } + if existingConfig == nil || len(existingConfig.KMSPlugins) == 0 { + return nil + } + + // Round-trip through ToEncryptionState → FromEncryptionState to pick up + // updated KMS plugin configs from key secrets while preserving the existing + // EncryptionConfiguration structure. ToEncryptionState enriches each key in + // the config with its backed secret data (including any in-place plugin config + // updates), and FromEncryptionState rebuilds the Config from the enriched state. + enrichedState, _ := encryptiondata.ToEncryptionState(existingConfig, keySecrets) + if enrichedState == nil { + return nil + } + + rebuiltConfig, err := encryptiondata.FromEncryptionState(enrichedState) + if err != nil { + return err + } + + // Only proceed if the provider list, key ordering, and write key designation + // are unchanged. Structural changes require convergence to avoid a server + // encrypting with a key another server hasn't observed. + if !equality.Semantic.DeepEqual(existingConfig.Encryption.Resources, rebuiltConfig.Encryption.Resources) { + return nil + } + + // Error if the plugin key set changed unexpectedly — this indicates + // a bug in the enrichment round-trip or corrupted state. + if len(existingConfig.KMSPlugins) != len(rebuiltConfig.KMSPlugins) { + return fmt.Errorf("KMS plugin key set size changed unexpectedly: existing=%d, rebuilt=%d", len(existingConfig.KMSPlugins), len(rebuiltConfig.KMSPlugins)) + } + for keyID := range existingConfig.KMSPlugins { + if _, ok := rebuiltConfig.KMSPlugins[keyID]; !ok { + return fmt.Errorf("KMS plugin config for keyID %s disappeared after enrichment round-trip", keyID) + } + } + + changed, err := c.applyEncryptionConfigSecret(ctx, rebuiltConfig, recorder) + if err != nil { + return err + } + if changed { + recorder.Eventf("EncryptionKMSPluginConfigPropagated", "Updated KMS plugin config in encryption-config secret %s/%s during non-convergence", namespace, name) + } + return nil +} + func (c *stateController) applyEncryptionConfigSecret(ctx context.Context, secretData *encryptiondata.Config, recorder events.Recorder) (bool, error) { s, err := encryptiondata.ToSecret("openshift-config-managed", fmt.Sprintf("%s-%s", encryptiondata.EncryptionConfSecretName, c.instanceName), secretData) if err != nil { diff --git a/pkg/operator/encryption/controllers/state_controller_test.go b/pkg/operator/encryption/controllers/state_controller_test.go index eb12247373..80cf8c1d04 100644 --- a/pkg/operator/encryption/controllers/state_controller_test.go +++ b/pkg/operator/encryption/controllers/state_controller_test.go @@ -1179,6 +1179,162 @@ func TestStateController(t *testing.T) { } }, }, + + // In-place KMS plugin config propagation during non-convergence + { + name: "KMS: propagates updated plugin config to encryption-config during non-convergence", + targetNamespace: "kms", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialResources: []runtime.Object{ + // No pods — non-converged + // Key secret with updated plugin config (new image) + encryptiontesting.CreateEncryptionKeySecretWithCustomKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, func() configv1.KMSPluginConfig { + updated := encryptiontesting.DefaultKMSPluginConfig + updated.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + return updated + }()), + // Existing encryption-config in openshift-config-managed with old plugin config + func() *corev1.Secret { + ec := &encryptiondata.Config{ + Encryption: &apiserverconfigv1.EncryptionConfiguration{ + Resources: []apiserverconfigv1.ResourceConfiguration{{ + Resources: []string{"secrets"}, + Providers: []apiserverconfigv1.ProviderConfiguration{{ + KMS: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "1_secrets", + Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Timeout: &metav1.Duration{Duration: 10 * time.Second}, + }, + }, { + Identity: &apiserverconfigv1.IdentityConfiguration{}, + }}, + }}, + }, + KMSPlugins: map[string]configv1.KMSPluginConfig{"1": encryptiontesting.DefaultKMSPluginConfig}, + } + ecs := createEncryptionCfgSecret(t, "openshift-config-managed", "1", ec) + ecs.Name = "encryption-config-kms" + return ecs + }(), + }, + expectedActions: []string{ + "list:pods:kms", + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "update:secrets:openshift-config-managed", + "create:events:kms", + "create:events:kms", + }, + validateFunc: func(ts *testing.T, actions []clientgotesting.Action, destName string, expectedEncryptionCfg *encryptiondata.Config) { + for _, action := range actions { + if action.Matches("update", "secrets") { + updateAction := action.(clientgotesting.UpdateAction) + actualSecret := updateAction.GetObject().(*corev1.Secret) + actualConfig, err := encryptiondata.FromSecret(actualSecret) + if err != nil { + ts.Fatalf("failed to parse updated encryption config: %v", err) + } + // Verify plugin config was updated + updatedPlugin := encryptiontesting.DefaultKMSPluginConfig + updatedPlugin.Vault.KMSPluginImage = "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111" + if !equality.Semantic.DeepEqual(actualConfig.KMSPlugins["1"], updatedPlugin) { + ts.Errorf("plugin config not propagated: got %+v", actualConfig.KMSPlugins["1"]) + } + // Verify EncryptionConfiguration (structural part) is unchanged + if len(actualConfig.Encryption.Resources) != 1 || len(actualConfig.Encryption.Resources[0].Providers) != 2 { + ts.Errorf("EncryptionConfiguration structure changed unexpectedly") + } + return + } + } + ts.Error("expected an update action for the encryption-config secret") + }, + }, + { + name: "KMS: no-op propagation when plugin configs match during non-convergence", + targetNamespace: "kms", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialResources: []runtime.Object{ + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + func() *corev1.Secret { + ec := &encryptiondata.Config{ + Encryption: &apiserverconfigv1.EncryptionConfiguration{ + Resources: []apiserverconfigv1.ResourceConfiguration{{ + Resources: []string{"secrets"}, + Providers: []apiserverconfigv1.ProviderConfiguration{{ + KMS: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "1_secrets", + Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Timeout: &metav1.Duration{Duration: 10 * time.Second}, + }, + }, { + Identity: &apiserverconfigv1.IdentityConfiguration{}, + }}, + }}, + }, + KMSPlugins: map[string]configv1.KMSPluginConfig{"1": encryptiontesting.DefaultKMSPluginConfig}, + } + ecs := createEncryptionCfgSecret(t, "openshift-config-managed", "1", ec) + ecs.Name = "encryption-config-kms" + return ecs + }(), + }, + expectedActions: []string{ + "list:pods:kms", + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + }, + }, + { + name: "KMS: no encryption-config secret yet during non-convergence", + targetNamespace: "kms", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialResources: []runtime.Object{ + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1), + }, + expectedActions: []string{ + "list:pods:kms", + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + }, + }, + { + name: "non-KMS mode skips propagation during non-convergence", + targetNamespace: "kms", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialResources: []runtime.Object{ + encryptiontesting.CreateEncryptionKeySecretWithRawKey("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, []byte("61def964fb967f5d7c44a2af8dab6865")), + func() *corev1.Secret { + ec := encryptiondatatesting.CreateEncryptionCfgWithWriteKey([]encryptiondatatesting.EncryptionKeysResourceTuple{{ + Resource: "secrets", + Keys: []apiserverconfigv1.Key{{ + Name: "1", + Secret: "NjFkZWY5NjRmYjk2N2Y1ZDdjNDRhMmFmOGRhYjY4NjU=", + }}, + }}) + ecs := createEncryptionCfgSecret(t, "openshift-config-managed", "1", ec) + ecs.Name = "encryption-config-kms" + return ecs + }(), + }, + expectedActions: []string{ + "list:pods:kms", + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + }, + }, } for _, scenario := range scenarios { diff --git a/pkg/operator/encryption/encryptiondata/config_test.go b/pkg/operator/encryption/encryptiondata/config_test.go index 1e796c99e3..90a987b53b 100644 --- a/pkg/operator/encryption/encryptiondata/config_test.go +++ b/pkg/operator/encryption/encryptiondata/config_test.go @@ -452,6 +452,33 @@ func TestToEncryptionState(t *testing.T) { } } +// ToEncryptionState must not mutate Config object. +// This test will catch it. +func TestToEncryptionStateDoesNotMutateInput(t *testing.T) { + input := encryptiondatatesting.CreateEncryptionCfgWithWriteKey([]encryptiondatatesting.EncryptionKeysResourceTuple{ + { + Resource: "secrets", + Keys: []apiserverconfigv1.Key{ + {Name: "2", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + {Name: "1", Secret: "MTcxNTgyYTBmY2Q2YzVmZGI2NWNiZjVhM2U5MjQ5ZDc="}, + }, + Modes: []string{"KMS", "aescbc"}, + }, + }) + inputCopy := input.Encryption.DeepCopy() + + keySecrets := []*corev1.Secret{ + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("test", []schema.GroupResource{{Resource: "secrets"}}, 2), + encryptiontesting.CreateEncryptionKeySecretWithRawKey("test", []schema.GroupResource{{Resource: "secrets"}}, 1, []byte("61def964fb967f5d7c44a2af8dab6865")), + } + + encryptiondata.ToEncryptionState(input, keySecrets) + + if diff := cmp.Diff(inputCopy, input.Encryption); diff != "" { + t.Fatalf("ToEncryptionState mutated the input Config.Encryption:\n%s", diff) + } +} + func TestFromEncryptionState(t *testing.T) { scenarios := []struct { name string diff --git a/test/e2e-encryption/encryption_test.go b/test/e2e-encryption/encryption_test.go index e05d6fe307..f7635e0321 100644 --- a/test/e2e-encryption/encryption_test.go +++ b/test/e2e-encryption/encryption_test.go @@ -13,6 +13,7 @@ import ( corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" v1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/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" @@ -285,20 +286,39 @@ func TestEncryptionIntegration(tt *testing.T) { } } - for keyID := range expectedKeyIDs { - pluginConfig, ok := cfg.KMSPlugins[keyID] - if !ok { - t.Fatalf("expected kms-plugin-config for keyID %s but not found in encryption-config secret", keyID) + err = wait.PollUntilContextTimeout(ctx, time.Millisecond*500, wait.ForeverTestTimeout, true, func(ctx context.Context) (bool, error) { + encryptionConfigSecret, err = kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-config-%s", component), metav1.GetOptions{}) + if err != nil { + return false, err } - - keySecret, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-key-%s-%s", component, keyID), metav1.GetOptions{}) - require.NoError(t, err) - keyPluginData := keySecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] - require.NotEmpty(t, keyPluginData, "key secret %s missing kms-plugin-config data", keyID) - keyPluginConfig, err := encoding.DecodeKMSPluginConfig(keyPluginData) - require.NoError(t, err) - require.Equal(t, keyPluginConfig, pluginConfig, "kms-plugin-config for keyID %s in encryption-config secret does not match key secret", keyID) - } + cfg, err = encryptiondata.FromSecret(encryptionConfigSecret) + if err != nil { + return false, nil + } + for keyID := range expectedKeyIDs { + pluginConfig, ok := cfg.KMSPlugins[keyID] + if !ok { + return false, nil + } + keySecret, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-key-%s-%s", component, keyID), metav1.GetOptions{}) + if err != nil { + return false, err + } + keyPluginData := keySecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] + if len(keyPluginData) == 0 { + return false, nil + } + keyPluginConfig, err := encoding.DecodeKMSPluginConfig(keyPluginData) + if err != nil { + return false, nil + } + if !equality.Semantic.DeepEqual(keyPluginConfig, pluginConfig) { + return false, nil + } + } + return true, nil + }) + require.NoError(t, err) for keyID := range cfg.KMSPlugins { if !expectedKeyIDs[keyID] { t.Fatalf("unexpected kms-plugin-config for keyID %s in encryption-config secret", keyID) @@ -685,6 +705,37 @@ func TestEncryptionIntegration(tt *testing.T) { ) waitForConditionStatus("Encrypted", operatorv1.ConditionTrue) + t.Logf("In-place KMS plugin config update: change image on key 13 (non-migration field)") + _, err = fakeApiServerClient.Patch(ctx, "cluster", types.MergePatchType, []byte(`{"spec":{"encryption":{"type":"KMS","kms":{"type":"Vault","vault":{"kmsPluginImage":"registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111","vaultAddress":"https://vault-new.example.com","authentication":{"type":"AppRole","appRole":{"secret":{"name":"vault-approle-secret"}}},"transitKey":"test-transit-key"}}}}}`), metav1.PatchOptions{}) + require.NoError(t, err) + + // Wait for the key secret's plugin config to be updated in-place + err = wait.PollUntilContextTimeout(ctx, time.Millisecond*500, 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, "13"), metav1.GetOptions{}) + if err != nil { + return false, err + } + pluginData := s.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] + if len(pluginData) == 0 { + return false, nil + } + cfg, err := encoding.DecodeKMSPluginConfig(pluginData) + if err != nil { + return false, fmt.Errorf("failed to decode kms-plugin-config for key 13: %w", err) + } + return cfg.Vault.KMSPluginImage == "registry.example.com/kms-plugin@sha256:1111111111111111111111111111111111111111111111111111111111111111", nil + }) + require.NoError(t, err) + // Verify no new key was created — in-place update should not trigger key creation + waitForKeys(12) + // Verify the encryption-config secret also has the updated plugin config + verifyKMSPlugins() + // The in-place plugin config update on key 13 triggers the state controller to + // propagate the updated plugin data into the encryption-config secret, consume it. + 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("Switch to identity from KMS") _, err = fakeApiServerClient.Patch(ctx, "cluster", types.MergePatchType, []byte(`{"spec":{"encryption":{"type":"identity","kms":null}}}`), metav1.PatchOptions{}) require.NoError(t, err)