CNTRLPLANE-4025: Add HyperShift Azure Managed HSM periodic - #82995
CNTRLPLANE-4025: Add HyperShift Azure Managed HSM periodic#82995hlipsig wants to merge 6 commits into
Conversation
Provision an ephemeral Azure Managed HSM for the release-5.0 HyperShift v2 encryption test and clean it up after the daily periodic run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Skipping CI for Draft Pull Request. |
|
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:
WalkthroughThe Azure self-managed HyperShift workflow now provisions an ephemeral Managed HSM, passes its encryption key to guest creation and tests, removes the resources, and runs through a daily periodic job. Azure deprovisioning also discovers and cleans up aged CI resource groups. ChangesAzure Managed HSM self-managed testing
Azure cluster deprovisioning
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PeriodicJob
participant SelfManagedWorkflow
participant ManagedHSMProvision
participant GuestCreation
participant E2ETests
participant ManagedHSMDeprovision
PeriodicJob->>SelfManagedWorkflow: Start daily Managed HSM test
SelfManagedWorkflow->>ManagedHSMProvision: Provision HSM and encryption key
ManagedHSMProvision-->>SelfManagedWorkflow: Write key ID to SHARED_DIR
SelfManagedWorkflow->>GuestCreation: Create guests with AZURE_ENCRYPTION_KEY_ID
SelfManagedWorkflow->>E2ETests: Run tests with AZURE_ENCRYPTION_KEY_ID
SelfManagedWorkflow->>ManagedHSMDeprovision: Delete and purge HSM
Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@ci-operator/step-registry/hypershift/azure/managed-hsm/deprovision/hypershift-azure-managed-hsm-deprovision-ref.yaml`:
- Line 3: Remove the best_effort setting from the Managed HSM deprovision step
so failures from login, vault operations, or resource-group deletion remain
visible in the job result. Keep the existing cleanup operations unchanged.
🪄 Autofix
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: 2a773b46-d603-4364-9b21-07106a1449b9
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/hypershift/openshift-hypershift-release-5.0-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (13)
ci-operator/config/openshift/hypershift/openshift-hypershift-release-5.0__periodics.yamlci-operator/step-registry/hypershift/azure/create-selfmanaged-guests/hypershift-azure-create-selfmanaged-guests-commands.shci-operator/step-registry/hypershift/azure/e2e/v2-self-managed/hypershift-azure-e2e-v2-self-managed-workflow.yamlci-operator/step-registry/hypershift/azure/managed-hsm/OWNERSci-operator/step-registry/hypershift/azure/managed-hsm/deprovision/OWNERSci-operator/step-registry/hypershift/azure/managed-hsm/deprovision/hypershift-azure-managed-hsm-deprovision-commands.shci-operator/step-registry/hypershift/azure/managed-hsm/deprovision/hypershift-azure-managed-hsm-deprovision-ref.metadata.jsonci-operator/step-registry/hypershift/azure/managed-hsm/deprovision/hypershift-azure-managed-hsm-deprovision-ref.yamlci-operator/step-registry/hypershift/azure/managed-hsm/provision/OWNERSci-operator/step-registry/hypershift/azure/managed-hsm/provision/hypershift-azure-managed-hsm-provision-commands.shci-operator/step-registry/hypershift/azure/managed-hsm/provision/hypershift-azure-managed-hsm-provision-ref.metadata.jsonci-operator/step-registry/hypershift/azure/managed-hsm/provision/hypershift-azure-managed-hsm-provision-ref.yamlci-operator/step-registry/hypershift/azure/run-e2e-v2-selfmanaged/hypershift-azure-run-e2e-v2-selfmanaged-chain.yaml
Wait for Azure to expose the soft-deleted HSM, purge it to stop billing, and retain partial-provision cleanup behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Disable shell tracing while reading and exporting the Managed HSM key URI in guest creation and v2 test execution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Authenticate HSM provisioning and cleanup with the hypershift-azure cluster profile while retaining the existing KMS workload identity input. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/pj-rehearse ack |
|
@hlipsig: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
| decoration_config: | ||
| skip_cloning: true | ||
| extra_refs: | ||
| - base_ref: release-5.0 |
There was a problem hiding this comment.
Note to my reviewer: Not sure if this is correct. I think the feature is going to land in 5.1 with backports. I'm a little unclear what this should be so I suspect you'll tell me how to fix it.
Add periodic-ipi-deprovision-azure to clean up leaked Azure IPI cluster resources, matching the existing AWS and GCP GC patterns. Uses a 6-hour TTL to avoid disrupting active tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@clusters/app.ci/ipi-deprovision/ipi-deprovision-bc.yaml`:
- Around line 50-52: Update the package installation command in the
ipi-deprovision image build to remove the --nogpg option, preserving GPG
signature verification for azure-cli and the other installed packages.
In `@core-services/ipi-deprovision/azure.sh`:
- Around line 64-80: Update the resource-group filtering around rg_name to
require the documented -rg suffix before proceeding. Remove the fallback that
assigns the full name to infraID, and skip non-matching names before metadata or
deletion logic; retain infraID extraction from the matched suffix format.
🪄 Autofix
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: 7f3a3d20-3c7d-4d10-87ba-a9be64d913b0
⛔ Files ignored due to path filters (1)
ci-operator/jobs/infra-build-farm-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (2)
clusters/app.ci/ipi-deprovision/ipi-deprovision-bc.yamlcore-services/ipi-deprovision/azure.sh
This reverts commit d102a21.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hlipsig The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse |
|
@hlipsig: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-v2-azure-self-managed |
|
@bryan-cox: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@hlipsig: The following tests 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. |
|
/retitle CNTRLPLANE-4025: Add HyperShift Azure Managed HSM periodic |
|
@hlipsig: This pull request references CNTRLPLANE-4025 which is a valid jira issue. 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. |
| approvers: | ||
| - csrwng | ||
| - enxebre | ||
| - sjenning |
There was a problem hiding this comment.
Remove sjenning from here please. This goes for the other owner files too.
bryan-cox
left a comment
There was a problem hiding this comment.
The HSM lifecycle scripts are well structured — writing cleanup inputs before provisioning, retrying with backoff on eventual-consistency operations, and guarding with set +x around sensitive values are all solid patterns.
One note on best_effort on the deprovision ref — see inline comment.
| @@ -0,0 +1,22 @@ | |||
| ref: | |||
| as: hypershift-azure-managed-hsm-deprovision | |||
| best_effort: true | |||
There was a problem hiding this comment.
Consider removing best_effort: true here. A leaked Managed HSM is expensive and should fail the job loudly so it gets investigated. Post steps all run regardless of earlier post step failures, so destroy-nested-management-cluster will still execute even if this step fails — there's no need for best-effort to protect downstream cleanup.
As-is best_effort is inert anyway (the workflow doesn't set allow_best_effort_post_steps: true), but if someone adds that flag later, deprovision failures would be silently swallowed.
Summary
Motivation
This provides end-to-end coverage for openshift/hypershift#9199. The v2 suite verifies the Managed HSM API selection, KMS v2 ciphertext in etcd, API decrypt/read-back, and persistence across a control-plane-operator restart.
Dependencies and discussion points
openshift/azure-kubernetes-kmsValidation
make ci-operator-checkconfigSummary by CodeRabbit
This PR adds Azure infrastructure support for HyperShift encryption testing and leaked-cluster cleanup.