Repository navigation
CNTRLPLANE-4025: feat(azure): support managed HSM for KMS encryption - #9199
Merged
openshift-merge-bot[bot] merged 6 commits intoAug 21, 2026
Merged
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
do-not-merge/needs-area
needs-rebase
Contributor
|
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:
📝 WalkthroughWalkthroughAzure KMS now supports Azure Managed HSM in addition to Azure Key Vault. The API adds an optional vault-type field with Key Vault as the default. Validation enforces matching vault types for active and backup keys and prevents changing the active key’s type. Azure utilities parse and generate service-specific endpoints. Cluster configuration, KAS runtime arguments, provider names, and fingerprints preserve the vault type. Compatibility tests cover legacy JSON and identity behavior. Sequence Diagram(s)sequenceDiagram
participant User
participant ClusterCLI
participant HostedCluster
participant AzureUtilities
participant KASKMS
User->>ClusterCLI: Provide Azure encryption key identifier
ClusterCLI->>HostedCluster: Set key and KeyVaultType
HostedCluster->>AzureUtilities: Parse key and resolve DNS suffix
AzureUtilities-->>HostedCluster: Return Azure encryption key metadata
HostedCluster->>KASKMS: Propagate active and target key metadata
KASKMS->>KASKMS: Add --managed-hsm when required
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms.go (1)
143-153: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd rotation-path coverage for
KeyVaultType.Add a
deriveKMSKeystest with Managed HSM active and target status keys. Cover both promotion branches. Assert that both returned keys retainManagedHSM; otherwise a future status-copy regression can remove the Managed HSM provider identity and runtime flag.As per coding guidelines, “Unit test any code changes and additions.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms.go` around lines 143 - 153, Add rotation-path coverage for KeyVaultType in deriveKMSKeys: create a test with active and target Azure status keys using ManagedHSM, exercise both key-promotion branches, and assert both returned keys preserve ManagedHSM. Ensure the test guards against status-copy regressions that drop the provider identity or runtime flag.Source: Coding guidelines
🧹 Nitpick comments (2)
control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.go (1)
532-548: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse fixed legacy identity fixtures.
Both tests calculate the baseline with the changed implementation. They prove normalization only. They do not prove compatibility with prior releases. Add fixed N-1 Key Vault provider-name and fingerprint fixtures. Assert that omitted and explicit
KeyVaultvalues match those fixtures.
control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.go#L532-L548: Assert the provider name against a fixed legacy value.support/secretencryption/fingerprint_test.go#L29-L43: Assert the fingerprint against a fixed legacy value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.go` around lines 532 - 548, The tests currently derive the legacy baseline from the changed implementation instead of verifying N-1 compatibility. In control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.go lines 532-548, update TestAzureKMSProviderName to compare both omitted and explicit KeyVault configurations against a fixed legacy provider-name fixture, while retaining the ManagedHSM distinction. In support/secretencryption/fingerprint_test.go lines 29-43, update the fingerprint test to compare omitted and explicit KeyVault configurations against a fixed legacy fingerprint fixture; do not calculate either baseline from the implementation under test.support/azureutil/azureutil.go (1)
448-451: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the DNS suffix resolution error.
Add operation context before returning
err. This identifiesGetKeyVaultFQDNas the failed operation.As per coding guidelines, wrap errors with context when rethrowing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@support/azureutil/azureutil.go` around lines 448 - 451, In GetKeyVaultFQDN, wrap the error returned by GetKeyVaultDNSSuffix with operation context identifying GetKeyVaultFQDN before returning it, while preserving the existing empty-string result and error flow.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/hypershift/v1beta1/azure.go`:
- Around line 917-921: Add envtest coverage for the AzureKMSSpec CEL
validations, exercising omitted vault types, Key Vault versus Managed HSM
mismatches, immutable activeKey.keyVaultType updates, and updates from an
omitted type to explicit KeyVault. Use the envtest API server to create and
update resources so CRD defaults and XValidation rules execute, and assert each
expected validation outcome.
In `@api/hypershift/v1beta1/etcdbackup_types.go`:
- Around line 181-187: Update the encryptionKeyURL validation patterns in
api/hypershift/v1beta1/etcdbackup_types.go at lines 181-187, 251-256, and
350-355 to accept Managed HSM hostnames ending in managedhsm.azure.net,
managedhsm.azure.cn, or managedhsm.usgovcloudapi.net, while preserving the
existing URL structure and validation constraints at all three sites.
In `@support/azureutil/azureutil_test.go`:
- Around line 694-739: Extend the TestGetKeyVaultDNSSuffix table with a China
cloud ManagedHSM case using the appropriate Azure China cloud identifier and
assert the expected managedhsm.azure.cn suffix. Keep the existing error and
other cloud cases unchanged.
In `@support/azureutil/azureutil.go`:
- Around line 487-496: Update the hostname classification before assigning
KeyVaultType in the AzureEncryptionKey construction: normalize host case and
validate it against exact approved Managed HSM and Key Vault hostname suffixes
using an allow-list. Set ManagedHSM only for an approved Managed HSM suffix,
KeyVault only for an approved Key Vault suffix, and reject or otherwise preserve
the existing invalid-host handling for unapproved domains; do not use an
unanchored substring check.
---
Outside diff comments:
In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms.go`:
- Around line 143-153: Add rotation-path coverage for KeyVaultType in
deriveKMSKeys: create a test with active and target Azure status keys using
ManagedHSM, exercise both key-promotion branches, and assert both returned keys
preserve ManagedHSM. Ensure the test guards against status-copy regressions that
drop the provider identity or runtime flag.
---
Nitpick comments:
In
`@control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.go`:
- Around line 532-548: The tests currently derive the legacy baseline from the
changed implementation instead of verifying N-1 compatibility. In
control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.go
lines 532-548, update TestAzureKMSProviderName to compare both omitted and
explicit KeyVault configurations against a fixed legacy provider-name fixture,
while retaining the ManagedHSM distinction. In
support/secretencryption/fingerprint_test.go lines 29-43, update the fingerprint
test to compare omitted and explicit KeyVault configurations against a fixed
legacy fingerprint fixture; do not calculate either baseline from the
implementation under test.
In `@support/azureutil/azureutil.go`:
- Around line 448-451: In GetKeyVaultFQDN, wrap the error returned by
GetKeyVaultDNSSuffix with operation context identifying GetKeyVaultFQDN before
returning it, while preserving the existing empty-string result and error flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e764f6e8-a327-4317-90a4-678e4eeb96c7
⛔ Files ignored due to path filters (41)
api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hcpetcdbackups.hypershift.openshift.io/HCPEtcdBackup.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ClusterUpdateAcceptRisks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ClusterVersionOperatorConfiguration.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDC.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDCWithUIDAndExtraClaimMappings.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ExternalOIDCWithUpstreamParity.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/GCPPlatform.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HCPEtcdBackup.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/ImageStreamImportMode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/KMSEncryptionProvider.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/OpenStack.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/TLSAdherence.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ClusterUpdateAcceptRisks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ClusterVersionOperatorConfiguration.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDC.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDCWithUIDAndExtraClaimMappings.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDCWithUpstreamParity.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/GCPPlatform.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HCPEtcdBackup.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ImageStreamImportMode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/KMSEncryptionProvider.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/OpenStack.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/TLSAdherence.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**client/applyconfiguration/hypershift/v1beta1/azurekmskey.gois excluded by!client/**cmd/install/assets/crds/hypershift-operator/tests/hcpetcdbackups.hypershift.openshift.io/techpreview.hcpetcdbackups.status.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hcpetcdbackups.hypershift.openshift.io/techpreview.hcpetcdbackups.validation.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.kms.testsuite.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hcpetcdbackups-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedclusters-Hypershift-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlvendor/github.com/openshift/hypershift/api/hypershift/v1beta1/azure.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/etcdbackup_types.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (14)
api/hypershift/v1beta1/azure.goapi/hypershift/v1beta1/azure_test.goapi/hypershift/v1beta1/etcdbackup_types.gocmd/cluster/azure/create.gocmd/util/azure_flag_descriptions.gocontrol-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/kms.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/reencryption/reencryption.gosupport/azureutil/azureutil.gosupport/azureutil/azureutil_test.gosupport/secretencryption/fingerprint.gosupport/secretencryption/fingerprint_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms_test.go (1)
119-171: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winKeep legacy Key Vault rotation coverage.
The table now covers Azure rotation only with
KeyVaultType: ManagedHSM. It does not exercise the API default whereKeyVaultTypeis omitted. Add equivalent read-only and promotion cases for the default Key Vault path. This protects legacy provider identity and rotation behavior.Based on the PR objective to preserve legacy Key Vault provider identities, keep a default-path regression case in this table.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms_test.go` around lines 119 - 171, Add equivalent read-only and promotion table cases alongside the existing Managed HSM cases, leaving Azure KeyVaultType omitted to exercise the legacy default Key Vault path. Use the same v1/v2 rotation states and validations in the kmsWriteReadKeys callbacks, including provider identity and key versions, so both rotation phases preserve legacy behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms_test.go`:
- Around line 193-197: Update mustAWSProviderName and mustAzureProviderName to
capture and assert errors returned by their respective KMS provider-name
helpers, failing the test explicitly when hashing fails; only return the
provider name after successful validation.
---
Nitpick comments:
In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms_test.go`:
- Around line 119-171: Add equivalent read-only and promotion table cases
alongside the existing Managed HSM cases, leaving Azure KeyVaultType omitted to
exercise the legacy default Key Vault path. Use the same v1/v2 rotation states
and validations in the kmsWriteReadKeys callbacks, including provider identity
and key versions, so both rotation phases preserve legacy behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 38821765-09e3-4ac2-893f-74944077ce9a
⛔ Files ignored due to path filters (1)
cmd/install/assets/crds/hypershift-operator/tests/hostedclusters.hypershift.openshift.io/stable.hostedclusters.kms.testsuite.yamlis excluded by!cmd/install/assets/**/*.yaml
📒 Files selected for processing (4)
control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/kms_test.gosupport/azureutil/azureutil.gosupport/secretencryption/fingerprint_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- support/secretencryption/fingerprint_test.go
- support/azureutil/azureutil.go
- control-plane-operator/controllers/hostedcontrolplane/v2/kas/kms/azure_test.go
openshift-ci
Bot
added
the
area/documentation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
docs/content/how-to/azure/create-azure-cluster-with-options.md (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNormalize the code blocks for Markdown validation.
Add appropriate language identifiers such as
bash,yaml, andtext. Apply the repository-configured code-block style consistently to resolve MD040 and MD046.Also applies to: 56-56, 67-67, 108-108, 138-138, 176-176, 181-181, 187-187, 200-200, 208-208, 217-217, 222-222, 244-244, 260-260, 269-269
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/how-to/azure/create-azure-cluster-with-options.md` at line 27, Normalize every fenced code block in create-azure-cluster-with-options.md by adding the appropriate language identifier, such as bash, yaml, or text, and apply the repository’s configured fence style consistently so all affected blocks satisfy MD040 and MD046.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/how-to/azure/create-azure-cluster-with-options.md`:
- Line 4: Update the link in the introductory sentence to use descriptive text
identifying the destination, such as “Azure cluster setup without additional
options,” while preserving the existing create-azure-cluster-on-aks.md target.
- Line 241: Remove the leading space inside the inline code span for
azure-kms-provider-active in the kube-apiserver verification sentence,
preserving the surrounding wording and formatting.
- Line 18: Correct the spelling in the documentation: update the line-18 wording
to “access the key vault” and capitalize “Ephemeral” on line 98. Run make
verify-codespell to verify the Markdown changes.
- Line 207: Update the HyperShift CLI command instructions around the step 1d
reference to point to step 1c, and remove backticks surrounding the shell
arguments in the fenced command block so they are passed literally rather than
interpreted as command substitution.
- Around line 157-164: Expand the Azure KMS procedure beyond the compatibility
note to document Managed HSM setup, including resource creation, key-identifier
configuration, supported endpoint forms, required role assignment, and Managed
HSM-specific provider or fingerprint verification output. Use the existing KMS
setup and verification sections as the integration points; if Managed HSM
details cannot be added, explicitly label those steps and outputs as Azure Key
Vault-only.
---
Nitpick comments:
In `@docs/content/how-to/azure/create-azure-cluster-with-options.md`:
- Line 27: Normalize every fenced code block in
create-azure-cluster-with-options.md by adding the appropriate language
identifier, such as bash, yaml, or text, and apply the repository’s configured
fence style consistently so all affected blocks satisfy MD040 and MD046.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: eceaa7a8-cda2-43fd-b977-f99c49ffdfb7
📒 Files selected for processing (1)
docs/content/how-to/azure/create-azure-cluster-with-options.md
bennerv
reviewed
Jul 31, 2026
hlipsig
force-pushed
the
hlipsig-azure-hsm-support
branch
3 times, most recently
from
August 4, 2026 21:31
4a9719a to
f58d3be
Compare
openshift-ci
Bot
added
the
area/testing
hlipsig
force-pushed
the
hlipsig-azure-hsm-support
branch
from
August 5, 2026 01:48
f58d3be to
1de9b4f
Compare
openshift-ci
Bot
removed
the
needs-rebase
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9199 +/- ##
==========================================
+ Coverage 45.85% 45.92% +0.06%
==========================================
Files 781 781
Lines 97936 98061 +125
==========================================
+ Hits 44911 45032 +121
- Misses 49959 49964 +5
+ Partials 3066 3065 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
Author
|
This is ready to review, but I don't have a prow job yet since we need to add some stuff to the Azure subscription. Manual verification passed and I have the environment available for use if anyone wants to take a look at it. |
hlipsig
marked this pull request as ready for review
August 5, 2026 02:57
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Contributor
Author
|
Fresh-cluster Azure end-to-end validation completed against commit Test setup:
Results:
|
Contributor
Author
|
/validated by @hlipsig see full evidence above |
Member
|
/pipeline required |
Contributor
|
Scheduling tests matching the |
Member
|
/cc |
Contributor
Author
|
/retest |
1 similar comment
Contributor
Author
|
/retest |
JoelSpeed
reviewed
Aug 20, 2026
Contributor
|
/approve |
- Add AzureKMSKeyVaultType to AzureKMSSpec with backward-compatible immutability validation - Support Public, US Government, China, German, and Bleu Azure clouds - Accept Managed HSM encryption key URLs and regenerate feature-gated API manifests Signed-off-by: Hilliary Lipsig <hlipsig@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Regenerate install CRDs and validation suites for Managed HSM and sovereign clouds - Cover immutable vault type transitions and all supported Azure key URL suffixes - Update the AzureKMSSpec client and vendored HyperShift API types Signed-off-by: Hilliary Lipsig <hlipsig@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Propagate the detected vault type into AzureKMSSpec during cluster creation - Expose all supported Azure cloud environments in CLI help - Document Key Vault and Managed HSM encryption key URL formats Signed-off-by: Hilliary Lipsig <hlipsig@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Resolve Key Vault and Managed HSM endpoints across all supported Azure clouds - Preserve legacy Key Vault fingerprints while distinguishing Managed HSM keys - Gate Managed HSM on OpenShift 4.22 without masking release resolution failures Signed-off-by: Hilliary Lipsig <hlipsig@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Pass Managed HSM mode to both active and backup Azure KMS provider sidecars - Use one immutable vault type for active, backup, and rotation target keys - Preserve legacy Key Vault provider names while distinguishing Managed HSM providers Signed-off-by: Hilliary Lipsig <hlipsig@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Describe supported Azure KMS services, clouds, and minimum OpenShift version - Regenerate the API reference and aggregated documentation Signed-off-by: Hilliary Lipsig <hlipsig@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/validated by @hlipsig recent changes were rebase only. |
Contributor
|
Scheduling tests matching the |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, hlipsig, JoelSpeed, muraee, yuqi-zhang 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 |
Contributor
|
@hlipsig: The following test failed, say
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. |
Member
|
/retest-required |
Contributor
Author
|
/verified by @hlipsig |
|
@hlipsig: 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. |
Contributor
Author
|
/cherry-pick release-5.0 |
|
@hlipsig: #9199 failed to apply on top of branch "release-5.0": 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. |
This was referenced Aug 21, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
Adds Azure Managed HSM support for control-plane KMS encryption. The API distinguishes Azure Key Vault from Managed HSM, and the selected type is carried through endpoint resolution, sidecar configuration, validation, status, rotation, and provider identity generation.
Existing Azure Key Vault provider names and fingerprints remain unchanged when
keyVaultTypeis omitted or explicitly set toKeyVault.Compatibility and limitations
.spec.secretEncryption.kms.azure.keyVaultTypeapplies to the entire Azure KMS configuration, including active, backup, status-derived, and rotation target keys.keyVaultTypeis immutable. Existing HostedClusters cannot migrate between Key Vault and Managed HSM; a new HostedCluster is required.Which issue(s) this PR fixes
Fixes https://redhat.atlassian.net/browse/OCPSTRAT-3549
Special notes for reviewers
The Azure Managed HSM path was manually verified against a live subscription before review. End-to-end revalidation of the review fixes is in progress.
Checklist