diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index 5ededc2ca1..e42ccb9a22 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -18,6 +18,7 @@ import ( apiserverv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/klog/v2" + "k8s.io/utils/clock" configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" @@ -193,6 +194,18 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact return nil } + if err := c.checkAndCreateKeysFromSnap(ctx, syncContext, planner, snap); err != nil { + return err + } + + if err := c.reconcileRemoteKeyRotationFromSnap(ctx, snap); err != nil { + return err + } + + return nil +} + +func (c *keyController) checkAndCreateKeysFromSnap(ctx context.Context, syncContext factory.SyncContext, planner *EncryptionPlanner, snap *KeyPlanningSnapshot) error { plan, err := planner.PlanNextKey(snap) if err != nil { return err @@ -224,6 +237,24 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact return nil } +func (c *keyController) reconcileRemoteKeyRotationFromSnap(ctx context.Context, snap *KeyPlanningSnapshot) error { + if c.encryptionStatusProvider == nil || snap.CurrentMode != state.KMS { + return nil + } + + writeKey, ok := writeKeyForRemoteKeyRotation(snap.State.DesiredBeforePlan) + if !ok || len(writeKey.RemoteKey().TargetRemoteKeyID) == 0 { + return nil + } + + encryptionStatus, err := c.encryptionStatusProvider.GetKMSEncryptionStatus(ctx) + if err != nil { + return fmt.Errorf("failed to get KMS encryption status for remote key rotation: %w", err) + } + + return reconcileRemoteKeyRotation(ctx, c.secretClient, c.instanceName, snap.State.EncryptedGRs, snap.State.DesiredBeforePlan, encryptionStatus, clock.RealClock{}) +} + func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *corev1.Secret, keyID uint64) error { actualKeySecret, err := c.secretClient.Secrets("openshift-config-managed").Get(ctx, keySecret.Name, metav1.GetOptions{}) if err != nil { @@ -267,13 +298,14 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur if err != nil { return nil, false, fmt.Errorf("failed to compute KMS config hash: %w", err) } - preflightPassed, err := c.ensureKMSPreflightPassed(ctx, configHash) + preflightPassed, remoteKeyID, err := c.ensureKMSPreflightPassed(ctx, configHash) if err != nil { return nil, false, err } if !preflightPassed { return nil, false, nil } + ks.KMS.RemoteKey.TargetRemoteKeyID = remoteKeyID } secret, err := secrets.FromKeyState(c.instanceName, ks) if err != nil { @@ -451,10 +483,10 @@ func (p *prefetchedKMSConfigHasherResourceProvider) getConfigMap(_ context.Conte // back off. // // Callers are responsible for requeuing when this returns (false, nil). -func (c *keyController) ensureKMSPreflightPassed(ctx context.Context, configHash string) (bool, error) { +func (c *keyController) ensureKMSPreflightPassed(ctx context.Context, configHash string) (bool, string, error) { encryptionStatus, err := c.encryptionStatusProvider.GetKMSEncryptionStatus(ctx) if err != nil { - return false, fmt.Errorf("failed to get KMS encryption status: %w", err) + return false, "", fmt.Errorf("failed to get KMS encryption status: %w", err) } // Scenario 1: ObservedConfigHash outdated — schedule the preflight check. @@ -462,22 +494,22 @@ func (c *keyController) ensureKMSPreflightPassed(ctx context.Context, configHash if err := c.encryptionStatusProvider.UpdateKMSEncryptionStatus(ctx, func(s *operatorv1.KMSEncryptionStatus) { s.Preflight.ObservedConfigHash = configHash }); err != nil { - return false, fmt.Errorf("failed to write preflight observed config hash: %w", err) + return false, "", fmt.Errorf("failed to write preflight observed config hash: %w", err) } // Scenario 1: back off: wait for the preflight controller to pick up the new hash. - return false, nil + return false, "", nil } // Scenario 2: preflight passed — proceed. if isPreflightResultSucceeded(&encryptionStatus.Preflight.Result, configHash) { - return true, nil + return true, encryptionStatus.Preflight.Result.RemoteKeyID, nil } // Scenario 3: preflight failed — surface the error. if isPreflightResultFailed(&encryptionStatus.Preflight.Result, configHash) { - return false, fmt.Errorf("KMS preflight check failed for config hash %s; fix the KMS configuration to proceed", configHash) + return false, "", fmt.Errorf("KMS preflight check failed for config hash %s; fix the KMS configuration to proceed", configHash) } // Scenario 4: back off: preflight check is still in progress. - return false, nil + return false, "", nil } type encryptionKeyPlan struct { @@ -594,6 +626,11 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern return latestKeyID, "kms-provider-changed", true, nil } + if secrets.NeedsRemoteKeyMigration(latestKey.RemoteKey()) { + // Block new encryption key minting while target and migrated remote key IDs differ. + return 0, "", false, nil + } + // 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. diff --git a/pkg/operator/encryption/controllers/migration_controller.go b/pkg/operator/encryption/controllers/migration_controller.go index 7fc649f020..a4ea814f9b 100644 --- a/pkg/operator/encryption/controllers/migration_controller.go +++ b/pkg/operator/encryption/controllers/migration_controller.go @@ -216,26 +216,49 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C // using a write key that another API server has not observed // this could lead to etcd storing data that not all API servers can decrypt var errs []error + var writeKeyState state.KeyState + var writeKeySecretName string + var migrationWriteKey string + var hadRemoteKeyMigration bool + var writeKeyGRs []schema.GroupResource + var remoteKeyMigratedGRs []schema.GroupResource for _, gr := range grs { grActualKeys := currentState[gr] if !grActualKeys.HasWriteKey() { continue // no write key to migrate to } - if alreadyMigrated, _, _ := state.MigratedFor([]schema.GroupResource{gr}, grActualKeys.WriteKey); alreadyMigrated { + writeKeyState = grActualKeys.WriteKey + writeKeySecret, err := secrets.FromKeyState(c.instanceName, grActualKeys.WriteKey) + if err != nil { + errs = append(errs, err) continue } + writeKeySecretName = writeKeySecret.Name + remoteKeyAnnotations := grActualKeys.WriteKey.RemoteKey() + migrationWriteKey = secrets.MigrationWriteKeyName(grActualKeys.WriteKey.Key.Name, remoteKeyAnnotations) + remoteKeyMigration := secrets.NeedsRemoteKeyMigration(remoteKeyAnnotations) + if remoteKeyMigration { + hadRemoteKeyMigration = true + writeKeyGRs = append(writeKeyGRs, gr) + } + + if !remoteKeyMigration { + if alreadyMigrated, _, _ := state.MigratedFor([]schema.GroupResource{gr}, grActualKeys.WriteKey); alreadyMigrated { + continue + } + } // idem-potent migration start - finished, result, when, err := c.migrator.EnsureMigration(gr, grActualKeys.WriteKey.Key.Name) + finished, result, when, err := c.migrator.EnsureMigration(gr, migrationWriteKey) if err == nil && finished && result != nil && time.Since(when) > migrationRetryDuration { // last migration error is far enough ago. Prune and retry. if err := c.migrator.PruneMigration(gr); err != nil { errs = append(errs, err) continue } - finished, result, when, err = c.migrator.EnsureMigration(gr, grActualKeys.WriteKey.Key.Name) - + // when is not used anymore below + finished, result, _, err = c.migrator.EnsureMigration(gr, migrationWriteKey) } if err != nil { errs = append(errs, err) @@ -251,6 +274,13 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C continue } + if remoteKeyMigration { + if result == nil { + remoteKeyMigratedGRs = append(remoteKeyMigratedGRs, gr) + } + continue + } + // update secret annotations oldWriteKey, err := secrets.FromKeyState(c.instanceName, grActualKeys.WriteKey) if err != nil { @@ -283,6 +313,23 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C } } + if hadRemoteKeyMigration && len(writeKeySecretName) > 0 && len(remoteKeyMigratedGRs) == len(writeKeyGRs) { + if remoteKeyID, ok := secrets.RemoteKeyIDFromMigrationWriteKey(writeKeyState.Key.Name, migrationWriteKey); ok { + if err := secrets.PatchRemoteKeyAnnotations(ctx, c.secretClient.Secrets("openshift-config-managed"), writeKeySecretName, func(rk *secrets.RemoteKeyAnnotations) (bool, error) { + if !secrets.NeedsRemoteKeyMigration(*rk) { + return false, nil + } + if rk.MigratedRemoteKeyID == remoteKeyID { + return false, nil + } + rk.MigratedRemoteKeyID = remoteKeyID + return true, nil + }); err != nil { + errs = append(errs, err) + } + } + } + return migratingResources, errors.NewAggregate(errs) } diff --git a/pkg/operator/encryption/controllers/migration_controller_remote_key_test.go b/pkg/operator/encryption/controllers/migration_controller_remote_key_test.go new file mode 100644 index 0000000000..17f3013a19 --- /dev/null +++ b/pkg/operator/encryption/controllers/migration_controller_remote_key_test.go @@ -0,0 +1,339 @@ +package controllers + +import ( + "context" + "encoding/json" + "testing" + "time" + + clocktesting "k8s.io/utils/clock/testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + apiserverconfigv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" + "k8s.io/client-go/kubernetes/fake" + clientgotesting "k8s.io/client-go/testing" + + operatorv1 "github.com/openshift/api/operator/v1" + + configv1clientfake "github.com/openshift/client-go/config/clientset/versioned/fake" + configv1informers "github.com/openshift/client-go/config/informers/externalversions" + "github.com/openshift/library-go/pkg/controller/factory" + encryptiondeployer "github.com/openshift/library-go/pkg/operator/encryption/deployer" + encryptiondatatesting "github.com/openshift/library-go/pkg/operator/encryption/encryptiondata/testing" + "github.com/openshift/library-go/pkg/operator/encryption/secrets" + "github.com/openshift/library-go/pkg/operator/encryption/state" + encryptiontesting "github.com/openshift/library-go/pkg/operator/encryption/testing" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/v1helpers" +) + +const ( + remoteKeyOld = "remote-old" + remoteKeyNew = "remote-new" +) + +func TestMigrationControllerRemoteKeyFirstEnablement(t *testing.T) { + targetGRs := []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + {Group: "", Resource: "configmaps"}, + } + firstEnablementWriteKey := "1-" + remoteKeyNew + + keySecret := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", targetGRs, 1) + secrets.ApplyRemoteKeyAnnotations(keySecret.Annotations, state.RemoteKeyState{TargetRemoteKeyID: remoteKeyNew}) + delete(keySecret.Annotations, secrets.EncryptionSecretMigratedResources) + + kmsKey := apiserverconfigv1.Key{Name: "1", Secret: "NzFlYTdjOTE0MTlhNjhmZDEyMjRmODhkNTAzMTZiNGU="} + keysResForSecrets := encryptiondatatesting.EncryptionKeysResourceTuple{ + Resource: "secrets", + Keys: []apiserverconfigv1.Key{kmsKey}, + Modes: []string{"KMS"}, + } + keysResForConfigMaps := encryptiondatatesting.EncryptionKeysResourceTuple{ + Resource: "configmaps", + Keys: []apiserverconfigv1.Key{kmsKey}, + Modes: []string{"KMS"}, + } + encryptionCfgSecret := createEncryptionCfgSecret(t, "kms", "1", encryptiondatatesting.CreateEncryptionCfgWithWriteKey([]encryptiondatatesting.EncryptionKeysResourceTuple{ + keysResForConfigMaps, keysResForSecrets, + })) + + migrator := &fakeMigrator{ + ensureReplies: map[schema.GroupResource]map[string]finishedResultErr{ + {Group: "", Resource: "secrets"}: {firstEnablementWriteKey: {finished: false}}, + {Group: "", Resource: "configmaps"}: {firstEnablementWriteKey: {finished: false}}, + }, + } + fakeOperatorClient := v1helpers.NewFakeStaticPodOperatorClient( + &operatorv1.StaticPodOperatorSpec{OperatorSpec: operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}}, + &operatorv1.StaticPodOperatorStatus{ + OperatorStatus: operatorv1.OperatorStatus{ + Conditions: []operatorv1.OperatorCondition{ + {Type: "EncryptionMigrationControllerDegraded", Status: operatorv1.ConditionFalse}, + {Type: "EncryptionMigrationControllerProgressing", Status: operatorv1.ConditionFalse}, + }, + }, + NodeStatuses: []operatorv1.NodeStatus{{NodeName: "node-1"}}, + }, + nil, + nil, + ) + fakeKubeClient := fake.NewSimpleClientset( + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + keySecret, + encryptionCfgSecret, + ) + eventRecorder := events.NewRecorder(fakeKubeClient.CoreV1().Events("operator"), "test-encryption-migration-controller", &corev1.ObjectReference{}, clocktesting.NewFakePassiveClock(time.Now())) + kubeInformers := v1helpers.NewKubeInformersForNamespaces(fakeKubeClient, "openshift-config-managed", "kms") + deployer, err := encryptiondeployer.NewRevisionLabelPodDeployer("revision", "kms", kubeInformers, fakeKubeClient.CoreV1(), fakeKubeClient.CoreV1(), encryptiondeployer.StaticPodNodeProvider{OperatorClient: fakeOperatorClient}) + if err != nil { + t.Fatal(err) + } + + target := NewMigrationController( + "kms", + newTestProvider(targetGRs), + deployer, + alwaysFulfilledPreconditions, + migrator, + fakeOperatorClient, + configv1informers.NewSharedInformerFactory(configv1clientfake.NewSimpleClientset(), time.Minute).Config().V1().APIServers(), + kubeInformers, + fakeKubeClient.CoreV1(), + metav1.ListOptions{}, + eventRecorder, + ) + if err := target.Sync(context.TODO(), factory.NewSyncContext("test", eventRecorder)); err != nil { + t.Fatal(err) + } + + expectedMigratorCalls := []string{ + "ensure:configmaps:" + firstEnablementWriteKey, + "ensure:secrets:" + firstEnablementWriteKey, + } + if !equalStringSlices(expectedMigratorCalls, migrator.calls) { + t.Fatalf("migrator calls:\n expected: %v\n got: %v", expectedMigratorCalls, migrator.calls) + } + for _, action := range fakeKubeClient.Actions() { + if !action.Matches("update", "secrets") { + continue + } + secret := action.(clientgotesting.UpdateAction).GetObject().(*corev1.Secret) + if secret.Name != keySecret.Name { + continue + } + rk, err := secrets.ReadRemoteKeyAnnotations(secret) + if err != nil { + t.Fatalf("read remote key annotations: %v", err) + } + if rk.MigratedRemoteKeyID != "" { + t.Fatal("first enablement must not set migrated-remote-key-id") + } + } +} + +func TestMigrationControllerRemoteKeyRotation(t *testing.T) { + targetGRs := []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + {Group: "", Resource: "configmaps"}, + } + remoteMigrationWriteKey := "1-" + remoteKeyNew + + scenarios := []struct { + name string + migratorEnsureReplies map[schema.GroupResource]map[string]finishedResultErr + expectedMigratorCalls []string + expectMigratedPatch bool + expectProgressing bool + }{ + { + name: "needsMigration runs suffixed SVM despite migrated-resources", + migratorEnsureReplies: map[schema.GroupResource]map[string]finishedResultErr{ + {Group: "", Resource: "secrets"}: {remoteMigrationWriteKey: {finished: false}}, + {Group: "", Resource: "configmaps"}: {remoteMigrationWriteKey: {finished: false}}, + }, + expectedMigratorCalls: []string{ + "ensure:configmaps:" + remoteMigrationWriteKey, + "ensure:secrets:" + remoteMigrationWriteKey, + }, + expectProgressing: true, + }, + { + name: "does not stamp migrated-remote-key-id until all suffixed SVMs finish", + migratorEnsureReplies: map[schema.GroupResource]map[string]finishedResultErr{ + {Group: "", Resource: "secrets"}: {remoteMigrationWriteKey: {finished: true}}, + {Group: "", Resource: "configmaps"}: {remoteMigrationWriteKey: {finished: false}}, + }, + expectedMigratorCalls: []string{ + "ensure:configmaps:" + remoteMigrationWriteKey, + "ensure:secrets:" + remoteMigrationWriteKey, + }, + expectProgressing: true, + }, + { + name: "stamps migrated-remote-key-id after all suffixed SVMs complete", + migratorEnsureReplies: map[schema.GroupResource]map[string]finishedResultErr{ + {Group: "", Resource: "secrets"}: {remoteMigrationWriteKey: {finished: true}}, + {Group: "", Resource: "configmaps"}: {remoteMigrationWriteKey: {finished: true}}, + }, + expectedMigratorCalls: []string{ + "ensure:configmaps:" + remoteMigrationWriteKey, + "ensure:secrets:" + remoteMigrationWriteKey, + }, + expectMigratedPatch: true, + }, + { + name: "plain write-key SVM does not satisfy remote-key rotation", + migratorEnsureReplies: map[schema.GroupResource]map[string]finishedResultErr{ + {Group: "", Resource: "secrets"}: {"1": {finished: true}, remoteMigrationWriteKey: {finished: false}}, + {Group: "", Resource: "configmaps"}: {"1": {finished: true}, remoteMigrationWriteKey: {finished: false}}, + }, + expectedMigratorCalls: []string{ + "ensure:configmaps:" + remoteMigrationWriteKey, + "ensure:secrets:" + remoteMigrationWriteKey, + }, + expectProgressing: true, + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + keySecret, encryptionCfgSecret := kmsRemoteKeyMigrationSecrets(t, targetGRs) + migrator := &fakeMigrator{ensureReplies: scenario.migratorEnsureReplies} + fakeOperatorClient := v1helpers.NewFakeStaticPodOperatorClient( + &operatorv1.StaticPodOperatorSpec{OperatorSpec: operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}}, + &operatorv1.StaticPodOperatorStatus{ + OperatorStatus: operatorv1.OperatorStatus{ + Conditions: []operatorv1.OperatorCondition{ + {Type: "EncryptionMigrationControllerDegraded", Status: operatorv1.ConditionFalse}, + {Type: "EncryptionMigrationControllerProgressing", Status: operatorv1.ConditionFalse}, + }, + }, + NodeStatuses: []operatorv1.NodeStatus{{NodeName: "node-1"}}, + }, + nil, + nil, + ) + fakeKubeClient := fake.NewSimpleClientset( + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + keySecret, + encryptionCfgSecret, + ) + eventRecorder := events.NewRecorder(fakeKubeClient.CoreV1().Events("operator"), "test-encryption-migration-controller", &corev1.ObjectReference{}, clocktesting.NewFakePassiveClock(time.Now())) + kubeInformers := v1helpers.NewKubeInformersForNamespaces(fakeKubeClient, "openshift-config-managed", "kms") + deployer, err := encryptiondeployer.NewRevisionLabelPodDeployer("revision", "kms", kubeInformers, fakeKubeClient.CoreV1(), fakeKubeClient.CoreV1(), encryptiondeployer.StaticPodNodeProvider{OperatorClient: fakeOperatorClient}) + if err != nil { + t.Fatal(err) + } + + target := NewMigrationController( + "kms", + newTestProvider(targetGRs), + deployer, + alwaysFulfilledPreconditions, + migrator, + fakeOperatorClient, + configv1informers.NewSharedInformerFactory(configv1clientfake.NewSimpleClientset(), time.Minute).Config().V1().APIServers(), + kubeInformers, + fakeKubeClient.CoreV1(), + metav1.ListOptions{}, + eventRecorder, + ) + if err := target.Sync(context.TODO(), factory.NewSyncContext("test", eventRecorder)); err != nil { + t.Fatal(err) + } + + if !equalStringSlices(scenario.expectedMigratorCalls, migrator.calls) { + t.Fatalf("migrator calls:\n expected: %v\n got: %v", scenario.expectedMigratorCalls, migrator.calls) + } + + patchedMigratedRemoteKeyID := false + for _, action := range fakeKubeClient.Actions() { + if !action.Matches("update", "secrets") { + continue + } + secret := action.(clientgotesting.UpdateAction).GetObject().(*corev1.Secret) + if secret.Name != keySecret.Name { + continue + } + rk, err := secrets.ReadRemoteKeyAnnotations(secret) + if err != nil { + t.Fatalf("read remote key annotations: %v", err) + } + if rk.MigratedRemoteKeyID == remoteKeyNew { + patchedMigratedRemoteKeyID = true + } + } + if scenario.expectMigratedPatch && !patchedMigratedRemoteKeyID { + t.Fatal("expected migrated-remote-key-id to be patched to remote-new") + } + if !scenario.expectMigratedPatch && patchedMigratedRemoteKeyID { + t.Fatal("unexpected migrated-remote-key-id patch before suffixed remote-key SVM completed") + } + + progressing := operatorv1.ConditionFalse + _, status, _, err := fakeOperatorClient.GetStaticPodOperatorState() + if err != nil { + t.Fatal(err) + } + for _, cond := range status.Conditions { + if cond.Type == "EncryptionMigrationControllerProgressing" { + progressing = cond.Status + } + } + if scenario.expectProgressing && progressing != operatorv1.ConditionTrue { + t.Fatalf("expected progressing=True, got %s", progressing) + } + if !scenario.expectProgressing && progressing != operatorv1.ConditionFalse { + t.Fatalf("expected progressing=False, got %s", progressing) + } + }) + } +} + +func kmsRemoteKeyMigrationSecrets(t *testing.T, targetGRs []schema.GroupResource) (*corev1.Secret, *corev1.Secret) { + t.Helper() + + keySecret := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", targetGRs, 1) + secrets.ApplyRemoteKeyAnnotations(keySecret.Annotations, state.RemoteKeyState{ + TargetRemoteKeyID: remoteKeyNew, + MigratedRemoteKeyID: remoteKeyOld, + }) + keySecret.Annotations[secrets.EncryptionSecretMigratedTimestamp] = time.Now().Format(time.RFC3339) + migrated := secrets.MigratedGroupResources{Resources: targetGRs} + bs, err := json.Marshal(migrated) + if err != nil { + t.Fatal(err) + } + keySecret.Annotations[secrets.EncryptionSecretMigratedResources] = string(bs) + + kmsKey := apiserverconfigv1.Key{Name: "1", Secret: "NzFlYTdjOTE0MTlhNjhmZDEyMjRmODhkNTAzMTZiNGU="} + keysResForSecrets := encryptiondatatesting.EncryptionKeysResourceTuple{ + Resource: "secrets", + Keys: []apiserverconfigv1.Key{kmsKey}, + Modes: []string{"KMS"}, + } + keysResForConfigMaps := encryptiondatatesting.EncryptionKeysResourceTuple{ + Resource: "configmaps", + Keys: []apiserverconfigv1.Key{kmsKey}, + Modes: []string{"KMS"}, + } + encryptionCfgSecret := createEncryptionCfgSecret(t, "kms", "1", encryptiondatatesting.CreateEncryptionCfgWithWriteKey([]encryptiondatatesting.EncryptionKeysResourceTuple{ + keysResForConfigMaps, keysResForSecrets, + })) + return keySecret, encryptionCfgSecret +} + +func equalStringSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/pkg/operator/encryption/controllers/remote_key_reconciler.go b/pkg/operator/encryption/controllers/remote_key_reconciler.go new file mode 100644 index 0000000000..e29691623f --- /dev/null +++ b/pkg/operator/encryption/controllers/remote_key_reconciler.go @@ -0,0 +1,163 @@ +package controllers + +import ( + "context" + "fmt" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/utils/clock" + + operatorv1 "github.com/openshift/api/operator/v1" + + "github.com/openshift/library-go/pkg/operator/encryption/kms/health" + "github.com/openshift/library-go/pkg/operator/encryption/secrets" + "github.com/openshift/library-go/pkg/operator/encryption/state" +) + +const remoteKeyConvergenceDuration = 5 * time.Minute + +// reconcileRemoteKeyRotation maintains KMS remote key rotation annotations on the +// encryption key secret for the current KMS write key. External remote-key rotation +// does not mint a new encryption key secret or trigger stateController; only +// migrationController re-encrypts etcd data. +func reconcileRemoteKeyRotation( + ctx context.Context, + secretClient corev1client.SecretsGetter, + instanceName string, + encryptedGRs []schema.GroupResource, + desiredState map[schema.GroupResource]state.GroupResourceState, + encryptionStatus *operatorv1.KMSEncryptionStatus, + clock clock.Clock, +) error { + if encryptionStatus == nil { + return nil + } + + writeKey, ok := writeKeyForRemoteKeyRotation(desiredState) + if !ok { + return nil + } + + secretName := fmt.Sprintf("encryption-key-%s-%s", instanceName, writeKey.Key.Name) + rk, err := readRemoteKeyAnnotationsFromSecret(ctx, secretClient, secretName) + if err != nil { + return err + } + if len(rk.TargetRemoteKeyID) == 0 { + return nil + } + + // During KMS-to-KMS migration multiple plugin key IDs can report at once; scope + // convergence to the current write key's keyID so backup/read-only plugins are ignored. + // TODO(thomas): we need to ensure the amount of reports match the number of operand pods + reports := health.ReportsForKeyID(encryptionStatus.HealthReports, writeKey.Key.Name) + convergedRemoteKeyID := health.ConvergedRemoteKeyID(reports) + if convergedRemoteKeyID == "" { + return nil + } + + if !secrets.IsBootstrapped(rk) { + allMigrated, _, _ := state.MigratedFor(encryptedGRs, writeKey) + if !allMigrated { + return nil + } + + err := secrets.PatchRemoteKeyAnnotations(ctx, secretClient.Secrets("openshift-config-managed"), secretName, func(rk *secrets.RemoteKeyAnnotations) (bool, error) { + if secrets.IsBootstrapped(*rk) { + return false, nil + } + if len(rk.TargetRemoteKeyID) == 0 { + return false, nil + } + rk.MigratedRemoteKeyID = rk.TargetRemoteKeyID + return true, nil + }) + + return err + } + + if convergedRemoteKeyID == rk.TargetRemoteKeyID { + return clearRemoteKeyConvergence(ctx, secretClient, secretName, rk) + } + + now := clock.Now() + promote, err := shouldPromoteConvergedKeyID(rk, convergedRemoteKeyID, now) + if err != nil { + return err + } + + if promote { + err = secrets.PatchRemoteKeyAnnotations(ctx, secretClient.Secrets("openshift-config-managed"), secretName, func(rk *secrets.RemoteKeyAnnotations) (bool, error) { + if rk.TargetRemoteKeyID == convergedRemoteKeyID { + return false, nil + } + if secrets.NeedsRemoteKeyMigration(*rk) { + return false, nil + } + rk.TargetRemoteKeyID = convergedRemoteKeyID + rk.ConvergedID = "" + rk.ConvergedAt = time.Time{} + return true, nil + }) + return err + } + + err = secrets.PatchRemoteKeyAnnotations(ctx, secretClient.Secrets("openshift-config-managed"), secretName, func(rk *secrets.RemoteKeyAnnotations) (bool, error) { + if rk.ConvergedID == convergedRemoteKeyID && !rk.ConvergedAt.IsZero() { + return false, nil + } + rk.ConvergedID = convergedRemoteKeyID + rk.ConvergedAt = now + return true, nil + }) + + return err +} + +func writeKeyForRemoteKeyRotation(desiredState map[schema.GroupResource]state.GroupResourceState) (state.KeyState, bool) { + for _, grState := range desiredState { + if grState.HasWriteKey() && grState.WriteKey.Mode == state.KMS { + return grState.WriteKey, true + } + } + return state.KeyState{}, false +} + +func readRemoteKeyAnnotationsFromSecret(ctx context.Context, secretClient corev1client.SecretsGetter, secretName string) (secrets.RemoteKeyAnnotations, error) { + s, err := secretClient.Secrets("openshift-config-managed").Get(ctx, secretName, metav1.GetOptions{}) + if err != nil { + return secrets.RemoteKeyAnnotations{}, err + } + return secrets.ReadRemoteKeyAnnotations(s) +} + +func shouldPromoteConvergedKeyID(rk secrets.RemoteKeyAnnotations, candidateRemoteKeyID string, now time.Time) (bool, error) { + if rk.ConvergedID != candidateRemoteKeyID || rk.ConvergedAt.IsZero() { + return false, nil + } + elapsed := now.Sub(rk.ConvergedAt) + if elapsed < remoteKeyConvergenceDuration { + return false, nil + } + if secrets.NeedsRemoteKeyMigration(rk) { + return false, nil + } + return true, nil +} + +func clearRemoteKeyConvergence(ctx context.Context, secretClient corev1client.SecretsGetter, secretName string, rk secrets.RemoteKeyAnnotations) error { + if rk.ConvergedID == "" && rk.ConvergedAt.IsZero() { + return nil + } + return secrets.PatchRemoteKeyAnnotations(ctx, secretClient.Secrets("openshift-config-managed"), secretName, func(rk *secrets.RemoteKeyAnnotations) (bool, error) { + if rk.ConvergedID == "" && rk.ConvergedAt.IsZero() { + return false, nil + } + rk.ConvergedID = "" + rk.ConvergedAt = time.Time{} + return true, nil + }) +} diff --git a/pkg/operator/encryption/controllers/remote_key_reconciler_test.go b/pkg/operator/encryption/controllers/remote_key_reconciler_test.go new file mode 100644 index 0000000000..ece7cbd049 --- /dev/null +++ b/pkg/operator/encryption/controllers/remote_key_reconciler_test.go @@ -0,0 +1,252 @@ +package controllers + +import ( + "context" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes/fake" + clocktesting "k8s.io/utils/clock/testing" + + operatorv1 "github.com/openshift/api/operator/v1" + apiserverconfigv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" + + "github.com/openshift/library-go/pkg/operator/encryption/secrets" + "github.com/openshift/library-go/pkg/operator/encryption/state" +) + +func TestReconcileRemoteKeyBootstrap(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "openshift-config-managed", + Name: "encryption-key-test-3", + Annotations: map[string]string{}, + }, + } + secrets.ApplyRemoteKeyAnnotations(secret.Annotations, state.RemoteKeyState{TargetRemoteKeyID: "remote-old"}) + client := fake.NewSimpleClientset(secret) + + writeKey := state.KeyState{ + Key: apiserverconfigv1.Key{Name: "3", Secret: "c2VjcmV0"}, + Mode: state.KMS, + Migrated: state.MigrationState{ + Resources: []schema.GroupResource{{Resource: "secrets"}}, + }, + KMS: &state.KMSState{ + RemoteKey: state.RemoteKeyState{TargetRemoteKeyID: "remote-old"}, + }, + } + desired := map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: {WriteKey: writeKey}, + } + status := &operatorv1.KMSEncryptionStatus{ + HealthReports: []operatorv1.KMSPluginHealthReport{ + {KeyID: "3", RemoteKeyID: "remote-new"}, + {KeyID: "3", RemoteKeyID: "remote-new"}, + }, + } + + err := reconcileRemoteKeyRotation(context.Background(), client.CoreV1(), "test", []schema.GroupResource{{Resource: "secrets"}}, desired, status, clocktesting.NewFakeClock(time.Now())) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updated, err := client.CoreV1().Secrets("openshift-config-managed").Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get secret: %v", err) + } + rk, err := secrets.ReadRemoteKeyAnnotations(updated) + if err != nil { + t.Fatalf("read remote key annotations: %v", err) + } + if rk.MigratedRemoteKeyID != "remote-old" { + t.Fatalf("expected bootstrap migrated-remote-key-id=remote-old, got %q", rk.MigratedRemoteKeyID) + } +} + +func TestReconcileRemoteKeyPromotion(t *testing.T) { + start := time.Date(2026, 8, 31, 10, 0, 0, 0, time.UTC) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "openshift-config-managed", + Name: "encryption-key-test-3", + Annotations: map[string]string{}, + }, + } + secrets.ApplyRemoteKeyAnnotations(secret.Annotations, state.RemoteKeyState{ + TargetRemoteKeyID: "remote-old", + MigratedRemoteKeyID: "remote-old", + ConvergedID: "remote-new", + ConvergedAt: start, + }) + client := fake.NewSimpleClientset(secret) + + writeKey := state.KeyState{ + Key: apiserverconfigv1.Key{Name: "3", Secret: "c2VjcmV0"}, + Mode: state.KMS, + Migrated: state.MigrationState{ + Resources: []schema.GroupResource{{Resource: "secrets"}}, + }, + KMS: &state.KMSState{ + RemoteKey: state.RemoteKeyState{ + TargetRemoteKeyID: "remote-old", + MigratedRemoteKeyID: "remote-old", + ConvergedID: "remote-new", + ConvergedAt: start, + }, + }, + } + desired := map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: {WriteKey: writeKey}, + } + status := &operatorv1.KMSEncryptionStatus{ + HealthReports: []operatorv1.KMSPluginHealthReport{ + {KeyID: "3", RemoteKeyID: "remote-new"}, + {KeyID: "3", RemoteKeyID: "remote-new"}, + }, + } + + err := reconcileRemoteKeyRotation(context.Background(), client.CoreV1(), "test", []schema.GroupResource{{Resource: "secrets"}}, desired, status, clocktesting.NewFakeClock(start.Add(6*time.Minute))) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updated, err := client.CoreV1().Secrets("openshift-config-managed").Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get secret: %v", err) + } + rk, err := secrets.ReadRemoteKeyAnnotations(updated) + if err != nil { + t.Fatalf("read remote key annotations: %v", err) + } + if rk.TargetRemoteKeyID != "remote-new" { + t.Fatalf("expected promoted target-remote-key-id=remote-new, got %q", rk.TargetRemoteKeyID) + } + if len(rk.ConvergedID) > 0 || !rk.ConvergedAt.IsZero() { + t.Fatal("expected convergence annotations to be cleared after promotion") + } +} + +func TestReconcileRemoteKeyIgnoresOtherKeyIDReports(t *testing.T) { + start := time.Date(2026, 8, 31, 10, 0, 0, 0, time.UTC) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "openshift-config-managed", + Name: "encryption-key-test-3", + Annotations: map[string]string{}, + }, + } + secrets.ApplyRemoteKeyAnnotations(secret.Annotations, state.RemoteKeyState{ + TargetRemoteKeyID: "remote-old", + MigratedRemoteKeyID: "remote-old", + ConvergedID: "remote-new", + ConvergedAt: start, + }) + client := fake.NewSimpleClientset(secret) + + writeKey := state.KeyState{ + Key: apiserverconfigv1.Key{Name: "3", Secret: "c2VjcmV0"}, + Mode: state.KMS, + Migrated: state.MigrationState{ + Resources: []schema.GroupResource{{Resource: "secrets"}}, + }, + KMS: &state.KMSState{ + RemoteKey: state.RemoteKeyState{ + TargetRemoteKeyID: "remote-old", + MigratedRemoteKeyID: "remote-old", + ConvergedID: "remote-new", + ConvergedAt: start, + }, + }, + } + desired := map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: {WriteKey: writeKey}, + } + status := &operatorv1.KMSEncryptionStatus{ + HealthReports: []operatorv1.KMSPluginHealthReport{ + {KeyID: "3", RemoteKeyID: "remote-new"}, + {KeyID: "3", RemoteKeyID: "remote-new"}, + {KeyID: "2", RemoteKeyID: "remote-old"}, + }, + } + + err := reconcileRemoteKeyRotation(context.Background(), client.CoreV1(), "test", []schema.GroupResource{{Resource: "secrets"}}, desired, status, clocktesting.NewFakeClock(start.Add(6*time.Minute))) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updated, err := client.CoreV1().Secrets("openshift-config-managed").Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get secret: %v", err) + } + rk, err := secrets.ReadRemoteKeyAnnotations(updated) + if err != nil { + t.Fatalf("read remote key annotations: %v", err) + } + if rk.TargetRemoteKeyID != "remote-new" { + t.Fatalf("expected promoted target-remote-key-id=remote-new, got %q", rk.TargetRemoteKeyID) + } +} + +func TestReconcileRemoteKeyDeferredPromotion(t *testing.T) { + start := time.Date(2026, 8, 31, 10, 0, 0, 0, time.UTC) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "openshift-config-managed", + Name: "encryption-key-test-3", + Annotations: map[string]string{}, + }, + } + secrets.ApplyRemoteKeyAnnotations(secret.Annotations, state.RemoteKeyState{ + TargetRemoteKeyID: "remote-a", + MigratedRemoteKeyID: "remote-old", + ConvergedID: "remote-b", + ConvergedAt: start, + }) + client := fake.NewSimpleClientset(secret) + + writeKey := state.KeyState{ + Key: apiserverconfigv1.Key{Name: "3", Secret: "c2VjcmV0"}, + Mode: state.KMS, + Migrated: state.MigrationState{ + Resources: []schema.GroupResource{{Resource: "secrets"}}, + }, + KMS: &state.KMSState{ + RemoteKey: state.RemoteKeyState{ + TargetRemoteKeyID: "remote-a", + MigratedRemoteKeyID: "remote-old", + ConvergedID: "remote-b", + ConvergedAt: start, + }, + }, + } + desired := map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: {WriteKey: writeKey}, + } + status := &operatorv1.KMSEncryptionStatus{ + HealthReports: []operatorv1.KMSPluginHealthReport{ + {KeyID: "3", RemoteKeyID: "remote-b"}, + {KeyID: "3", RemoteKeyID: "remote-b"}, + }, + } + + err := reconcileRemoteKeyRotation(context.Background(), client.CoreV1(), "test", []schema.GroupResource{{Resource: "secrets"}}, desired, status, clocktesting.NewFakeClock(start.Add(6*time.Minute))) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updated, err := client.CoreV1().Secrets("openshift-config-managed").Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get secret: %v", err) + } + rk, err := secrets.ReadRemoteKeyAnnotations(updated) + if err != nil { + t.Fatalf("read remote key annotations: %v", err) + } + if rk.TargetRemoteKeyID != "remote-a" { + t.Fatalf("expected target to remain remote-a during in-flight migration, got %q", rk.TargetRemoteKeyID) + } +} diff --git a/pkg/operator/encryption/secrets/remote_key.go b/pkg/operator/encryption/secrets/remote_key.go new file mode 100644 index 0000000000..788995b7ed --- /dev/null +++ b/pkg/operator/encryption/secrets/remote_key.go @@ -0,0 +1,125 @@ +package secrets + +import ( + "context" + "fmt" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/util/retry" + + "github.com/openshift/library-go/pkg/operator/encryption/state" +) + +// RemoteKeyAnnotations is the annotation-backed view of state.RemoteKeyState. +type RemoteKeyAnnotations = state.RemoteKeyState + +// ReadRemoteKeyAnnotations reads remote key rotation annotations from a key secret. +func ReadRemoteKeyAnnotations(s *corev1.Secret) (RemoteKeyAnnotations, error) { + if s == nil { + return RemoteKeyAnnotations{}, nil + } + return readRemoteKeyAnnotations(s.Annotations, s.Namespace, s.Name) +} + +func readRemoteKeyAnnotations(annotations map[string]string, namespace, name string) (RemoteKeyAnnotations, error) { + rk := RemoteKeyAnnotations{ + TargetRemoteKeyID: annotations[encryptionSecretTargetRemoteKeyID], + MigratedRemoteKeyID: annotations[encryptionSecretMigratedRemoteKeyID], + ConvergedID: annotations[encryptionSecretRemoteKeyConvergedID], + } + if v, ok := annotations[encryptionSecretRemoteKeyConvergedAt]; ok && len(v) > 0 { + ts, err := time.Parse(time.RFC3339, v) + if err != nil { + return RemoteKeyAnnotations{}, fmt.Errorf("secret %s/%s has invalid %s annotation: %v", namespace, name, encryptionSecretRemoteKeyConvergedAt, err) + } + rk.ConvergedAt = ts + } + return rk, nil +} + +// ApplyRemoteKeyAnnotations writes remote key rotation annotations into the given map. +// Empty values remove the corresponding annotation keys. +func ApplyRemoteKeyAnnotations(annotations map[string]string, rk RemoteKeyAnnotations) { + setOrDeleteAnnotation(annotations, encryptionSecretTargetRemoteKeyID, rk.TargetRemoteKeyID) + setOrDeleteAnnotation(annotations, encryptionSecretMigratedRemoteKeyID, rk.MigratedRemoteKeyID) + setOrDeleteAnnotation(annotations, encryptionSecretRemoteKeyConvergedID, rk.ConvergedID) + if rk.ConvergedAt.IsZero() { + delete(annotations, encryptionSecretRemoteKeyConvergedAt) + } else { + annotations[encryptionSecretRemoteKeyConvergedAt] = rk.ConvergedAt.Format(time.RFC3339) + } +} + +func setOrDeleteAnnotation(annotations map[string]string, key, value string) { + if len(value) == 0 { + delete(annotations, key) + return + } + annotations[key] = value +} + +// NeedsRemoteKeyMigration reports whether migrated-remote-key-id is set, +// target-remote-key-id is non-empty, and they differ (needsMigration). +func NeedsRemoteKeyMigration(rk RemoteKeyAnnotations) bool { + return len(rk.MigratedRemoteKeyID) > 0 && + len(rk.TargetRemoteKeyID) > 0 && + rk.MigratedRemoteKeyID != rk.TargetRemoteKeyID +} + +// IsBootstrapped reports whether the initial remote key bootstrap has completed. +func IsBootstrapped(rk RemoteKeyAnnotations) bool { + return len(rk.MigratedRemoteKeyID) > 0 +} + +// MigrationWriteKeyName returns the StorageVersionMigration write-key annotation value. +// When target-remote-key-id is set, the write-key is always suffixed with that ID +// (first enablement and remote-key rotation). Plain keyName is used only when +// target-remote-key-id is unset. +func MigrationWriteKeyName(keyName string, rk RemoteKeyAnnotations) string { + if len(rk.TargetRemoteKeyID) == 0 { + return keyName + } + return keyName + "-" + rk.TargetRemoteKeyID +} + +// RemoteKeyIDFromMigrationWriteKey extracts the remote key ID suffix from a migration write-key value. +func RemoteKeyIDFromMigrationWriteKey(keyName, migrationWriteKey string) (string, bool) { + prefix := keyName + "-" + if !strings.HasPrefix(migrationWriteKey, prefix) { + return "", false + } + remoteKeyID := strings.TrimPrefix(migrationWriteKey, prefix) + if len(remoteKeyID) == 0 { + return "", false + } + return remoteKeyID, true +} + +// PatchRemoteKeyAnnotations updates remote key annotations on a key secret using +// get-modify-update with conflict retry. Other annotations are preserved. +func PatchRemoteKeyAnnotations(ctx context.Context, client corev1client.SecretInterface, secretName string, mutate func(*RemoteKeyAnnotations) (bool, error)) error { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + s, err := client.Get(ctx, secretName, metav1.GetOptions{}) + if err != nil { + return err + } + rk, err := ReadRemoteKeyAnnotations(s) + if err != nil { + return err + } + changed, err := mutate(&rk) + if err != nil || !changed { + return err + } + if s.Annotations == nil { + s.Annotations = map[string]string{} + } + ApplyRemoteKeyAnnotations(s.Annotations, rk) + _, updateErr := client.Update(ctx, s, metav1.UpdateOptions{}) + return updateErr + }) +} diff --git a/pkg/operator/encryption/secrets/remote_key_patch_test.go b/pkg/operator/encryption/secrets/remote_key_patch_test.go new file mode 100644 index 0000000000..f96f7b1712 --- /dev/null +++ b/pkg/operator/encryption/secrets/remote_key_patch_test.go @@ -0,0 +1,83 @@ +package secrets + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + + "github.com/openshift/library-go/pkg/operator/encryption/state" +) + +func TestPatchRemoteKeyAnnotationsPreservesOtherAnnotations(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "openshift-config-managed", + Name: "encryption-key-test-3", + Annotations: map[string]string{ + encryptionSecretTargetRemoteKeyID: "remote-old", + encryptionSecretMigratedRemoteKeyID: "remote-old", + EncryptionSecretMigratedTimestamp: "2026-08-31T10:00:00Z", + }, + }, + } + client := fake.NewSimpleClientset(secret) + + err := PatchRemoteKeyAnnotations(context.Background(), client.CoreV1().Secrets(secret.Namespace), secret.Name, func(rk *RemoteKeyAnnotations) (bool, error) { + rk.TargetRemoteKeyID = "remote-new" + return true, nil + }) + if err != nil { + t.Fatalf("patch failed: %v", err) + } + + updated, err := client.CoreV1().Secrets(secret.Namespace).Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get secret: %v", err) + } + if updated.Annotations[encryptionSecretTargetRemoteKeyID] != "remote-new" { + t.Fatalf("target not updated") + } + if updated.Annotations[EncryptionSecretMigratedTimestamp] == "" { + t.Fatal("expected migrated timestamp annotation to be preserved") + } +} + +func TestPatchRemoteKeyAnnotationsConcurrentWriters(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "openshift-config-managed", + Name: "encryption-key-test-3", + Annotations: map[string]string{}, + }, + } + client := fake.NewSimpleClientset(secret) + + if err := PatchRemoteKeyAnnotations(context.Background(), client.CoreV1().Secrets(secret.Namespace), secret.Name, func(rk *RemoteKeyAnnotations) (bool, error) { + rk.TargetRemoteKeyID = "remote-new" + return true, nil + }); err != nil { + t.Fatalf("first patch failed: %v", err) + } + + if err := PatchRemoteKeyAnnotations(context.Background(), client.CoreV1().Secrets(secret.Namespace), secret.Name, func(rk *RemoteKeyAnnotations) (bool, error) { + rk.MigratedRemoteKeyID = "remote-new" + return true, nil + }); err != nil { + t.Fatalf("second patch failed: %v", err) + } + + updated, err := client.CoreV1().Secrets(secret.Namespace).Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get secret: %v", err) + } + rk := state.RemoteKeyState{ + TargetRemoteKeyID: updated.Annotations[encryptionSecretTargetRemoteKeyID], + MigratedRemoteKeyID: updated.Annotations[encryptionSecretMigratedRemoteKeyID], + } + if rk.TargetRemoteKeyID != "remote-new" || rk.MigratedRemoteKeyID != "remote-new" { + t.Fatalf("unexpected annotations: %#v", updated.Annotations) + } +} diff --git a/pkg/operator/encryption/secrets/remote_key_test.go b/pkg/operator/encryption/secrets/remote_key_test.go new file mode 100644 index 0000000000..68fc8e8ce6 --- /dev/null +++ b/pkg/operator/encryption/secrets/remote_key_test.go @@ -0,0 +1,119 @@ +package secrets + +import ( + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/openshift/library-go/pkg/operator/encryption/state" +) + +func TestReadRemoteKeyAnnotations(t *testing.T) { + ts := time.Date(2026, 8, 31, 10, 0, 0, 0, time.UTC) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "openshift-config-managed", + Name: "encryption-key-test-1", + Annotations: map[string]string{ + encryptionSecretTargetRemoteKeyID: "remote-old", + encryptionSecretMigratedRemoteKeyID: "remote-old", + encryptionSecretRemoteKeyConvergedID: "remote-new", + encryptionSecretRemoteKeyConvergedAt: ts.Format(time.RFC3339), + }, + }, + } + + got, err := ReadRemoteKeyAnnotations(secret) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.TargetRemoteKeyID != "remote-old" || got.MigratedRemoteKeyID != "remote-old" { + t.Fatalf("unexpected ids: %#v", got) + } + if got.ConvergedID != "remote-new" || !got.ConvergedAt.Equal(ts) { + t.Fatalf("unexpected convergence: %#v", got) + } +} + +func TestNeedsRemoteKeyMigration(t *testing.T) { + scenarios := []struct { + name string + rk state.RemoteKeyState + want bool + }{ + {name: "unset migrated", rk: state.RemoteKeyState{TargetRemoteKeyID: "a"}, want: false}, + {name: "equal", rk: state.RemoteKeyState{TargetRemoteKeyID: "a", MigratedRemoteKeyID: "a"}, want: false}, + {name: "differs", rk: state.RemoteKeyState{TargetRemoteKeyID: "b", MigratedRemoteKeyID: "a"}, want: true}, + } + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + if got := NeedsRemoteKeyMigration(scenario.rk); got != scenario.want { + t.Fatalf("got %v want %v", got, scenario.want) + } + }) + } +} + +func TestMigrationWriteKeyName(t *testing.T) { + scenarios := []struct { + name string + keyName string + rk state.RemoteKeyState + want string + }{ + { + name: "first enablement", + keyName: "3", + rk: state.RemoteKeyState{TargetRemoteKeyID: "remote-old"}, + want: "3-remote-old", + }, + { + name: "no target", + keyName: "3", + rk: state.RemoteKeyState{}, + want: "3", + }, + { + name: "steady state", + keyName: "3", + rk: state.RemoteKeyState{TargetRemoteKeyID: "remote-old", MigratedRemoteKeyID: "remote-old"}, + want: "3-remote-old", + }, + { + name: "rotation", + keyName: "3", + rk: state.RemoteKeyState{TargetRemoteKeyID: "remote-new", MigratedRemoteKeyID: "remote-old"}, + want: "3-remote-new", + }, + } + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + if got := MigrationWriteKeyName(scenario.keyName, scenario.rk); got != scenario.want { + t.Fatalf("got %q want %q", got, scenario.want) + } + }) + } +} + +func TestRemoteKeyIDFromMigrationWriteKey(t *testing.T) { + got, ok := RemoteKeyIDFromMigrationWriteKey("3", "3-remote-new") + if !ok || got != "remote-new" { + t.Fatalf("got %q ok=%v", got, ok) + } + _, ok = RemoteKeyIDFromMigrationWriteKey("3", "3") + if ok { + t.Fatal("expected false for plain key name") + } +} + +func TestApplyRemoteKeyAnnotationsClearsEmptyValues(t *testing.T) { + annotations := map[string]string{ + encryptionSecretTargetRemoteKeyID: "old", + } + ApplyRemoteKeyAnnotations(annotations, state.RemoteKeyState{}) + if _, ok := annotations[encryptionSecretTargetRemoteKeyID]; ok { + t.Fatal("expected target annotation to be removed") + } +} diff --git a/pkg/operator/encryption/secrets/secrets.go b/pkg/operator/encryption/secrets/secrets.go index 716764f08d..b058d51000 100644 --- a/pkg/operator/encryption/secrets/secrets.go +++ b/pkg/operator/encryption/secrets/secrets.go @@ -66,6 +66,11 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) { key.Mode = keyMode case state.KMS: key.KMS = &state.KMSState{} + remoteKey, err := readRemoteKeyAnnotations(s.Annotations, s.Namespace, s.Name) + if err != nil { + return state.KeyState{}, err + } + key.KMS.RemoteKey = remoteKey if v, ok := s.Data[EncryptionSecretKMSEncryptionConfig]; ok && len(v) > 0 { kmsConfiguration, err := encoding.DecodeKMSConfiguration(v) if err != nil { @@ -162,6 +167,10 @@ func FromKeyState(component string, ks state.KeyState) (*corev1.Secret, error) { s.Annotations[EncryptionSecretMigratedResources] = string(bs) } + if ks.Mode == state.KMS { + ApplyRemoteKeyAnnotations(s.Annotations, ks.RemoteKey()) + } + if ks.HasKMSEncryption() { encryptionConfigurationData, err := encoding.EncodeKMSConfiguration(ks.KMS.Encryption) if err != nil { diff --git a/pkg/operator/encryption/secrets/secrets_test.go b/pkg/operator/encryption/secrets/secrets_test.go index 0051e77f33..a2826f0562 100644 --- a/pkg/operator/encryption/secrets/secrets_test.go +++ b/pkg/operator/encryption/secrets/secrets_test.go @@ -151,6 +151,10 @@ func TestRoundtrip(t *testing.T) { Timeout: &metav1.Duration{Duration: 10 * time.Second}, }, Plugin: defaultKMSPluginConfig, + RemoteKey: state.RemoteKeyState{ + TargetRemoteKeyID: "remote-new", + MigratedRemoteKeyID: "remote-old", + }, }, Migrated: state.MigrationState{ Timestamp: now, diff --git a/test/library/encryption/helpers.go b/test/library/encryption/helpers.go index 5ebe0ad924..4112676ffa 100644 --- a/test/library/encryption/helpers.go +++ b/test/library/encryption/helpers.go @@ -28,6 +28,8 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" + "github.com/openshift/library-go/pkg/operator/encryption/secrets" + oauthapiv1 "github.com/openshift/api/oauth/v1" routev1 "github.com/openshift/api/route/v1" "github.com/openshift/library-go/test/library" @@ -72,7 +74,7 @@ type ForceRotationFunc func(t testing.TB, ctx context.Context) // WaitForRotationCompleteFunc waits until re-migration after rotation has finished. // Static encryption waits for the next encryption key secret to be migrated; -// KMS waits for a new finished entry in KeyRotationStatus, created by the rotation controller. +// KMS waits until target-remote-key-id and migrated-remote-key-id converge on the new remote key ID. type WaitForRotationCompleteFunc func(t testing.TB, clientSet ClientSet, prevKeyMeta EncryptionKeyMeta, scenario BasicScenario) // StaticEncryptionForceRotation returns a ForceRotationFunc that mints a new key via encryption.reason. @@ -91,6 +93,61 @@ func WaitForNextEncryptionKeyRotation() WaitForRotationCompleteFunc { } } +// WaitForKMSRemoteKeyRotationComplete waits until the write-key secret's target and migrated +// remote key IDs converge. Unlike static encryption rotation, KMS remote key rotation does +// not mint a new encryption key secret. The helper first waits for migrated-remote-key-id +// to be populated, records that value, then waits for target-remote-key-id and +// migrated-remote-key-id to match on a different remote key ID. +func WaitForKMSRemoteKeyRotationComplete() WaitForRotationCompleteFunc { + return func(t testing.TB, clientSet ClientSet, prevKeyMeta EncryptionKeyMeta, scenario BasicScenario) { + t.Helper() + require.NotEmpty(t, prevKeyMeta.Name, "previous key name is required") + + t.Logf("Waiting for KMS remote key rotation to complete on secret %q", prevKeyMeta.Name) + var baselineRemoteKeyID string + var baselineCaptured bool + // Do not use t.Context(): Ginkgo's GinkgoTBWrapper promotes Context() to a nil + // embedded testing.TB and panics (nil pointer dereference). The poll timeout + // below is the cancellation bound, matching other helpers in this file. + err := wait.PollUntilContextTimeout(context.Background(), waitPollInterval, waitPollTimeout, true, func(ctx context.Context) (bool, error) { + secret, err := clientSet.Kube.CoreV1().Secrets(scenario.Namespace).Get(ctx, prevKeyMeta.Name, metav1.GetOptions{}) + if err != nil { + return false, nil + } + rk, err := secrets.ReadRemoteKeyAnnotations(secret) + if err != nil { + return false, err + } + if !baselineCaptured { + if len(rk.MigratedRemoteKeyID) == 0 { + t.Logf("KMS remote key pending bootstrap on %q: target=%q converged=%q", + prevKeyMeta.Name, rk.TargetRemoteKeyID, rk.ConvergedID) + return false, nil + } + baselineRemoteKeyID = rk.MigratedRemoteKeyID + baselineCaptured = true + t.Logf("Observed migrated-remote-key-id=%q on %q before waiting for rotation convergence", + baselineRemoteKeyID, prevKeyMeta.Name) + } + if len(rk.TargetRemoteKeyID) == 0 || len(rk.MigratedRemoteKeyID) == 0 { + return false, nil + } + if rk.TargetRemoteKeyID != rk.MigratedRemoteKeyID { + t.Logf("KMS remote key rotation pending on %q: target=%q migrated=%q converged=%q (baseline=%q)", + prevKeyMeta.Name, rk.TargetRemoteKeyID, rk.MigratedRemoteKeyID, rk.ConvergedID, baselineRemoteKeyID) + return false, nil + } + if rk.MigratedRemoteKeyID == baselineRemoteKeyID { + return false, nil + } + t.Logf("KMS remote key rotation complete on %q: target=migrated=%q (baseline=%q)", + prevKeyMeta.Name, rk.MigratedRemoteKeyID, baselineRemoteKeyID) + return true, nil + }) + require.NoError(t, err) + } +} + func SetAndWaitForEncryptionType(ctx context.Context, t testing.TB, provider EncryptionProvider, defaultTargetGRs []schema.GroupResource, namespace, labelSelector string) ClientSet { t.Helper()