diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/AROSwift/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/AROSwift/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yaml index 1d6ec0b4cf15..00ede7ef4443 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/AROSwift/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/AROSwift/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yaml @@ -30,6 +30,7 @@ spec: cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: bootstrap-manifests,logs,kms-socket,tmp-dir component.hypershift.openshift.io/config-hash: 0fd3eed819dc307e1d8949e2360eec75741638a5741638a5741638a5741638a5794dc8cc8b110e88a2a4098ca2a4098ca7ac042de7e69167 hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64 + kube-apiserver.hypershift.openshift.io/encryption-config-hash: sha256:89909e55b7962a3be09e580ed103db17f5b38ae558366dd4d87b5245de8c79b6 labels: app: kube-apiserver hypershift.openshift.io/control-plane-component: kube-apiserver diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go b/control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go index 62f615c67557..556f985b650c 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/kas/deployment.go @@ -15,6 +15,7 @@ import ( "github.com/openshift/hypershift/support/config" component "github.com/openshift/hypershift/support/controlplane-component" "github.com/openshift/hypershift/support/proxy" + "github.com/openshift/hypershift/support/secretencryption" "github.com/openshift/hypershift/support/util" configv1 "github.com/openshift/api/config/v1" @@ -123,14 +124,20 @@ func adaptDeployment(cpContext component.WorkloadContext, deployment *appsv1.Dep if secretEncryption := hcp.Spec.SecretEncryption; secretEncryption != nil { applyGenericSecretEncryptionConfig(&deployment.Spec.Template.Spec) + encConfigSecret := manifests.KASSecretEncryptionConfigFile(hcp.Namespace) + currentConfig, configBytes, err := readCurrentEncryptionConfig(cpContext, encConfigSecret) + if err != nil { + return fmt.Errorf("failed to read current encryption config: %w", err) + } + // Record a dedicated encryption-config hash on the pod template. The CPO v2 + // framework already computes a composite config-hash from all mounted secrets, + // but that hash mixes multiple secrets together. This separate annotation lets + // the HCCO verify that KAS has rolled out with a specific encryption config. + secretencryption.SetEncryptionConfigHashAnnotation(&deployment.Spec.Template, configBytes) switch secretEncryption.Type { case hyperv1.KMS: - encConfigSecret := manifests.KASSecretEncryptionConfigFile(hcp.Namespace) - currentConfig, err := readCurrentEncryptionConfig(cpContext, encConfigSecret) - if err != nil { - return fmt.Errorf("failed to read current encryption config: %w", err) - } - kasReady, err := isKASConverged(cpContext) + configHash := secretencryption.EncryptionConfigHash(configBytes) + kasReady, err := isKASConverged(cpContext, configHash) if err != nil { return fmt.Errorf("failed to check KAS convergence: %w", err) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/kas/secretencryption.go b/control-plane-operator/controllers/hostedcontrolplane/v2/kas/secretencryption.go index 32e6a89644ea..c2eb53c1fee5 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/kas/secretencryption.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/kas/secretencryption.go @@ -7,7 +7,6 @@ import ( "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" component "github.com/openshift/hypershift/support/controlplane-component" "github.com/openshift/hypershift/support/secretencryption" - "github.com/openshift/hypershift/support/util" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -35,13 +34,14 @@ func adaptSecretEncryptionConfig(cpContext component.WorkloadContext, secret *co encStatus := &cpContext.HCP.Status.SecretEncryption // Read the live encryption config from the cluster to derive the two-stage rollout state. - currentConfig, err := readCurrentEncryptionConfig(cpContext, secret) + currentConfig, configBytes, err := readCurrentEncryptionConfig(cpContext, secret) if err != nil { return fmt.Errorf("failed to read current encryption config: %w", err) } // Check KAS convergence — needed to decide whether to promote the target key. - kasConverged, err := isKASConverged(cpContext) + configHash := secretencryption.EncryptionConfigHash(configBytes) + kasConverged, err := isKASConverged(cpContext, configHash) if err != nil { return fmt.Errorf("failed to check KAS convergence: %w", err) } @@ -67,9 +67,10 @@ func adaptSecretEncryptionConfig(cpContext component.WorkloadContext, secret *co return nil } -// isKASConverged checks if the KAS Deployment has fully rolled out. +// isKASConverged checks if the KAS Deployment has fully rolled out with the +// encryption config snapshot identified by expectedConfigHash. // Returns false (not error) if the deployment doesn't exist yet. -func isKASConverged(cpContext component.WorkloadContext) (bool, error) { +func isKASConverged(cpContext component.WorkloadContext, expectedConfigHash string) (bool, error) { kasDeployment := &appsv1.Deployment{} kasRef := manifests.KASDeployment(cpContext.HCP.Namespace) if err := cpContext.Client.Get(cpContext, client.ObjectKeyFromObject(kasRef), kasDeployment); err != nil { @@ -78,27 +79,31 @@ func isKASConverged(cpContext component.WorkloadContext) (bool, error) { } return false, fmt.Errorf("failed to get KAS deployment: %w", err) } - return util.IsDeploymentReady(cpContext, kasDeployment), nil + return secretencryption.KASDeploymentConvergedWithEncryptionConfig(cpContext, cpContext.Client, kasDeployment, expectedConfigHash), nil } // readCurrentEncryptionConfig reads the live encryption config secret from the // cluster and parses its EncryptionConfiguration. Returns nil (not an error) // if the secret does not exist yet. -func readCurrentEncryptionConfig(cpContext component.WorkloadContext, templateSecret *corev1.Secret) (*apiserverv1.EncryptionConfiguration, error) { +func readCurrentEncryptionConfig(cpContext component.WorkloadContext, templateSecret *corev1.Secret) (*apiserverv1.EncryptionConfiguration, []byte, error) { existingSecret := &corev1.Secret{} if err := cpContext.Client.Get(cpContext, client.ObjectKeyFromObject(templateSecret), existingSecret); err != nil { if apierrors.IsNotFound(err) { - return nil, nil + return nil, nil, nil } - return nil, err + return nil, nil, err } configBytes := existingSecret.Data[secretEncryptionConfigurationKey] if len(configBytes) == 0 { - return nil, nil + return nil, nil, nil } - return secretencryption.DecodeEncryptionConfiguration(configBytes) + currentConfig, err := secretencryption.DecodeEncryptionConfiguration(configBytes) + if err != nil { + return nil, configBytes, err + } + return currentConfig, configBytes, nil } // deriveAESCBCEncryptionConfig determines the AESCBC write and read keys using diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption.go b/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption.go index 4b81eed480bb..d36c9b1e2a84 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption.go @@ -213,12 +213,12 @@ func (r *Reconciler) handleInProgressRotation(ctx context.Context, log logr.Logg } // Derive the current phase from observable state. - role, err := r.deriveTargetKeyRole(ctx, hcp) + role, configHash, err := r.deriveTargetKeyRole(ctx, hcp) if err != nil { return reconcile.Result{}, fmt.Errorf("failed to derive target key role from EncryptionConfiguration: %w", err) } - converged, err := r.isKASConverged(ctx, log, hcp) + converged, err := r.isKASConverged(ctx, log, hcp, configHash) if err != nil { return reconcile.Result{}, fmt.Errorf("failed to check KAS convergence: %w", err) } @@ -289,7 +289,9 @@ func (r *Reconciler) handleInProgressRotation(ctx context.Context, log logr.Logg // deriveTargetKeyRole reads the kas-secret-encryption-config Secret, parses the // EncryptionConfiguration, and determines where the target key appears. -func (r *Reconciler) deriveTargetKeyRole(ctx context.Context, hcp *hyperv1.HostedControlPlane) (secretencryption.TargetKeyRole, error) { +// It also returns a hash of the config bytes it read so callers can verify that +// KAS has rolled out with that exact config snapshot. +func (r *Reconciler) deriveTargetKeyRole(ctx context.Context, hcp *hyperv1.HostedControlPlane) (secretencryption.TargetKeyRole, string, error) { secret := &corev1.Secret{} secretKey := crclient.ObjectKey{ Namespace: hcp.Namespace, @@ -297,27 +299,29 @@ func (r *Reconciler) deriveTargetKeyRole(ctx context.Context, hcp *hyperv1.Hoste } if err := r.cpClient.Get(ctx, secretKey, secret); err != nil { if apierrors.IsNotFound(err) { - return secretencryption.TargetKeyAbsent, nil + return secretencryption.TargetKeyAbsent, "", nil } - return secretencryption.TargetKeyAbsent, fmt.Errorf("failed to get encryption config secret: %w", err) + return secretencryption.TargetKeyAbsent, "", fmt.Errorf("failed to get encryption config secret: %w", err) } configBytes := secret.Data[secretencryption.EncryptionConfigurationKey] if len(configBytes) == 0 { - return secretencryption.TargetKeyAbsent, nil + return secretencryption.TargetKeyAbsent, "", nil } + configHash := secretencryption.EncryptionConfigHash(configBytes) + currentConfig, err := secretencryption.DecodeEncryptionConfiguration(configBytes) if err != nil { - return secretencryption.TargetKeyAbsent, err + return secretencryption.TargetKeyAbsent, configHash, err } targetName, err := r.computeTargetKeyProviderName(ctx, hcp) if err != nil { - return secretencryption.TargetKeyAbsent, fmt.Errorf("failed to compute target key provider name: %w", err) + return secretencryption.TargetKeyAbsent, configHash, fmt.Errorf("failed to compute target key provider name: %w", err) } - return secretencryption.FindKeyRole(currentConfig, targetName, hcp.Spec.SecretEncryption.Type), nil + return secretencryption.FindKeyRole(currentConfig, targetName, hcp.Spec.SecretEncryption.Type), configHash, nil } // computeTargetKeyProviderName computes the provider name that the CPO would @@ -464,8 +468,9 @@ func (r *Reconciler) completeRotation(log logr.Logger, hcp *hyperv1.HostedContro return reconcile.Result{}, nil } -// isKASConverged checks if the KAS Deployment has fully rolled out. -func (r *Reconciler) isKASConverged(ctx context.Context, log logr.Logger, hcp *hyperv1.HostedControlPlane) (bool, error) { +// isKASConverged checks if the KAS Deployment has fully rolled out with the +// encryption config snapshot identified by expectedConfigHash. +func (r *Reconciler) isKASConverged(ctx context.Context, log logr.Logger, hcp *hyperv1.HostedControlPlane, expectedConfigHash string) (bool, error) { deployment := &appsv1.Deployment{} kasRef := manifests.KASDeployment(hcp.Namespace) if err := r.cpClient.Get(ctx, crclient.ObjectKeyFromObject(kasRef), deployment); err != nil { @@ -475,11 +480,13 @@ func (r *Reconciler) isKASConverged(ctx context.Context, log logr.Logger, hcp *h return false, fmt.Errorf("failed to get KAS deployment: %w", err) } - ready := util.IsDeploymentReady(ctx, deployment) - if !ready { - log.V(2).Info("KAS not converged") + converged := secretencryption.KASDeploymentConvergedWithEncryptionConfig(ctx, r.cpClient, deployment, expectedConfigHash) + if !converged { + log.V(2).Info("KAS not converged with encryption config", + "expectedConfigHash", expectedConfigHash, + "deploymentConfigHash", deployment.Spec.Template.Annotations[secretencryption.EncryptionConfigHashAnnotation]) } - return ready, nil + return converged, nil } // keyStatusFromSpec computes a SecretEncryptionKeyStatus from the HCP spec. diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption_test.go b/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption_test.go index b4ecc1adac48..0cc894ba1dbb 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption_test.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption_test.go @@ -298,8 +298,12 @@ func encryptionConfigSecret(namespace string, encType hyperv1.SecretEncryptionTy } } -func convergedKASDeployment(namespace string) *appsv1.Deployment { +func convergedKASDeployment(namespace string, encSecret ...*corev1.Secret) *appsv1.Deployment { replicas := int32(3) + template := corev1.PodTemplateSpec{} + if len(encSecret) > 0 && encSecret[0] != nil { + secretencryption.SetEncryptionConfigHashAnnotation(&template, encSecret[0].Data[secretencryption.EncryptionConfigurationKey]) + } return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "kube-apiserver", @@ -308,6 +312,7 @@ func convergedKASDeployment(namespace string) *appsv1.Deployment { }, Spec: appsv1.DeploymentSpec{ Replicas: &replicas, + Template: template, }, Status: appsv1.DeploymentStatus{ ObservedGeneration: 1, @@ -503,12 +508,15 @@ func TestReconcile(t *testing.T) { }, }, { - name: "When in ReadOnlyDeploy phase and KAS is converged it should advance to WritePromote", + name: "When in ReadOnlyDeploy phase and KAS deployment is ready but config hash mismatches it should wait", cpObjects: func() []client.Object { oldHash := secretencryption.DataHash([]byte("old-key-data")) newHash := secretencryption.DataHash([]byte("new-key-data")) oldKS := aescbcKeyStatus("aescbc-key-1", oldHash) newKS := aescbcKeyStatus("aescbc-key-1", newHash) + encSecret := encryptionConfigSecret(testNamespace, hyperv1.AESCBC, + aescbcProviderKeyName("old-key-data"), + aescbcProviderKeyName("new-key-data")) return []client.Object{ newHCP( withAESCBCEncryption("aescbc-key-1"), @@ -523,10 +531,46 @@ func TestReconcile(t *testing.T) { ), aescbcKeySecret("aescbc-key-1", testNamespace, "new-key-data"), convergedKASDeployment(testNamespace), - // Target key is read-only (second), old key is write (first). - encryptionConfigSecret(testNamespace, hyperv1.AESCBC, - aescbcProviderKeyName("old-key-data"), - aescbcProviderKeyName("new-key-data")), + encSecret, + } + }(), + migrator: newFakeMigrator, + expectResult: reconcile.Result{RequeueAfter: 30 * time.Second}, + validate: func(t *testing.T, g Gomega, cl client.Client, _ *fakeMigrator) { + hcp := getHCP(context.Background(), g, cl) + g.Expect(hcp.Status.SecretEncryption.History[0].State).To(Equal(hyperv1.EncryptionMigrationStateReadOnlyDeploy)) + + cond := meta.FindStatusCondition(hcp.Status.Conditions, string(hyperv1.EtcdDataEncryptionUpToDate)) + g.Expect(cond).ToNot(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(hyperv1.ReEncryptionWaitingForKASReason)) + }, + }, + { + name: "When in ReadOnlyDeploy phase and KAS is converged it should advance to WritePromote", + cpObjects: func() []client.Object { + oldHash := secretencryption.DataHash([]byte("old-key-data")) + newHash := secretencryption.DataHash([]byte("new-key-data")) + oldKS := aescbcKeyStatus("aescbc-key-1", oldHash) + newKS := aescbcKeyStatus("aescbc-key-1", newHash) + encSecret := encryptionConfigSecret(testNamespace, hyperv1.AESCBC, + aescbcProviderKeyName("old-key-data"), + aescbcProviderKeyName("new-key-data")) + return []client.Object{ + newHCP( + withAESCBCEncryption("aescbc-key-1"), + withActiveKey(oldKS), + withTargetKey(newKS), + withHistory(hyperv1.EncryptionMigrationHistory{ + From: secretencryption.KeyReferenceFromStatus(oldKS), + To: secretencryption.KeyReferenceFromStatus(newKS), + State: hyperv1.EncryptionMigrationStateReadOnlyDeploy, + StartedTime: metav1.Time{Time: fixedTime}, + }), + ), + aescbcKeySecret("aescbc-key-1", testNamespace, "new-key-data"), + convergedKASDeployment(testNamespace, encSecret), + encSecret, } }(), migrator: newFakeMigrator, @@ -547,6 +591,9 @@ func TestReconcile(t *testing.T) { newHash := secretencryption.DataHash([]byte("new-key-data")) oldKS := aescbcKeyStatus("aescbc-key-1", oldHash) newKS := aescbcKeyStatus("aescbc-key-1", newHash) + encSecret := encryptionConfigSecret(testNamespace, hyperv1.AESCBC, + aescbcProviderKeyName("new-key-data"), + aescbcProviderKeyName("old-key-data")) return []client.Object{ newHCP( withAESCBCEncryption("aescbc-key-1"), @@ -560,11 +607,8 @@ func TestReconcile(t *testing.T) { }), ), aescbcKeySecret("aescbc-key-1", testNamespace, "new-key-data"), - convergedKASDeployment(testNamespace), - // Target key promoted to write (first), old key demoted to read-only (second). - encryptionConfigSecret(testNamespace, hyperv1.AESCBC, - aescbcProviderKeyName("new-key-data"), - aescbcProviderKeyName("old-key-data")), + convergedKASDeployment(testNamespace, encSecret), + encSecret, } }(), migrator: newFakeMigrator, @@ -588,6 +632,9 @@ func TestReconcile(t *testing.T) { newHash := secretencryption.DataHash([]byte("new-key-data")) oldKS := aescbcKeyStatus("aescbc-key-1", oldHash) newKS := aescbcKeyStatus("aescbc-key-1", newHash) + encSecret := encryptionConfigSecret(testNamespace, hyperv1.AESCBC, + aescbcProviderKeyName("new-key-data"), + aescbcProviderKeyName("old-key-data")) return []client.Object{ newHCP( withAESCBCEncryption("aescbc-key-1"), @@ -601,11 +648,8 @@ func TestReconcile(t *testing.T) { }), ), aescbcKeySecret("aescbc-key-1", testNamespace, "new-key-data"), - convergedKASDeployment(testNamespace), - // Target key is write (first), old key is read-only (second). - encryptionConfigSecret(testNamespace, hyperv1.AESCBC, - aescbcProviderKeyName("new-key-data"), - aescbcProviderKeyName("old-key-data")), + convergedKASDeployment(testNamespace, encSecret), + encSecret, } }(), migrator: newFakeMigrator, @@ -622,6 +666,9 @@ func TestReconcile(t *testing.T) { newHash := secretencryption.DataHash([]byte("new-key-data")) oldKS := aescbcKeyStatus("aescbc-key-1", oldHash) newKS := aescbcKeyStatus("aescbc-key-1", newHash) + encSecret := encryptionConfigSecret(testNamespace, hyperv1.AESCBC, + aescbcProviderKeyName("new-key-data"), + aescbcProviderKeyName("old-key-data")) return []client.Object{ newHCP( withAESCBCEncryption("aescbc-key-1"), @@ -635,11 +682,8 @@ func TestReconcile(t *testing.T) { }), ), aescbcKeySecret("aescbc-key-1", testNamespace, "new-key-data"), - convergedKASDeployment(testNamespace), - // Target key is write (first), old key is read-only (second). - encryptionConfigSecret(testNamespace, hyperv1.AESCBC, - aescbcProviderKeyName("new-key-data"), - aescbcProviderKeyName("old-key-data")), + convergedKASDeployment(testNamespace, encSecret), + encSecret, } }(), migrator: func() *fakeMigrator { @@ -675,6 +719,9 @@ func TestReconcile(t *testing.T) { newHash := secretencryption.DataHash([]byte("new-key-data")) oldKS := aescbcKeyStatus("aescbc-key-1", oldHash) newKS := aescbcKeyStatus("aescbc-key-1", newHash) + encSecret := encryptionConfigSecret(testNamespace, hyperv1.AESCBC, + aescbcProviderKeyName("new-key-data"), + aescbcProviderKeyName("old-key-data")) return []client.Object{ newHCP( withAESCBCEncryption("aescbc-key-1"), @@ -688,11 +735,8 @@ func TestReconcile(t *testing.T) { }), ), aescbcKeySecret("aescbc-key-1", testNamespace, "new-key-data"), - convergedKASDeployment(testNamespace), - // Target key is write (first), old key is read-only (second). - encryptionConfigSecret(testNamespace, hyperv1.AESCBC, - aescbcProviderKeyName("new-key-data"), - aescbcProviderKeyName("old-key-data")), + convergedKASDeployment(testNamespace, encSecret), + encSecret, } }(), migrator: func() *fakeMigrator { @@ -742,6 +786,9 @@ func TestReconcile(t *testing.T) { cpObjects: func() []client.Object { oldKS := awsKeyStatus("arn:aws:kms:us-east-1:123456789012:key/old-key") newKS := awsKeyStatus("arn:aws:kms:us-east-1:123456789012:key/test-key-1") + encSecret := encryptionConfigSecret(testNamespace, hyperv1.KMS, + awsProviderKeyName("arn:aws:kms:us-east-1:123456789012:key/test-key-1"), + awsProviderKeyName("arn:aws:kms:us-east-1:123456789012:key/old-key")) return []client.Object{ newHCP( withKMSEncryption(), @@ -754,11 +801,8 @@ func TestReconcile(t *testing.T) { StartedTime: metav1.Time{Time: fixedTime}, }), ), - convergedKASDeployment(testNamespace), - // Target key is write (first KMS provider), old key is read-only (second). - encryptionConfigSecret(testNamespace, hyperv1.KMS, - awsProviderKeyName("arn:aws:kms:us-east-1:123456789012:key/test-key-1"), - awsProviderKeyName("arn:aws:kms:us-east-1:123456789012:key/old-key")), + convergedKASDeployment(testNamespace, encSecret), + encSecret, } }(), migrator: func() *fakeMigrator { @@ -813,15 +857,15 @@ func TestReconcile(t *testing.T) { ActiveKey: corev1.LocalObjectReference{Name: "aescbc-key-3"}, }, } + encSecret := encryptionConfigSecret(testNamespace, hyperv1.AESCBC, + aescbcProviderKeyName("mid-key-data"), + aescbcProviderKeyName("old-key-data")) return []client.Object{ hcp, aescbcKeySecret("aescbc-key-2", testNamespace, "mid-key-data"), aescbcKeySecret("aescbc-key-3", testNamespace, "third-key-data"), - convergedKASDeployment(testNamespace), - // Target (mid) key is write (first), old key is read-only (second). - encryptionConfigSecret(testNamespace, hyperv1.AESCBC, - aescbcProviderKeyName("mid-key-data"), - aescbcProviderKeyName("old-key-data")), + convergedKASDeployment(testNamespace, encSecret), + encSecret, } }(), migrator: newFakeMigrator, @@ -841,6 +885,9 @@ func TestReconcile(t *testing.T) { cpObjects: func() []client.Object { oldKS := awsKeyStatus("arn:aws:kms:us-east-1:123456789012:key/old-key") newKS := awsKeyStatus("arn:aws:kms:us-east-1:123456789012:key/test-key-1") + encSecret := encryptionConfigSecret(testNamespace, hyperv1.KMS, + awsProviderKeyName("arn:aws:kms:us-east-1:123456789012:key/test-key-1"), + awsProviderKeyName("arn:aws:kms:us-east-1:123456789012:key/old-key")) return []client.Object{ newHCP( withKMSEncryption(), @@ -853,10 +900,8 @@ func TestReconcile(t *testing.T) { StartedTime: metav1.Time{Time: fixedTime}, }), ), - convergedKASDeployment(testNamespace), - encryptionConfigSecret(testNamespace, hyperv1.KMS, - awsProviderKeyName("arn:aws:kms:us-east-1:123456789012:key/test-key-1"), - awsProviderKeyName("arn:aws:kms:us-east-1:123456789012:key/old-key")), + convergedKASDeployment(testNamespace, encSecret), + encSecret, } }(), migrator: func() *fakeMigrator { diff --git a/support/secretencryption/encryptionconfig.go b/support/secretencryption/encryptionconfig.go index b7bd57e45c02..b9f3d5dce1ab 100644 --- a/support/secretencryption/encryptionconfig.go +++ b/support/secretencryption/encryptionconfig.go @@ -1,13 +1,23 @@ package secretencryption import ( + "context" + "crypto/sha256" "fmt" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/support/util" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer/json" apiserverv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" + + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/go-logr/logr" ) const ( @@ -16,6 +26,10 @@ const ( // EncryptionConfigurationKind is the Kind used in EncryptionConfiguration manifests. EncryptionConfigurationKind = "EncryptionConfiguration" + + // EncryptionConfigHashAnnotation is set on the KAS pod template to record the + // encryption config content that triggered the current rollout. + EncryptionConfigHashAnnotation = "kube-apiserver.hypershift.openshift.io/encryption-config-hash" ) var ( @@ -28,6 +42,70 @@ func init() { yamlDecoder = json.NewSerializerWithOptions(json.DefaultMetaFactory, encScheme, encScheme, json.SerializerOptions{Yaml: true}) } +// EncryptionConfigHash returns a stable hash of encryption config bytes. +// The format matches the kube-apiserver encryption config controller. +func EncryptionConfigHash(data []byte) string { + if len(data) == 0 { + return "" + } + return fmt.Sprintf("sha256:%x", sha256.Sum256(data)) +} + +// SetEncryptionConfigHashAnnotation records the encryption config hash on a pod template. +func SetEncryptionConfigHashAnnotation(podTemplate *corev1.PodTemplateSpec, configBytes []byte) { + hash := EncryptionConfigHash(configBytes) + if hash == "" { + return + } + if podTemplate.Annotations == nil { + podTemplate.Annotations = map[string]string{} + } + podTemplate.Annotations[EncryptionConfigHashAnnotation] = hash +} + +// KASDeploymentConvergedWithEncryptionConfig reports whether the KAS deployment +// has fully rolled out with the given encryption config hash and all old pods +// have finished terminating. Kubernetes deployment status fields exclude +// terminating pods, so IsDeploymentReady alone returns true while old pods are +// still running — this function additionally checks for their absence. +func KASDeploymentConvergedWithEncryptionConfig(ctx context.Context, c client.Reader, deployment *appsv1.Deployment, expectedConfigHash string) bool { + if expectedConfigHash == "" || !util.IsDeploymentReady(ctx, deployment) { + return false + } + if deployment.Spec.Template.Annotations[EncryptionConfigHashAnnotation] != expectedConfigHash { + return false + } + return !hasTerminatingPods(ctx, c, deployment) +} + +// hasTerminatingPods returns true if any pods matching the deployment's selector +// have a non-nil DeletionTimestamp (i.e. are still shutting down). Returns true +// on error to fail closed. +func hasTerminatingPods(ctx context.Context, c client.Reader, deployment *appsv1.Deployment) bool { + logger := logr.FromContextOrDiscard(ctx) + if deployment.Spec.Selector == nil { + return false + } + selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector) + if err != nil { + logger.Error(err, "Failed to parse deployment selector, failing closed") + return true + } + podList := &corev1.PodList{} + if err := c.List(ctx, podList, + client.InNamespace(deployment.Namespace), + client.MatchingLabelsSelector{Selector: selector}); err != nil { + logger.Error(err, "Failed to list pods for termination check, failing closed") + return true + } + for i := range podList.Items { + if podList.Items[i].DeletionTimestamp != nil { + return true + } + } + return false +} + // DecodeEncryptionConfiguration parses raw YAML bytes into an EncryptionConfiguration. func DecodeEncryptionConfiguration(data []byte) (*apiserverv1.EncryptionConfiguration, error) { cfg := &apiserverv1.EncryptionConfiguration{} diff --git a/support/secretencryption/encryptionconfig_test.go b/support/secretencryption/encryptionconfig_test.go index 5960bc6f7514..60a79169c317 100644 --- a/support/secretencryption/encryptionconfig_test.go +++ b/support/secretencryption/encryptionconfig_test.go @@ -2,12 +2,19 @@ package secretencryption import ( "testing" + "time" . "github.com/onsi/gomega" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" apiserverv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" + + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func TestDecodeEncryptionConfiguration(t *testing.T) { @@ -89,6 +96,119 @@ func aescbcKey(name string) apiserverv1.Key { return apiserverv1.Key{Name: name, Secret: "dW51c2Vk"} } +func TestEncryptionConfigHash(t *testing.T) { + t.Parallel() + + t.Run("When config bytes are empty it should return an empty hash", func(t *testing.T) { + g := NewWithT(t) + g.Expect(EncryptionConfigHash(nil)).To(BeEmpty()) + }) + + t.Run("When config bytes are provided it should return a stable sha256 hash", func(t *testing.T) { + g := NewWithT(t) + data := []byte("apiVersion: apiserver.config.k8s.io/v1\nkind: EncryptionConfiguration\n") + hash := EncryptionConfigHash(data) + g.Expect(hash).To(HavePrefix("sha256:")) + g.Expect(EncryptionConfigHash(data)).To(Equal(hash)) + }) +} + +func TestKASDeploymentConvergedWithEncryptionConfig(t *testing.T) { + t.Parallel() + + configBytes := []byte("config") + configHash := EncryptionConfigHash(configBytes) + replicas := int32(1) + + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + _ = appsv1.AddToScheme(scheme) + + kasLabels := map[string]string{"app": "kube-apiserver"} + + readyDeployment := func() *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-apiserver", + Namespace: "clusters-test", + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + Selector: &metav1.LabelSelector{MatchLabels: kasLabels}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + EncryptionConfigHashAnnotation: configHash, + }, + }, + }, + }, + Status: appsv1.DeploymentStatus{ + Replicas: 1, + UpdatedReplicas: 1, + ReadyReplicas: 1, + AvailableReplicas: 1, + }, + } + } + + t.Run("When deployment is ready, hash matches, and no terminating pods it should report converged", func(t *testing.T) { + g := NewWithT(t) + deployment := readyDeployment() + activePod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-apiserver-abc123", + Namespace: "clusters-test", + Labels: kasLabels, + }, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + } + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(activePod).Build() + g.Expect(KASDeploymentConvergedWithEncryptionConfig(t.Context(), c, deployment, configHash)).To(BeTrue()) + }) + + t.Run("When deployment is ready but hash mismatches it should not report converged", func(t *testing.T) { + g := NewWithT(t) + deployment := readyDeployment() + deployment.Spec.Template.Annotations = nil + c := fake.NewClientBuilder().WithScheme(scheme).Build() + g.Expect(KASDeploymentConvergedWithEncryptionConfig(t.Context(), c, deployment, configHash)).To(BeFalse()) + }) + + t.Run("When deployment is ready and hash matches but a terminating pod exists it should not report converged", func(t *testing.T) { + g := NewWithT(t) + deployment := readyDeployment() + now := metav1.NewTime(time.Now()) + terminatingPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-apiserver-old", + Namespace: "clusters-test", + Labels: kasLabels, + DeletionTimestamp: &now, + Finalizers: []string{"test-finalizer"}, + }, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + } + activePod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-apiserver-new", + Namespace: "clusters-test", + Labels: kasLabels, + }, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + } + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(terminatingPod, activePod).Build() + g.Expect(KASDeploymentConvergedWithEncryptionConfig(t.Context(), c, deployment, configHash)).To(BeFalse()) + }) + + t.Run("When expected config hash is empty it should not report converged", func(t *testing.T) { + g := NewWithT(t) + deployment := readyDeployment() + c := fake.NewClientBuilder().WithScheme(scheme).Build() + g.Expect(KASDeploymentConvergedWithEncryptionConfig(t.Context(), c, deployment, "")).To(BeFalse()) + }) +} + func TestFindKeyRole(t *testing.T) { t.Parallel()