From 09b25bc8b608c7c8efbd2fcc29ed5d55a284d5ce Mon Sep 17 00:00:00 2001 From: Thomas Jungblut Date: Thu, 11 Jun 2026 13:10:37 +0200 Subject: [PATCH 1/3] NO-JIRA: kms rotation prototype --- .../controllerset/apiservercontrollerset.go | 7 + pkg/operator/encryption/controllers.go | 14 + .../encryption/controllers/key_controller.go | 5 + .../controllers/key_controller_test.go | 18 ++ .../controllers/kms_rotation_controller.go | 272 ++++++++++++++++++ .../kms_rotation_controller_test.go | 151 ++++++++++ .../controllers/migration_controller.go | 83 +++++- pkg/operator/encryption/secrets/kek.go | 58 ++++ pkg/operator/encryption/secrets/kek_test.go | 107 +++++++ pkg/operator/encryption/secrets/secrets.go | 10 + pkg/operator/encryption/secrets/types.go | 15 + pkg/operator/encryption/state/types.go | 21 ++ .../encryption/statemachine/transition.go | 4 + .../statemachine/transition_test.go | 40 +++ test/e2e-encryption/encryption_test.go | 1 + 15 files changed, 802 insertions(+), 4 deletions(-) create mode 100644 pkg/operator/encryption/controllers/kms_rotation_controller.go create mode 100644 pkg/operator/encryption/controllers/kms_rotation_controller_test.go create mode 100644 pkg/operator/encryption/secrets/kek.go create mode 100644 pkg/operator/encryption/secrets/kek_test.go diff --git a/pkg/operator/apiserver/controllerset/apiservercontrollerset.go b/pkg/operator/apiserver/controllerset/apiservercontrollerset.go index 938fd965f7..70fa0e5648 100644 --- a/pkg/operator/apiserver/controllerset/apiservercontrollerset.go +++ b/pkg/operator/apiserver/controllerset/apiservercontrollerset.go @@ -394,6 +394,11 @@ func (cs *APIServerControllerSet) WithUnsupportedConfigPrefixForEncryptionContro return cs } +func (cs *APIServerControllerSet) WithConvergedKEKReporterForEncryptionControllers(reporter controllers.ConvergedKEKReporter) *APIServerControllerSet { + cs.encryptionControllers.convergedKEKReporter = reporter + return cs +} + func (cs *APIServerControllerSet) WithoutEncryptionControllers() *APIServerControllerSet { cs.encryptionControllers.controller = nil cs.encryptionControllers.emptyAllowed = true @@ -485,6 +490,7 @@ type encryptionControllerBuilder struct { provider controllers.Provider deployer statemachine.Deployer migrator migrators.Migrator + convergedKEKReporter controllers.ConvergedKEKReporter secretsClient corev1.SecretsGetter configMapClient corev1.ConfigMapsGetter apiServerClient configv1client.APIServerInterface @@ -506,6 +512,7 @@ func (e *encryptionControllerBuilder) build() []controllerWrapper { e.provider, e.deployer, e.migrator, + e.convergedKEKReporter, e.operatorClient, e.apiServerClient, e.apiServerInformer, diff --git a/pkg/operator/encryption/controllers.go b/pkg/operator/encryption/controllers.go index 38c8357b33..9e6f655572 100644 --- a/pkg/operator/encryption/controllers.go +++ b/pkg/operator/encryption/controllers.go @@ -29,6 +29,7 @@ func NewControllers( provider controllers.Provider, deployer statemachine.Deployer, migrator migrators.Migrator, + convergedKEKReporter controllers.ConvergedKEKReporter, operatorClient operatorv1helpers.OperatorClient, apiServerClient configv1client.APIServerInterface, apiServerInformer configv1informers.APIServerInformer, @@ -125,6 +126,19 @@ func NewControllers( encryptionSecretSelector, eventRecorder, ), + controllers.NewKMSRotationController( + component, + provider, + deployer, + encryptionEnabledChecker.PreconditionFulfilled, + convergedKEKReporter, + operatorClient, + apiServerInformer, + kubeInformersForNamespaces, + secretsClient, + encryptionSecretSelector, + eventRecorder, + ), }, nil } diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index af38a8d59c..7482e6e752 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -391,6 +391,11 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern return 0, "", false } + // KMS KEK rotation migration must complete before minting a new key + if latestKey.NeedsKekMigration() { + return 0, "", false + } + // if the most recent secret was encrypted in a mode different than the current mode, we need to generate a new key if latestKey.Mode != currentMode { return latestKeyID, "encryption-mode-changed", true diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index 56fda9e0f1..8dee7ce8eb 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -521,6 +521,24 @@ func TestKeyController(t *testing.T) { // Should be no-op because migration hasn't completed yet expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, + { + name: "no-op when KMS kek migration is in flight", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: func() []runtime.Object { + secret := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5) + secret.Annotations["encryption.apiserver.operator.openshift.io/target-kek-id"] = "kek-new" + secret.Annotations["encryption.apiserver.operator.openshift.io/migrated-kek-id"] = "kek-old" + return []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + secret, + } + }(), + apiServerObjects: []runtime.Object{apiServerWithKMS}, + targetNamespace: "kms", + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, + }, { name: "creates a new KMS key when switching from Identity to KMS", diff --git a/pkg/operator/encryption/controllers/kms_rotation_controller.go b/pkg/operator/encryption/controllers/kms_rotation_controller.go new file mode 100644 index 0000000000..232b214e46 --- /dev/null +++ b/pkg/operator/encryption/controllers/kms_rotation_controller.go @@ -0,0 +1,272 @@ +package controllers + +import ( + "context" + "fmt" + "time" + + corev1 "k8s.io/api/core/v1" + 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/client-go/util/retry" + "k8s.io/klog/v2" + + 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/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" + "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" + operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers" +) + +// ConvergedKEKReporter provides the cluster-converged KMS KEK identity from health aggregation. +type ConvergedKEKReporter interface { + ConvergedKekID() (kekID string, converged bool) +} + +type kmsRotationController struct { + instanceName string + controllerInstanceName string + operatorClient operatorv1helpers.OperatorClient + secretClient corev1client.SecretsGetter + encryptionSecretSelector metav1.ListOptions + deployer statemachine.Deployer + provider Provider + preconditionsFulfilledFn preconditionsFulfilled + convergedKEKReporter ConvergedKEKReporter + now func() time.Time +} + +func NewKMSRotationController( + instanceName string, + provider Provider, + deployer statemachine.Deployer, + preconditionsFulfilledFn preconditionsFulfilled, + convergedKEKReporter ConvergedKEKReporter, + operatorClient operatorv1helpers.OperatorClient, + apiServerConfigInformer configv1informers.APIServerInformer, + kubeInformersForNamespaces operatorv1helpers.KubeInformersForNamespaces, + secretClient corev1client.SecretsGetter, + encryptionSecretSelector metav1.ListOptions, + eventRecorder events.Recorder, +) factory.Controller { + c := &kmsRotationController{ + instanceName: instanceName, + controllerInstanceName: factory.ControllerInstanceName(instanceName, "EncryptionKMSRotation"), + operatorClient: operatorClient, + encryptionSecretSelector: encryptionSecretSelector, + secretClient: secretClient, + deployer: deployer, + provider: provider, + preconditionsFulfilledFn: preconditionsFulfilledFn, + convergedKEKReporter: convergedKEKReporter, + now: time.Now, + } + + return factory.New().ResyncEvery(time.Minute).WithSync(c.sync).WithControllerInstanceName(c.controllerInstanceName).WithInformers( + operatorClient.Informer(), + kubeInformersForNamespaces.InformersFor("openshift-config-managed").Core().V1().Secrets().Informer(), + apiServerConfigInformer.Informer(), + deployer, + ).ToController( + c.controllerInstanceName, + eventRecorder.WithComponentSuffix("encryption-kms-rotation-controller"), + ) +} + +func (c *kmsRotationController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if c.convergedKEKReporter == nil { + return nil + } + + if ready, err := shouldRunEncryptionController(c.operatorClient, c.preconditionsFulfilledFn, c.provider.ShouldRunEncryptionControllers); err != nil || !ready { + return err + } + + _, _, encryptionSecrets, isTransitionalReason, err := statemachine.GetEncryptionConfigAndState( + ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, c.provider.EncryptedGRs(), + ) + if err != nil { + return err + } + if len(isTransitionalReason) > 0 { + return nil + } + + writeKeySecret, writeKeyState, ok := latestKMSWriteKeySecret(encryptionSecrets) + if !ok { + return nil + } + + convergedKekID, converged := c.convergedKEKReporter.ConvergedKekID() + if !converged || convergedKekID == "" { + return c.updateWriteKeySecret(ctx, syncCtx, writeKeySecret, clearKekConvergenceAnnotations) + } + + return c.reconcileKekAnnotations(ctx, syncCtx, writeKeySecret, writeKeyState, convergedKekID, c.provider.EncryptedGRs()) +} + +func latestKMSWriteKeySecret(encryptionSecrets []*corev1.Secret) (*corev1.Secret, state.KeyState, bool) { + var latestSecret *corev1.Secret + var latestKey state.KeyState + var latestKeyID uint64 + for _, s := range encryptionSecrets { + ks, err := secrets.ToKeyState(s) + if err != nil || ks.Mode != state.KMS { + continue + } + keyID, valid := state.NameToKeyID(s.Name) + if !valid { + continue + } + if latestSecret == nil || keyID > latestKeyID { + latestSecret = s + latestKey = ks + latestKeyID = keyID + } + } + return latestSecret, latestKey, latestSecret != nil +} + +func (c *kmsRotationController) reconcileKekAnnotations( + ctx context.Context, + syncCtx factory.SyncContext, + writeKeySecret *corev1.Secret, + writeKeyState state.KeyState, + convergedKekID string, + encryptedGRs []schema.GroupResource, +) error { + kekMigration := secrets.KekMigrationFromSecret(writeKeySecret) + + // Bootstrap: initial migration complete, no kekId annotations yet. + if kekMigration.TargetKekID == "" && kekMigration.MigratedKekID == "" { + allMigrated, _, _ := state.MigratedFor(encryptedGRs, writeKeyState) + if !allMigrated { + return nil + } + return c.updateWriteKeySecret(ctx, syncCtx, writeKeySecret, func(s *corev1.Secret) (bool, error) { + return setKekBootstrapAnnotations(s, convergedKekID) + }) + } + + // Steady state or migration in flight: converged kekId matches current target. + if convergedKekID == kekMigration.TargetKekID { + if kekMigration.KekConvergedID != "" || !kekMigration.KekConvergedAt.IsZero() { + return c.updateWriteKeySecret(ctx, syncCtx, writeKeySecret, clearKekConvergenceAnnotations) + } + return nil + } + + // Candidate kekId differs from target: start or maintain the convergence clock. + if convergedKekID != kekMigration.KekConvergedID { + return c.updateWriteKeySecret(ctx, syncCtx, writeKeySecret, func(s *corev1.Secret) (bool, error) { + return setKekConvergenceClock(s, convergedKekID, c.now()) + }) + } + + if c.now().Sub(kekMigration.KekConvergedAt) >= secrets.KekConvergenceDelay { + return c.updateWriteKeySecret(ctx, syncCtx, writeKeySecret, func(s *corev1.Secret) (bool, error) { + return promoteConvergedKekToTarget(s, convergedKekID) + }) + } + + return nil +} + +type secretAnnotationMutator func(s *corev1.Secret) (changed bool, err error) + +func (c *kmsRotationController) updateWriteKeySecret(ctx context.Context, syncCtx factory.SyncContext, writeKeySecret *corev1.Secret, mutate secretAnnotationMutator) error { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + s, err := c.secretClient.Secrets(writeKeySecret.Namespace).Get(ctx, writeKeySecret.Name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get key secret %s/%s: %w", writeKeySecret.Namespace, writeKeySecret.Name, err) + } + + changed, err := mutate(s) + if err != nil { + return err + } + if !changed { + return nil + } + + _, updateErr := c.secretClient.Secrets(s.Namespace).Update(ctx, s, metav1.UpdateOptions{}) + resourcehelper.ReportUpdateEvent(syncCtx.Recorder(), s, updateErr) + return updateErr + }) +} + +func setKekBootstrapAnnotations(s *corev1.Secret, kekID string) (bool, error) { + if kekID == "" { + return false, nil + } + if s.Annotations == nil { + s.Annotations = map[string]string{} + } + if s.Annotations[secrets.EncryptionSecretTargetKekID] == kekID && + s.Annotations[secrets.EncryptionSecretMigratedKekID] == kekID { + return false, nil + } + s.Annotations[secrets.EncryptionSecretTargetKekID] = kekID + s.Annotations[secrets.EncryptionSecretMigratedKekID] = kekID + delete(s.Annotations, secrets.EncryptionSecretKekConvergedAt) + delete(s.Annotations, secrets.EncryptionSecretKekConvergedID) + klog.V(2).Infof("bootstrapped KMS kekId annotations on secret %s/%s to %q", s.Namespace, s.Name, kekID) + return true, nil +} + +func setKekConvergenceClock(s *corev1.Secret, kekID string, now time.Time) (bool, error) { + if kekID == "" { + return false, nil + } + if s.Annotations == nil { + s.Annotations = map[string]string{} + } + changed := false + if s.Annotations[secrets.EncryptionSecretKekConvergedID] != kekID { + s.Annotations[secrets.EncryptionSecretKekConvergedID] = kekID + s.Annotations[secrets.EncryptionSecretKekConvergedAt] = now.Format(time.RFC3339) + changed = true + } + if changed { + klog.V(2).Infof("started KMS kekId convergence clock on secret %s/%s for candidate %q", s.Namespace, s.Name, kekID) + } + return changed, nil +} + +func promoteConvergedKekToTarget(s *corev1.Secret, kekID string) (bool, error) { + if kekID == "" { + return false, nil + } + if s.Annotations == nil { + s.Annotations = map[string]string{} + } + if s.Annotations[secrets.EncryptionSecretTargetKekID] == kekID && + s.Annotations[secrets.EncryptionSecretKekConvergedID] == "" && + s.Annotations[secrets.EncryptionSecretKekConvergedAt] == "" { + return false, nil + } + s.Annotations[secrets.EncryptionSecretTargetKekID] = kekID + delete(s.Annotations, secrets.EncryptionSecretKekConvergedAt) + delete(s.Annotations, secrets.EncryptionSecretKekConvergedID) + klog.V(2).Infof("updated target-kek-id on secret %s/%s to %q after convergence delay", s.Namespace, s.Name, kekID) + return true, nil +} + +func clearKekConvergenceAnnotations(s *corev1.Secret) (bool, error) { + if s.Annotations == nil { + return false, nil + } + _, hasID := s.Annotations[secrets.EncryptionSecretKekConvergedID] + _, hasAt := s.Annotations[secrets.EncryptionSecretKekConvergedAt] + if !hasID && !hasAt { + return false, nil + } + delete(s.Annotations, secrets.EncryptionSecretKekConvergedID) + delete(s.Annotations, secrets.EncryptionSecretKekConvergedAt) + klog.V(4).Infof("cleared KMS kekId convergence clock on secret %s/%s", s.Namespace, s.Name) + return true, nil +} diff --git a/pkg/operator/encryption/controllers/kms_rotation_controller_test.go b/pkg/operator/encryption/controllers/kms_rotation_controller_test.go new file mode 100644 index 0000000000..3fadff700b --- /dev/null +++ b/pkg/operator/encryption/controllers/kms_rotation_controller_test.go @@ -0,0 +1,151 @@ +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" + clocktesting "k8s.io/utils/clock/testing" + + "k8s.io/client-go/kubernetes/fake" + + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/encryption/secrets" + encryptiontesting "github.com/openshift/library-go/pkg/operator/encryption/testing" + "github.com/openshift/library-go/pkg/operator/events" +) + +type fakeConvergedKEKReporter struct { + kekID string + converged bool +} + +func (f *fakeConvergedKEKReporter) ConvergedKekID() (string, bool) { + return f.kekID, f.converged +} + +func TestKMSRotationControllerAnnotationMutators(t *testing.T) { + t.Run("bootstrap sets equal target and migrated", func(t *testing.T) { + s := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}} + changed, err := setKekBootstrapAnnotations(s, "kek-1") + if err != nil || !changed { + t.Fatalf("setKekBootstrapAnnotations() changed=%v err=%v", changed, err) + } + if s.Annotations[secrets.EncryptionSecretTargetKekID] != "kek-1" || s.Annotations[secrets.EncryptionSecretMigratedKekID] != "kek-1" { + t.Fatalf("unexpected annotations: %#v", s.Annotations) + } + }) + + t.Run("convergence clock starts on new candidate", func(t *testing.T) { + now := time.Date(2026, 6, 11, 12, 0, 0, 0, time.UTC) + s := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ + secrets.EncryptionSecretTargetKekID: "kek-old", + }}} + changed, err := setKekConvergenceClock(s, "kek-new", now) + if err != nil || !changed { + t.Fatalf("setKekConvergenceClock() changed=%v err=%v", changed, err) + } + if s.Annotations[secrets.EncryptionSecretKekConvergedID] != "kek-new" { + t.Fatalf("unexpected converged id: %q", s.Annotations[secrets.EncryptionSecretKekConvergedID]) + } + if s.Annotations[secrets.EncryptionSecretKekConvergedAt] != now.Format(time.RFC3339) { + t.Fatalf("unexpected converged at: %q", s.Annotations[secrets.EncryptionSecretKekConvergedAt]) + } + }) + + t.Run("promote target after convergence delay", func(t *testing.T) { + s := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ + secrets.EncryptionSecretTargetKekID: "kek-old", + secrets.EncryptionSecretKekConvergedID: "kek-new", + secrets.EncryptionSecretKekConvergedAt: time.Now().Format(time.RFC3339), + secrets.EncryptionSecretMigratedKekID: "kek-old", + }}} + changed, err := promoteConvergedKekToTarget(s, "kek-new") + if err != nil || !changed { + t.Fatalf("promoteConvergedKekToTarget() changed=%v err=%v", changed, err) + } + if s.Annotations[secrets.EncryptionSecretTargetKekID] != "kek-new" { + t.Fatalf("target = %q", s.Annotations[secrets.EncryptionSecretTargetKekID]) + } + if _, ok := s.Annotations[secrets.EncryptionSecretKekConvergedID]; ok { + t.Fatalf("expected converged id cleared") + } + }) + + t.Run("clear convergence annotations", func(t *testing.T) { + s := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ + secrets.EncryptionSecretKekConvergedID: "kek-new", + secrets.EncryptionSecretKekConvergedAt: time.Now().Format(time.RFC3339), + }}} + changed, err := clearKekConvergenceAnnotations(s) + if err != nil || !changed { + t.Fatalf("clearKekConvergenceAnnotations() changed=%v err=%v", changed, err) + } + if len(s.Annotations) != 0 { + t.Fatalf("expected annotations cleared, got %#v", s.Annotations) + } + }) +} + +func TestKMSRotationControllerReconcileKekAnnotations(t *testing.T) { + grs := []schema.GroupResource{{Group: "", Resource: "secrets"}} + now := time.Date(2026, 6, 11, 12, 0, 0, 0, time.UTC) + + t.Run("bootstrap after initial migration", func(t *testing.T) { + secret := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", grs, 1) + secret.Annotations[secrets.EncryptionSecretMigratedTimestamp] = now.Format(time.RFC3339) + c := &kmsRotationController{ + now: func() time.Time { return now }, + } + writeKey, err := secrets.ToKeyState(secret) + if err != nil { + t.Fatal(err) + } + fakeClient := fake.NewSimpleClientset(secret) + c.secretClient = fakeClient.CoreV1() + eventRecorder := events.NewRecorder(fakeClient.CoreV1().Events("operator"), "test-kms-rotation", &corev1.ObjectReference{}, clocktesting.NewFakePassiveClock(now)) + syncCtx := factory.NewSyncContext("test", eventRecorder) + if err := c.reconcileKekAnnotations(context.Background(), syncCtx, secret, writeKey, "kek-1", grs); err != nil { + t.Fatal(err) + } + updated, err := fakeClient.CoreV1().Secrets(secret.Namespace).Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if updated.Annotations[secrets.EncryptionSecretTargetKekID] != "kek-1" || updated.Annotations[secrets.EncryptionSecretMigratedKekID] != "kek-1" { + t.Fatalf("unexpected annotations: %#v", updated.Annotations) + } + }) + + t.Run("promote after convergence delay", func(t *testing.T) { + secret := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", grs, 1) + secret.Annotations[secrets.EncryptionSecretTargetKekID] = "kek-old" + secret.Annotations[secrets.EncryptionSecretMigratedKekID] = "kek-old" + secret.Annotations[secrets.EncryptionSecretKekConvergedID] = "kek-new" + secret.Annotations[secrets.EncryptionSecretKekConvergedAt] = now.Add(-secrets.KekConvergenceDelay).Format(time.RFC3339) + c := &kmsRotationController{ + now: func() time.Time { return now }, + } + writeKey, err := secrets.ToKeyState(secret) + if err != nil { + t.Fatal(err) + } + fakeClient := fake.NewSimpleClientset(secret) + c.secretClient = fakeClient.CoreV1() + eventRecorder := events.NewRecorder(fakeClient.CoreV1().Events("operator"), "test-kms-rotation", &corev1.ObjectReference{}, clocktesting.NewFakePassiveClock(now)) + syncCtx := factory.NewSyncContext("test", eventRecorder) + if err := c.reconcileKekAnnotations(context.Background(), syncCtx, secret, writeKey, "kek-new", grs); err != nil { + t.Fatal(err) + } + updated, err := fakeClient.CoreV1().Secrets(secret.Namespace).Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if updated.Annotations[secrets.EncryptionSecretTargetKekID] != "kek-new" { + t.Fatalf("target = %q", updated.Annotations[secrets.EncryptionSecretTargetKekID]) + } + }) +} diff --git a/pkg/operator/encryption/controllers/migration_controller.go b/pkg/operator/encryption/controllers/migration_controller.go index 7fc649f020..682fb8c57d 100644 --- a/pkg/operator/encryption/controllers/migration_controller.go +++ b/pkg/operator/encryption/controllers/migration_controller.go @@ -215,39 +215,57 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C // we never want to migrate during an intermediate state because that could lead to one API server // 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 + writeKeySecret, err := writeKeySecretForState(encryptionSecrets, currentState) + if err != nil { + return nil, err + } + needsKekMigration := secrets.NeedsKekMigration(writeKeySecret) + var errs []error + kekMigrationComplete := needsKekMigration for _, gr := range grs { grActualKeys := currentState[gr] if !grActualKeys.HasWriteKey() { + kekMigrationComplete = false continue // no write key to migrate to } - if alreadyMigrated, _, _ := state.MigratedFor([]schema.GroupResource{gr}, grActualKeys.WriteKey); alreadyMigrated { - continue + if !needsKekMigration { + if alreadyMigrated, _, _ := state.MigratedFor([]schema.GroupResource{gr}, grActualKeys.WriteKey); alreadyMigrated { + continue + } + } + + writeKeyForGR := grActualKeys.WriteKey.Key.Name + if needsKekMigration { + writeKeyForGR = secrets.MigrationWriteKey(grActualKeys.WriteKey.Key.Name, writeKeySecret) } // idem-potent migration start - finished, result, when, err := c.migrator.EnsureMigration(gr, grActualKeys.WriteKey.Key.Name) + finished, result, when, err := c.migrator.EnsureMigration(gr, writeKeyForGR) 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) + finished, result, when, err = c.migrator.EnsureMigration(gr, writeKeyForGR) } if err != nil { errs = append(errs, err) + kekMigrationComplete = false continue } if finished && result != nil { errs = append(errs, result) + kekMigrationComplete = false continue } if !finished { migratingResources = append(migratingResources, gr) + kekMigrationComplete = false continue } @@ -279,13 +297,70 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C return updateErr }); err != nil { errs = append(errs, err) + kekMigrationComplete = false continue } } + if kekMigrationComplete && writeKeySecret != nil { + if err := c.setMigratedKekID(ctx, syncContext, writeKeySecret); err != nil { + errs = append(errs, err) + } + } + return migratingResources, errors.NewAggregate(errs) } +func writeKeySecretForState(encryptionSecrets []*corev1.Secret, currentState map[schema.GroupResource]state.GroupResourceState) (*corev1.Secret, error) { + var writeKey state.KeyState + for _, grState := range currentState { + if grState.HasWriteKey() { + writeKey = grState.WriteKey + break + } + } + if len(writeKey.Key.Name) == 0 { + return nil, nil + } + + for _, s := range encryptionSecrets { + keyID, valid := state.NameToKeyID(s.Name) + if !valid { + continue + } + writeKeyID, ok := state.NameToKeyID(writeKey.Key.Name) + if !ok || keyID != writeKeyID { + continue + } + return s, nil + } + return nil, fmt.Errorf("write key secret for key ID %s not found", writeKey.Key.Name) +} + +func (c *migrationController) setMigratedKekID(ctx context.Context, syncContext factory.SyncContext, writeKeySecret *corev1.Secret) error { + targetKekID := writeKeySecret.Annotations[secrets.EncryptionSecretTargetKekID] + if targetKekID == "" { + return nil + } + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + s, err := c.secretClient.Secrets(writeKeySecret.Namespace).Get(ctx, writeKeySecret.Name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get key secret %s/%s: %v", writeKeySecret.Namespace, writeKeySecret.Name, err) + } + if s.Annotations[secrets.EncryptionSecretMigratedKekID] == targetKekID { + return nil + } + if s.Annotations == nil { + s.Annotations = map[string]string{} + } + s.Annotations[secrets.EncryptionSecretMigratedKekID] = targetKekID + + _, updateErr := c.secretClient.Secrets(s.Namespace).Update(ctx, s, metav1.UpdateOptions{}) + resourcehelper.ReportUpdateEvent(syncContext.Recorder(), s, updateErr) + return updateErr + }) +} + func setResourceMigrated(gr schema.GroupResource, s *corev1.Secret) (bool, error) { migratedGRs := secrets.MigratedGroupResources{} if existing, found := s.Annotations[secrets.EncryptionSecretMigratedResources]; found { diff --git a/pkg/operator/encryption/secrets/kek.go b/pkg/operator/encryption/secrets/kek.go new file mode 100644 index 0000000000..2e2a8a729c --- /dev/null +++ b/pkg/operator/encryption/secrets/kek.go @@ -0,0 +1,58 @@ +package secrets + +import ( + "fmt" + "time" + + corev1 "k8s.io/api/core/v1" +) + +// KekMigrationState holds KMS KEK rotation annotations from a write-key secret. +type KekMigrationState struct { + TargetKekID string + MigratedKekID string + KekConvergedAt time.Time + KekConvergedID string +} + +// KekMigrationFromSecret parses KMS KEK rotation annotations from a secret. +func KekMigrationFromSecret(s *corev1.Secret) KekMigrationState { + if s == nil || s.Annotations == nil { + return KekMigrationState{} + } + state := KekMigrationState{ + TargetKekID: s.Annotations[EncryptionSecretTargetKekID], + MigratedKekID: s.Annotations[EncryptionSecretMigratedKekID], + KekConvergedID: s.Annotations[EncryptionSecretKekConvergedID], + } + if v, ok := s.Annotations[EncryptionSecretKekConvergedAt]; ok && len(v) > 0 { + if ts, err := time.Parse(time.RFC3339, v); err == nil { + state.KekConvergedAt = ts + } + } + return state +} + +// NeedsKekMigration reports whether target-kek-id is set and differs from migrated-kek-id. +func NeedsKekMigration(s *corev1.Secret) bool { + if s == nil || s.Annotations == nil { + return false + } + target := s.Annotations[EncryptionSecretTargetKekID] + if target == "" { + return false + } + return target != s.Annotations[EncryptionSecretMigratedKekID] +} + +// MigrationWriteKey returns the migrator write-key identity for the given key name. +// When target-kek-id is set, the format is {keyName}-{kekId}; otherwise {keyName}. +func MigrationWriteKey(keyName string, s *corev1.Secret) string { + if s == nil || s.Annotations == nil { + return keyName + } + if target := s.Annotations[EncryptionSecretTargetKekID]; target != "" { + return fmt.Sprintf("%s-%s", keyName, target) + } + return keyName +} diff --git a/pkg/operator/encryption/secrets/kek_test.go b/pkg/operator/encryption/secrets/kek_test.go new file mode 100644 index 0000000000..7c5369c337 --- /dev/null +++ b/pkg/operator/encryption/secrets/kek_test.go @@ -0,0 +1,107 @@ +package secrets + +import ( + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestNeedsKekMigration(t *testing.T) { + tests := []struct { + name string + secret *corev1.Secret + want bool + }{ + { + name: "nil secret", + secret: nil, + want: false, + }, + { + name: "no annotations", + secret: &corev1.Secret{}, + want: false, + }, + { + name: "steady state", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + EncryptionSecretTargetKekID: "kek-old", + EncryptionSecretMigratedKekID: "kek-old", + }, + }, + }, + want: false, + }, + { + name: "rotation in flight", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + EncryptionSecretTargetKekID: "kek-new", + EncryptionSecretMigratedKekID: "kek-old", + }, + }, + }, + want: true, + }, + { + name: "target only", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + EncryptionSecretTargetKekID: "kek-new", + }, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := NeedsKekMigration(tt.secret); got != tt.want { + t.Fatalf("NeedsKekMigration() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMigrationWriteKey(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + EncryptionSecretTargetKekID: "kek-new", + }, + }, + } + if got := MigrationWriteKey("8", secret); got != "8-kek-new" { + t.Fatalf("MigrationWriteKey() = %q, want %q", got, "8-kek-new") + } + if got := MigrationWriteKey("8", &corev1.Secret{}); got != "8" { + t.Fatalf("MigrationWriteKey() without target = %q, want %q", got, "8") + } +} + +func TestKekMigrationFromSecret(t *testing.T) { + ts := time.Date(2026, 6, 11, 12, 0, 0, 0, time.UTC) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + EncryptionSecretTargetKekID: "kek-target", + EncryptionSecretMigratedKekID: "kek-migrated", + EncryptionSecretKekConvergedID: "kek-candidate", + EncryptionSecretKekConvergedAt: ts.Format(time.RFC3339), + }, + }, + } + got := KekMigrationFromSecret(secret) + if got.TargetKekID != "kek-target" || got.MigratedKekID != "kek-migrated" || got.KekConvergedID != "kek-candidate" { + t.Fatalf("unexpected kek migration state: %#v", got) + } + if !got.KekConvergedAt.Equal(ts) { + t.Fatalf("KekConvergedAt = %v, want %v", got.KekConvergedAt, ts) + } +} diff --git a/pkg/operator/encryption/secrets/secrets.go b/pkg/operator/encryption/secrets/secrets.go index 716764f08d..7db4404ae9 100644 --- a/pkg/operator/encryption/secrets/secrets.go +++ b/pkg/operator/encryption/secrets/secrets.go @@ -60,6 +60,16 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) { key.ExternalReason = v } + kekMigration := KekMigrationFromSecret(s) + if kekMigration.TargetKekID != "" || kekMigration.MigratedKekID != "" || kekMigration.KekConvergedID != "" || !kekMigration.KekConvergedAt.IsZero() { + key.KekMigration = &state.KekMigrationState{ + TargetKekID: kekMigration.TargetKekID, + MigratedKekID: kekMigration.MigratedKekID, + KekConvergedAt: kekMigration.KekConvergedAt, + KekConvergedID: kekMigration.KekConvergedID, + } + } + keyMode := state.Mode(s.Annotations[encryptionSecretMode]) switch keyMode { case state.AESCBC, state.AESGCM, state.SecretBox, state.Identity: diff --git a/pkg/operator/encryption/secrets/types.go b/pkg/operator/encryption/secrets/types.go index 5a49727cf3..ff9f7eee2b 100644 --- a/pkg/operator/encryption/secrets/types.go +++ b/pkg/operator/encryption/secrets/types.go @@ -1,6 +1,8 @@ package secrets import ( + "time" + "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -67,8 +69,21 @@ const ( // values fetched from the referenced configmap in openshift-config. The full data key is // constructed as prefix + configMapName + separator + dataKey. encryptionSecretKMSConfigMapDataPrefix = "encryption.apiserver.operator.openshift.io-kms-plugin-configmap-" + + // EncryptionSecretTargetKekID is the target KMS KEK identity to migrate toward. + EncryptionSecretTargetKekID = "encryption.apiserver.operator.openshift.io/target-kek-id" + // EncryptionSecretMigratedKekID is the last fully migrated KMS KEK identity. + EncryptionSecretMigratedKekID = "encryption.apiserver.operator.openshift.io/migrated-kek-id" + // EncryptionSecretKekConvergedAt records when a candidate kekId first achieved cluster convergence (RFC3339). + EncryptionSecretKekConvergedAt = "encryption.apiserver.operator.openshift.io/kek-converged-at" + // EncryptionSecretKekConvergedID is the candidate kekId the kek-converged-at timestamp belongs to. + EncryptionSecretKekConvergedID = "encryption.apiserver.operator.openshift.io/kek-converged-id" ) +// KekConvergenceDelay is the minimum duration a candidate kekId must remain cluster-converged +// before target-kek-id is updated to a new value (KEP-3299). +const KekConvergenceDelay = 5 * time.Minute + // MigratedGroupResources is the data structured stored in the // encryption.apiserver.operator.openshift.io/migrated-resources // of a key secret. diff --git a/pkg/operator/encryption/state/types.go b/pkg/operator/encryption/state/types.go index fd1af2ebe6..0226dca5f3 100644 --- a/pkg/operator/encryption/state/types.go +++ b/pkg/operator/encryption/state/types.go @@ -50,6 +50,27 @@ type KeyState struct { ExternalReason string // stores all the KMS encryption mode related configurations KMS *KMSState + // KekMigration stores KMS KEK rotation annotations from the write-key secret. + KekMigration *KekMigrationState +} + +// KekMigrationState holds KMS KEK rotation annotations parsed from a key secret. +type KekMigrationState struct { + TargetKekID string + MigratedKekID string + KekConvergedAt time.Time + KekConvergedID string +} + +// NeedsKekMigration reports whether target-kek-id is set and differs from migrated-kek-id. +func (k KeyState) NeedsKekMigration() bool { + if k.Mode != KMS || k.KekMigration == nil { + return false + } + if k.KekMigration.TargetKekID == "" { + return false + } + return k.KekMigration.TargetKekID != k.KekMigration.MigratedKekID } func (k *KeyState) HasKMSEncryption() bool { diff --git a/pkg/operator/encryption/statemachine/transition.go b/pkg/operator/encryption/statemachine/transition.go index 12eb5b8b90..51e9ba8be6 100644 --- a/pkg/operator/encryption/statemachine/transition.go +++ b/pkg/operator/encryption/statemachine/transition.go @@ -193,6 +193,10 @@ func getDesiredEncryptionState(oldSecretData *encryptiondata.Config, encryptionS // STEP 4: with consistent read-keys and write-keys, remove every read-key other than the write-key and one last read key. // // Note: because read-keys are consistent, currentlyEncryptedGRs equals toBeEncryptedGRs + if writeKey.NeedsKekMigration() { + klog.V(4).Infof("KMS kek migration in progress for key ID %s", writeKey.Key.Name) + return desiredEncryptionState + } allMigrated, _, reason := state.MigratedFor(currentlyEncryptedGRs, writeKey) if !allMigrated { klog.V(4).Infof("%s", reason) diff --git a/pkg/operator/encryption/statemachine/transition_test.go b/pkg/operator/encryption/statemachine/transition_test.go index 11f0beed5e..9bcb56fab5 100644 --- a/pkg/operator/encryption/statemachine/transition_test.go +++ b/pkg/operator/encryption/statemachine/transition_test.go @@ -14,6 +14,7 @@ import ( "github.com/openshift/library-go/pkg/operator/encryption/encryptiondata" 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" ) @@ -1299,6 +1300,45 @@ func TestGetDesiredEncryptionState(t *testing.T) { }}, }), }, + { + "KMS kek migration in flight blocks STEP 4 pruning", + args{ + toSecretData(&apiserverconfigv1.EncryptionConfiguration{ + Resources: []apiserverconfigv1.ResourceConfiguration{{ + Resources: []string{"secrets"}, + Providers: []apiserverconfigv1.ProviderConfiguration{{ + KMS: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "2_secrets", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", + Timeout: &metav1.Duration{Duration: 10 * time.Second}, + }, + }, { + AESCBC: &apiserverconfigv1.AESConfiguration{ + Keys: []apiserverconfigv1.Key{{ + Name: "1", + Secret: base64.StdEncoding.EncodeToString([]byte("21ea7c91419a68fd1224f88d50316b4e")), + }}, + }, + }, { + Identity: &apiserverconfigv1.IdentityConfiguration{}, + }}, + }}, + }), + "kms", + func() []*corev1.Secret { + secretsList := []*corev1.Secret{ + encryptiontesting.CreateEncryptionKeySecretWithRawKey("kms", nil, 1, []byte("21ea7c91419a68fd1224f88d50316b4e")), + encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 2), + } + secretsList[1].Annotations[secrets.EncryptionSecretTargetKekID] = "kek-new" + secretsList[1].Annotations[secrets.EncryptionSecretMigratedKekID] = "kek-old" + return secretsList + }(), + []schema.GroupResource{{Group: "", Resource: "secrets"}}, + }, + outputMatchingInputConfig, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/test/e2e-encryption/encryption_test.go b/test/e2e-encryption/encryption_test.go index f659f51489..3998184025 100644 --- a/test/e2e-encryption/encryption_test.go +++ b/test/e2e-encryption/encryption_test.go @@ -159,6 +159,7 @@ func TestEncryptionIntegration(tt *testing.T) { provider, deployer, migrator, + nil, operatorClient, fakeApiServerClient, fakeConfigInformer.Config().V1().APIServers(), From e3c679dbcee56296ab8d04d7c35807ea00fa3e32 Mon Sep 17 00:00:00 2001 From: Thomas Jungblut Date: Thu, 11 Jun 2026 13:19:54 +0200 Subject: [PATCH 2/3] add a mock convergence reporter Signed-off-by: Thomas Jungblut --- .../controllerset/apiservercontrollerset.go | 7 -- pkg/operator/encryption/controllers.go | 7 +- .../controllers/kms_rotation_controller.go | 5 +- .../kms/health/configmap_reporter.go | 70 ++++++++++++ .../kms/health/configmap_reporter_test.go | 102 ++++++++++++++++++ test/e2e-encryption/encryption_test.go | 1 - 6 files changed, 179 insertions(+), 13 deletions(-) create mode 100644 pkg/operator/encryption/kms/health/configmap_reporter.go create mode 100644 pkg/operator/encryption/kms/health/configmap_reporter_test.go diff --git a/pkg/operator/apiserver/controllerset/apiservercontrollerset.go b/pkg/operator/apiserver/controllerset/apiservercontrollerset.go index 70fa0e5648..938fd965f7 100644 --- a/pkg/operator/apiserver/controllerset/apiservercontrollerset.go +++ b/pkg/operator/apiserver/controllerset/apiservercontrollerset.go @@ -394,11 +394,6 @@ func (cs *APIServerControllerSet) WithUnsupportedConfigPrefixForEncryptionContro return cs } -func (cs *APIServerControllerSet) WithConvergedKEKReporterForEncryptionControllers(reporter controllers.ConvergedKEKReporter) *APIServerControllerSet { - cs.encryptionControllers.convergedKEKReporter = reporter - return cs -} - func (cs *APIServerControllerSet) WithoutEncryptionControllers() *APIServerControllerSet { cs.encryptionControllers.controller = nil cs.encryptionControllers.emptyAllowed = true @@ -490,7 +485,6 @@ type encryptionControllerBuilder struct { provider controllers.Provider deployer statemachine.Deployer migrator migrators.Migrator - convergedKEKReporter controllers.ConvergedKEKReporter secretsClient corev1.SecretsGetter configMapClient corev1.ConfigMapsGetter apiServerClient configv1client.APIServerInterface @@ -512,7 +506,6 @@ func (e *encryptionControllerBuilder) build() []controllerWrapper { e.provider, e.deployer, e.migrator, - e.convergedKEKReporter, e.operatorClient, e.apiServerClient, e.apiServerInformer, diff --git a/pkg/operator/encryption/controllers.go b/pkg/operator/encryption/controllers.go index 9e6f655572..6bb04d7a73 100644 --- a/pkg/operator/encryption/controllers.go +++ b/pkg/operator/encryption/controllers.go @@ -19,6 +19,7 @@ import ( operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers" "github.com/openshift/library-go/pkg/operator/encryption/controllers" + kmshealth "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/statemachine" ) @@ -29,7 +30,6 @@ func NewControllers( provider controllers.Provider, deployer statemachine.Deployer, migrator migrators.Migrator, - convergedKEKReporter controllers.ConvergedKEKReporter, operatorClient operatorv1helpers.OperatorClient, apiServerClient configv1client.APIServerInterface, apiServerInformer configv1informers.APIServerInformer, @@ -50,6 +50,11 @@ func NewControllers( return nil, err } + convergedKEKReporter := kmshealth.NewMOCK_ConfigMapConvergedKEKReporter( + kubeInformersForNamespaces.ConfigMapLister(), + "", + ) + // for testing resourceSyncer might be nil if resourceSyncer != nil { if err := resourceSyncer.SyncSecretConditionally( diff --git a/pkg/operator/encryption/controllers/kms_rotation_controller.go b/pkg/operator/encryption/controllers/kms_rotation_controller.go index 232b214e46..96f5d49a0f 100644 --- a/pkg/operator/encryption/controllers/kms_rotation_controller.go +++ b/pkg/operator/encryption/controllers/kms_rotation_controller.go @@ -69,6 +69,7 @@ func NewKMSRotationController( return factory.New().ResyncEvery(time.Minute).WithSync(c.sync).WithControllerInstanceName(c.controllerInstanceName).WithInformers( operatorClient.Informer(), kubeInformersForNamespaces.InformersFor("openshift-config-managed").Core().V1().Secrets().Informer(), + kubeInformersForNamespaces.InformersFor("openshift-config").Core().V1().ConfigMaps().Informer(), apiServerConfigInformer.Informer(), deployer, ).ToController( @@ -78,10 +79,6 @@ func NewKMSRotationController( } func (c *kmsRotationController) sync(ctx context.Context, syncCtx factory.SyncContext) error { - if c.convergedKEKReporter == nil { - return nil - } - if ready, err := shouldRunEncryptionController(c.operatorClient, c.preconditionsFulfilledFn, c.provider.ShouldRunEncryptionControllers); err != nil || !ready { return err } diff --git a/pkg/operator/encryption/kms/health/configmap_reporter.go b/pkg/operator/encryption/kms/health/configmap_reporter.go new file mode 100644 index 0000000000..6549b61e63 --- /dev/null +++ b/pkg/operator/encryption/kms/health/configmap_reporter.go @@ -0,0 +1,70 @@ +package health + +import ( + "strconv" + "strings" + + corev1 "k8s.io/api/core/v1" + corev1listers "k8s.io/client-go/listers/core/v1" + "k8s.io/klog/v2" +) + +const ( + // ConvergedKekConfigMapNamespace is where the mock converged-kek ConfigMap lives. + ConvergedKekConfigMapNamespace = "openshift-config" + // DefaultConvergedKekConfigMapName is the default ConfigMap name for mock KEK health input. + DefaultConvergedKekConfigMapName = "encryption-kms-converged-kek" + // ConvergedKekConfigMapDataKeyKekID holds the cluster-converged KMS kekId. + ConvergedKekConfigMapDataKeyKekID = "converged-kek-id" + // ConvergedKekConfigMapDataKeyConverged is an optional "true"/"false" override. + // When omitted, a non-empty converged-kek-id is treated as converged. + ConvergedKekConfigMapDataKeyConverged = "converged" +) + +// MOCK_ConfigMapConvergedKEKReporter reads cluster-converged kekId from a ConfigMap in openshift-config. +// It is intended for development and testing until kms-health-reporter publishes real health input. +type MOCK_ConfigMapConvergedKEKReporter struct { + lister corev1listers.ConfigMapLister + namespace string + name string +} + +// NewMOCK_ConfigMapConvergedKEKReporter returns a mock reporter backed by the named ConfigMap. +// An empty name uses DefaultConvergedKekConfigMapName. +func NewMOCK_ConfigMapConvergedKEKReporter(lister corev1listers.ConfigMapLister, name string) *MOCK_ConfigMapConvergedKEKReporter { + if name == "" { + name = DefaultConvergedKekConfigMapName + } + return &MOCK_ConfigMapConvergedKEKReporter{ + lister: lister, + namespace: ConvergedKekConfigMapNamespace, + name: name, + } +} + +func (r *MOCK_ConfigMapConvergedKEKReporter) ConvergedKekID() (string, bool) { + cm, err := r.lister.ConfigMaps(r.namespace).Get(r.name) + if err != nil { + klog.V(4).InfoS("converged kek configmap not available", "namespace", r.namespace, "name", r.name, "err", err) + return "", false + } + return ConvergedKekFromConfigMap(cm) +} + +// ConvergedKekFromConfigMap parses mock health input from a ConfigMap. +func ConvergedKekFromConfigMap(cm *corev1.ConfigMap) (kekID string, converged bool) { + if cm == nil || cm.Data == nil { + return "", false + } + kekID = strings.TrimSpace(cm.Data[ConvergedKekConfigMapDataKeyKekID]) + if kekID == "" { + return "", false + } + if v, ok := cm.Data[ConvergedKekConfigMapDataKeyConverged]; ok { + parsed, err := strconv.ParseBool(strings.TrimSpace(v)) + if err != nil || !parsed { + return "", false + } + } + return kekID, true +} diff --git a/pkg/operator/encryption/kms/health/configmap_reporter_test.go b/pkg/operator/encryption/kms/health/configmap_reporter_test.go new file mode 100644 index 0000000000..aaacaffe54 --- /dev/null +++ b/pkg/operator/encryption/kms/health/configmap_reporter_test.go @@ -0,0 +1,102 @@ +package health + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1listers "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" +) + +func TestConvergedKekFromConfigMap(t *testing.T) { + tests := []struct { + name string + cm *corev1.ConfigMap + wantKekID string + wantConverged bool + }{ + { + name: "nil configmap", + cm: nil, + wantConverged: false, + }, + { + name: "empty data", + cm: &corev1.ConfigMap{}, + wantConverged: false, + }, + { + name: "converged with kek id only", + cm: &corev1.ConfigMap{ + Data: map[string]string{ + ConvergedKekConfigMapDataKeyKekID: "kek-old", + }, + }, + wantKekID: "kek-old", + wantConverged: true, + }, + { + name: "explicit converged true", + cm: &corev1.ConfigMap{ + Data: map[string]string{ + ConvergedKekConfigMapDataKeyKekID: "kek-new", + ConvergedKekConfigMapDataKeyConverged: "true", + }, + }, + wantKekID: "kek-new", + wantConverged: true, + }, + { + name: "explicit converged false", + cm: &corev1.ConfigMap{ + Data: map[string]string{ + ConvergedKekConfigMapDataKeyKekID: "kek-new", + ConvergedKekConfigMapDataKeyConverged: "false", + }, + }, + wantConverged: false, + }, + { + name: "whitespace trimmed", + cm: &corev1.ConfigMap{ + Data: map[string]string{ + ConvergedKekConfigMapDataKeyKekID: " kek-1 ", + }, + }, + wantKekID: "kek-1", + wantConverged: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotKekID, gotConverged := ConvergedKekFromConfigMap(tt.cm) + if gotKekID != tt.wantKekID || gotConverged != tt.wantConverged { + t.Fatalf("ConvergedKekFromConfigMap() = (%q, %v), want (%q, %v)", gotKekID, gotConverged, tt.wantKekID, tt.wantConverged) + } + }) + } +} + +func TestMOCK_ConfigMapConvergedKEKReporter(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultConvergedKekConfigMapName, + Namespace: ConvergedKekConfigMapNamespace, + }, + Data: map[string]string{ + ConvergedKekConfigMapDataKeyKekID: "kek-test", + }, + } + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) + if err := indexer.Add(cm); err != nil { + t.Fatal(err) + } + lister := corev1listers.NewConfigMapLister(indexer) + reporter := NewMOCK_ConfigMapConvergedKEKReporter(lister, "") + + gotKekID, gotConverged := reporter.ConvergedKekID() + if gotKekID != "kek-test" || !gotConverged { + t.Fatalf("ConvergedKekID() = (%q, %v), want (%q, true)", gotKekID, gotConverged, "kek-test") + } +} diff --git a/test/e2e-encryption/encryption_test.go b/test/e2e-encryption/encryption_test.go index 3998184025..f659f51489 100644 --- a/test/e2e-encryption/encryption_test.go +++ b/test/e2e-encryption/encryption_test.go @@ -159,7 +159,6 @@ func TestEncryptionIntegration(tt *testing.T) { provider, deployer, migrator, - nil, operatorClient, fakeApiServerClient, fakeConfigInformer.Config().V1().APIServers(), From 374bd672722fdce663e8faa0048f5a41e4705d35 Mon Sep 17 00:00:00 2001 From: Thomas Jungblut Date: Thu, 11 Jun 2026 13:46:44 +0200 Subject: [PATCH 3/3] bot code review suggestion --- .../controllers/kms_rotation_controller.go | 9 ++++++ .../kms_rotation_controller_test.go | 31 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/pkg/operator/encryption/controllers/kms_rotation_controller.go b/pkg/operator/encryption/controllers/kms_rotation_controller.go index 96f5d49a0f..e1c5c1d1c1 100644 --- a/pkg/operator/encryption/controllers/kms_rotation_controller.go +++ b/pkg/operator/encryption/controllers/kms_rotation_controller.go @@ -164,6 +164,12 @@ func (c *kmsRotationController) reconcileKekAnnotations( }) } + if kekMigration.KekConvergedAt.IsZero() { + return c.updateWriteKeySecret(ctx, syncCtx, writeKeySecret, func(s *corev1.Secret) (bool, error) { + return setKekConvergenceClock(s, convergedKekID, c.now()) + }) + } + if c.now().Sub(kekMigration.KekConvergedAt) >= secrets.KekConvergenceDelay { return c.updateWriteKeySecret(ctx, syncCtx, writeKeySecret, func(s *corev1.Secret) (bool, error) { return promoteConvergedKekToTarget(s, convergedKekID) @@ -227,6 +233,9 @@ func setKekConvergenceClock(s *corev1.Secret, kekID string, now time.Time) (bool s.Annotations[secrets.EncryptionSecretKekConvergedID] = kekID s.Annotations[secrets.EncryptionSecretKekConvergedAt] = now.Format(time.RFC3339) changed = true + } else if s.Annotations[secrets.EncryptionSecretKekConvergedAt] == "" { + s.Annotations[secrets.EncryptionSecretKekConvergedAt] = now.Format(time.RFC3339) + changed = true } if changed { klog.V(2).Infof("started KMS kekId convergence clock on secret %s/%s for candidate %q", s.Namespace, s.Name, kekID) diff --git a/pkg/operator/encryption/controllers/kms_rotation_controller_test.go b/pkg/operator/encryption/controllers/kms_rotation_controller_test.go index 3fadff700b..683b00b0b5 100644 --- a/pkg/operator/encryption/controllers/kms_rotation_controller_test.go +++ b/pkg/operator/encryption/controllers/kms_rotation_controller_test.go @@ -120,6 +120,37 @@ func TestKMSRotationControllerReconcileKekAnnotations(t *testing.T) { } }) + t.Run("missing converged-at starts clock instead of promoting", func(t *testing.T) { + secret := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", grs, 1) + secret.Annotations[secrets.EncryptionSecretTargetKekID] = "kek-old" + secret.Annotations[secrets.EncryptionSecretMigratedKekID] = "kek-old" + secret.Annotations[secrets.EncryptionSecretKekConvergedID] = "kek-new" + c := &kmsRotationController{ + now: func() time.Time { return now }, + } + writeKey, err := secrets.ToKeyState(secret) + if err != nil { + t.Fatal(err) + } + fakeClient := fake.NewSimpleClientset(secret) + c.secretClient = fakeClient.CoreV1() + eventRecorder := events.NewRecorder(fakeClient.CoreV1().Events("operator"), "test-kms-rotation", &corev1.ObjectReference{}, clocktesting.NewFakePassiveClock(now)) + syncCtx := factory.NewSyncContext("test", eventRecorder) + if err := c.reconcileKekAnnotations(context.Background(), syncCtx, secret, writeKey, "kek-new", grs); err != nil { + t.Fatal(err) + } + updated, err := fakeClient.CoreV1().Secrets(secret.Namespace).Get(context.Background(), secret.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if updated.Annotations[secrets.EncryptionSecretTargetKekID] != "kek-old" { + t.Fatalf("target = %q, want kek-old", updated.Annotations[secrets.EncryptionSecretTargetKekID]) + } + if updated.Annotations[secrets.EncryptionSecretKekConvergedAt] != now.Format(time.RFC3339) { + t.Fatalf("converged-at = %q, want %q", updated.Annotations[secrets.EncryptionSecretKekConvergedAt], now.Format(time.RFC3339)) + } + }) + t.Run("promote after convergence delay", func(t *testing.T) { secret := encryptiontesting.CreateEncryptionKeySecretWithKMSPluginConfig("kms", grs, 1) secret.Annotations[secrets.EncryptionSecretTargetKekID] = "kek-old"