feature: Enable encryption x manual rollback flow #460
Conversation
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
ndubchak
commented
Jan 16, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enables manual rollback functionality for hosts using PCR encryption by:
- Updating pcrlock policy generation to support rollback scenarios with encrypted volumes
- Adding binary path ordering by volume label (AZLA/AZLB) for consistent pcrlock generation
- Creating a new manual rollback helper for testing rollback with encryption
- Removing the restriction that prevented manual rollback with encryption
Changes:
- Enabled encryption support for manual A/B rollbacks by regenerating pcrlock policies with both current and rollback boot binaries
- Added a new
manual-rollbackstorm helper to test manual rollback flows, including artifact collection - Updated binary path sorting to ensure consistent ordering by volume label for deterministic pcrlock generation
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/trident/src/engine/storage/encryption.rs | Added volume label-based sorting for UKI and bootloader binaries; enhanced get_uki_paths and get_bootloader_paths to handle rollback scenarios |
| crates/trident/src/engine/manual_rollback/mod.rs | Uncommented and enabled pcrlock policy regeneration during manual rollback staging |
| crates/trident/src/engine/manual_rollback/utils.rs | Removed encryption with volume change restriction and updated logging with consistent quote formatting |
| tools/storm/utils/ssh/sftp/sftp.go | Added DownloadRemoteFile function to download remote files via SFTP with sudo privileges |
| tools/storm/helpers/manual_rollback.go | New helper to test manual rollback, including artifact collection and rollback validation |
| tools/storm/helpers/init.go | Added ManualRollbackHelper and alphabetically sorted helper list |
| .pipelines/templates/stages/testing_common/e2e-test-run.yml | Added pipeline steps to test manual rollback with encryption in combined configuration |
1 task
Member
|
remove |
ndubchak
commented
Jan 17, 2026
bfjelds
previously approved these changes
Jan 17, 2026
ndubchak
changed the title
Contributor
Author
|
/AzurePipelines run [GITHUB]-trident-pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ayaegashi
approved these changes
Jan 17, 2026
Contributor
ayaegashi
left a comment
ayaegashi
left a comment
There was a problem hiding this comment.
lgtm as long as pr-e2e passes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔍 Description
This PR enables running A/B manual rollback on hosts that are using PCR encryption.
🤔 Rationale
This PR is a follow up to the recent commit to enable manual rollback in Trident. This PR allows the users to perform a manual rollback that requires a reboot on a host that has PCR encryption enabled.
Fundamentally, manual rollback x encryption would fail because
systemd-cryptsetupwould be unable to decrypt the encrypted volumes at boot, as the pcrlock JSON in the rollback OS would not correspond to the actual TPM 2.0 access policy. This would happen because the (A or B) (in the servicing OS) and (B or A) (in the rollback OS) pcrlock JSONs are not equivalent. This PR solves this problem by modifying logic related to how.pcrlockfiles are generated. In particular, we now always sort the lists of UKI and bootloader binaries to: AZLA, then AZLB. This means that whensystemd-pcrlockparses the directory searching for.pcrlockfiles, it will include first AZLA, and then AZLB into the pcrlock policy. As a result, the servicing OS and the rollback OS will now have an identical pcrlock JSON (A or B), so that the encrypted volumes can be decrypted when the host is booting into the rollback OS.Logic for manual rollback testing as a storm scenario was taken from a commit by @bfjelds .