CNTRLPLANE-3367: Add KMS key rotation section by tjungblu · Pull Request #2036 · openshift/enhancements

tjungblu · 2026-06-10T14:50:13Z

This covers an annotation-based approach to detect and migrate on external KEK changes in the KMS plugin architecture.
Replaces a previous PR in #2000

This covers the KEK rotation mechanism through external KMS via the plugin architecture. Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>

This covers an annotation-based approach to detect and migrate on external KEK changes in the KMS plugin architecture. Replaces a previous PR in openshift#2000. Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>

openshift-ci-robot · 2026-06-10T14:50:18Z

@tjungblu: This pull request references CNTRLPLANE-3367 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

This covers an annotation-based approach to detect and migrate on external KEK changes in the KMS plugin architecture.
Replaces a previous PR in #2000

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2026-06-10T14:50:48Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jan--f for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ardaguclu

This is very well-written PR, thank you. Dropped a comment. Other than that looks good to me.

ardaguclu · 2026-06-10T15:42:10Z

+|---------------------|--------|-------|---------|
+| `encryption.apiserver.operator.openshift.io/target-kek-id` | KMS rotation controller | `kekId` | Target kekId to migrate toward (after 5m delay) |
+| `encryption.apiserver.operator.openshift.io/migrated-kek-id` | migrationController | `kekId` | Last fully migrated kekId |
+| `encryption.apiserver.operator.openshift.io/kek-converged-at` | KMS rotation controller | RFC3339 | When candidate `kekId` first achieved cluster convergence |


Just a comment about the implementation: Since these 2 annotations are only used by rotation controller, maybe we don't need to carry them in KeyState.

ardaguclu · 2026-06-11T08:25:15Z

/lgtm

tjungblu · 2026-06-11T08:25:42Z

/hold

for others to review

p0lyn0mial · 2026-06-15T09:56:38Z

+4. For **≥ 5m** converged on `kek-new`: if health diverges, clears `kek-converged-id` and `kek-converged-at`; if stable, sets **`encryption.apiserver.operator.openshift.io/target-kek-id = kek-new`** and clears convergence pair.
+5. **migrationController** sees `needsMigration`. **Migrator** runs with `encryption.apiserver.operator.openshift.io/write-key = {keyName}-kek-new`.
+6. **State machine** and **keyController** hold while `needsMigration`.
+7. **migrationController** completes all GRs → **`encryption.apiserver.operator.openshift.io/migrated-kek-id = kek-new`**.


What happens in the following scenario ?

target-kek-id = kek-A, migration runs, SVMs annotated with write-key = {keyName}-kek-A

All SVMs complete migration controller is about to write migrated-kek-id

Rotation controller sets target-kek-id = kek-B right at this moment

does migrationController read target-kek-id to determine the value for migrated-kek-id, or does it use the kekId from the SVM's write-key annotation?

If it re-reads then kek-B migration might be skipped which is a bug.

Getting the latest encryption key secret will certainly be a bug here, which we also can't avoid, e.g. when the operator restarts while the migration is running.

What do you think about ensuring there is no change to target-kek-id while migrated-kek-id != target-kek-id?

p0lyn0mial · 2026-06-15T10:05:10Z

+
+| Full annotation key | Writer | Value | Meaning |
+|---------------------|--------|-------|---------|
+| `encryption.apiserver.operator.openshift.io/target-kek-id` | KMS rotation controller | `kekId` | Target kekId to migrate toward (after 5m delay on rotation); equals `migrated-kek-id` in steady state |


didn't we want to delegate this to the key controller? I think that would solve the race condition we discussed, wouldn't it?

pong :) one change at a time, let's get the other things sorted out. I'll dedicate an alternative for this.

p0lyn0mial · 2026-06-15T12:44:32Z

+Rotation progress is tracked on the write-key secret via annotations.
+
+**Responsibilities:**
+- **KMSPreflightController** writes the first `encryption.apiserver.operator.openshift.io/target-kek-id` on the write-key secret, using the `kekId` observed from the plugin `Status` check during [pre-flight](#pre-flight-checker-tech-preview-v2).


note that the "write-key" will be created after the preflight phase. so the preflight cannot store this information on the "write-key" secret.

When new key is created, it will not be immediately the write key.

It will start as backup key;

current write key (1)

new key as read key (2)

After that new roll out, it will be promoted as write key

new key as new write key (2)

previous write key is now read key (1)

In that case, key controller can write the kekID retrieved from preflight checker into the new key.

This is out of scope of this PR: This can be the integration logic between preflight checker and key controller. Key controller won't create the new key, until it sees a kekID for the calculated hash of the given KMS plugin.

changed this interaction to the key controller

openshift-ci · 2026-06-15T13:28:36Z

@tjungblu: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

This covers an annotation-based approach to detect and migrate on external KEK changes in the KMS plugin architecture. This design differs from openshift#2036 by centralizing everything in the existing key controller instead of creating a new rotation controller.

ardaguclu · 2026-06-16T13:48:27Z

/lgtm
we definitely need this design in GA

ardaguclu · 2026-06-17T08:20:58Z

/lgtm cancel

tjungblu · 2026-06-18T11:16:53Z

/close

we can circle back here if needed, otherwise this is superseded by #2041

openshift-ci · 2026-06-18T11:17:07Z

@tjungblu: Closed this PR.

Details

In response to this:

/close

we can circle back here if needed, otherwise this is superseded by #2041

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

tjungblu added 2 commits June 10, 2026 15:23

CNTRLPLANE-3367: Add KMS key rotation section

862f7b8

This covers the KEK rotation mechanism through external KMS via the plugin architecture. Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>

CNTRLPLANE-3367: Add KMS key rotation section

26fc00f

This covers an annotation-based approach to detect and migrate on external KEK changes in the KMS plugin architecture. Replaces a previous PR in openshift#2000. Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 10, 2026

openshift-ci Bot requested review from jupierce and stleerh June 10, 2026 14:50

ardaguclu reviewed Jun 10, 2026

View reviewed changes

This was referenced Jun 11, 2026

[Auto-Generated] Personal Workflow Brief tjungblu/agent#1

Open

[Auto-Generated] KMS Team PR Dashboard openshift/library-go#2268

Open

also set target kek on bootstrap

1eb5015

openshift-ci Bot assigned ardaguclu Jun 11, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2026

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2026

p0lyn0mial reviewed Jun 15, 2026

View reviewed changes

Comment thread enhancements/kube-apiserver/kms-encryption-foundations.md

lukaszs comments

4201f8d

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 15, 2026

p0lyn0mial reviewed Jun 15, 2026

View reviewed changes

preflight vs. keycontroller

3d654cf

tjungblu mentioned this pull request Jun 15, 2026

CNTRLPLANE-3367: Add KMS key rotation section #2041

Open

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2026

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2026

openshift-ci Bot closed this Jun 18, 2026

Uh oh!

Conversation

tjungblu commented Jun 10, 2026

Uh oh!

openshift-ci-robot commented Jun 10, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci Bot commented Jun 10, 2026

Uh oh!

ardaguclu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ardaguclu commented Jun 11, 2026

Uh oh!

tjungblu commented Jun 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Jun 15, 2026

Uh oh!

ardaguclu commented Jun 16, 2026

Uh oh!

ardaguclu commented Jun 17, 2026

Uh oh!

tjungblu commented Jun 18, 2026

Uh oh!

openshift-ci Bot commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

openshift-ci-robot commented Jun 10, 2026 •

edited by openshift-ci Bot

Loading