CNTRLPLANE-3070: Support KMS on self-managed Azure without affecting ARO HCP#8088
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
Skipping CI for Draft Pull Request. |
|
@bryan-cox: This pull request references CNTRLPLANE-3070 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 "4.22.0" version, but no target version was set. DetailsIn response to this:
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
Bot
added
the
do-not-merge/work-in-progress
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request adds support for self-managed Azure KMS encryption at rest and wiring for KMS workload identities. Changes include making AzureKMSSpec.KMS optional and adding a mutually-exclusive SelfManagedKMS; adding --enable-kms to IAM creation and plumbing kmsClientID through IAM/infra outputs; conditionally including a kms workload identity; control-plane/operator changes to detect self-managed Azure and render token-minter/cloud-token or managed secret-store CSI volumes accordingly; centralizing the cloud token mount path with CloudTokenMountPath; and extensive tests and docs for both managed and self-managed Azure KMS flows. Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI/IAM Cmd
participant WI as Workload Identity Mgr
participant Infra as Infra Manager
participant Secret as K8s Secrets
User->>CLI: hypershift create iam azure --enable-kms
CLI->>WI: CreateWorkloadIdentitiesFromIAMOptions(enableKMS=true)
WI->>WI: GetWorkloadIdentityDefinitions(opts={IncludeKMS:true})
WI->>Infra: Create Azure identities (includes kms)
Infra->>Infra: Assign roles (if applicable)
Infra-->>WI: Return identities + kmsClientID
WI-->>CLI: Return IAM output with kmsClientID
CLI->>Secret: Write workload-identities.json (includes kmsClientID)
sequenceDiagram
participant User
participant Cluster as Cluster Create
participant Infra as Infra Extract
participant CPO as Control Plane Operator
participant KMS as KMS Provider
User->>Infra: Load workload-identities.json
Infra->>Infra: Extract kmsClientID from JSON
Infra-->>Cluster: Pass infra output (kmsClientID)
User->>Cluster: hypershift create cluster azure --encryption-key-id=<key>
Cluster->>Cluster: Build AzureKMSSpec (SelfManagedKMS if kmsClientID present else KMS)
Cluster-->>CPO: Apply SecretEncryption with AzureKMSSpec
CPO->>CPO: Detect self-managed vs managed
alt Self-Managed
CPO->>KMS: Add token-minter container + cloud-token volume
CPO->>KMS: Set AZURE_CLIENT_ID/AZURE_TENANT_ID and federated token file
KMS->>KMS: Mint federated token at runtime and authenticate to Key Vault
else Managed
CPO->>KMS: Mount credentials secret via Secret Store CSI
KMS->>KMS: Use credentials secret to authenticate to Key Vault
end
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@bryan-cox: This pull request references CNTRLPLANE-3070 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 "4.22.0" version, but no target version was set. DetailsIn response to this:
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
Bot
added
do-not-merge/needs-area
area/api
bryan-cox
force-pushed
the
CNTRLPLANE-3070
branch
2 times, most recently
from
1890fdc to
a3d1f65
Compare
openshift-ci
Bot
added
needs-rebase
bryan-cox
force-pushed
the
CNTRLPLANE-3070
branch
2 times, most recently
from
21007e3 to
2d16bd3
Compare
openshift-ci
Bot
removed
the
needs-rebase
bryan-cox
force-pushed
the
CNTRLPLANE-3070
branch
from
2d16bd3 to
82e995f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8088 +/- ##
==========================================
+ Coverage 40.00% 40.34% +0.33%
==========================================
Files 751 753 +2
Lines 92838 93134 +296
==========================================
+ Hits 37137 37572 +435
+ Misses 53014 52862 -152
- Partials 2687 2700 +13
... and 7 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
openshift-ci
Bot
added
the
needs-rebase
- Add AzureKMSSpec with mutually exclusive auth modes: `kms` (ManagedIdentity for ARO HCP) and `workloadIdentity` (WorkloadIdentity for self-managed via token-minter) - Add CEL XValidation rules enforcing mutual exclusivity, at-least-one, and immutability between the two authentication modes - Add AzureKeyVaultAccessType enum for Key Vault access mechanism selection - Add HostedCluster-level CEL rule requiring `selfManagedKMS` when using WorkloadIdentities authentication with Azure KMS - Reuse existing WorkloadIdentity type for KMS auth to maintain consistency with other Azure workload identity fields Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
- Add --azure-kms-client-id and --azure-kms-tenant-id flags for workload identity-based KMS authentication - Add KMS client ID to IAM identity creation output - Add Azure flag descriptions for new KMS parameters - Update stable envtest validation test cases for Azure KMS Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
…ed clusters - Add KMS secret provider class reconciliation for self-managed Azure using workload identity credentials - Add IsSelfManagedAzure helper to azureutil for platform detection - Add KMS-related constants for config paths and identifiers - Add token-minter sidecar support for KMS workload identity authentication - Update HO azure platform controller to handle both managed identity and workload identity KMS modes Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
…naged clusters - Add Azure KMS provider with self-managed mode support using token-minter sidecar for workload identity authentication - Add KMS pod configuration for both managed identity and workload identity authentication modes - Add encryption config generation for Azure KMS with key hashing - Update KAS deployment to mount KMS-specific volumes and containers - Add self-managed Azure KMS unit tests for encryption config and pod configuration Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
- Add e2e test for Azure KMS encryption on self-managed clusters - Add envtest validation test cases for Azure KMS mutual exclusivity, immutability (both directions), and key version update scenarios - Add reverse immutability test: switching from workloadIdentity to kms must also fail Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
- Add Azure KMS setup guide for self-managed clusters including workload identity configuration and Key Vault access - Update azure-workload-identity-setup with KMS identity creation - Regenerate API reference and aggregated docs Signed-off-by: Bryan Cox <brcox@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
bryan-cox
force-pushed
the
CNTRLPLANE-3070
branch
from
fbe5dfc to
c313825
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, enxebre The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/lgtm |
|
Scheduling tests matching the |
|
/verified by @bryan-cox I verified this works and the report verification report is located here - https://redhat.atlassian.net/browse/CNTRLPLANE-3070?focusedCommentId=16980328 |
openshift-ci-robot
added
the
verified
|
@bryan-cox: This PR has been marked as verified by DetailsIn response to this:
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. |
| // +required | ||
| KMS ManagedIdentity `json:"kms"` | ||
| // +optional | ||
| KMS ManagedIdentity `json:"kms,omitzero"` |
There was a problem hiding this comment.
I know this field was added before, but why is it called kms? with the new field, it should be called managedIdentity or smth
There was a problem hiding this comment.
.spec.secretEncryption.kms.azure.kms doesn't sound correct.
There was a problem hiding this comment.
I'm not sure how we can change this now since ARO HCP is in flight other than introducing a new field and trying to deprecated this.
|
/override "Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main" |
|
/override "Red Hat Konflux / hypershift-operator-enterprise-contract / hypershift-operator-main" |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: Red Hat Konflux / hypershift-operator-main-enterprise-contract / hypershift-operator-main DetailsIn response to this:
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. |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: Red Hat Konflux / hypershift-operator-enterprise-contract / hypershift-operator-main DetailsIn response to this:
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. |
|
@bryan-cox: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
openshift-merge-bot
Bot
merged commit 9e283ae
into
openshift:main
6 participants
What this PR does / why we need it:
Enables Azure Key Vault KMS encryption (etcd encryption at rest) for self-managed Azure HyperShift clusters using workload identity federation, without breaking the existing ARO HCP (managed Azure) KMS path that uses managed identities with CSI secret store volumes.
Key Changes
API: Added
SelfManagedKMSfield (typeSelfManagedAzureKMS) toAzureKMSSpecwith aClientIDfor the workload identity that hasKey Vault Crypto Userrole on the Key Vault. CEL validation rules enforce mutual exclusivity betweenkms(managed) andselfManagedKMS(self-managed), and immutability once set.Control Plane Operator:
UseWorkloadIdentityExtension) for self-managedHyperShift Operator:
CLI:
create cluster azure: Only setsManagedIdentityKMS creds for managed Azure; self-managed usesAzureKMSSpec.SelfManagedKMScreate iam azure: Creates KMS workload identity with federated credential forkms-providerservice accountE2E: Updated
TestCreateClusterCustomConfigto handle self-managed Azure KMS assertions. Added envtest coverage for CEL validation rules (mutual exclusivity, immutability).Documentation: Added KMS encryption section to self-managed Azure cluster guide with Key Vault setup and workload identity federation instructions.
Which issue(s) this PR fixes:
Fixes CNTRLPLANE-3070
Special notes for your reviewer:
The self-managed Azure KMS authentication pattern follows the same approach used by Cloud Controller Manager (CCM) and Azure CSI storage drivers, which already support self-managed Azure with workload identity federation.
The token-minter sidecar mints OIDC tokens for the
kms-providerservice account inkube-systemnamespace, matching the pattern used by AWS KMS.kmsandselfManagedKMSare mutually exclusive and immutable once set — switching between managed and self-managed KMS auth after cluster creation is not supported.Checklist: