Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 45 additions & 8 deletions pkg/operator/encryption/controllers/key_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
apiserverv1 "k8s.io/apiserver/pkg/apis/apiserver/v1"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/klog/v2"
"k8s.io/utils/clock"

configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
Expand Down Expand Up @@ -193,6 +194,18 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
return nil
}

if err := c.checkAndCreateKeysFromSnap(ctx, syncContext, planner, snap); err != nil {
return err
}

if err := c.reconcileRemoteKeyRotationFromSnap(ctx, snap); err != nil {
return err
}

return nil
}

func (c *keyController) checkAndCreateKeysFromSnap(ctx context.Context, syncContext factory.SyncContext, planner *EncryptionPlanner, snap *KeyPlanningSnapshot) error {
plan, err := planner.PlanNextKey(snap)
if err != nil {
return err
Expand Down Expand Up @@ -224,6 +237,24 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
return nil
}

func (c *keyController) reconcileRemoteKeyRotationFromSnap(ctx context.Context, snap *KeyPlanningSnapshot) error {
if c.encryptionStatusProvider == nil || snap.CurrentMode != state.KMS {
return nil
}

writeKey, ok := writeKeyForRemoteKeyRotation(snap.State.DesiredBeforePlan)
if !ok || len(writeKey.RemoteKey().TargetRemoteKeyID) == 0 {
return nil
}

encryptionStatus, err := c.encryptionStatusProvider.GetKMSEncryptionStatus(ctx)
if err != nil {
return fmt.Errorf("failed to get KMS encryption status for remote key rotation: %w", err)
}

return reconcileRemoteKeyRotation(ctx, c.secretClient, c.instanceName, snap.State.EncryptedGRs, snap.State.DesiredBeforePlan, encryptionStatus, clock.RealClock{})
}

func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *corev1.Secret, keyID uint64) error {
actualKeySecret, err := c.secretClient.Secrets("openshift-config-managed").Get(ctx, keySecret.Name, metav1.GetOptions{})
if err != nil {
Expand Down Expand Up @@ -267,13 +298,14 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur
if err != nil {
return nil, false, fmt.Errorf("failed to compute KMS config hash: %w", err)
}
preflightPassed, err := c.ensureKMSPreflightPassed(ctx, configHash)
preflightPassed, remoteKeyID, err := c.ensureKMSPreflightPassed(ctx, configHash)
if err != nil {
return nil, false, err
}
if !preflightPassed {
return nil, false, nil
}
ks.KMS.RemoteKey.TargetRemoteKeyID = remoteKeyID
}
secret, err := secrets.FromKeyState(c.instanceName, ks)
if err != nil {
Expand Down Expand Up @@ -451,33 +483,33 @@ func (p *prefetchedKMSConfigHasherResourceProvider) getConfigMap(_ context.Conte
// back off.
//
// Callers are responsible for requeuing when this returns (false, nil).
func (c *keyController) ensureKMSPreflightPassed(ctx context.Context, configHash string) (bool, error) {
func (c *keyController) ensureKMSPreflightPassed(ctx context.Context, configHash string) (bool, string, error) {
encryptionStatus, err := c.encryptionStatusProvider.GetKMSEncryptionStatus(ctx)
if err != nil {
return false, fmt.Errorf("failed to get KMS encryption status: %w", err)
return false, "", fmt.Errorf("failed to get KMS encryption status: %w", err)
}

// Scenario 1: ObservedConfigHash outdated — schedule the preflight check.
if encryptionStatus.Preflight.ObservedConfigHash != configHash {
if err := c.encryptionStatusProvider.UpdateKMSEncryptionStatus(ctx, func(s *operatorv1.KMSEncryptionStatus) {
s.Preflight.ObservedConfigHash = configHash
}); err != nil {
return false, fmt.Errorf("failed to write preflight observed config hash: %w", err)
return false, "", fmt.Errorf("failed to write preflight observed config hash: %w", err)
}
// Scenario 1: back off: wait for the preflight controller to pick up the new hash.
return false, nil
return false, "", nil
}

// Scenario 2: preflight passed — proceed.
if isPreflightResultSucceeded(&encryptionStatus.Preflight.Result, configHash) {
return true, nil
return true, encryptionStatus.Preflight.Result.RemoteKeyID, nil
}
// Scenario 3: preflight failed — surface the error.
if isPreflightResultFailed(&encryptionStatus.Preflight.Result, configHash) {
return false, fmt.Errorf("KMS preflight check failed for config hash %s; fix the KMS configuration to proceed", configHash)
return false, "", fmt.Errorf("KMS preflight check failed for config hash %s; fix the KMS configuration to proceed", configHash)
}
// Scenario 4: back off: preflight check is still in progress.
return false, nil
return false, "", nil
}

type encryptionKeyPlan struct {
Expand Down Expand Up @@ -594,6 +626,11 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern
return latestKeyID, "kms-provider-changed", true, nil
}

if secrets.NeedsRemoteKeyMigration(latestKey.RemoteKey()) {
// Block new encryption key minting while target and migrated remote key IDs differ.
return 0, "", false, nil
}

// For KMS mode, we don't do time-based rotation. KMS keys are rotated
// externally by the KMS provider. Moreover, we don't trigger new key when external reason is changed.
// Because it would lead to duplicate providers which is not allowed.
Expand Down
55 changes: 51 additions & 4 deletions pkg/operator/encryption/controllers/migration_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,26 +216,49 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C
// using a write key that another API server has not observed
// this could lead to etcd storing data that not all API servers can decrypt
var errs []error
var writeKeyState state.KeyState
var writeKeySecretName string
var migrationWriteKey string
var hadRemoteKeyMigration bool
var writeKeyGRs []schema.GroupResource
var remoteKeyMigratedGRs []schema.GroupResource
for _, gr := range grs {
grActualKeys := currentState[gr]
if !grActualKeys.HasWriteKey() {
continue // no write key to migrate to
}

if alreadyMigrated, _, _ := state.MigratedFor([]schema.GroupResource{gr}, grActualKeys.WriteKey); alreadyMigrated {
writeKeyState = grActualKeys.WriteKey
writeKeySecret, err := secrets.FromKeyState(c.instanceName, grActualKeys.WriteKey)
if err != nil {
errs = append(errs, err)
continue
}
writeKeySecretName = writeKeySecret.Name
remoteKeyAnnotations := grActualKeys.WriteKey.RemoteKey()
migrationWriteKey = secrets.MigrationWriteKeyName(grActualKeys.WriteKey.Key.Name, remoteKeyAnnotations)
remoteKeyMigration := secrets.NeedsRemoteKeyMigration(remoteKeyAnnotations)
if remoteKeyMigration {
hadRemoteKeyMigration = true
writeKeyGRs = append(writeKeyGRs, gr)
}

if !remoteKeyMigration {
if alreadyMigrated, _, _ := state.MigratedFor([]schema.GroupResource{gr}, grActualKeys.WriteKey); alreadyMigrated {
continue
}
}
Comment on lines +237 to +250

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open for a more elegant solution here, we need to get the target key that has triggered the migration


// idem-potent migration start
finished, result, when, err := c.migrator.EnsureMigration(gr, grActualKeys.WriteKey.Key.Name)
finished, result, when, err := c.migrator.EnsureMigration(gr, migrationWriteKey)
if err == nil && finished && result != nil && time.Since(when) > migrationRetryDuration {
// last migration error is far enough ago. Prune and retry.
if err := c.migrator.PruneMigration(gr); err != nil {
errs = append(errs, err)
continue
}
finished, result, when, err = c.migrator.EnsureMigration(gr, grActualKeys.WriteKey.Key.Name)

// when is not used anymore below
finished, result, _, err = c.migrator.EnsureMigration(gr, migrationWriteKey)
}
if err != nil {
errs = append(errs, err)
Expand All @@ -251,6 +274,13 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C
continue
}

if remoteKeyMigration {
if result == nil {
remoteKeyMigratedGRs = append(remoteKeyMigratedGRs, gr)
}
continue
}

// update secret annotations
oldWriteKey, err := secrets.FromKeyState(c.instanceName, grActualKeys.WriteKey)
if err != nil {
Expand Down Expand Up @@ -283,6 +313,23 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C
}
}

if hadRemoteKeyMigration && len(writeKeySecretName) > 0 && len(remoteKeyMigratedGRs) == len(writeKeyGRs) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// TODO(thomas): this probably should be reconciled in the above loop, so we can keep track of the migrated GRs in our own annotation

if remoteKeyID, ok := secrets.RemoteKeyIDFromMigrationWriteKey(writeKeyState.Key.Name, migrationWriteKey); ok {
if err := secrets.PatchRemoteKeyAnnotations(ctx, c.secretClient.Secrets("openshift-config-managed"), writeKeySecretName, func(rk *secrets.RemoteKeyAnnotations) (bool, error) {
if !secrets.NeedsRemoteKeyMigration(*rk) {
return false, nil
}
if rk.MigratedRemoteKeyID == remoteKeyID {
return false, nil
}
rk.MigratedRemoteKeyID = remoteKeyID
return true, nil
}); err != nil {
errs = append(errs, err)
}
}
}

return migratingResources, errors.NewAggregate(errs)
}

Expand Down
Loading