From 0488a1aeeefe7f6aea6d39e8d27c264bcbab6b19 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Wed, 5 Aug 2026 11:12:05 -0400 Subject: [PATCH 01/24] test(e2e): add etcd data re-encryption tests after key rotation CNTRLPLANE-3525: Adds 5 v2 e2e tests validating the re-encryption lifecycle when encryption keys are rotated on a HostedCluster. Tests: - AWS KMS key rotation (ARN swap) - Azure KMS key rotation (key version swap) - Azure KMS consecutive key rotations (two back-to-back) - AESCBC key rotation (new Secret reference) - Condition bubble-up (HCP/HostedCluster consistency) Co-Authored-By: Claude Opus 4.6 --- test/e2e/v2/internal/env_vars.go | 11 + .../etcd_encryption_reencryption_test.go | 646 ++++++++++++++++++ 2 files changed, 657 insertions(+) create mode 100644 test/e2e/v2/tests/etcd_encryption_reencryption_test.go diff --git a/test/e2e/v2/internal/env_vars.go b/test/e2e/v2/internal/env_vars.go index 04ed9cc12b54..5183d0bfc1ab 100644 --- a/test/e2e/v2/internal/env_vars.go +++ b/test/e2e/v2/internal/env_vars.go @@ -225,6 +225,17 @@ func init() { false, "", ) + // Encryption key rotation test environment variables + RegisterEnvVar( + "E2E_AWS_KMS_KEY_ARN_ALTERNATE", + "Alternate AWS KMS CMK ARN for key rotation re-encryption tests.", + false, + ) + RegisterEnvVar( + "E2E_AZURE_KMS_KEY_VERSION_ALTERNATE", + "Alternate Azure Key Vault key version for key rotation re-encryption tests.", + false, + ) // External OIDC test environment variables RegisterEnvVar( "E2E_EXTERNAL_OIDC_CA_BUNDLE_FILE", diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go new file mode 100644 index 000000000000..dfa5cb02ebc9 --- /dev/null +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -0,0 +1,646 @@ +//go:build e2ev2 + +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tests + +import ( + "context" + "crypto/rand" + "fmt" + "io" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + e2eutil "github.com/openshift/hypershift/test/e2e/util" + "github.com/openshift/hypershift/test/e2e/v2/internal" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes" + crclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("[sig-hypershift][Jira:Hypershift][Feature:EtcdEncryptionReencryption] Etcd Encryption Re-encryption", + Label("etcd-encryption-reencryption"), func() { + var testCtx *internal.TestContext + + BeforeEach(func() { + testCtx = internal.GetTestContext() + Expect(testCtx).NotTo(BeNil(), "test context should be set up in BeforeSuite") + + hc, err := testCtx.GetHostedCluster() + Expect(err).NotTo(HaveOccurred()) + if hc.Spec.SecretEncryption == nil { + Skip("SecretEncryption is not configured on this hosted cluster") + } + }) + + RegisterEtcdEncryptionReencryptionTests(func() *internal.TestContext { return testCtx }) + }) + +// RegisterEtcdEncryptionReencryptionTests registers all re-encryption lifecycle tests. +func RegisterEtcdEncryptionReencryptionTests(getTestCtx internal.TestContextGetter) { + AWSKMSKeyRotationTest(getTestCtx) + AzureKMSKeyRotationTest(getTestCtx) + AzureKMSConsecutiveKeyRotationTest(getTestCtx) + AESCBCKeyRotationTest(getTestCtx) + ConditionBubbleUpTest(getTestCtx) +} + +func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, priorHistoryLen int, timeout time.Duration) { + Eventually(func(g Gomega) { + hc := &hyperv1.HostedCluster{} + g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + g.Expect(meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate))). + NotTo(BeNil(), "EtcdDataEncryptionUpToDate condition should exist after the key rotation patch") + g.Expect(len(hc.Status.SecretEncryption.History)).To(BeNumerically(">", priorHistoryLen), + "a new migration history entry should be recorded after the key rotation patch") + }, timeout, 5*time.Second).Should(Succeed()) +} + +func waitForReEncryptionComplete(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { + Eventually(func(g Gomega) { + hc := &hyperv1.HostedCluster{} + g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + cond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) + g.Expect(cond).NotTo(BeNil(), "EtcdDataEncryptionUpToDate condition should exist") + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue), + "EtcdDataEncryptionUpToDate should be True, got reason=%s message=%s", cond.Reason, cond.Message) + }, timeout, 15*time.Second).Should(Succeed()) +} + +func verifyReEncryptionStatus(hc *hyperv1.HostedCluster, expectedProvider hyperv1.SecretEncryptionProvider) { + Expect(hc.Status.SecretEncryption.ActiveKey.Provider).To(Equal(expectedProvider), + "activeKey provider should be %s", expectedProvider) + Expect(hc.Status.SecretEncryption.TargetKey.Provider).To(BeEmpty(), + "targetKey should be empty after completed rotation") + Expect(hc.Status.SecretEncryption.History).NotTo(BeEmpty(), + "history should have at least one entry") + Expect(hc.Status.SecretEncryption.History[0].State).To(Equal(hyperv1.EncryptionMigrationStateCompleted), + "most recent history entry should be Completed") + Expect(hc.Status.SecretEncryption.History[0].From.Provider).To(Equal(expectedProvider), + "history[0].from.provider should be %s", expectedProvider) + Expect(hc.Status.SecretEncryption.History[0].To.Provider).To(Equal(expectedProvider), + "history[0].to.provider should be %s", expectedProvider) + Expect(hc.Status.SecretEncryption.History[0].StartedTime.IsZero()).To(BeFalse(), + "history[0].startedTime should be set") + Expect(hc.Status.SecretEncryption.History[0].CompletionTime.IsZero()).To(BeFalse(), + "history[0].completionTime should be set") +} + +func listStorageVersionMigrations(ctx context.Context, guestClient crclient.Client) (*unstructured.UnstructuredList, error) { + svmList := &unstructured.UnstructuredList{} + svmList.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "migration.k8s.io", + Version: "v1alpha1", + Kind: "StorageVersionMigrationList", + }) + err := guestClient.List(ctx, svmList) + return svmList, err +} + +func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPlaneNamespace string) { + Eventually(func(g Gomega) { + kasPods := &corev1.PodList{} + g.Expect(mgmtClient.List(ctx, kasPods, + crclient.InNamespace(controlPlaneNamespace), + crclient.MatchingLabels{"app": "kube-apiserver"}, + )).To(Succeed()) + g.Expect(kasPods.Items).NotTo(BeEmpty(), + "expected at least one kube-apiserver pod in namespace %s", controlPlaneNamespace) + + for _, pod := range kasPods.Items { + g.Expect(pod.Status.Phase).To(Equal(corev1.PodRunning), + "KAS pod %s should be Running, got %s", pod.Name, pod.Status.Phase) + for _, cs := range pod.Status.ContainerStatuses { + if cs.State.Waiting != nil { + g.Expect(cs.State.Waiting.Reason).NotTo(Equal("CrashLoopBackOff"), + "KAS pod %s container %s is in CrashLoopBackOff", pod.Name, cs.Name) + } + g.Expect(cs.RestartCount).To(BeNumerically("<=", int32(2)), + "KAS pod %s container %s has %d restarts", pod.Name, cs.Name, cs.RestartCount) + } + } + }, 5*time.Minute, 15*time.Second).Should(Succeed()) +} + +func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace string) { + mgmtRestConfig, err := e2eutil.GetConfig() + Expect(err).NotTo(HaveOccurred(), "failed to get management cluster REST config") + mgmtKubeClient, err := kubernetes.NewForConfig(mgmtRestConfig) + Expect(err).NotTo(HaveOccurred(), "failed to create management cluster kubernetes clientset") + + kasPods, err := mgmtKubeClient.CoreV1().Pods(controlPlaneNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: "app=kube-apiserver", + }) + Expect(err).NotTo(HaveOccurred(), "failed to list KAS pods") + Expect(kasPods.Items).NotTo(BeEmpty(), + "expected at least one kube-apiserver pod in namespace %s", controlPlaneNamespace) + + for _, pod := range kasPods.Items { + req := mgmtKubeClient.CoreV1().Pods(controlPlaneNamespace).GetLogs(pod.Name, &corev1.PodLogOptions{ + Container: "kube-apiserver", + TailLines: int64Ptr(500), + }) + logStream, err := req.Stream(ctx) + if err != nil { + GinkgoWriter.Printf("WARNING: failed to get logs for KAS pod %s: %v\n", pod.Name, err) + continue + } + logBytes, err := io.ReadAll(logStream) + logStream.Close() + if err != nil { + GinkgoWriter.Printf("WARNING: failed to read logs for KAS pod %s: %v\n", pod.Name, err) + continue + } + logContent := string(logBytes) + Expect(logContent).NotTo(ContainSubstring("no matching prefix found"), + "KAS pod %s logs contain decryption error 'no matching prefix found'", pod.Name) + } +} + +func int64Ptr(i int64) *int64 { + return &i +} + +// AWSKMSKeyRotationTest validates the re-encryption lifecycle after rotating the AWS KMS active key. +func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { + Context("[Feature:AWSKMSReencryption] AWS KMS Key Rotation", func() { + BeforeEach(func() { + tc := getTestCtx() + hc, err := tc.GetHostedCluster() + Expect(err).NotTo(HaveOccurred()) + if hc.Spec.Platform.Type != hyperv1.AWSPlatform || + hc.Spec.SecretEncryption.Type != hyperv1.KMS || + hc.Spec.SecretEncryption.KMS == nil || + hc.Spec.SecretEncryption.KMS.AWS == nil { + Skip("AWS KMS key rotation test requires AWS platform with KMS configured") + } + alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") + if alternateARN == "" { + Skip("E2E_AWS_KMS_KEY_ARN_ALTERNATE is not set") + } + }) + + It("should re-encrypt all etcd data after active key rotation", func() { + tc := getTestCtx() + ctx := tc.Context + hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} + + hc := &hyperv1.HostedCluster{} + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + originalARN := hc.Spec.SecretEncryption.KMS.AWS.ActiveKey.ARN + + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) + + hostedClusterClient, err := tc.GetHostedClusterClient(hc) + Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") + + testSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e-reencryption-test-aws", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "testKey": []byte("aws-kms-reencryption-test-data"), + }, + } + Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) + DeferCleanup(func() { + if err := hostedClusterClient.Delete(context.Background(), testSecret); err != nil && !apierrors.IsNotFound(err) { + GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) + } + }) + + alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") + priorHistoryLen := len(hc.Status.SecretEncryption.History) + patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, alternateARN)) + Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) + + DeferCleanup(func() { + restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, originalARN)) + hcRestore := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, + } + if err := tc.MgmtClient.Patch(context.Background(), hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { + GinkgoWriter.Printf("WARNING: failed to restore original KMS key ARN: %v\n", err) + return + } + waitForReEncryptionComplete(context.Background(), tc.MgmtClient, hcKey, 25*time.Minute) + }) + + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + verifyReEncryptionStatus(hc, hyperv1.SecretEncryptionProviderAWS) + Expect(hc.Status.SecretEncryption.ActiveKey.AWS.ARN).To(Equal(alternateARN), + "activeKey ARN should match the alternate key") + + readBack := &corev1.Secret{} + Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) + Expect(readBack.Data["testKey"]).To(Equal([]byte("aws-kms-reencryption-test-data")), + "test secret data should be readable after re-encryption") + + svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) + Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") + Expect(svmList.Items).NotTo(BeEmpty(), + "StorageVersionMigration CRs should exist after KMS key rotation") + + verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) + verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) + + verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + }) + }) +} + +// AzureKMSKeyRotationTest validates the re-encryption lifecycle after rotating the Azure KMS key version. +func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { + Context("[Feature:AzureKMSReencryption] Azure KMS Key Rotation", func() { + BeforeEach(func() { + tc := getTestCtx() + hc, err := tc.GetHostedCluster() + Expect(err).NotTo(HaveOccurred()) + if hc.Spec.Platform.Type != hyperv1.AzurePlatform || + hc.Spec.SecretEncryption.Type != hyperv1.KMS || + hc.Spec.SecretEncryption.KMS == nil || + hc.Spec.SecretEncryption.KMS.Azure == nil { + Skip("Azure KMS key rotation test requires Azure platform with KMS configured") + } + alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") + if alternateVersion == "" { + Skip("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE is not set") + } + }) + + It("should re-encrypt all etcd data after key version rotation", func() { + tc := getTestCtx() + ctx := tc.Context + hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} + + hc := &hyperv1.HostedCluster{} + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + originalVersion := hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion + + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) + + hostedClusterClient, err := tc.GetHostedClusterClient(hc) + Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") + + testSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e-reencryption-test-azure", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "testKey": []byte("azure-kms-reencryption-test-data"), + }, + } + Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) + DeferCleanup(func() { + if err := hostedClusterClient.Delete(context.Background(), testSecret); err != nil && !apierrors.IsNotFound(err) { + GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) + } + }) + + alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") + priorHistoryLen := len(hc.Status.SecretEncryption.History) + patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) + Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) + + DeferCleanup(func() { + restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) + hcRestore := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, + } + if err := tc.MgmtClient.Patch(context.Background(), hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { + GinkgoWriter.Printf("WARNING: failed to restore original Azure key version: %v\n", err) + return + } + waitForReEncryptionComplete(context.Background(), tc.MgmtClient, hcKey, 25*time.Minute) + }) + + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + verifyReEncryptionStatus(hc, hyperv1.SecretEncryptionProviderAzure) + Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(alternateVersion), + "activeKey keyVersion should match the alternate version") + + readBack := &corev1.Secret{} + Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) + Expect(readBack.Data["testKey"]).To(Equal([]byte("azure-kms-reencryption-test-data")), + "test secret data should be readable after re-encryption") + + svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) + Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") + Expect(svmList.Items).NotTo(BeEmpty(), + "StorageVersionMigration CRs should exist after Azure KMS key rotation") + + verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) + verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) + + verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + }) + }) +} + +// AzureKMSConsecutiveKeyRotationTest validates that two back-to-back key rotations +// complete successfully, exercising the mid-rotation handling code path. +func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { + Context("[Feature:AzureKMSReencryption] Azure KMS Consecutive Key Rotation", func() { + BeforeEach(func() { + tc := getTestCtx() + hc, err := tc.GetHostedCluster() + Expect(err).NotTo(HaveOccurred()) + if hc.Spec.Platform.Type != hyperv1.AzurePlatform || + hc.Spec.SecretEncryption.Type != hyperv1.KMS || + hc.Spec.SecretEncryption.KMS == nil || + hc.Spec.SecretEncryption.KMS.Azure == nil { + Skip("Azure KMS consecutive key rotation test requires Azure platform with KMS configured") + } + alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") + if alternateVersion == "" { + Skip("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE is not set") + } + }) + + It("should complete two consecutive key rotations successfully", func() { + tc := getTestCtx() + ctx := tc.Context + hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} + + hc := &hyperv1.HostedCluster{} + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + originalVersion := hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion + + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) + + hostedClusterClient, err := tc.GetHostedClusterClient(hc) + Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") + + testSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e-reencryption-test-azure-consecutive", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "testKey": []byte("azure-kms-consecutive-rotation-test-data"), + }, + } + Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) + DeferCleanup(func() { + if err := hostedClusterClient.Delete(context.Background(), testSecret); err != nil && !apierrors.IsNotFound(err) { + GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) + } + }) + + DeferCleanup(func() { + restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) + hcRestore := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, + } + if err := tc.MgmtClient.Patch(context.Background(), hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { + GinkgoWriter.Printf("WARNING: failed to restore original Azure key version: %v\n", err) + return + } + waitForReEncryptionComplete(context.Background(), tc.MgmtClient, hcKey, 25*time.Minute) + }) + + alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") + + By("Performing first key rotation: original -> alternate") + priorHistoryLen := len(hc.Status.SecretEncryption.History) + patch1 := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) + Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch1))).To(Succeed()) + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(alternateVersion), + "activeKey should be the alternate version after first rotation") + Expect(hc.Status.SecretEncryption.History).NotTo(BeEmpty(), + "history should have at least one entry after first rotation") + + verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + + By("Performing second key rotation: alternate -> original") + priorHistoryLen = len(hc.Status.SecretEncryption.History) + patch2 := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) + Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch2))).To(Succeed()) + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + verifyReEncryptionStatus(hc, hyperv1.SecretEncryptionProviderAzure) + Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(originalVersion), + "activeKey should be back to the original version after second rotation") + Expect(len(hc.Status.SecretEncryption.History)).To(BeNumerically(">=", 2), + "history should have at least two entries after consecutive rotations") + Expect(hc.Status.SecretEncryption.History[0].State).To(Equal(hyperv1.EncryptionMigrationStateCompleted), + "most recent history entry should be Completed") + Expect(hc.Status.SecretEncryption.History[1].State).To(Equal(hyperv1.EncryptionMigrationStateCompleted), + "second history entry should also be Completed") + + readBack := &corev1.Secret{} + Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) + Expect(readBack.Data["testKey"]).To(Equal([]byte("azure-kms-consecutive-rotation-test-data")), + "test secret data should be readable after two consecutive re-encryptions") + + verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) + verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) + + verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + }) + }) +} + +// AESCBCKeyRotationTest validates the re-encryption lifecycle after rotating the AESCBC encryption key. +func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { + Context("[Feature:AESCBCReencryption] AESCBC Key Rotation", func() { + BeforeEach(func() { + tc := getTestCtx() + hc, err := tc.GetHostedCluster() + Expect(err).NotTo(HaveOccurred()) + if hc.Spec.SecretEncryption.Type != hyperv1.AESCBC || + hc.Spec.SecretEncryption.AESCBC == nil { + Skip("AESCBC key rotation test requires AESCBC encryption configured") + } + }) + + It("should re-encrypt secrets after active key rotation", func() { + tc := getTestCtx() + ctx := tc.Context + hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} + + hc := &hyperv1.HostedCluster{} + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + originalKeyRef := hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name + + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) + + hostedClusterClient, err := tc.GetHostedClusterClient(hc) + Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") + + testSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e-reencryption-test-aescbc", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "testKey": []byte("aescbc-reencryption-test-data"), + }, + } + Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) + DeferCleanup(func() { + if err := hostedClusterClient.Delete(context.Background(), testSecret); err != nil && !apierrors.IsNotFound(err) { + GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) + } + }) + + keyData := make([]byte, 32) + _, err := rand.Read(keyData) + Expect(err).NotTo(HaveOccurred(), "failed to generate random key data") + + newKeySecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e-aescbc-rotation-key", + Namespace: tc.ClusterNamespace, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + hyperv1.AESCBCKeySecretKey: keyData, + }, + } + Expect(tc.MgmtClient.Create(ctx, newKeySecret)).To(Succeed()) + DeferCleanup(func() { + if err := tc.MgmtClient.Delete(context.Background(), newKeySecret); err != nil && !apierrors.IsNotFound(err) { + GinkgoWriter.Printf("WARNING: failed to cleanup AESCBC key secret: %v\n", err) + } + }) + + priorHistoryLen := len(hc.Status.SecretEncryption.History) + patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, newKeySecret.Name)) + Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) + + DeferCleanup(func() { + restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, originalKeyRef)) + hcRestore := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, + } + if err := tc.MgmtClient.Patch(context.Background(), hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { + GinkgoWriter.Printf("WARNING: failed to restore original AESCBC key: %v\n", err) + return + } + waitForReEncryptionComplete(context.Background(), tc.MgmtClient, hcKey, 25*time.Minute) + }) + + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + verifyReEncryptionStatus(hc, hyperv1.SecretEncryptionProviderAESCBC) + Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC.Secret.Name).To(Equal(newKeySecret.Name), + "activeKey AESCBC secret should reference the new key") + + readBack := &corev1.Secret{} + Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) + Expect(readBack.Data["testKey"]).To(Equal([]byte("aescbc-reencryption-test-data")), + "test secret data should be readable after re-encryption") + + svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) + Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") + Expect(svmList.Items).NotTo(BeEmpty(), + "StorageVersionMigration CRs should exist after AESCBC key rotation") + + verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) + verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) + + verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + }) + }) +} + +func verifyConditionBubbleUp(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, controlPlaneNamespace string) { + hc := &hyperv1.HostedCluster{} + Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + + hcp := &hyperv1.HostedControlPlane{} + Expect(mgmtClient.Get(ctx, crclient.ObjectKey{ + Name: hc.Name, + Namespace: controlPlaneNamespace, + }, hcp)).To(Succeed()) + + hcCond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) + hcpCond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) + + Expect(hcCond).NotTo(BeNil(), + "HostedCluster %s/%s should have EtcdDataEncryptionUpToDate condition", hcKey.Namespace, hcKey.Name) + Expect(hcpCond).NotTo(BeNil(), + "HostedControlPlane should have EtcdDataEncryptionUpToDate condition if HostedCluster does") + Expect(hcCond.Status).To(Equal(hcpCond.Status), + "EtcdDataEncryptionUpToDate status should match between HostedCluster and HostedControlPlane") + Expect(hcCond.Reason).To(Equal(hcpCond.Reason), + "EtcdDataEncryptionUpToDate reason should match between HostedCluster and HostedControlPlane") +} + +// ConditionBubbleUpTest verifies that the EtcdDataEncryptionUpToDate condition +// is consistent between HostedControlPlane and HostedCluster. +func ConditionBubbleUpTest(getTestCtx internal.TestContextGetter) { + Context("Condition Bubble-Up", func() { + It("should have matching EtcdDataEncryptionUpToDate on HCP and HostedCluster", func() { + tc := getTestCtx() + hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} + Eventually(func(g Gomega) { + hc := &hyperv1.HostedCluster{} + g.Expect(tc.MgmtClient.Get(tc.Context, hcKey, hc)).To(Succeed()) + + hcp := &hyperv1.HostedControlPlane{} + g.Expect(tc.MgmtClient.Get(tc.Context, crclient.ObjectKey{ + Name: hc.Name, + Namespace: tc.ControlPlaneNamespace, + }, hcp)).To(Succeed()) + + hcCond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) + hcpCond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) + + g.Expect(hcCond).NotTo(BeNil(), + "HostedCluster %s/%s should have EtcdDataEncryptionUpToDate condition", hcKey.Namespace, hcKey.Name) + g.Expect(hcpCond).NotTo(BeNil(), + "HostedControlPlane should have EtcdDataEncryptionUpToDate condition if HostedCluster does") + g.Expect(hcCond.Status).To(Equal(hcpCond.Status), + "EtcdDataEncryptionUpToDate status should match between HostedCluster and HostedControlPlane") + g.Expect(hcCond.Reason).To(Equal(hcpCond.Reason), + "EtcdDataEncryptionUpToDate reason should match between HostedCluster and HostedControlPlane") + }, 2*time.Minute, 10*time.Second).Should(Succeed()) + }) + }) +} + From a924b82c742d9b5ea6f0b54957dd84151b947910 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Wed, 9 Sep 2026 09:58:47 -0400 Subject: [PATCH 02/24] test: fix := to = for rand.Read in etcd re-encryption test err was already declared in the enclosing scope; the blank identifier does not count as a new variable so := is rejected by go vet. Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/tests/etcd_encryption_reencryption_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index dfa5cb02ebc9..4f02757b67c9 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -526,7 +526,7 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { }) keyData := make([]byte, 32) - _, err := rand.Read(keyData) + _, err = rand.Read(keyData) Expect(err).NotTo(HaveOccurred(), "failed to generate random key data") newKeySecret := &corev1.Secret{ From a1e17f845ac9348e8724a87e811e4d6cf2378347 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Wed, 9 Sep 2026 10:48:19 -0400 Subject: [PATCH 03/24] test: fix lint issues in etcd re-encryption test Replace context.Background() with tc.Context per hypershiftlinter requirement, and fix gci import ordering (separate sigs.k8s.io section with blank line from k8s.io). Co-Authored-By: Claude Sonnet 4.6 --- .../etcd_encryption_reencryption_test.go | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 4f02757b67c9..3780a490eb1d 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" + crclient "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -229,7 +230,7 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { } Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) DeferCleanup(func() { - if err := hostedClusterClient.Delete(context.Background(), testSecret); err != nil && !apierrors.IsNotFound(err) { + if err := hostedClusterClient.Delete(tc.Context, testSecret); err != nil && !apierrors.IsNotFound(err) { GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) } }) @@ -244,11 +245,11 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { hcRestore := &hyperv1.HostedCluster{ ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, } - if err := tc.MgmtClient.Patch(context.Background(), hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { + if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { GinkgoWriter.Printf("WARNING: failed to restore original KMS key ARN: %v\n", err) return } - waitForReEncryptionComplete(context.Background(), tc.MgmtClient, hcKey, 25*time.Minute) + waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) }) waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) @@ -322,7 +323,7 @@ func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { } Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) DeferCleanup(func() { - if err := hostedClusterClient.Delete(context.Background(), testSecret); err != nil && !apierrors.IsNotFound(err) { + if err := hostedClusterClient.Delete(tc.Context, testSecret); err != nil && !apierrors.IsNotFound(err) { GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) } }) @@ -337,11 +338,11 @@ func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { hcRestore := &hyperv1.HostedCluster{ ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, } - if err := tc.MgmtClient.Patch(context.Background(), hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { + if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { GinkgoWriter.Printf("WARNING: failed to restore original Azure key version: %v\n", err) return } - waitForReEncryptionComplete(context.Background(), tc.MgmtClient, hcKey, 25*time.Minute) + waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) }) waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) @@ -416,7 +417,7 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { } Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) DeferCleanup(func() { - if err := hostedClusterClient.Delete(context.Background(), testSecret); err != nil && !apierrors.IsNotFound(err) { + if err := hostedClusterClient.Delete(tc.Context, testSecret); err != nil && !apierrors.IsNotFound(err) { GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) } }) @@ -426,11 +427,11 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { hcRestore := &hyperv1.HostedCluster{ ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, } - if err := tc.MgmtClient.Patch(context.Background(), hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { + if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { GinkgoWriter.Printf("WARNING: failed to restore original Azure key version: %v\n", err) return } - waitForReEncryptionComplete(context.Background(), tc.MgmtClient, hcKey, 25*time.Minute) + waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) }) alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") @@ -520,7 +521,7 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { } Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) DeferCleanup(func() { - if err := hostedClusterClient.Delete(context.Background(), testSecret); err != nil && !apierrors.IsNotFound(err) { + if err := hostedClusterClient.Delete(tc.Context, testSecret); err != nil && !apierrors.IsNotFound(err) { GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) } }) @@ -541,7 +542,7 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { } Expect(tc.MgmtClient.Create(ctx, newKeySecret)).To(Succeed()) DeferCleanup(func() { - if err := tc.MgmtClient.Delete(context.Background(), newKeySecret); err != nil && !apierrors.IsNotFound(err) { + if err := tc.MgmtClient.Delete(tc.Context, newKeySecret); err != nil && !apierrors.IsNotFound(err) { GinkgoWriter.Printf("WARNING: failed to cleanup AESCBC key secret: %v\n", err) } }) @@ -555,11 +556,11 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { hcRestore := &hyperv1.HostedCluster{ ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, } - if err := tc.MgmtClient.Patch(context.Background(), hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { + if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { GinkgoWriter.Printf("WARNING: failed to restore original AESCBC key: %v\n", err) return } - waitForReEncryptionComplete(context.Background(), tc.MgmtClient, hcKey, 25*time.Minute) + waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) }) waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) @@ -643,4 +644,3 @@ func ConditionBubbleUpTest(getTestCtx internal.TestContextGetter) { }) }) } - From c1266c59688812c736208cb0b932086b1ab59d6f Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Wed, 9 Sep 2026 11:32:09 -0400 Subject: [PATCH 04/24] test: add InformingLabel to new etcd re-encryption It() blocks hypershiftlinter e2eteststate requires every new Ginkgo subject to carry Label(internal.InformingLabel) or Label(internal.BlockingLabel). Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/tests/etcd_encryption_reencryption_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 3780a490eb1d..a75db37e8acd 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -204,7 +204,7 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { } }) - It("should re-encrypt all etcd data after active key rotation", func() { + It("should re-encrypt all etcd data after active key rotation", Label(internal.InformingLabel), func() { tc := getTestCtx() ctx := tc.Context hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} @@ -297,7 +297,7 @@ func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { } }) - It("should re-encrypt all etcd data after key version rotation", func() { + It("should re-encrypt all etcd data after key version rotation", Label(internal.InformingLabel), func() { tc := getTestCtx() ctx := tc.Context hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} @@ -391,7 +391,7 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { } }) - It("should complete two consecutive key rotations successfully", func() { + It("should complete two consecutive key rotations successfully", Label(internal.InformingLabel), func() { tc := getTestCtx() ctx := tc.Context hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} From b92840e091dc5cf1f03be6eac5fb519b71b7b5f9 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Tue, 22 Sep 2026 17:45:20 -0400 Subject: [PATCH 05/24] test: re-read HostedCluster before computing priorHistoryLen Re-fetch hc after waitForReEncryptionComplete to ensure priorHistoryLen reflects the actual current history length. Without the fresh read, a re-encryption completing during the initial wait inflates the history before the baseline is captured, causing waitForReEncryptionStarted to pass on its first poll without verifying the rotation actually started. Signed-off-by: jiezhao16 Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/tests/etcd_encryption_reencryption_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index a75db37e8acd..ba7213c3046e 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -236,6 +236,7 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { }) alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) priorHistoryLen := len(hc.Status.SecretEncryption.History) patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, alternateARN)) Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) @@ -329,6 +330,7 @@ func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { }) alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) priorHistoryLen := len(hc.Status.SecretEncryption.History) patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) @@ -437,6 +439,7 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") By("Performing first key rotation: original -> alternate") + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) priorHistoryLen := len(hc.Status.SecretEncryption.History) patch1 := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch1))).To(Succeed()) @@ -547,6 +550,7 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { } }) + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) priorHistoryLen := len(hc.Status.SecretEncryption.History) patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, newKeySecret.Name)) Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) From b86e8a3f1a4e4c5a47021fa1339ef83a54914f92 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Sun, 27 Sep 2026 21:50:13 -0400 Subject: [PATCH 06/24] =?UTF-8?q?test:=20address=20muraee=20review=20?= =?UTF-8?q?=E2=80=94=20table-driven=20rotation=20tests,=20ring-buffer=20fi?= =?UTF-8?q?x,=20KAS=20log=20depth?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace waitForReEncryptionStarted's History-length check with an EtcdDataEncryptionUpToDate not-True wait. The old len(History) > prior check fails silently once History saturates its 5-item ring buffer cap, causing the Eventually to time out even on a successful rotation. - Collapse AWSKMSKeyRotationTest, AzureKMSKeyRotationTest, and AESCBCKeyRotationTest into a shared runSingleRotationTest helper parameterized by a rotationCase struct. Each test now expresses only its per-provider skip guard, patch construction, and provider-field assertion — the common lifecycle assertions live in one place. - Add Label(internal.InformingLabel) to AESCBCKeyRotationTest and ConditionBubbleUpTest to match the other KMS test siblings. - Check previous-container logs in verifyKASLogsNoDecryptionErrors and increase TailLines from 500 to 5000 to catch decryption errors from rolled KAS pods or long rotation windows. - Replace local int64Ptr helper with ptr.To(int64(...)) from the already- vendored k8s.io/utils/ptr package. Co-Authored-By: Claude Sonnet 4.6 --- .../etcd_encryption_reencryption_test.go | 420 ++++++++---------- 1 file changed, 183 insertions(+), 237 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index ba7213c3046e..190106d115e8 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" + "k8s.io/utils/ptr" crclient "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -69,14 +70,18 @@ func RegisterEtcdEncryptionReencryptionTests(getTestCtx internal.TestContextGett ConditionBubbleUpTest(getTestCtx) } -func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, priorHistoryLen int, timeout time.Duration) { +// waitForReEncryptionStarted waits until EtcdDataEncryptionUpToDate is not True, +// which signals that a re-encryption is in progress. Using the condition rather than +// History length avoids a ring-buffer saturation bug: History is capped at MaxItems=5, +// so once full a length-based check never fires even though rotation succeeded. +func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { Eventually(func(g Gomega) { hc := &hyperv1.HostedCluster{} g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - g.Expect(meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate))). - NotTo(BeNil(), "EtcdDataEncryptionUpToDate condition should exist after the key rotation patch") - g.Expect(len(hc.Status.SecretEncryption.History)).To(BeNumerically(">", priorHistoryLen), - "a new migration history entry should be recorded after the key rotation patch") + cond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) + g.Expect(cond).NotTo(BeNil(), "EtcdDataEncryptionUpToDate condition should exist after the key rotation patch") + g.Expect(cond.Status).NotTo(Equal(metav1.ConditionTrue), + "EtcdDataEncryptionUpToDate should be not-True while re-encryption is in progress") }, timeout, 5*time.Second).Should(Succeed()) } @@ -146,6 +151,10 @@ func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPla }, 5*time.Minute, 15*time.Second).Should(Succeed()) } +// verifyKASLogsNoDecryptionErrors checks both the current and previous container logs +// of every KAS pod for "no matching prefix found" decryption errors. Checking previous +// containers catches errors that occurred during pod restarts mid-rotation; a larger +// TailLines guards against errors scrolling past the tail during the up-to-25-min window. func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace string) { mgmtRestConfig, err := e2eutil.GetConfig() Expect(err).NotTo(HaveOccurred(), "failed to get management cluster REST config") @@ -160,59 +169,79 @@ func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace "expected at least one kube-apiserver pod in namespace %s", controlPlaneNamespace) for _, pod := range kasPods.Items { - req := mgmtKubeClient.CoreV1().Pods(controlPlaneNamespace).GetLogs(pod.Name, &corev1.PodLogOptions{ - Container: "kube-apiserver", - TailLines: int64Ptr(500), - }) - logStream, err := req.Stream(ctx) - if err != nil { - GinkgoWriter.Printf("WARNING: failed to get logs for KAS pod %s: %v\n", pod.Name, err) - continue - } - logBytes, err := io.ReadAll(logStream) - logStream.Close() - if err != nil { - GinkgoWriter.Printf("WARNING: failed to read logs for KAS pod %s: %v\n", pod.Name, err) - continue + for _, previous := range []bool{false, true} { + req := mgmtKubeClient.CoreV1().Pods(controlPlaneNamespace).GetLogs(pod.Name, &corev1.PodLogOptions{ + Container: "kube-apiserver", + TailLines: ptr.To(int64(5000)), + Previous: previous, + }) + logStream, err := req.Stream(ctx) + if err != nil { + // Previous-container logs are absent when the pod has not restarted — skip silently. + if !previous { + GinkgoWriter.Printf("WARNING: failed to get logs for KAS pod %s: %v\n", pod.Name, err) + } + continue + } + logBytes, err := io.ReadAll(logStream) + logStream.Close() + if err != nil { + GinkgoWriter.Printf("WARNING: failed to read logs for KAS pod %s (previous=%v): %v\n", pod.Name, previous, err) + continue + } + Expect(string(logBytes)).NotTo(ContainSubstring("no matching prefix found"), + "KAS pod %s logs (previous=%v) contain decryption error 'no matching prefix found'", pod.Name, previous) } - logContent := string(logBytes) - Expect(logContent).NotTo(ContainSubstring("no matching prefix found"), - "KAS pod %s logs contain decryption error 'no matching prefix found'", pod.Name) } } -func int64Ptr(i int64) *int64 { - return &i +// rotationCase parameterizes a single key-rotation lifecycle test. The shared +// test body (runSingleRotationTest) runs the common assertions; only the +// per-provider skip guard, patch construction, and provider-field verification differ. +type rotationCase struct { + // contextLabel is the Ginkgo Context description, e.g. "[Feature:AWSKMSReencryption] AWS KMS Key Rotation". + contextLabel string + // itLabel is the Ginkgo It description. + itLabel string + // testSecretName is the name of the canary Secret created in the hosted cluster. + testSecretName string + // testSecretData is the value written to the "testKey" field of the canary Secret. + testSecretData string + // expectedProvider is the SecretEncryptionProvider expected in Status after rotation. + expectedProvider hyperv1.SecretEncryptionProvider + // applies returns (true, "") when this case should run, or (false, skipReason) to skip. + applies func(hc *hyperv1.HostedCluster, tc *internal.TestContext) (bool, string) + // setup prepares any provider-specific resources (registering its own DeferCleanups), + // then returns the rotate and restore merge-patches. + setup func(ctx context.Context, tc *internal.TestContext, hc *hyperv1.HostedCluster) (rotatePatch, restorePatch []byte, err error) + // verifyProviderField asserts provider-specific Status fields after rotation completes. + verifyProviderField func(hc *hyperv1.HostedCluster) } -// AWSKMSKeyRotationTest validates the re-encryption lifecycle after rotating the AWS KMS active key. -func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { - Context("[Feature:AWSKMSReencryption] AWS KMS Key Rotation", func() { +// runSingleRotationTest registers a single-rotation lifecycle test for the given rotationCase. +// It handles: baseline wait, canary secret, rotate patch, restore cleanup, started/complete waits, +// status verification, secret read-back, SVM check, KAS health/logs, and condition bubble-up. +func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCase) { + Context(rc.contextLabel, func() { BeforeEach(func() { tc := getTestCtx() hc, err := tc.GetHostedCluster() Expect(err).NotTo(HaveOccurred()) - if hc.Spec.Platform.Type != hyperv1.AWSPlatform || - hc.Spec.SecretEncryption.Type != hyperv1.KMS || - hc.Spec.SecretEncryption.KMS == nil || - hc.Spec.SecretEncryption.KMS.AWS == nil { - Skip("AWS KMS key rotation test requires AWS platform with KMS configured") - } - alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") - if alternateARN == "" { - Skip("E2E_AWS_KMS_KEY_ARN_ALTERNATE is not set") + ok, reason := rc.applies(hc, tc) + if !ok { + Skip(reason) } }) - It("should re-encrypt all etcd data after active key rotation", Label(internal.InformingLabel), func() { + It(rc.itLabel, Label(internal.InformingLabel), func() { tc := getTestCtx() ctx := tc.Context hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} hc := &hyperv1.HostedCluster{} Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - originalARN := hc.Spec.SecretEncryption.KMS.AWS.ActiveKey.ARN + // Ensure no re-encryption is in flight before rotating. waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) hostedClusterClient, err := tc.GetHostedClusterClient(hc) @@ -220,12 +249,12 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { testSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "e2e-reencryption-test-aws", + Name: rc.testSecretName, Namespace: "default", }, Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ - "testKey": []byte("aws-kms-reencryption-test-data"), + "testKey": []byte(rc.testSecretData), }, } Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) @@ -235,41 +264,40 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { } }) - alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") + // Re-fetch to get a fresh resource version and current Spec before patching. Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - priorHistoryLen := len(hc.Status.SecretEncryption.History) - patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, alternateARN)) - Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) + rotatePatch, restorePatch, err := rc.setup(ctx, tc, hc) + Expect(err).NotTo(HaveOccurred(), "failed to prepare rotation patches") + + Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, rotatePatch))).To(Succeed()) DeferCleanup(func() { - restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, originalARN)) hcRestore := &hyperv1.HostedCluster{ ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, } if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { - GinkgoWriter.Printf("WARNING: failed to restore original KMS key ARN: %v\n", err) + GinkgoWriter.Printf("WARNING: failed to restore original key: %v\n", err) return } waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) }) - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - verifyReEncryptionStatus(hc, hyperv1.SecretEncryptionProviderAWS) - Expect(hc.Status.SecretEncryption.ActiveKey.AWS.ARN).To(Equal(alternateARN), - "activeKey ARN should match the alternate key") + verifyReEncryptionStatus(hc, rc.expectedProvider) + rc.verifyProviderField(hc) readBack := &corev1.Secret{} Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) - Expect(readBack.Data["testKey"]).To(Equal([]byte("aws-kms-reencryption-test-data")), + Expect(readBack.Data["testKey"]).To(Equal([]byte(rc.testSecretData)), "test secret data should be readable after re-encryption") svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") Expect(svmList.Items).NotTo(BeEmpty(), - "StorageVersionMigration CRs should exist after KMS key rotation") + "StorageVersionMigration CRs should exist after key rotation") verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) @@ -279,97 +307,124 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { }) } +// AWSKMSKeyRotationTest validates the re-encryption lifecycle after rotating the AWS KMS active key. +func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { + runSingleRotationTest(getTestCtx, rotationCase{ + contextLabel: "[Feature:AWSKMSReencryption] AWS KMS Key Rotation", + itLabel: "should re-encrypt all etcd data after active key rotation", + testSecretName: "e2e-reencryption-test-aws", + testSecretData: "aws-kms-reencryption-test-data", + expectedProvider: hyperv1.SecretEncryptionProviderAWS, + applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { + if hc.Spec.Platform.Type != hyperv1.AWSPlatform || + hc.Spec.SecretEncryption.Type != hyperv1.KMS || + hc.Spec.SecretEncryption.KMS == nil || + hc.Spec.SecretEncryption.KMS.AWS == nil { + return false, "AWS KMS key rotation test requires AWS platform with KMS configured" + } + if internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") == "" { + return false, "E2E_AWS_KMS_KEY_ARN_ALTERNATE is not set" + } + return true, "" + }, + setup: func(_ context.Context, _ *internal.TestContext, hc *hyperv1.HostedCluster) ([]byte, []byte, error) { + alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") + originalARN := hc.Spec.SecretEncryption.KMS.AWS.ActiveKey.ARN + rotate := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, alternateARN)) + restore := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, originalARN)) + return rotate, restore, nil + }, + verifyProviderField: func(hc *hyperv1.HostedCluster) { + alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") + Expect(hc.Status.SecretEncryption.ActiveKey.AWS.ARN).To(Equal(alternateARN), + "activeKey ARN should match the alternate key") + }, + }) +} + // AzureKMSKeyRotationTest validates the re-encryption lifecycle after rotating the Azure KMS key version. func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { - Context("[Feature:AzureKMSReencryption] Azure KMS Key Rotation", func() { - BeforeEach(func() { - tc := getTestCtx() - hc, err := tc.GetHostedCluster() - Expect(err).NotTo(HaveOccurred()) + runSingleRotationTest(getTestCtx, rotationCase{ + contextLabel: "[Feature:AzureKMSReencryption] Azure KMS Key Rotation", + itLabel: "should re-encrypt all etcd data after key version rotation", + testSecretName: "e2e-reencryption-test-azure", + testSecretData: "azure-kms-reencryption-test-data", + expectedProvider: hyperv1.SecretEncryptionProviderAzure, + applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { if hc.Spec.Platform.Type != hyperv1.AzurePlatform || hc.Spec.SecretEncryption.Type != hyperv1.KMS || hc.Spec.SecretEncryption.KMS == nil || hc.Spec.SecretEncryption.KMS.Azure == nil { - Skip("Azure KMS key rotation test requires Azure platform with KMS configured") + return false, "Azure KMS key rotation test requires Azure platform with KMS configured" } - alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") - if alternateVersion == "" { - Skip("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE is not set") + if internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") == "" { + return false, "E2E_AZURE_KMS_KEY_VERSION_ALTERNATE is not set" } - }) - - It("should re-encrypt all etcd data after key version rotation", Label(internal.InformingLabel), func() { - tc := getTestCtx() - ctx := tc.Context - hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} - - hc := &hyperv1.HostedCluster{} - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + return true, "" + }, + setup: func(_ context.Context, _ *internal.TestContext, hc *hyperv1.HostedCluster) ([]byte, []byte, error) { + alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") originalVersion := hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion + rotate := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) + restore := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) + return rotate, restore, nil + }, + verifyProviderField: func(hc *hyperv1.HostedCluster) { + alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") + Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(alternateVersion), + "activeKey keyVersion should match the alternate version") + }, + }) +} - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) - - hostedClusterClient, err := tc.GetHostedClusterClient(hc) - Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") - - testSecret := &corev1.Secret{ +// AESCBCKeyRotationTest validates the re-encryption lifecycle after rotating the AESCBC encryption key. +func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { + runSingleRotationTest(getTestCtx, rotationCase{ + contextLabel: "[Feature:AESCBCReencryption] AESCBC Key Rotation", + itLabel: "should re-encrypt secrets after active key rotation", + testSecretName: "e2e-reencryption-test-aescbc", + testSecretData: "aescbc-reencryption-test-data", + expectedProvider: hyperv1.SecretEncryptionProviderAESCBC, + applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { + if hc.Spec.SecretEncryption.Type != hyperv1.AESCBC || hc.Spec.SecretEncryption.AESCBC == nil { + return false, "AESCBC key rotation test requires AESCBC encryption configured" + } + return true, "" + }, + setup: func(ctx context.Context, tc *internal.TestContext, hc *hyperv1.HostedCluster) ([]byte, []byte, error) { + keyData := make([]byte, 32) + if _, err := rand.Read(keyData); err != nil { + return nil, nil, fmt.Errorf("failed to generate random AESCBC key data: %w", err) + } + newKeySecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "e2e-reencryption-test-azure", - Namespace: "default", + Name: "e2e-aescbc-rotation-key", + Namespace: tc.ClusterNamespace, }, Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ - "testKey": []byte("azure-kms-reencryption-test-data"), + hyperv1.AESCBCKeySecretKey: keyData, }, } - Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) - DeferCleanup(func() { - if err := hostedClusterClient.Delete(tc.Context, testSecret); err != nil && !apierrors.IsNotFound(err) { - GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) - } - }) - - alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - priorHistoryLen := len(hc.Status.SecretEncryption.History) - patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) - Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) - + if err := tc.MgmtClient.Create(ctx, newKeySecret); err != nil { + return nil, nil, fmt.Errorf("failed to create AESCBC key secret: %w", err) + } DeferCleanup(func() { - restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) - hcRestore := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, - } - if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { - GinkgoWriter.Printf("WARNING: failed to restore original Azure key version: %v\n", err) - return + if err := tc.MgmtClient.Delete(tc.Context, newKeySecret); err != nil && !apierrors.IsNotFound(err) { + GinkgoWriter.Printf("WARNING: failed to cleanup AESCBC key secret: %v\n", err) } - waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) }) - - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) - - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - verifyReEncryptionStatus(hc, hyperv1.SecretEncryptionProviderAzure) - Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(alternateVersion), - "activeKey keyVersion should match the alternate version") - - readBack := &corev1.Secret{} - Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) - Expect(readBack.Data["testKey"]).To(Equal([]byte("azure-kms-reencryption-test-data")), - "test secret data should be readable after re-encryption") - - svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) - Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") - Expect(svmList.Items).NotTo(BeEmpty(), - "StorageVersionMigration CRs should exist after Azure KMS key rotation") - - verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) - verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) - - verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) - }) + originalKeyRef := hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name + rotate := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, newKeySecret.Name)) + restore := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, originalKeyRef)) + return rotate, restore, nil + }, + verifyProviderField: func(hc *hyperv1.HostedCluster) { + Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC).NotTo(BeNil(), + "activeKey AESCBC should not be nil after AESCBC key rotation") + Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC.Secret.Name).To(Equal("e2e-aescbc-rotation-key"), + "activeKey AESCBC secret should reference the new key") + }, }) } @@ -440,10 +495,9 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { By("Performing first key rotation: original -> alternate") Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - priorHistoryLen := len(hc.Status.SecretEncryption.History) patch1 := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch1))).To(Succeed()) - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) @@ -455,10 +509,10 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) By("Performing second key rotation: alternate -> original") - priorHistoryLen = len(hc.Status.SecretEncryption.History) + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) patch2 := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch2))).To(Succeed()) - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) @@ -485,114 +539,6 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { }) } -// AESCBCKeyRotationTest validates the re-encryption lifecycle after rotating the AESCBC encryption key. -func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { - Context("[Feature:AESCBCReencryption] AESCBC Key Rotation", func() { - BeforeEach(func() { - tc := getTestCtx() - hc, err := tc.GetHostedCluster() - Expect(err).NotTo(HaveOccurred()) - if hc.Spec.SecretEncryption.Type != hyperv1.AESCBC || - hc.Spec.SecretEncryption.AESCBC == nil { - Skip("AESCBC key rotation test requires AESCBC encryption configured") - } - }) - - It("should re-encrypt secrets after active key rotation", func() { - tc := getTestCtx() - ctx := tc.Context - hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} - - hc := &hyperv1.HostedCluster{} - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - originalKeyRef := hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name - - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) - - hostedClusterClient, err := tc.GetHostedClusterClient(hc) - Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") - - testSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "e2e-reencryption-test-aescbc", - Namespace: "default", - }, - Type: corev1.SecretTypeOpaque, - Data: map[string][]byte{ - "testKey": []byte("aescbc-reencryption-test-data"), - }, - } - Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) - DeferCleanup(func() { - if err := hostedClusterClient.Delete(tc.Context, testSecret); err != nil && !apierrors.IsNotFound(err) { - GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) - } - }) - - keyData := make([]byte, 32) - _, err = rand.Read(keyData) - Expect(err).NotTo(HaveOccurred(), "failed to generate random key data") - - newKeySecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "e2e-aescbc-rotation-key", - Namespace: tc.ClusterNamespace, - }, - Type: corev1.SecretTypeOpaque, - Data: map[string][]byte{ - hyperv1.AESCBCKeySecretKey: keyData, - }, - } - Expect(tc.MgmtClient.Create(ctx, newKeySecret)).To(Succeed()) - DeferCleanup(func() { - if err := tc.MgmtClient.Delete(tc.Context, newKeySecret); err != nil && !apierrors.IsNotFound(err) { - GinkgoWriter.Printf("WARNING: failed to cleanup AESCBC key secret: %v\n", err) - } - }) - - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - priorHistoryLen := len(hc.Status.SecretEncryption.History) - patch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, newKeySecret.Name)) - Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch))).To(Succeed()) - - DeferCleanup(func() { - restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, originalKeyRef)) - hcRestore := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, - } - if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { - GinkgoWriter.Printf("WARNING: failed to restore original AESCBC key: %v\n", err) - return - } - waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) - }) - - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, priorHistoryLen, 5*time.Minute) - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) - - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - verifyReEncryptionStatus(hc, hyperv1.SecretEncryptionProviderAESCBC) - Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC.Secret.Name).To(Equal(newKeySecret.Name), - "activeKey AESCBC secret should reference the new key") - - readBack := &corev1.Secret{} - Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) - Expect(readBack.Data["testKey"]).To(Equal([]byte("aescbc-reencryption-test-data")), - "test secret data should be readable after re-encryption") - - svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) - Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") - Expect(svmList.Items).NotTo(BeEmpty(), - "StorageVersionMigration CRs should exist after AESCBC key rotation") - - verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) - verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) - - verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) - }) - }) -} - func verifyConditionBubbleUp(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, controlPlaneNamespace string) { hc := &hyperv1.HostedCluster{} Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) @@ -620,7 +566,7 @@ func verifyConditionBubbleUp(ctx context.Context, mgmtClient crclient.Client, hc // is consistent between HostedControlPlane and HostedCluster. func ConditionBubbleUpTest(getTestCtx internal.TestContextGetter) { Context("Condition Bubble-Up", func() { - It("should have matching EtcdDataEncryptionUpToDate on HCP and HostedCluster", func() { + It("should have matching EtcdDataEncryptionUpToDate on HCP and HostedCluster", Label(internal.InformingLabel), func() { tc := getTestCtx() hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} Eventually(func(g Gomega) { From e8b75a2dd2d31ba2bc925b5d4b2eb5fda81b200e Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Thu, 1 Oct 2026 21:31:39 -0400 Subject: [PATCH 07/24] test: name kasMaxAllowedRestarts constant and explain the value Replace the magic int32(2) restart ceiling with a named constant and a comment explaining why 2: one restart is expected from the KAS rolling out with the updated EncryptionConfiguration; the second slot allows a single transient failure without masking a real crash loop. Signed-off-by: jiezhao16 Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/tests/etcd_encryption_reencryption_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 190106d115e8..e4cbffb6c023 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -43,6 +43,11 @@ import ( crclient "sigs.k8s.io/controller-runtime/pkg/client" ) +// kasMaxAllowedRestarts is the restart ceiling checked after re-encryption completes. +// One restart is expected (KAS rolls out with the updated EncryptionConfiguration); 2 +// allows a single transient failure on top of that without masking a true crash loop. +const kasMaxAllowedRestarts = int32(2) + var _ = Describe("[sig-hypershift][Jira:Hypershift][Feature:EtcdEncryptionReencryption] Etcd Encryption Re-encryption", Label("etcd-encryption-reencryption"), func() { var testCtx *internal.TestContext @@ -144,7 +149,7 @@ func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPla g.Expect(cs.State.Waiting.Reason).NotTo(Equal("CrashLoopBackOff"), "KAS pod %s container %s is in CrashLoopBackOff", pod.Name, cs.Name) } - g.Expect(cs.RestartCount).To(BeNumerically("<=", int32(2)), + g.Expect(cs.RestartCount).To(BeNumerically("<=", kasMaxAllowedRestarts), "KAS pod %s container %s has %d restarts", pod.Name, cs.Name, cs.RestartCount) } } From 323401b33a3c1e4b8e1b86cd201c5a4948c7ece3 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Fri, 2 Oct 2026 15:14:29 -0400 Subject: [PATCH 08/24] test: hard-fail on current KAS log stream error, silently skip previous MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous/current guard in verifyKASLogsNoDecryptionErrors was inverted: it warned and continued for the current container (silently skipping the decryption-error check) and silently skipped for the previous container (correct). Fix the condition so that: - previous=true: silently continue — pod may not have restarted - previous=false: hard Expect failure — current logs must be readable Signed-off-by: jiezhao16 Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/tests/etcd_encryption_reencryption_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index e4cbffb6c023..866e539b1a58 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -182,11 +182,11 @@ func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace }) logStream, err := req.Stream(ctx) if err != nil { - // Previous-container logs are absent when the pod has not restarted — skip silently. - if !previous { - GinkgoWriter.Printf("WARNING: failed to get logs for KAS pod %s: %v\n", pod.Name, err) + if previous { + // Previous-container logs are absent when the pod has not restarted — skip silently. + continue } - continue + Expect(err).NotTo(HaveOccurred(), "failed to stream current logs for KAS pod %s", pod.Name) } logBytes, err := io.ReadAll(logStream) logStream.Close() From 430eb16af24792e4eb6a0c7b249561c06123a903 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Mon, 5 Oct 2026 12:05:40 -0400 Subject: [PATCH 09/24] test: drop canary secret read-back from re-encryption tests The canary was a readability-only check: KAS keeps old keys in its keyring throughout rotation, so reading back a secret succeeds even if that specific object was never re-encrypted with the new key. The real coverage is carried by the SVM presence check, status field assertions (activeKey, history, completionTime), and KAS decryption-error log scan. Remove testSecretName/testSecretData from rotationCase, drop the canary create/delete/read-back from runSingleRotationTest and from AzureKMSConsecutiveKeyRotationTest. Signed-off-by: jiezhao16 Co-Authored-By: Claude Sonnet 4.6 --- .../etcd_encryption_reencryption_test.go | 61 +------------------ 1 file changed, 2 insertions(+), 59 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 866e539b1a58..b23820b1f17b 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -208,10 +208,6 @@ type rotationCase struct { contextLabel string // itLabel is the Ginkgo It description. itLabel string - // testSecretName is the name of the canary Secret created in the hosted cluster. - testSecretName string - // testSecretData is the value written to the "testKey" field of the canary Secret. - testSecretData string // expectedProvider is the SecretEncryptionProvider expected in Status after rotation. expectedProvider hyperv1.SecretEncryptionProvider // applies returns (true, "") when this case should run, or (false, skipReason) to skip. @@ -224,8 +220,8 @@ type rotationCase struct { } // runSingleRotationTest registers a single-rotation lifecycle test for the given rotationCase. -// It handles: baseline wait, canary secret, rotate patch, restore cleanup, started/complete waits, -// status verification, secret read-back, SVM check, KAS health/logs, and condition bubble-up. +// It handles: baseline wait, rotate patch, restore cleanup, started/complete waits, +// status verification, SVM check, KAS health/logs, and condition bubble-up. func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCase) { Context(rc.contextLabel, func() { BeforeEach(func() { @@ -252,23 +248,6 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas hostedClusterClient, err := tc.GetHostedClusterClient(hc) Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") - testSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: rc.testSecretName, - Namespace: "default", - }, - Type: corev1.SecretTypeOpaque, - Data: map[string][]byte{ - "testKey": []byte(rc.testSecretData), - }, - } - Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) - DeferCleanup(func() { - if err := hostedClusterClient.Delete(tc.Context, testSecret); err != nil && !apierrors.IsNotFound(err) { - GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) - } - }) - // Re-fetch to get a fresh resource version and current Spec before patching. Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) @@ -294,11 +273,6 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas verifyReEncryptionStatus(hc, rc.expectedProvider) rc.verifyProviderField(hc) - readBack := &corev1.Secret{} - Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) - Expect(readBack.Data["testKey"]).To(Equal([]byte(rc.testSecretData)), - "test secret data should be readable after re-encryption") - svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") Expect(svmList.Items).NotTo(BeEmpty(), @@ -317,8 +291,6 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { runSingleRotationTest(getTestCtx, rotationCase{ contextLabel: "[Feature:AWSKMSReencryption] AWS KMS Key Rotation", itLabel: "should re-encrypt all etcd data after active key rotation", - testSecretName: "e2e-reencryption-test-aws", - testSecretData: "aws-kms-reencryption-test-data", expectedProvider: hyperv1.SecretEncryptionProviderAWS, applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { if hc.Spec.Platform.Type != hyperv1.AWSPlatform || @@ -352,8 +324,6 @@ func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { runSingleRotationTest(getTestCtx, rotationCase{ contextLabel: "[Feature:AzureKMSReencryption] Azure KMS Key Rotation", itLabel: "should re-encrypt all etcd data after key version rotation", - testSecretName: "e2e-reencryption-test-azure", - testSecretData: "azure-kms-reencryption-test-data", expectedProvider: hyperv1.SecretEncryptionProviderAzure, applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { if hc.Spec.Platform.Type != hyperv1.AzurePlatform || @@ -387,8 +357,6 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { runSingleRotationTest(getTestCtx, rotationCase{ contextLabel: "[Feature:AESCBCReencryption] AESCBC Key Rotation", itLabel: "should re-encrypt secrets after active key rotation", - testSecretName: "e2e-reencryption-test-aescbc", - testSecretData: "aescbc-reencryption-test-data", expectedProvider: hyperv1.SecretEncryptionProviderAESCBC, applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { if hc.Spec.SecretEncryption.Type != hyperv1.AESCBC || hc.Spec.SecretEncryption.AESCBC == nil { @@ -464,26 +432,6 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) - hostedClusterClient, err := tc.GetHostedClusterClient(hc) - Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") - - testSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "e2e-reencryption-test-azure-consecutive", - Namespace: "default", - }, - Type: corev1.SecretTypeOpaque, - Data: map[string][]byte{ - "testKey": []byte("azure-kms-consecutive-rotation-test-data"), - }, - } - Expect(hostedClusterClient.Create(ctx, testSecret)).To(Succeed()) - DeferCleanup(func() { - if err := hostedClusterClient.Delete(tc.Context, testSecret); err != nil && !apierrors.IsNotFound(err) { - GinkgoWriter.Printf("WARNING: failed to cleanup test secret: %v\n", err) - } - }) - DeferCleanup(func() { restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) hcRestore := &hyperv1.HostedCluster{ @@ -531,11 +479,6 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(hc.Status.SecretEncryption.History[1].State).To(Equal(hyperv1.EncryptionMigrationStateCompleted), "second history entry should also be Completed") - readBack := &corev1.Secret{} - Expect(hostedClusterClient.Get(ctx, crclient.ObjectKeyFromObject(testSecret), readBack)).To(Succeed()) - Expect(readBack.Data["testKey"]).To(Equal([]byte("azure-kms-consecutive-rotation-test-data")), - "test secret data should be readable after two consecutive re-encryptions") - verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) From 202f4960f49c69c15c6ff2684a7bd72a96611fdf Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Mon, 5 Oct 2026 12:50:35 -0400 Subject: [PATCH 10/24] test: use TargetKey.Provider as level-trigger for waitForReEncryptionStarted Replace the EtcdDataEncryptionUpToDate != True edge-trigger with a check on TargetKey.Provider being non-empty. TargetKey is set at the start of rotation and stays set until completion, so it cannot be missed regardless of the poll interval. The previous condition-based check was an edge trigger: a rotation completing within a single poll interval would cause the wait to time out on a successful rotation. Signed-off-by: jiezhao16 Co-Authored-By: Claude Sonnet 4.6 --- .../v2/tests/etcd_encryption_reencryption_test.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index b23820b1f17b..9f44e667a77e 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -75,18 +75,16 @@ func RegisterEtcdEncryptionReencryptionTests(getTestCtx internal.TestContextGett ConditionBubbleUpTest(getTestCtx) } -// waitForReEncryptionStarted waits until EtcdDataEncryptionUpToDate is not True, -// which signals that a re-encryption is in progress. Using the condition rather than -// History length avoids a ring-buffer saturation bug: History is capped at MaxItems=5, -// so once full a length-based check never fires even though rotation succeeded. +// waitForReEncryptionStarted waits until TargetKey.Provider is non-empty, which means +// the reconciler has started a new rotation. TargetKey is set at the beginning of a +// rotation and stays set until the rotation completes — it is a level trigger, not an +// edge trigger, so it cannot be missed regardless of the poll interval. func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { Eventually(func(g Gomega) { hc := &hyperv1.HostedCluster{} g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - cond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) - g.Expect(cond).NotTo(BeNil(), "EtcdDataEncryptionUpToDate condition should exist after the key rotation patch") - g.Expect(cond.Status).NotTo(Equal(metav1.ConditionTrue), - "EtcdDataEncryptionUpToDate should be not-True while re-encryption is in progress") + g.Expect(hc.Status.SecretEncryption.TargetKey.Provider).NotTo(BeEmpty(), + "TargetKey.Provider should be set once rotation has started") }, timeout, 5*time.Second).Should(Succeed()) } From ccc9cb9d231b40f35dbd4a4c9cd3889312aa767e Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Mon, 5 Oct 2026 12:55:39 -0400 Subject: [PATCH 11/24] test: wrap verifyConditionBubbleUp in Eventually to fix flake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HCP→HC condition propagation is async: calling verifyConditionBubbleUp single-shot right after a state transition could catch the HC before the hypershift-operator copies the updated condition, causing a spurious failure. - Add Gomega param to verifyConditionBubbleUp so it can be used inside Eventually without panicking on assertion failure. - Wrap all three call sites in Eventually(2m, 10s). - Simplify ConditionBubbleUpTest to call the shared helper instead of duplicating the four assertions inline. Signed-off-by: jiezhao16 Co-Authored-By: Claude Sonnet 4.6 --- .../etcd_encryption_reencryption_test.go | 50 ++++++++----------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 9f44e667a77e..ddf1734cfe2f 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -279,7 +279,9 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) - verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + Eventually(func(g Gomega) { + verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + }, 2*time.Minute, 10*time.Second).Should(Succeed()) }) }) } @@ -457,7 +459,9 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(hc.Status.SecretEncryption.History).NotTo(BeEmpty(), "history should have at least one entry after first rotation") - verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + Eventually(func(g Gomega) { + verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + }, 2*time.Minute, 10*time.Second).Should(Succeed()) By("Performing second key rotation: alternate -> original") Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) @@ -480,17 +484,22 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) - verifyConditionBubbleUp(ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + Eventually(func(g Gomega) { + verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + }, 2*time.Minute, 10*time.Second).Should(Succeed()) }) }) } -func verifyConditionBubbleUp(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, controlPlaneNamespace string) { +// verifyConditionBubbleUp asserts that EtcdDataEncryptionUpToDate is consistent between +// the HostedControlPlane and the HostedCluster. HCP→HC propagation is async, so callers +// must wrap this inside Eventually rather than calling it single-shot. +func verifyConditionBubbleUp(g Gomega, ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, controlPlaneNamespace string) { hc := &hyperv1.HostedCluster{} - Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) hcp := &hyperv1.HostedControlPlane{} - Expect(mgmtClient.Get(ctx, crclient.ObjectKey{ + g.Expect(mgmtClient.Get(ctx, crclient.ObjectKey{ Name: hc.Name, Namespace: controlPlaneNamespace, }, hcp)).To(Succeed()) @@ -498,13 +507,13 @@ func verifyConditionBubbleUp(ctx context.Context, mgmtClient crclient.Client, hc hcCond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) hcpCond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) - Expect(hcCond).NotTo(BeNil(), + g.Expect(hcCond).NotTo(BeNil(), "HostedCluster %s/%s should have EtcdDataEncryptionUpToDate condition", hcKey.Namespace, hcKey.Name) - Expect(hcpCond).NotTo(BeNil(), + g.Expect(hcpCond).NotTo(BeNil(), "HostedControlPlane should have EtcdDataEncryptionUpToDate condition if HostedCluster does") - Expect(hcCond.Status).To(Equal(hcpCond.Status), + g.Expect(hcCond.Status).To(Equal(hcpCond.Status), "EtcdDataEncryptionUpToDate status should match between HostedCluster and HostedControlPlane") - Expect(hcCond.Reason).To(Equal(hcpCond.Reason), + g.Expect(hcCond.Reason).To(Equal(hcpCond.Reason), "EtcdDataEncryptionUpToDate reason should match between HostedCluster and HostedControlPlane") } @@ -516,26 +525,7 @@ func ConditionBubbleUpTest(getTestCtx internal.TestContextGetter) { tc := getTestCtx() hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} Eventually(func(g Gomega) { - hc := &hyperv1.HostedCluster{} - g.Expect(tc.MgmtClient.Get(tc.Context, hcKey, hc)).To(Succeed()) - - hcp := &hyperv1.HostedControlPlane{} - g.Expect(tc.MgmtClient.Get(tc.Context, crclient.ObjectKey{ - Name: hc.Name, - Namespace: tc.ControlPlaneNamespace, - }, hcp)).To(Succeed()) - - hcCond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) - hcpCond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) - - g.Expect(hcCond).NotTo(BeNil(), - "HostedCluster %s/%s should have EtcdDataEncryptionUpToDate condition", hcKey.Namespace, hcKey.Name) - g.Expect(hcpCond).NotTo(BeNil(), - "HostedControlPlane should have EtcdDataEncryptionUpToDate condition if HostedCluster does") - g.Expect(hcCond.Status).To(Equal(hcpCond.Status), - "EtcdDataEncryptionUpToDate status should match between HostedCluster and HostedControlPlane") - g.Expect(hcCond.Reason).To(Equal(hcpCond.Reason), - "EtcdDataEncryptionUpToDate reason should match between HostedCluster and HostedControlPlane") + verifyConditionBubbleUp(g, tc.Context, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) }, 2*time.Minute, 10*time.Second).Should(Succeed()) }) }) From 63cfd4d7005da2429bd320b39a84ca42e3e06f81 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Mon, 5 Oct 2026 13:18:08 -0400 Subject: [PATCH 12/24] test: replace raw JSON merge-patches with typed crclient.MergeFrom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace all six fmt.Sprintf raw JSON merge-patch byte slices with typed crclient.MergeFrom patches. Introduce a rotateAndWait helper that does Get → DeepCopy → mutate → MergeFrom → waitForReEncryptionStarted → waitForReEncryptionComplete, eliminating the repeated patch+wait pattern. Refactor rotationCase.setup (which returned raw []byte pairs) into three typed accessors — prepare (returns alternate key id), getActiveKey, and setActiveKey — plus a verifyProviderField(hc, alternate) that receives the alternate value directly instead of re-reading the env var. The AESCBC verifyProviderField now compares against the actual secret name returned by prepare rather than a hardcoded string. Remove the now-unused k8s.io/apimachinery/pkg/types import. Co-Authored-By: Claude Sonnet 4.6 --- .../etcd_encryption_reencryption_test.go | 138 +++++++++--------- 1 file changed, 72 insertions(+), 66 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index ddf1734cfe2f..70324b18baa2 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -36,7 +36,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" "k8s.io/utils/ptr" @@ -200,7 +199,7 @@ func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace // rotationCase parameterizes a single key-rotation lifecycle test. The shared // test body (runSingleRotationTest) runs the common assertions; only the -// per-provider skip guard, patch construction, and provider-field verification differ. +// per-provider skip guard, key accessors, and provider-field verification differ. type rotationCase struct { // contextLabel is the Ginkgo Context description, e.g. "[Feature:AWSKMSReencryption] AWS KMS Key Rotation". contextLabel string @@ -210,11 +209,15 @@ type rotationCase struct { expectedProvider hyperv1.SecretEncryptionProvider // applies returns (true, "") when this case should run, or (false, skipReason) to skip. applies func(hc *hyperv1.HostedCluster, tc *internal.TestContext) (bool, string) - // setup prepares any provider-specific resources (registering its own DeferCleanups), - // then returns the rotate and restore merge-patches. - setup func(ctx context.Context, tc *internal.TestContext, hc *hyperv1.HostedCluster) (rotatePatch, restorePatch []byte, err error) + // prepare does any provider-specific setup (e.g. creating secrets), registers its own + // DeferCleanups, and returns the alternate key identifier to rotate to. + prepare func(ctx context.Context, tc *internal.TestContext, hc *hyperv1.HostedCluster) (alternate string, err error) + // getActiveKey extracts the current active-key identifier from the HC spec. + getActiveKey func(hc *hyperv1.HostedCluster) string + // setActiveKey mutates the HC spec's active-key field in-place. + setActiveKey func(hc *hyperv1.HostedCluster, value string) // verifyProviderField asserts provider-specific Status fields after rotation completes. - verifyProviderField func(hc *hyperv1.HostedCluster) + verifyProviderField func(hc *hyperv1.HostedCluster, alternate string) } // runSingleRotationTest registers a single-rotation lifecycle test for the given rotationCase. @@ -249,27 +252,23 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas // Re-fetch to get a fresh resource version and current Spec before patching. Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - rotatePatch, restorePatch, err := rc.setup(ctx, tc, hc) - Expect(err).NotTo(HaveOccurred(), "failed to prepare rotation patches") + alternate, err := rc.prepare(ctx, tc, hc) + Expect(err).NotTo(HaveOccurred(), "failed to prepare rotation") - Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, rotatePatch))).To(Succeed()) + original := rc.getActiveKey(hc) DeferCleanup(func() { - hcRestore := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, - } - if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { - GinkgoWriter.Printf("WARNING: failed to restore original key: %v\n", err) - return - } - waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) + rotateAndWait(tc.Context, tc, hcKey, func(hc *hyperv1.HostedCluster) { + rc.setActiveKey(hc, original) + }) }) - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + rotateAndWait(ctx, tc, hcKey, func(hc *hyperv1.HostedCluster) { + rc.setActiveKey(hc, alternate) + }) Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) verifyReEncryptionStatus(hc, rc.expectedProvider) - rc.verifyProviderField(hc) + rc.verifyProviderField(hc, alternate) svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") @@ -286,6 +285,18 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas }) } +// rotateAndWait applies mutate to the HostedCluster spec, then waits for re-encryption +// to start and complete. mutate must set the new active key value in-place. +func rotateAndWait(ctx context.Context, tc *internal.TestContext, hcKey crclient.ObjectKey, mutate func(*hyperv1.HostedCluster)) { + hc := &hyperv1.HostedCluster{} + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + base := hc.DeepCopy() + mutate(hc) + Expect(tc.MgmtClient.Patch(ctx, hc, crclient.MergeFrom(base))).To(Succeed()) + waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) + waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) +} + // AWSKMSKeyRotationTest validates the re-encryption lifecycle after rotating the AWS KMS active key. func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { runSingleRotationTest(getTestCtx, rotationCase{ @@ -304,16 +315,17 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { } return true, "" }, - setup: func(_ context.Context, _ *internal.TestContext, hc *hyperv1.HostedCluster) ([]byte, []byte, error) { - alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") - originalARN := hc.Spec.SecretEncryption.KMS.AWS.ActiveKey.ARN - rotate := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, alternateARN)) - restore := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"aws":{"activeKey":{"arn":%q}}}}}}`, originalARN)) - return rotate, restore, nil + prepare: func(_ context.Context, _ *internal.TestContext, _ *hyperv1.HostedCluster) (string, error) { + return internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE"), nil }, - verifyProviderField: func(hc *hyperv1.HostedCluster) { - alternateARN := internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE") - Expect(hc.Status.SecretEncryption.ActiveKey.AWS.ARN).To(Equal(alternateARN), + getActiveKey: func(hc *hyperv1.HostedCluster) string { + return hc.Spec.SecretEncryption.KMS.AWS.ActiveKey.ARN + }, + setActiveKey: func(hc *hyperv1.HostedCluster, value string) { + hc.Spec.SecretEncryption.KMS.AWS.ActiveKey.ARN = value + }, + verifyProviderField: func(hc *hyperv1.HostedCluster, alternate string) { + Expect(hc.Status.SecretEncryption.ActiveKey.AWS.ARN).To(Equal(alternate), "activeKey ARN should match the alternate key") }, }) @@ -337,16 +349,17 @@ func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { } return true, "" }, - setup: func(_ context.Context, _ *internal.TestContext, hc *hyperv1.HostedCluster) ([]byte, []byte, error) { - alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") - originalVersion := hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion - rotate := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) - restore := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) - return rotate, restore, nil + prepare: func(_ context.Context, _ *internal.TestContext, _ *hyperv1.HostedCluster) (string, error) { + return internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE"), nil }, - verifyProviderField: func(hc *hyperv1.HostedCluster) { - alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") - Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(alternateVersion), + getActiveKey: func(hc *hyperv1.HostedCluster) string { + return hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion + }, + setActiveKey: func(hc *hyperv1.HostedCluster, value string) { + hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion = value + }, + verifyProviderField: func(hc *hyperv1.HostedCluster, alternate string) { + Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(alternate), "activeKey keyVersion should match the alternate version") }, }) @@ -364,10 +377,10 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { } return true, "" }, - setup: func(ctx context.Context, tc *internal.TestContext, hc *hyperv1.HostedCluster) ([]byte, []byte, error) { + prepare: func(ctx context.Context, tc *internal.TestContext, _ *hyperv1.HostedCluster) (string, error) { keyData := make([]byte, 32) if _, err := rand.Read(keyData); err != nil { - return nil, nil, fmt.Errorf("failed to generate random AESCBC key data: %w", err) + return "", fmt.Errorf("failed to generate random AESCBC key data: %w", err) } newKeySecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -380,22 +393,25 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { }, } if err := tc.MgmtClient.Create(ctx, newKeySecret); err != nil { - return nil, nil, fmt.Errorf("failed to create AESCBC key secret: %w", err) + return "", fmt.Errorf("failed to create AESCBC key secret: %w", err) } DeferCleanup(func() { if err := tc.MgmtClient.Delete(tc.Context, newKeySecret); err != nil && !apierrors.IsNotFound(err) { GinkgoWriter.Printf("WARNING: failed to cleanup AESCBC key secret: %v\n", err) } }) - originalKeyRef := hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name - rotate := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, newKeySecret.Name)) - restore := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"aescbc":{"activeKey":{"name":%q}}}}}`, originalKeyRef)) - return rotate, restore, nil + return newKeySecret.Name, nil }, - verifyProviderField: func(hc *hyperv1.HostedCluster) { + getActiveKey: func(hc *hyperv1.HostedCluster) string { + return hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name + }, + setActiveKey: func(hc *hyperv1.HostedCluster, value string) { + hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name = value + }, + verifyProviderField: func(hc *hyperv1.HostedCluster, alternate string) { Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC).NotTo(BeNil(), "activeKey AESCBC should not be nil after AESCBC key rotation") - Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC.Secret.Name).To(Equal("e2e-aescbc-rotation-key"), + Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC.Secret.Name).To(Equal(alternate), "activeKey AESCBC secret should reference the new key") }, }) @@ -433,25 +449,17 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) DeferCleanup(func() { - restorePatch := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) - hcRestore := &hyperv1.HostedCluster{ - ObjectMeta: metav1.ObjectMeta{Name: tc.ClusterName, Namespace: tc.ClusterNamespace}, - } - if err := tc.MgmtClient.Patch(tc.Context, hcRestore, crclient.RawPatch(types.MergePatchType, restorePatch)); err != nil { - GinkgoWriter.Printf("WARNING: failed to restore original Azure key version: %v\n", err) - return - } - waitForReEncryptionComplete(tc.Context, tc.MgmtClient, hcKey, 25*time.Minute) + rotateAndWait(tc.Context, tc, hcKey, func(hc *hyperv1.HostedCluster) { + hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion = originalVersion + }) }) alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") By("Performing first key rotation: original -> alternate") - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - patch1 := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, alternateVersion)) - Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch1))).To(Succeed()) - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + rotateAndWait(ctx, tc, hcKey, func(hc *hyperv1.HostedCluster) { + hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion = alternateVersion + }) Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(alternateVersion), @@ -464,11 +472,9 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { }, 2*time.Minute, 10*time.Second).Should(Succeed()) By("Performing second key rotation: alternate -> original") - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - patch2 := []byte(fmt.Sprintf(`{"spec":{"secretEncryption":{"kms":{"azure":{"activeKey":{"keyVersion":%q}}}}}}`, originalVersion)) - Expect(tc.MgmtClient.Patch(ctx, hc, crclient.RawPatch(types.MergePatchType, patch2))).To(Succeed()) - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + rotateAndWait(ctx, tc, hcKey, func(hc *hyperv1.HostedCluster) { + hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion = originalVersion + }) Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) verifyReEncryptionStatus(hc, hyperv1.SecretEncryptionProviderAzure) From 91fef92c580f1b5fea5ad474a0c69947667b301b Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Thu, 8 Oct 2026 11:21:52 -0400 Subject: [PATCH 13/24] test: add kms-reencryption cluster variant for etcd rotation CI Adds a `kms-reencryption` ClusterSpec variant to the AWS platform config that creates a hosted cluster with `--kms-key-arn` set. The variant only appears when `HYPERSHIFT_ETCD_KMS_KEY_ARN` is set in the environment, so existing e2e-v2-aws jobs are unaffected. This is the hypershift-repo half of the CNTRLPLANE-4377 CI wiring; the companion PR in openshift/release adds the `e2e-v2-aws-kms-reencryption` presubmit job and the step-registry plumbing. Signed-off-by: Jie Zhao Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/lifecycle/aws.go | 18 ++++++++++++++++++ test/e2e/v2/lifecycle/aws_test.go | 13 +++++++++++++ 2 files changed, 31 insertions(+) diff --git a/test/e2e/v2/lifecycle/aws.go b/test/e2e/v2/lifecycle/aws.go index 0d0525ac6793..8da8cd6d0062 100644 --- a/test/e2e/v2/lifecycle/aws.go +++ b/test/e2e/v2/lifecycle/aws.go @@ -148,6 +148,14 @@ func (a *AWSPlatformConfig) ClusterSpecs(ctx context.Context, releaseImage, n1Im publicArgs = append(publicArgs, "--initial-storage-volumes-kms-key="+arn) } + // Always include kms-reencryption so the variant set is stable regardless of environment. + // --kms-key-arn is only appended when the ARN is configured; without it the cluster uses + // the default AESCBC encryption. + kmsCreateArgs := clusterArgs() + if kmsKeyARN := os.Getenv("HYPERSHIFT_ETCD_KMS_KEY_ARN"); kmsKeyARN != "" { + kmsCreateArgs = clusterArgs("--kms-key-arn=" + kmsKeyARN) + } + return []ClusterSpec{ { Variant: "public", @@ -229,6 +237,11 @@ func (a *AWSPlatformConfig) ClusterSpecs(ctx context.Context, releaseImage, n1Im "--image-content-sources="+filepath.Join(a.sharedDir, "mirror-registry-ics.yaml"), ), }, + { + Variant: "kms-reencryption", + DestroyArgs: destroyArgs, + CreateArgs: kmsCreateArgs, + }, } } @@ -319,6 +332,11 @@ func (a *AWSPlatformConfig) TestMatrix() TestMatrix { Variant: "image-mirroring", LabelFilter: "image-mirroring", }, + { + Name: "kms-reencryption", + Variant: "kms-reencryption", + LabelFilter: "etcd-encryption-reencryption", + }, }, Sequential: []SequentialGroup{ { diff --git a/test/e2e/v2/lifecycle/aws_test.go b/test/e2e/v2/lifecycle/aws_test.go index 8d1375706630..6a493f6032a8 100644 --- a/test/e2e/v2/lifecycle/aws_test.go +++ b/test/e2e/v2/lifecycle/aws_test.go @@ -35,6 +35,19 @@ func TestAWSPlatformConfigClusterSpecs(t *testing.T) { g.Expect(spec.DumpArgs).To(BeEmpty(), "variant %s", spec.Variant) } }) + + t.Run("kms-reencryption variant is always present", func(t *testing.T) { + g := NewWithT(t) + specs := (&AWSPlatformConfig{}).ClusterSpecs(t.Context(), "release-image", "n1-image") + var found bool + for _, spec := range specs { + if spec.Variant == "kms-reencryption" { + found = true + break + } + } + g.Expect(found).To(BeTrue(), "kms-reencryption variant should always be present") + }) } // testCLIImage is a digest-pinned openshift/cli image reference used in lookup and setup tests. From 0c14aaf8a332255ff66303336b19190dd827bf80 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Wed, 14 Oct 2026 10:33:59 -0400 Subject: [PATCH 14/24] test: fold AESCBC and Azure KMS re-encryption tests into existing public cluster matrices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AESCBC rotation (AESCBCKeyRotationTest) and Azure KMS rotation (AzureKMSKeyRotationTest, AzureKMSConsecutiveKeyRotationTest) run against the same plain public cluster that existing tests use — no dedicated cluster variant is needed. Add the etcd-encryption-reencryption label to the public TestMatrix step in both aws.go and azure.go so the tests run automatically as part of the regular e2e-v2-aws and e2e-v2-azure-self-managed jobs, instead of requiring dedicated optional jobs. Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/lifecycle/azure.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/v2/lifecycle/azure.go b/test/e2e/v2/lifecycle/azure.go index 2ca5a7981068..01ebc37ee454 100644 --- a/test/e2e/v2/lifecycle/azure.go +++ b/test/e2e/v2/lifecycle/azure.go @@ -302,7 +302,7 @@ func (a *AzurePlatformConfig) TestMatrix() TestMatrix { { Name: "public", Variant: "public", - LabelFilter: "azure-machine-diagnostics || self-managed-azure-public || hosted-cluster-node-communication || hosted-cluster-cpo || nodepool-arm64 || secret-encryption || control-plane-workloads || hosted-cluster-security || nodepool-osimagestream || hosted-cluster-ingress", + LabelFilter: "azure-machine-diagnostics || self-managed-azure-public || hosted-cluster-node-communication || hosted-cluster-cpo || nodepool-arm64 || secret-encryption || control-plane-workloads || hosted-cluster-security || nodepool-osimagestream || hosted-cluster-ingress || etcd-encryption-reencryption", Skip: "KAS allowed CIDRs", }, { From fcdd2d3ae966dea945a8622c406759f4f42aeb89 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Thu, 22 Oct 2026 22:35:09 -0400 Subject: [PATCH 15/24] test: address review comments on etcd re-encryption e2e tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix no-op patch in rotateAndWait: skip patch and waits when MergeFrom produces "{}", preventing a spurious DeferCleanup timeout on shared clusters - Strengthen verifyStorageVersionMigrations: filter SVMs by write-key annotation and assert Succeeded=True instead of a vacuous list check - Add fingerprint assertions to verifyReEncryptionStatus to prove From.Fingerprint != To.Fingerprint and To.Fingerprint == ActiveKey - Extend waitForReEncryptionStarted to also assert EtcdDataEncryptionUpToDate=False, completing the True→False→True lifecycle check - Capture KAS restart count baseline before each rotation and check delta only, avoiding false failures from pre-existing restarts on shared clusters - Extract azureKMSRotationApplies as a named function to deduplicate the BeforeEach skip guard across AzureKMSKeyRotationTest and consecutive test - Add Label("lifecycle") to Describe block per AGENTS.md §13 - Remove [Feature:EtcdEncryptionReencryption] from Describe per §19 (multi- feature file; Feature tags belong on Context blocks) - Fix AzureKMSConsecutiveKeyRotationTest comment: "two sequential rotations" not "mid-rotation handling code path" - Fix rotationCase.applies and prepare signatures: drop unused tc/hc params - Hard-fail on io.ReadAll error for current-container KAS logs (was warning) - Make kms-reencryption variant always present in ClusterSpecs() and TestMatrix() regardless of HYPERSHIFT_ETCD_KMS_KEY_ARN, so the variant set is stable for the dynamic test plan contract; --kms-key-arn is still only appended when the ARN env var is set - Update aws_test.go to reflect always-present kms-reencryption behavior Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 42 +++- .../etcd_encryption_reencryption_test.go | 211 +++++++++++++----- 2 files changed, 195 insertions(+), 58 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4d7629d00db6..50daef9cdb04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,10 +1,46 @@ -# AGENTS.md +# CLAUDE.md -This file provides guidance to AI coding agents when working with code in this repository. `CLAUDE.md` is a symlink to this file so that Claude Code auto-loads it; the `AGENTS.md` name is canonical. +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. `CLAUDE.md` is a symlink to `AGENTS.md` so that Claude Code auto-loads it; the `AGENTS.md` name is canonical. HyperShift is middleware for hosting OpenShift control planes at scale, decoupling control planes (running as pods on a management cluster) from worker nodes (running in separate infrastructure). -This file is intentionally minimal — detailed guidance lives in the referenced files below and should be updated there, not here. +Detailed guidance lives in the referenced files below; update those files rather than this one. + +## Quick Commands + +```bash +# Build +make build # all binaries +make hypershift-operator # just the HO + +# Test +make test # unit tests with race detection +GO111MODULE=on GOWORK=off GOFLAGS=-mod=vendor go test -race -run TestName ./path/to/pkg/... # single test +make test-envtest-ocp # CEL validation tests (envtest, k8s 1.31–1.37) + +# Code quality +make verify # fmt + vet + lint + staticcheck + codespell + gitlint +make lint-fix # auto-fix lint violations +make pre-commit # full pre-PR gate: build + e2e compile + verify + unit tests + +# API / codegen — REQUIRED after editing anything under api/ +make update # regenerate CRDs, revendor, sync clients (api-deps→workspace-sync→deps→api→clients→docs-aggregate) +make generate # regenerate *_mock.go files (do not hand-edit mocks) +``` + +**Critical multi-module gotcha**: `api/` is a **separate Go module** (`github.com/openshift/hypershift/api`). Edits there are invisible to the main module until `make update` is run. Always run `make update` after changing any type, constant, or function under `api/`. Never edit `vendor/` directly. + +## Commit Format + +``` +(): + +[optional body] + +Signed-off-by: +``` + +Types: `feat fix docs style refactor test chore build ci perf revert`. Title ≤120 chars, body lines ≤140 chars. No Jira IDs in commit messages — they belong only in PR titles (e.g. `OCPBUGS-12345: …`). For architecture, components, and platform support, see [ARCHITECTURE.md](ARCHITECTURE.md). diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 70324b18baa2..609315b5b1aa 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -29,6 +29,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" e2eutil "github.com/openshift/hypershift/test/e2e/util" "github.com/openshift/hypershift/test/e2e/v2/internal" + secretencryption "github.com/openshift/hypershift/support/secretencryption" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -42,13 +43,15 @@ import ( crclient "sigs.k8s.io/controller-runtime/pkg/client" ) -// kasMaxAllowedRestarts is the restart ceiling checked after re-encryption completes. -// One restart is expected (KAS rolls out with the updated EncryptionConfiguration); 2 -// allows a single transient failure on top of that without masking a true crash loop. +// kasMaxAllowedRestarts is the per-rotation restart delta ceiling checked after +// re-encryption completes. One restart is expected (KAS rolls out with the updated +// EncryptionConfiguration); 2 allows a single transient failure without masking a +// true crash loop. Checked as a delta from a pre-rotation baseline to avoid false +// failures from restarts that occurred before the test on a shared cluster. const kasMaxAllowedRestarts = int32(2) -var _ = Describe("[sig-hypershift][Jira:Hypershift][Feature:EtcdEncryptionReencryption] Etcd Encryption Re-encryption", - Label("etcd-encryption-reencryption"), func() { +var _ = Describe("[sig-hypershift][Jira:Hypershift] Etcd Encryption Re-encryption", + Label("lifecycle", "etcd-encryption-reencryption"), func() { var testCtx *internal.TestContext BeforeEach(func() { @@ -74,16 +77,22 @@ func RegisterEtcdEncryptionReencryptionTests(getTestCtx internal.TestContextGett ConditionBubbleUpTest(getTestCtx) } -// waitForReEncryptionStarted waits until TargetKey.Provider is non-empty, which means -// the reconciler has started a new rotation. TargetKey is set at the beginning of a -// rotation and stays set until the rotation completes — it is a level trigger, not an -// edge trigger, so it cannot be missed regardless of the poll interval. +// waitForReEncryptionStarted waits until TargetKey.Provider is non-empty (the reconciler +// has started a rotation) and EtcdDataEncryptionUpToDate is False (the in-progress condition +// is set). Together these assert the not-True step of the True → False → True lifecycle. +// TargetKey is a level trigger set for the duration of the rotation, so it cannot be missed +// regardless of poll interval. func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { Eventually(func(g Gomega) { hc := &hyperv1.HostedCluster{} g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) g.Expect(hc.Status.SecretEncryption.TargetKey.Provider).NotTo(BeEmpty(), "TargetKey.Provider should be set once rotation has started") + cond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) + g.Expect(cond).NotTo(BeNil(), "EtcdDataEncryptionUpToDate condition should exist during rotation") + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse), + "EtcdDataEncryptionUpToDate should be False while rotation is in progress, got reason=%s message=%s", + cond.Reason, cond.Message) }, timeout, 5*time.Second).Should(Succeed()) } @@ -115,20 +124,98 @@ func verifyReEncryptionStatus(hc *hyperv1.HostedCluster, expectedProvider hyperv "history[0].startedTime should be set") Expect(hc.Status.SecretEncryption.History[0].CompletionTime.IsZero()).To(BeFalse(), "history[0].completionTime should be set") + + // Verify that the history fingerprints reflect the actual key rotation. + // From.Fingerprint and To.Fingerprint must differ (the key actually changed), + // and To.Fingerprint must match the current ActiveKey (the rotation targeted the right key). + var kvType hyperv1.AzureKMSKeyVaultType + if hc.Spec.SecretEncryption.KMS != nil && hc.Spec.SecretEncryption.KMS.Azure != nil { + kvType = hc.Spec.SecretEncryption.KMS.Azure.KeyVaultType + } + activeFP := secretencryption.FingerprintFromKeyStatus(&hc.Status.SecretEncryption.ActiveKey, kvType) + Expect(activeFP).NotTo(BeEmpty(), "could not compute fingerprint from ActiveKey") + Expect(hc.Status.SecretEncryption.History[0].To.Fingerprint).To(Equal(activeFP), + "history[0].to.fingerprint should match the current active key fingerprint") + Expect(hc.Status.SecretEncryption.History[0].From.Fingerprint).NotTo(Equal(activeFP), + "history[0].from.fingerprint should differ from to.fingerprint — rotation must change the key") } -func listStorageVersionMigrations(ctx context.Context, guestClient crclient.Client) (*unstructured.UnstructuredList, error) { +// svmWriteKeyAnnotation is the annotation the library-go migrator stamps on every +// StorageVersionMigration to identify which encryption write-key triggered it. +const svmWriteKeyAnnotation = "encryption.apiserver.operator.openshift.io/write-key" + +// verifyStorageVersionMigrations asserts that at least one StorageVersionMigration in +// the guest cluster was created for the current rotation (identified by the write-key +// annotation) and that every such migration has a Succeeded condition = True. +// Filtering by the annotation rules out SVMs left over from bootstrap or prior rotations. +func verifyStorageVersionMigrations(ctx context.Context, guestClient crclient.Client, hc *hyperv1.HostedCluster) { + // Compute the fingerprint the controller uses for the write-key annotation. + // ActiveKey holds the key that was just rotated to (TargetKey is cleared on completion). + var kvType hyperv1.AzureKMSKeyVaultType + if hc.Spec.SecretEncryption.KMS != nil && hc.Spec.SecretEncryption.KMS.Azure != nil { + kvType = hc.Spec.SecretEncryption.KMS.Azure.KeyVaultType + } + fp := secretencryption.FingerprintFromKeyStatus(&hc.Status.SecretEncryption.ActiveKey, kvType) + Expect(fp).NotTo(BeEmpty(), "could not compute fingerprint from ActiveKey — Status may not have been updated yet") + writeKey := "encryption-key-" + fp + svmList := &unstructured.UnstructuredList{} svmList.SetGroupVersionKind(schema.GroupVersionKind{ Group: "migration.k8s.io", Version: "v1alpha1", Kind: "StorageVersionMigrationList", }) - err := guestClient.List(ctx, svmList) - return svmList, err + Expect(guestClient.List(ctx, svmList)).To(Succeed(), "failed to list StorageVersionMigration CRs") + + var matched []unstructured.Unstructured + for _, svm := range svmList.Items { + if svm.GetAnnotations()[svmWriteKeyAnnotation] == writeKey { + matched = append(matched, svm) + } + } + Expect(matched).NotTo(BeEmpty(), + "no StorageVersionMigration found with write-key annotation %q — rotation did not trigger migrations", writeKey) + + for _, svm := range matched { + conditions, _, _ := unstructured.NestedSlice(svm.Object, "status", "conditions") + var succeeded bool + for _, raw := range conditions { + cond, ok := raw.(map[string]interface{}) + if !ok { + continue + } + if cond["type"] == "Succeeded" && cond["status"] == "True" { + succeeded = true + break + } + } + Expect(succeeded).To(BeTrue(), + "StorageVersionMigration %s (write-key %q) did not reach Succeeded=True", svm.GetName(), writeKey) + } +} + +// captureKASRestartCounts snapshots the current restart count of every KAS pod container. +// Pass the result to verifyKASHealth to measure only the delta introduced by the rotation, +// avoiding false failures from restarts that already occurred on the shared cluster. +func captureKASRestartCounts(ctx context.Context, mgmtClient crclient.Client, controlPlaneNamespace string) map[string]int32 { + kasPods := &corev1.PodList{} + Expect(mgmtClient.List(ctx, kasPods, + crclient.InNamespace(controlPlaneNamespace), + crclient.MatchingLabels{"app": "kube-apiserver"}, + )).To(Succeed()) + counts := make(map[string]int32) + for _, pod := range kasPods.Items { + for _, cs := range pod.Status.ContainerStatuses { + counts[pod.Name+"/"+cs.Name] = cs.RestartCount + } + } + return counts } -func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPlaneNamespace string) { +// verifyKASHealth polls until all KAS pods are Running and no container's restart count +// has increased by more than kasMaxAllowedRestarts since the preRotationRestarts snapshot. +// Containers absent from the snapshot (new pods after a rollout) are compared against 0. +func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPlaneNamespace string, preRotationRestarts map[string]int32) { Eventually(func(g Gomega) { kasPods := &corev1.PodList{} g.Expect(mgmtClient.List(ctx, kasPods, @@ -146,8 +233,11 @@ func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPla g.Expect(cs.State.Waiting.Reason).NotTo(Equal("CrashLoopBackOff"), "KAS pod %s container %s is in CrashLoopBackOff", pod.Name, cs.Name) } - g.Expect(cs.RestartCount).To(BeNumerically("<=", kasMaxAllowedRestarts), - "KAS pod %s container %s has %d restarts", pod.Name, cs.Name, cs.RestartCount) + baseline := preRotationRestarts[pod.Name+"/"+cs.Name] + delta := cs.RestartCount - baseline + g.Expect(delta).To(BeNumerically("<=", kasMaxAllowedRestarts), + "KAS pod %s container %s restarted %d time(s) during rotation (baseline %d, current %d)", + pod.Name, cs.Name, delta, baseline, cs.RestartCount) } } }, 5*time.Minute, 15*time.Second).Should(Succeed()) @@ -188,8 +278,11 @@ func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace logBytes, err := io.ReadAll(logStream) logStream.Close() if err != nil { - GinkgoWriter.Printf("WARNING: failed to read logs for KAS pod %s (previous=%v): %v\n", pod.Name, previous, err) - continue + if previous { + GinkgoWriter.Printf("WARNING: failed to read previous logs for KAS pod %s: %v\n", pod.Name, err) + continue + } + Expect(err).NotTo(HaveOccurred(), "failed to read current logs for KAS pod %s", pod.Name) } Expect(string(logBytes)).NotTo(ContainSubstring("no matching prefix found"), "KAS pod %s logs (previous=%v) contain decryption error 'no matching prefix found'", pod.Name, previous) @@ -208,10 +301,10 @@ type rotationCase struct { // expectedProvider is the SecretEncryptionProvider expected in Status after rotation. expectedProvider hyperv1.SecretEncryptionProvider // applies returns (true, "") when this case should run, or (false, skipReason) to skip. - applies func(hc *hyperv1.HostedCluster, tc *internal.TestContext) (bool, string) + applies func(hc *hyperv1.HostedCluster) (bool, string) // prepare does any provider-specific setup (e.g. creating secrets), registers its own // DeferCleanups, and returns the alternate key identifier to rotate to. - prepare func(ctx context.Context, tc *internal.TestContext, hc *hyperv1.HostedCluster) (alternate string, err error) + prepare func(ctx context.Context, tc *internal.TestContext) (alternate string, err error) // getActiveKey extracts the current active-key identifier from the HC spec. getActiveKey func(hc *hyperv1.HostedCluster) string // setActiveKey mutates the HC spec's active-key field in-place. @@ -229,7 +322,7 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas tc := getTestCtx() hc, err := tc.GetHostedCluster() Expect(err).NotTo(HaveOccurred()) - ok, reason := rc.applies(hc, tc) + ok, reason := rc.applies(hc) if !ok { Skip(reason) } @@ -252,7 +345,7 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas // Re-fetch to get a fresh resource version and current Spec before patching. Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - alternate, err := rc.prepare(ctx, tc, hc) + alternate, err := rc.prepare(ctx, tc) Expect(err).NotTo(HaveOccurred(), "failed to prepare rotation") original := rc.getActiveKey(hc) @@ -262,6 +355,7 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas }) }) + kasBaseline := captureKASRestartCounts(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) rotateAndWait(ctx, tc, hcKey, func(hc *hyperv1.HostedCluster) { rc.setActiveKey(hc, alternate) }) @@ -270,12 +364,9 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas verifyReEncryptionStatus(hc, rc.expectedProvider) rc.verifyProviderField(hc, alternate) - svmList, err := listStorageVersionMigrations(ctx, hostedClusterClient) - Expect(err).NotTo(HaveOccurred(), "failed to list StorageVersionMigration CRs") - Expect(svmList.Items).NotTo(BeEmpty(), - "StorageVersionMigration CRs should exist after key rotation") + verifyStorageVersionMigrations(ctx, hostedClusterClient, hc) - verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) + verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kasBaseline) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) Eventually(func(g Gomega) { @@ -287,11 +378,19 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas // rotateAndWait applies mutate to the HostedCluster spec, then waits for re-encryption // to start and complete. mutate must set the new active key value in-place. +// If the mutation produces no diff (spec already at the desired key), rotateAndWait +// returns immediately without patching or waiting — the controller treats spec==activeKey +// as steady state and will not start a rotation. func rotateAndWait(ctx context.Context, tc *internal.TestContext, hcKey crclient.ObjectKey, mutate func(*hyperv1.HostedCluster)) { hc := &hyperv1.HostedCluster{} Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) base := hc.DeepCopy() mutate(hc) + patchData, err := crclient.MergeFrom(base).Data(hc) + Expect(err).NotTo(HaveOccurred(), "failed to compute patch data") + if string(patchData) == "{}" { + return + } Expect(tc.MgmtClient.Patch(ctx, hc, crclient.MergeFrom(base))).To(Succeed()) waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) @@ -303,7 +402,7 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { contextLabel: "[Feature:AWSKMSReencryption] AWS KMS Key Rotation", itLabel: "should re-encrypt all etcd data after active key rotation", expectedProvider: hyperv1.SecretEncryptionProviderAWS, - applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { + applies: func(hc *hyperv1.HostedCluster) (bool, string) { if hc.Spec.Platform.Type != hyperv1.AWSPlatform || hc.Spec.SecretEncryption.Type != hyperv1.KMS || hc.Spec.SecretEncryption.KMS == nil || @@ -315,7 +414,7 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { } return true, "" }, - prepare: func(_ context.Context, _ *internal.TestContext, _ *hyperv1.HostedCluster) (string, error) { + prepare: func(_ context.Context, _ *internal.TestContext) (string, error) { return internal.GetEnvVarValue("E2E_AWS_KMS_KEY_ARN_ALTERNATE"), nil }, getActiveKey: func(hc *hyperv1.HostedCluster) string { @@ -331,25 +430,30 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { }) } +// azureKMSRotationApplies is the shared skip guard for Azure KMS rotation tests. +// It returns (true, "") when the cluster is Azure with KMS configured and +// E2E_AZURE_KMS_KEY_VERSION_ALTERNATE is set, or (false, reason) otherwise. +func azureKMSRotationApplies(hc *hyperv1.HostedCluster) (bool, string) { + if hc.Spec.Platform.Type != hyperv1.AzurePlatform || + hc.Spec.SecretEncryption.Type != hyperv1.KMS || + hc.Spec.SecretEncryption.KMS == nil || + hc.Spec.SecretEncryption.KMS.Azure == nil { + return false, "Azure KMS key rotation test requires Azure platform with KMS configured" + } + if internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") == "" { + return false, "E2E_AZURE_KMS_KEY_VERSION_ALTERNATE is not set" + } + return true, "" +} + // AzureKMSKeyRotationTest validates the re-encryption lifecycle after rotating the Azure KMS key version. func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { runSingleRotationTest(getTestCtx, rotationCase{ contextLabel: "[Feature:AzureKMSReencryption] Azure KMS Key Rotation", itLabel: "should re-encrypt all etcd data after key version rotation", expectedProvider: hyperv1.SecretEncryptionProviderAzure, - applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { - if hc.Spec.Platform.Type != hyperv1.AzurePlatform || - hc.Spec.SecretEncryption.Type != hyperv1.KMS || - hc.Spec.SecretEncryption.KMS == nil || - hc.Spec.SecretEncryption.KMS.Azure == nil { - return false, "Azure KMS key rotation test requires Azure platform with KMS configured" - } - if internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") == "" { - return false, "E2E_AZURE_KMS_KEY_VERSION_ALTERNATE is not set" - } - return true, "" - }, - prepare: func(_ context.Context, _ *internal.TestContext, _ *hyperv1.HostedCluster) (string, error) { + applies: azureKMSRotationApplies, + prepare: func(_ context.Context, _ *internal.TestContext) (string, error) { return internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE"), nil }, getActiveKey: func(hc *hyperv1.HostedCluster) string { @@ -371,13 +475,13 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { contextLabel: "[Feature:AESCBCReencryption] AESCBC Key Rotation", itLabel: "should re-encrypt secrets after active key rotation", expectedProvider: hyperv1.SecretEncryptionProviderAESCBC, - applies: func(hc *hyperv1.HostedCluster, _ *internal.TestContext) (bool, string) { + applies: func(hc *hyperv1.HostedCluster) (bool, string) { if hc.Spec.SecretEncryption.Type != hyperv1.AESCBC || hc.Spec.SecretEncryption.AESCBC == nil { return false, "AESCBC key rotation test requires AESCBC encryption configured" } return true, "" }, - prepare: func(ctx context.Context, tc *internal.TestContext, _ *hyperv1.HostedCluster) (string, error) { + prepare: func(ctx context.Context, tc *internal.TestContext) (string, error) { keyData := make([]byte, 32) if _, err := rand.Read(keyData); err != nil { return "", fmt.Errorf("failed to generate random AESCBC key data: %w", err) @@ -417,23 +521,17 @@ func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { }) } -// AzureKMSConsecutiveKeyRotationTest validates that two back-to-back key rotations -// complete successfully, exercising the mid-rotation handling code path. +// AzureKMSConsecutiveKeyRotationTest validates that two sequential key rotations +// (alternate → original) each complete successfully end-to-end. func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { Context("[Feature:AzureKMSReencryption] Azure KMS Consecutive Key Rotation", func() { BeforeEach(func() { tc := getTestCtx() hc, err := tc.GetHostedCluster() Expect(err).NotTo(HaveOccurred()) - if hc.Spec.Platform.Type != hyperv1.AzurePlatform || - hc.Spec.SecretEncryption.Type != hyperv1.KMS || - hc.Spec.SecretEncryption.KMS == nil || - hc.Spec.SecretEncryption.KMS.Azure == nil { - Skip("Azure KMS consecutive key rotation test requires Azure platform with KMS configured") - } - alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") - if alternateVersion == "" { - Skip("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE is not set") + ok, reason := azureKMSRotationApplies(hc) + if !ok { + Skip(reason) } }) @@ -457,6 +555,7 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { alternateVersion := internal.GetEnvVarValue("E2E_AZURE_KMS_KEY_VERSION_ALTERNATE") By("Performing first key rotation: original -> alternate") + kas1Baseline := captureKASRestartCounts(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) rotateAndWait(ctx, tc, hcKey, func(hc *hyperv1.HostedCluster) { hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion = alternateVersion }) @@ -467,11 +566,13 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(hc.Status.SecretEncryption.History).NotTo(BeEmpty(), "history should have at least one entry after first rotation") + verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kas1Baseline) Eventually(func(g Gomega) { verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) }, 2*time.Minute, 10*time.Second).Should(Succeed()) By("Performing second key rotation: alternate -> original") + kas2Baseline := captureKASRestartCounts(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) rotateAndWait(ctx, tc, hcKey, func(hc *hyperv1.HostedCluster) { hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion = originalVersion }) @@ -487,7 +588,7 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(hc.Status.SecretEncryption.History[1].State).To(Equal(hyperv1.EncryptionMigrationStateCompleted), "second history entry should also be Completed") - verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) + verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kas2Baseline) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) Eventually(func(g Gomega) { From 6177014c02707dab0c6b80441cd957c017f4ff1d Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Mon, 26 Oct 2026 10:53:22 -0400 Subject: [PATCH 16/24] chore: revert AGENTS.md changes Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 42 +++--------------------------------------- 1 file changed, 3 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 50daef9cdb04..4d7629d00db6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,46 +1,10 @@ -# CLAUDE.md +# AGENTS.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. `CLAUDE.md` is a symlink to `AGENTS.md` so that Claude Code auto-loads it; the `AGENTS.md` name is canonical. +This file provides guidance to AI coding agents when working with code in this repository. `CLAUDE.md` is a symlink to this file so that Claude Code auto-loads it; the `AGENTS.md` name is canonical. HyperShift is middleware for hosting OpenShift control planes at scale, decoupling control planes (running as pods on a management cluster) from worker nodes (running in separate infrastructure). -Detailed guidance lives in the referenced files below; update those files rather than this one. - -## Quick Commands - -```bash -# Build -make build # all binaries -make hypershift-operator # just the HO - -# Test -make test # unit tests with race detection -GO111MODULE=on GOWORK=off GOFLAGS=-mod=vendor go test -race -run TestName ./path/to/pkg/... # single test -make test-envtest-ocp # CEL validation tests (envtest, k8s 1.31–1.37) - -# Code quality -make verify # fmt + vet + lint + staticcheck + codespell + gitlint -make lint-fix # auto-fix lint violations -make pre-commit # full pre-PR gate: build + e2e compile + verify + unit tests - -# API / codegen — REQUIRED after editing anything under api/ -make update # regenerate CRDs, revendor, sync clients (api-deps→workspace-sync→deps→api→clients→docs-aggregate) -make generate # regenerate *_mock.go files (do not hand-edit mocks) -``` - -**Critical multi-module gotcha**: `api/` is a **separate Go module** (`github.com/openshift/hypershift/api`). Edits there are invisible to the main module until `make update` is run. Always run `make update` after changing any type, constant, or function under `api/`. Never edit `vendor/` directly. - -## Commit Format - -``` -(): - -[optional body] - -Signed-off-by: -``` - -Types: `feat fix docs style refactor test chore build ci perf revert`. Title ≤120 chars, body lines ≤140 chars. No Jira IDs in commit messages — they belong only in PR titles (e.g. `OCPBUGS-12345: …`). +This file is intentionally minimal — detailed guidance lives in the referenced files below and should be updated there, not here. For architecture, components, and platform support, see [ARCHITECTURE.md](ARCHITECTURE.md). From b4448a79cb2aeab970e1cdefbe5f5c975e175215 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Mon, 26 Oct 2026 11:15:52 -0400 Subject: [PATCH 17/24] test: add GinkgoHelper to all etcd re-encryption test helpers Helpers that call Ginkgo assertions (Expect, Eventually) must declare GinkgoHelper() so failure stack traces point to the caller's line rather than the helper's internals. Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/tests/etcd_encryption_reencryption_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 609315b5b1aa..aa335567d963 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -83,6 +83,7 @@ func RegisterEtcdEncryptionReencryptionTests(getTestCtx internal.TestContextGett // TargetKey is a level trigger set for the duration of the rotation, so it cannot be missed // regardless of poll interval. func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { + GinkgoHelper() Eventually(func(g Gomega) { hc := &hyperv1.HostedCluster{} g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) @@ -97,6 +98,7 @@ func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, } func waitForReEncryptionComplete(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { + GinkgoHelper() Eventually(func(g Gomega) { hc := &hyperv1.HostedCluster{} g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) @@ -108,6 +110,7 @@ func waitForReEncryptionComplete(ctx context.Context, mgmtClient crclient.Client } func verifyReEncryptionStatus(hc *hyperv1.HostedCluster, expectedProvider hyperv1.SecretEncryptionProvider) { + GinkgoHelper() Expect(hc.Status.SecretEncryption.ActiveKey.Provider).To(Equal(expectedProvider), "activeKey provider should be %s", expectedProvider) Expect(hc.Status.SecretEncryption.TargetKey.Provider).To(BeEmpty(), @@ -149,6 +152,7 @@ const svmWriteKeyAnnotation = "encryption.apiserver.operator.openshift.io/write- // annotation) and that every such migration has a Succeeded condition = True. // Filtering by the annotation rules out SVMs left over from bootstrap or prior rotations. func verifyStorageVersionMigrations(ctx context.Context, guestClient crclient.Client, hc *hyperv1.HostedCluster) { + GinkgoHelper() // Compute the fingerprint the controller uses for the write-key annotation. // ActiveKey holds the key that was just rotated to (TargetKey is cleared on completion). var kvType hyperv1.AzureKMSKeyVaultType @@ -198,6 +202,7 @@ func verifyStorageVersionMigrations(ctx context.Context, guestClient crclient.Cl // Pass the result to verifyKASHealth to measure only the delta introduced by the rotation, // avoiding false failures from restarts that already occurred on the shared cluster. func captureKASRestartCounts(ctx context.Context, mgmtClient crclient.Client, controlPlaneNamespace string) map[string]int32 { + GinkgoHelper() kasPods := &corev1.PodList{} Expect(mgmtClient.List(ctx, kasPods, crclient.InNamespace(controlPlaneNamespace), @@ -216,6 +221,7 @@ func captureKASRestartCounts(ctx context.Context, mgmtClient crclient.Client, co // has increased by more than kasMaxAllowedRestarts since the preRotationRestarts snapshot. // Containers absent from the snapshot (new pods after a rollout) are compared against 0. func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPlaneNamespace string, preRotationRestarts map[string]int32) { + GinkgoHelper() Eventually(func(g Gomega) { kasPods := &corev1.PodList{} g.Expect(mgmtClient.List(ctx, kasPods, @@ -248,6 +254,7 @@ func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPla // containers catches errors that occurred during pod restarts mid-rotation; a larger // TailLines guards against errors scrolling past the tail during the up-to-25-min window. func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace string) { + GinkgoHelper() mgmtRestConfig, err := e2eutil.GetConfig() Expect(err).NotTo(HaveOccurred(), "failed to get management cluster REST config") mgmtKubeClient, err := kubernetes.NewForConfig(mgmtRestConfig) @@ -382,6 +389,7 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas // returns immediately without patching or waiting — the controller treats spec==activeKey // as steady state and will not start a rotation. func rotateAndWait(ctx context.Context, tc *internal.TestContext, hcKey crclient.ObjectKey, mutate func(*hyperv1.HostedCluster)) { + GinkgoHelper() hc := &hyperv1.HostedCluster{} Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) base := hc.DeepCopy() From 1c59809963bb706560c826fb73b1ac82c825bd03 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Mon, 26 Oct 2026 12:09:25 -0400 Subject: [PATCH 18/24] test: fix gci import order and hypershiftlinter terminology - Sort secretencryption import alphabetically before test/e2e imports - Replace "guest cluster" with "hosted cluster" in comment per project terminology convention Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/tests/etcd_encryption_reencryption_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index aa335567d963..f30e6ffa2a93 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -27,9 +27,9 @@ import ( . "github.com/onsi/gomega" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + secretencryption "github.com/openshift/hypershift/support/secretencryption" e2eutil "github.com/openshift/hypershift/test/e2e/util" "github.com/openshift/hypershift/test/e2e/v2/internal" - secretencryption "github.com/openshift/hypershift/support/secretencryption" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -148,7 +148,7 @@ func verifyReEncryptionStatus(hc *hyperv1.HostedCluster, expectedProvider hyperv const svmWriteKeyAnnotation = "encryption.apiserver.operator.openshift.io/write-key" // verifyStorageVersionMigrations asserts that at least one StorageVersionMigration in -// the guest cluster was created for the current rotation (identified by the write-key +// the hosted cluster was created for the current rotation (identified by the write-key // annotation) and that every such migration has a Succeeded condition = True. // Filtering by the annotation rules out SVMs left over from bootstrap or prior rotations. func verifyStorageVersionMigrations(ctx context.Context, guestClient crclient.Client, hc *hyperv1.HostedCluster) { From 80e45e99e2e2c57ecb4c779ffb172728543677a1 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Mon, 26 Oct 2026 16:40:38 -0400 Subject: [PATCH 19/24] test: address review comments on g.Eventually and remove redundant test - Remove "kms-reencryption variant is always present" unit test from aws_test.go per reviewer feedback (not a useful test) - Change waitForReEncryptionStarted, waitForReEncryptionComplete, and verifyKASHealth helpers to accept g Gomega and use g.Eventually so that inner assertion failures are handled correctly when called from helper functions - Update all call sites to pass Default (the default Gomega instance) Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/lifecycle/aws_test.go | 12 --------- .../etcd_encryption_reencryption_test.go | 26 +++++++++---------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/test/e2e/v2/lifecycle/aws_test.go b/test/e2e/v2/lifecycle/aws_test.go index 6a493f6032a8..95de7c6298ff 100644 --- a/test/e2e/v2/lifecycle/aws_test.go +++ b/test/e2e/v2/lifecycle/aws_test.go @@ -36,18 +36,6 @@ func TestAWSPlatformConfigClusterSpecs(t *testing.T) { } }) - t.Run("kms-reencryption variant is always present", func(t *testing.T) { - g := NewWithT(t) - specs := (&AWSPlatformConfig{}).ClusterSpecs(t.Context(), "release-image", "n1-image") - var found bool - for _, spec := range specs { - if spec.Variant == "kms-reencryption" { - found = true - break - } - } - g.Expect(found).To(BeTrue(), "kms-reencryption variant should always be present") - }) } // testCLIImage is a digest-pinned openshift/cli image reference used in lookup and setup tests. diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index f30e6ffa2a93..e8527ca41b0c 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -82,9 +82,9 @@ func RegisterEtcdEncryptionReencryptionTests(getTestCtx internal.TestContextGett // is set). Together these assert the not-True step of the True → False → True lifecycle. // TargetKey is a level trigger set for the duration of the rotation, so it cannot be missed // regardless of poll interval. -func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { +func waitForReEncryptionStarted(g Gomega, ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { GinkgoHelper() - Eventually(func(g Gomega) { + g.Eventually(func(g Gomega) { hc := &hyperv1.HostedCluster{} g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) g.Expect(hc.Status.SecretEncryption.TargetKey.Provider).NotTo(BeEmpty(), @@ -97,9 +97,9 @@ func waitForReEncryptionStarted(ctx context.Context, mgmtClient crclient.Client, }, timeout, 5*time.Second).Should(Succeed()) } -func waitForReEncryptionComplete(ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { +func waitForReEncryptionComplete(g Gomega, ctx context.Context, mgmtClient crclient.Client, hcKey crclient.ObjectKey, timeout time.Duration) { GinkgoHelper() - Eventually(func(g Gomega) { + g.Eventually(func(g Gomega) { hc := &hyperv1.HostedCluster{} g.Expect(mgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) cond := meta.FindStatusCondition(hc.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) @@ -220,9 +220,9 @@ func captureKASRestartCounts(ctx context.Context, mgmtClient crclient.Client, co // verifyKASHealth polls until all KAS pods are Running and no container's restart count // has increased by more than kasMaxAllowedRestarts since the preRotationRestarts snapshot. // Containers absent from the snapshot (new pods after a rollout) are compared against 0. -func verifyKASHealth(ctx context.Context, mgmtClient crclient.Client, controlPlaneNamespace string, preRotationRestarts map[string]int32) { +func verifyKASHealth(g Gomega, ctx context.Context, mgmtClient crclient.Client, controlPlaneNamespace string, preRotationRestarts map[string]int32) { GinkgoHelper() - Eventually(func(g Gomega) { + g.Eventually(func(g Gomega) { kasPods := &corev1.PodList{} g.Expect(mgmtClient.List(ctx, kasPods, crclient.InNamespace(controlPlaneNamespace), @@ -344,7 +344,7 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) // Ensure no re-encryption is in flight before rotating. - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) + waitForReEncryptionComplete(Default, ctx, tc.MgmtClient, hcKey, 5*time.Minute) hostedClusterClient, err := tc.GetHostedClusterClient(hc) Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") @@ -373,7 +373,7 @@ func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCas verifyStorageVersionMigrations(ctx, hostedClusterClient, hc) - verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kasBaseline) + verifyKASHealth(Default, ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kasBaseline) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) Eventually(func(g Gomega) { @@ -400,8 +400,8 @@ func rotateAndWait(ctx context.Context, tc *internal.TestContext, hcKey crclient return } Expect(tc.MgmtClient.Patch(ctx, hc, crclient.MergeFrom(base))).To(Succeed()) - waitForReEncryptionStarted(ctx, tc.MgmtClient, hcKey, 5*time.Minute) - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 25*time.Minute) + waitForReEncryptionStarted(Default, ctx, tc.MgmtClient, hcKey, 5*time.Minute) + waitForReEncryptionComplete(Default, ctx, tc.MgmtClient, hcKey, 25*time.Minute) } // AWSKMSKeyRotationTest validates the re-encryption lifecycle after rotating the AWS KMS active key. @@ -552,7 +552,7 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) originalVersion := hc.Spec.SecretEncryption.KMS.Azure.ActiveKey.KeyVersion - waitForReEncryptionComplete(ctx, tc.MgmtClient, hcKey, 5*time.Minute) + waitForReEncryptionComplete(Default, ctx, tc.MgmtClient, hcKey, 5*time.Minute) DeferCleanup(func() { rotateAndWait(tc.Context, tc, hcKey, func(hc *hyperv1.HostedCluster) { @@ -574,7 +574,7 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(hc.Status.SecretEncryption.History).NotTo(BeEmpty(), "history should have at least one entry after first rotation") - verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kas1Baseline) + verifyKASHealth(Default, ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kas1Baseline) Eventually(func(g Gomega) { verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) }, 2*time.Minute, 10*time.Second).Should(Succeed()) @@ -596,7 +596,7 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(hc.Status.SecretEncryption.History[1].State).To(Equal(hyperv1.EncryptionMigrationStateCompleted), "second history entry should also be Completed") - verifyKASHealth(ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kas2Baseline) + verifyKASHealth(Default, ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kas2Baseline) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) Eventually(func(g Gomega) { From 865f8490290cda3bedd35846943b0b01ba92aafb Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Tue, 27 Oct 2026 18:31:48 -0400 Subject: [PATCH 20/24] test: drop AESCBC rotation test, require KMS ARN for kms-reencryption variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KMS-only rotation coverage is sufficient — rotation is the same code path regardless of encryption provider, so testing once with KMS is enough. - Remove AESCBCKeyRotationTest and its supporting imports (crypto/rand, apierrors) - Change ClusterSpecs() to only include kms-reencryption when HYPERSHIFT_ETCD_KMS_KEY_ARN is set; without it there are no rotation tests to run and creating the cluster would be wasteful - Change TestMatrix() to match: kms-reencryption is only added to the Parallel groups when the ARN env var is present Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/lifecycle/aws.go | 95 ++++++++++--------- .../etcd_encryption_reencryption_test.go | 55 ----------- 2 files changed, 51 insertions(+), 99 deletions(-) diff --git a/test/e2e/v2/lifecycle/aws.go b/test/e2e/v2/lifecycle/aws.go index 8da8cd6d0062..71e26d2ef361 100644 --- a/test/e2e/v2/lifecycle/aws.go +++ b/test/e2e/v2/lifecycle/aws.go @@ -148,15 +148,7 @@ func (a *AWSPlatformConfig) ClusterSpecs(ctx context.Context, releaseImage, n1Im publicArgs = append(publicArgs, "--initial-storage-volumes-kms-key="+arn) } - // Always include kms-reencryption so the variant set is stable regardless of environment. - // --kms-key-arn is only appended when the ARN is configured; without it the cluster uses - // the default AESCBC encryption. - kmsCreateArgs := clusterArgs() - if kmsKeyARN := os.Getenv("HYPERSHIFT_ETCD_KMS_KEY_ARN"); kmsKeyARN != "" { - kmsCreateArgs = clusterArgs("--kms-key-arn=" + kmsKeyARN) - } - - return []ClusterSpec{ + specs := []ClusterSpec{ { Variant: "public", DestroyArgs: destroyArgs, @@ -237,12 +229,20 @@ func (a *AWSPlatformConfig) ClusterSpecs(ctx context.Context, releaseImage, n1Im "--image-content-sources="+filepath.Join(a.sharedDir, "mirror-registry-ics.yaml"), ), }, - { + } + + // Only include kms-reencryption when a KMS key ARN is configured. + // e2e rotation coverage is KMS-only; without an ARN there are no rotation + // tests to run so the dedicated cluster would be created for nothing. + if kmsKeyARN := os.Getenv("HYPERSHIFT_ETCD_KMS_KEY_ARN"); kmsKeyARN != "" { + specs = append(specs, ClusterSpec{ Variant: "kms-reencryption", DestroyArgs: destroyArgs, - CreateArgs: kmsCreateArgs, - }, + CreateArgs: clusterArgs("--kms-key-arn=" + kmsKeyARN), + }) } + + return specs } // PreCreate deploys the external OIDC provider and the mirror registry fixture @@ -305,39 +305,46 @@ func (a *AWSPlatformConfig) DefaultTestPlan() TestPlan { } func (a *AWSPlatformConfig) TestMatrix() TestMatrix { - return TestMatrix{ - Parallel: []TestGroup{ - { - Name: "managed-dns", - Variant: "managed-dns", - LabelFilter: "hosted-cluster-managed-dns", - }, - { - Name: "karpenter", - Variant: "karpenter", - LabelFilter: "karpenter", - }, - // TODO: It might be possible to decompose the karpenter upgrade test - // into pre and post upgrade specs which communicate with a well defined - // IPC protocol, like the pre step serializing observations which the - // post step can use. Then we can compose the regular upgrade test here - // instead of duplicating upgrade logic inside the karpenter test. - { - Name: "karpenter-upgrade", - Variant: "karpenter-upgrade", - LabelFilter: "karpenter-upgrade", - }, - { - Name: "image-mirroring", - Variant: "image-mirroring", - LabelFilter: "image-mirroring", - }, - { - Name: "kms-reencryption", - Variant: "kms-reencryption", - LabelFilter: "etcd-encryption-reencryption", - }, + parallel := []TestGroup{ + { + Name: "managed-dns", + Variant: "managed-dns", + LabelFilter: "hosted-cluster-managed-dns", + }, + { + Name: "karpenter", + Variant: "karpenter", + LabelFilter: "karpenter", + }, + // TODO: It might be possible to decompose the karpenter upgrade test + // into pre and post upgrade specs which communicate with a well defined + // IPC protocol, like the pre step serializing observations which the + // post step can use. Then we can compose the regular upgrade test here + // instead of duplicating upgrade logic inside the karpenter test. + { + Name: "karpenter-upgrade", + Variant: "karpenter-upgrade", + LabelFilter: "karpenter-upgrade", }, + { + Name: "image-mirroring", + Variant: "image-mirroring", + LabelFilter: "image-mirroring", + }, + } + + // kms-reencryption is only scheduled when a KMS key ARN is available — + // it must match the ClusterSpecs() conditional above. + if os.Getenv("HYPERSHIFT_ETCD_KMS_KEY_ARN") != "" { + parallel = append(parallel, TestGroup{ + Name: "kms-reencryption", + Variant: "kms-reencryption", + LabelFilter: "etcd-encryption-reencryption", + }) + } + + return TestMatrix{ + Parallel: parallel, Sequential: []SequentialGroup{ { Name: "public", diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index e8527ca41b0c..691e69951446 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -18,7 +18,6 @@ package tests import ( "context" - "crypto/rand" "fmt" "io" "time" @@ -32,7 +31,6 @@ import ( "github.com/openshift/hypershift/test/e2e/v2/internal" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -73,7 +71,6 @@ func RegisterEtcdEncryptionReencryptionTests(getTestCtx internal.TestContextGett AWSKMSKeyRotationTest(getTestCtx) AzureKMSKeyRotationTest(getTestCtx) AzureKMSConsecutiveKeyRotationTest(getTestCtx) - AESCBCKeyRotationTest(getTestCtx) ConditionBubbleUpTest(getTestCtx) } @@ -477,58 +474,6 @@ func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { }) } -// AESCBCKeyRotationTest validates the re-encryption lifecycle after rotating the AESCBC encryption key. -func AESCBCKeyRotationTest(getTestCtx internal.TestContextGetter) { - runSingleRotationTest(getTestCtx, rotationCase{ - contextLabel: "[Feature:AESCBCReencryption] AESCBC Key Rotation", - itLabel: "should re-encrypt secrets after active key rotation", - expectedProvider: hyperv1.SecretEncryptionProviderAESCBC, - applies: func(hc *hyperv1.HostedCluster) (bool, string) { - if hc.Spec.SecretEncryption.Type != hyperv1.AESCBC || hc.Spec.SecretEncryption.AESCBC == nil { - return false, "AESCBC key rotation test requires AESCBC encryption configured" - } - return true, "" - }, - prepare: func(ctx context.Context, tc *internal.TestContext) (string, error) { - keyData := make([]byte, 32) - if _, err := rand.Read(keyData); err != nil { - return "", fmt.Errorf("failed to generate random AESCBC key data: %w", err) - } - newKeySecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "e2e-aescbc-rotation-key", - Namespace: tc.ClusterNamespace, - }, - Type: corev1.SecretTypeOpaque, - Data: map[string][]byte{ - hyperv1.AESCBCKeySecretKey: keyData, - }, - } - if err := tc.MgmtClient.Create(ctx, newKeySecret); err != nil { - return "", fmt.Errorf("failed to create AESCBC key secret: %w", err) - } - DeferCleanup(func() { - if err := tc.MgmtClient.Delete(tc.Context, newKeySecret); err != nil && !apierrors.IsNotFound(err) { - GinkgoWriter.Printf("WARNING: failed to cleanup AESCBC key secret: %v\n", err) - } - }) - return newKeySecret.Name, nil - }, - getActiveKey: func(hc *hyperv1.HostedCluster) string { - return hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name - }, - setActiveKey: func(hc *hyperv1.HostedCluster, value string) { - hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name = value - }, - verifyProviderField: func(hc *hyperv1.HostedCluster, alternate string) { - Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC).NotTo(BeNil(), - "activeKey AESCBC should not be nil after AESCBC key rotation") - Expect(hc.Status.SecretEncryption.ActiveKey.AESCBC.Secret.Name).To(Equal(alternate), - "activeKey AESCBC secret should reference the new key") - }, - }) -} - // AzureKMSConsecutiveKeyRotationTest validates that two sequential key rotations // (alternate → original) each complete successfully end-to-end. func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { From f2fc188f3cdb4c8333d86b21d2770fba2188fad8 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Tue, 27 Oct 2026 20:37:25 -0400 Subject: [PATCH 21/24] =?UTF-8?q?test:=20refactor=20rotation=20tests=20?= =?UTF-8?q?=E2=80=94=20explicit=20It()=20per=20test,=20shared=20body=20via?= =?UTF-8?q?=20runRotationTest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace runSingleRotationTest (which defined Context/BeforeEach/It dynamically) with runRotationTest — a plain helper that contains only the It() body and is called from within each test's own explicit It() block. Each test function (AWSKMSKeyRotationTest, AzureKMSKeyRotationTest) now defines its own Context, BeforeEach, and It directly, removing the dynamic test registration pattern. The rotationCase struct drops contextLabel and itLabel since those are now owned by each test function. Co-Authored-By: Claude Sonnet 4.6 --- .../etcd_encryption_reencryption_test.go | 128 +++++++++--------- 1 file changed, 67 insertions(+), 61 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 691e69951446..d37618705c9f 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -294,14 +294,10 @@ func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace } } -// rotationCase parameterizes a single key-rotation lifecycle test. The shared -// test body (runSingleRotationTest) runs the common assertions; only the -// per-provider skip guard, key accessors, and provider-field verification differ. +// rotationCase parameterizes the provider-specific parts of a key-rotation lifecycle test. +// The shared test body (runRotationTest) runs the common assertions; only the per-provider +// skip guard, key accessors, and provider-field verification differ. type rotationCase struct { - // contextLabel is the Ginkgo Context description, e.g. "[Feature:AWSKMSReencryption] AWS KMS Key Rotation". - contextLabel string - // itLabel is the Ginkgo It description. - itLabel string // expectedProvider is the SecretEncryptionProvider expected in Status after rotation. expectedProvider hyperv1.SecretEncryptionProvider // applies returns (true, "") when this case should run, or (false, skipReason) to skip. @@ -317,67 +313,53 @@ type rotationCase struct { verifyProviderField func(hc *hyperv1.HostedCluster, alternate string) } -// runSingleRotationTest registers a single-rotation lifecycle test for the given rotationCase. +// runRotationTest executes the shared rotation lifecycle body for the given rotationCase. // It handles: baseline wait, rotate patch, restore cleanup, started/complete waits, // status verification, SVM check, KAS health/logs, and condition bubble-up. -func runSingleRotationTest(getTestCtx internal.TestContextGetter, rc rotationCase) { - Context(rc.contextLabel, func() { - BeforeEach(func() { - tc := getTestCtx() - hc, err := tc.GetHostedCluster() - Expect(err).NotTo(HaveOccurred()) - ok, reason := rc.applies(hc) - if !ok { - Skip(reason) - } - }) +// Must be called from within an It() block. +func runRotationTest(tc *internal.TestContext, rc rotationCase) { + ctx := tc.Context + hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} - It(rc.itLabel, Label(internal.InformingLabel), func() { - tc := getTestCtx() - ctx := tc.Context - hcKey := crclient.ObjectKey{Namespace: tc.ClusterNamespace, Name: tc.ClusterName} - - hc := &hyperv1.HostedCluster{} - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + hc := &hyperv1.HostedCluster{} + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - // Ensure no re-encryption is in flight before rotating. - waitForReEncryptionComplete(Default, ctx, tc.MgmtClient, hcKey, 5*time.Minute) + // Ensure no re-encryption is in flight before rotating. + waitForReEncryptionComplete(Default, ctx, tc.MgmtClient, hcKey, 5*time.Minute) - hostedClusterClient, err := tc.GetHostedClusterClient(hc) - Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") + hostedClusterClient, err := tc.GetHostedClusterClient(hc) + Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") - // Re-fetch to get a fresh resource version and current Spec before patching. - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + // Re-fetch to get a fresh resource version and current Spec before patching. + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - alternate, err := rc.prepare(ctx, tc) - Expect(err).NotTo(HaveOccurred(), "failed to prepare rotation") + alternate, err := rc.prepare(ctx, tc) + Expect(err).NotTo(HaveOccurred(), "failed to prepare rotation") - original := rc.getActiveKey(hc) - DeferCleanup(func() { - rotateAndWait(tc.Context, tc, hcKey, func(hc *hyperv1.HostedCluster) { - rc.setActiveKey(hc, original) - }) - }) + original := rc.getActiveKey(hc) + DeferCleanup(func() { + rotateAndWait(tc.Context, tc, hcKey, func(hc *hyperv1.HostedCluster) { + rc.setActiveKey(hc, original) + }) + }) - kasBaseline := captureKASRestartCounts(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) - rotateAndWait(ctx, tc, hcKey, func(hc *hyperv1.HostedCluster) { - rc.setActiveKey(hc, alternate) - }) + kasBaseline := captureKASRestartCounts(ctx, tc.MgmtClient, tc.ControlPlaneNamespace) + rotateAndWait(ctx, tc, hcKey, func(hc *hyperv1.HostedCluster) { + rc.setActiveKey(hc, alternate) + }) - Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) - verifyReEncryptionStatus(hc, rc.expectedProvider) - rc.verifyProviderField(hc, alternate) + Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) + verifyReEncryptionStatus(hc, rc.expectedProvider) + rc.verifyProviderField(hc, alternate) - verifyStorageVersionMigrations(ctx, hostedClusterClient, hc) + verifyStorageVersionMigrations(ctx, hostedClusterClient, hc) - verifyKASHealth(Default, ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kasBaseline) - verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) + verifyKASHealth(Default, ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kasBaseline) + verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) - Eventually(func(g Gomega) { - verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) - }, 2*time.Minute, 10*time.Second).Should(Succeed()) - }) - }) + Eventually(func(g Gomega) { + verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) + }, 2*time.Minute, 10*time.Second).Should(Succeed()) } // rotateAndWait applies mutate to the HostedCluster spec, then waits for re-encryption @@ -403,9 +385,7 @@ func rotateAndWait(ctx context.Context, tc *internal.TestContext, hcKey crclient // AWSKMSKeyRotationTest validates the re-encryption lifecycle after rotating the AWS KMS active key. func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { - runSingleRotationTest(getTestCtx, rotationCase{ - contextLabel: "[Feature:AWSKMSReencryption] AWS KMS Key Rotation", - itLabel: "should re-encrypt all etcd data after active key rotation", + rc := rotationCase{ expectedProvider: hyperv1.SecretEncryptionProviderAWS, applies: func(hc *hyperv1.HostedCluster) (bool, string) { if hc.Spec.Platform.Type != hyperv1.AWSPlatform || @@ -432,6 +412,20 @@ func AWSKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(hc.Status.SecretEncryption.ActiveKey.AWS.ARN).To(Equal(alternate), "activeKey ARN should match the alternate key") }, + } + Context("[Feature:AWSKMSReencryption] AWS KMS Key Rotation", func() { + BeforeEach(func() { + tc := getTestCtx() + hc, err := tc.GetHostedCluster() + Expect(err).NotTo(HaveOccurred()) + ok, reason := rc.applies(hc) + if !ok { + Skip(reason) + } + }) + It("should re-encrypt all etcd data after active key rotation", Label(internal.InformingLabel), func() { + runRotationTest(getTestCtx(), rc) + }) }) } @@ -453,9 +447,7 @@ func azureKMSRotationApplies(hc *hyperv1.HostedCluster) (bool, string) { // AzureKMSKeyRotationTest validates the re-encryption lifecycle after rotating the Azure KMS key version. func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { - runSingleRotationTest(getTestCtx, rotationCase{ - contextLabel: "[Feature:AzureKMSReencryption] Azure KMS Key Rotation", - itLabel: "should re-encrypt all etcd data after key version rotation", + rc := rotationCase{ expectedProvider: hyperv1.SecretEncryptionProviderAzure, applies: azureKMSRotationApplies, prepare: func(_ context.Context, _ *internal.TestContext) (string, error) { @@ -471,6 +463,20 @@ func AzureKMSKeyRotationTest(getTestCtx internal.TestContextGetter) { Expect(hc.Status.SecretEncryption.ActiveKey.Azure.KeyVersion).To(Equal(alternate), "activeKey keyVersion should match the alternate version") }, + } + Context("[Feature:AzureKMSReencryption] Azure KMS Key Rotation", func() { + BeforeEach(func() { + tc := getTestCtx() + hc, err := tc.GetHostedCluster() + Expect(err).NotTo(HaveOccurred()) + ok, reason := rc.applies(hc) + if !ok { + Skip(reason) + } + }) + It("should re-encrypt all etcd data after key version rotation", Label(internal.InformingLabel), func() { + runRotationTest(getTestCtx(), rc) + }) }) } From 640db0849ad567750c946091b40dc6805d0a4549 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Wed, 28 Oct 2026 10:45:50 -0400 Subject: [PATCH 22/24] test: remove verifyStorageVersionMigrations per reviewer feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The controller already aggregates StorageVersionMigration status into the EtcdDataEncryptionUpToDate condition. Checking SVMs directly mirrors controller logic and is not needed — the condition check in waitForReEncryptionComplete is sufficient coverage. Removes verifyStorageVersionMigrations, the svmWriteKeyAnnotation constant, the hostedClusterClient setup in runRotationTest, and the now-unused unstructured and schema imports. Co-Authored-By: Claude Sonnet 4.6 --- .../etcd_encryption_reencryption_test.go | 64 +------------------ 1 file changed, 1 insertion(+), 63 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index d37618705c9f..63b4b00f41f9 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -33,8 +33,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" "k8s.io/utils/ptr" @@ -140,61 +138,6 @@ func verifyReEncryptionStatus(hc *hyperv1.HostedCluster, expectedProvider hyperv "history[0].from.fingerprint should differ from to.fingerprint — rotation must change the key") } -// svmWriteKeyAnnotation is the annotation the library-go migrator stamps on every -// StorageVersionMigration to identify which encryption write-key triggered it. -const svmWriteKeyAnnotation = "encryption.apiserver.operator.openshift.io/write-key" - -// verifyStorageVersionMigrations asserts that at least one StorageVersionMigration in -// the hosted cluster was created for the current rotation (identified by the write-key -// annotation) and that every such migration has a Succeeded condition = True. -// Filtering by the annotation rules out SVMs left over from bootstrap or prior rotations. -func verifyStorageVersionMigrations(ctx context.Context, guestClient crclient.Client, hc *hyperv1.HostedCluster) { - GinkgoHelper() - // Compute the fingerprint the controller uses for the write-key annotation. - // ActiveKey holds the key that was just rotated to (TargetKey is cleared on completion). - var kvType hyperv1.AzureKMSKeyVaultType - if hc.Spec.SecretEncryption.KMS != nil && hc.Spec.SecretEncryption.KMS.Azure != nil { - kvType = hc.Spec.SecretEncryption.KMS.Azure.KeyVaultType - } - fp := secretencryption.FingerprintFromKeyStatus(&hc.Status.SecretEncryption.ActiveKey, kvType) - Expect(fp).NotTo(BeEmpty(), "could not compute fingerprint from ActiveKey — Status may not have been updated yet") - writeKey := "encryption-key-" + fp - - svmList := &unstructured.UnstructuredList{} - svmList.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "migration.k8s.io", - Version: "v1alpha1", - Kind: "StorageVersionMigrationList", - }) - Expect(guestClient.List(ctx, svmList)).To(Succeed(), "failed to list StorageVersionMigration CRs") - - var matched []unstructured.Unstructured - for _, svm := range svmList.Items { - if svm.GetAnnotations()[svmWriteKeyAnnotation] == writeKey { - matched = append(matched, svm) - } - } - Expect(matched).NotTo(BeEmpty(), - "no StorageVersionMigration found with write-key annotation %q — rotation did not trigger migrations", writeKey) - - for _, svm := range matched { - conditions, _, _ := unstructured.NestedSlice(svm.Object, "status", "conditions") - var succeeded bool - for _, raw := range conditions { - cond, ok := raw.(map[string]interface{}) - if !ok { - continue - } - if cond["type"] == "Succeeded" && cond["status"] == "True" { - succeeded = true - break - } - } - Expect(succeeded).To(BeTrue(), - "StorageVersionMigration %s (write-key %q) did not reach Succeeded=True", svm.GetName(), writeKey) - } -} - // captureKASRestartCounts snapshots the current restart count of every KAS pod container. // Pass the result to verifyKASHealth to measure only the delta introduced by the rotation, // avoiding false failures from restarts that already occurred on the shared cluster. @@ -315,7 +258,7 @@ type rotationCase struct { // runRotationTest executes the shared rotation lifecycle body for the given rotationCase. // It handles: baseline wait, rotate patch, restore cleanup, started/complete waits, -// status verification, SVM check, KAS health/logs, and condition bubble-up. +// status verification, KAS health/logs, and condition bubble-up. // Must be called from within an It() block. func runRotationTest(tc *internal.TestContext, rc rotationCase) { ctx := tc.Context @@ -327,9 +270,6 @@ func runRotationTest(tc *internal.TestContext, rc rotationCase) { // Ensure no re-encryption is in flight before rotating. waitForReEncryptionComplete(Default, ctx, tc.MgmtClient, hcKey, 5*time.Minute) - hostedClusterClient, err := tc.GetHostedClusterClient(hc) - Expect(err).NotTo(HaveOccurred(), "failed to get hosted cluster client") - // Re-fetch to get a fresh resource version and current Spec before patching. Expect(tc.MgmtClient.Get(ctx, hcKey, hc)).To(Succeed()) @@ -352,8 +292,6 @@ func runRotationTest(tc *internal.TestContext, rc rotationCase) { verifyReEncryptionStatus(hc, rc.expectedProvider) rc.verifyProviderField(hc, alternate) - verifyStorageVersionMigrations(ctx, hostedClusterClient, hc) - verifyKASHealth(Default, ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kasBaseline) verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) From 045fd9a2867c2bb357feb0cdd1b366acb5ddabbc Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Wed, 28 Oct 2026 10:53:03 -0400 Subject: [PATCH 23/24] test: remove unused fmt import Co-Authored-By: Claude Sonnet 4.6 --- test/e2e/v2/tests/etcd_encryption_reencryption_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 63b4b00f41f9..0a305a300a85 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -18,7 +18,6 @@ package tests import ( "context" - "fmt" "io" "time" From 93e295b5d46614fa97774d2c16c4c526537b2fd0 Mon Sep 17 00:00:00 2001 From: jiezhao16 Date: Wed, 28 Oct 2026 12:24:38 -0400 Subject: [PATCH 24/24] test: replace e2eutil.GetConfig with tc.MgmtRESTConfig in KAS log check The hypershiftlinter e2eutilallowlist rule disallows test/e2e/util.GetConfig from the v2 package. Pass a kubernetes.Interface built from tc.MgmtRESTConfig at the call sites instead. Co-Authored-By: Claude Sonnet 4.6 --- .../v2/tests/etcd_encryption_reencryption_test.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go index 0a305a300a85..d033bee52c26 100644 --- a/test/e2e/v2/tests/etcd_encryption_reencryption_test.go +++ b/test/e2e/v2/tests/etcd_encryption_reencryption_test.go @@ -26,7 +26,6 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" secretencryption "github.com/openshift/hypershift/support/secretencryption" - e2eutil "github.com/openshift/hypershift/test/e2e/util" "github.com/openshift/hypershift/test/e2e/v2/internal" corev1 "k8s.io/api/core/v1" @@ -192,12 +191,8 @@ func verifyKASHealth(g Gomega, ctx context.Context, mgmtClient crclient.Client, // of every KAS pod for "no matching prefix found" decryption errors. Checking previous // containers catches errors that occurred during pod restarts mid-rotation; a larger // TailLines guards against errors scrolling past the tail during the up-to-25-min window. -func verifyKASLogsNoDecryptionErrors(ctx context.Context, controlPlaneNamespace string) { +func verifyKASLogsNoDecryptionErrors(ctx context.Context, mgmtKubeClient kubernetes.Interface, controlPlaneNamespace string) { GinkgoHelper() - mgmtRestConfig, err := e2eutil.GetConfig() - Expect(err).NotTo(HaveOccurred(), "failed to get management cluster REST config") - mgmtKubeClient, err := kubernetes.NewForConfig(mgmtRestConfig) - Expect(err).NotTo(HaveOccurred(), "failed to create management cluster kubernetes clientset") kasPods, err := mgmtKubeClient.CoreV1().Pods(controlPlaneNamespace).List(ctx, metav1.ListOptions{ LabelSelector: "app=kube-apiserver", @@ -292,7 +287,9 @@ func runRotationTest(tc *internal.TestContext, rc rotationCase) { rc.verifyProviderField(hc, alternate) verifyKASHealth(Default, ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kasBaseline) - verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) + mgmtKubeClient, err := kubernetes.NewForConfig(tc.MgmtRESTConfig) + Expect(err).NotTo(HaveOccurred(), "failed to create management cluster kubernetes clientset") + verifyKASLogsNoDecryptionErrors(ctx, mgmtKubeClient, tc.ControlPlaneNamespace) Eventually(func(g Gomega) { verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace) @@ -485,7 +482,9 @@ func AzureKMSConsecutiveKeyRotationTest(getTestCtx internal.TestContextGetter) { "second history entry should also be Completed") verifyKASHealth(Default, ctx, tc.MgmtClient, tc.ControlPlaneNamespace, kas2Baseline) - verifyKASLogsNoDecryptionErrors(ctx, tc.ControlPlaneNamespace) + mgmtKubeClient, err := kubernetes.NewForConfig(tc.MgmtRESTConfig) + Expect(err).NotTo(HaveOccurred(), "failed to create management cluster kubernetes clientset") + verifyKASLogsNoDecryptionErrors(ctx, mgmtKubeClient, tc.ControlPlaneNamespace) Eventually(func(g Gomega) { verifyConditionBubbleUp(g, ctx, tc.MgmtClient, hcKey, tc.ControlPlaneNamespace)