bug: Check extension image path in HC; remove incompatible confext testing#387
bug: Check extension image path in HC; remove incompatible confext testing#387
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where Trident did not validate the paths of sysexts and confexts in the Host Configuration, which prevented proper merging in the target OS. The validation now ensures that extension image paths have filenames matching their extension-release names.
- Added path validation in Trident to check that extension image filenames match their extension-release names
- Updated the AB update helper to correctly update extension paths when transitioning from version 1 to version 2
- Modified E2E tests to use
systemd-sysext statusinstead ofsystemd-sysext listto verify only actively merged extensions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/trident/src/subsystems/extensions/release.rs | Adds validation to ensure extension paths end with the correct filename format ({name}.raw) matching the extension-release file |
| tools/storm/helpers/ab_update.go | Updates extension path updating logic to transition paths from version 1 to version 2 (e.g., -1.raw to -2.raw) |
| tests/e2e_tests/extensions_test.py | Changes test to use systemd-{extType} status instead of list to verify only actively merged extensions, and updates assertion logic to check extension names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ayaegashi
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /// The absolute path of the extension image in the target OS. | ||
| /// The absolute path of the extension image in the target OS. File names | ||
| /// must match the file extension of the 'extension-release' file in the |
There was a problem hiding this comment.
Sorry, what does this mean actually? Can you add examples here of a file name matching the "file extension of the extension-release file" and not matching it?
There was a problem hiding this comment.
I reworded this and the error message in release.rs, hopefully more clear but let me know if still confusing
There was a problem hiding this comment.
Yes, this is very clear now!!
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🔍 Description
This PR fixes a couple of bugs in the extensions testing flow:
systemd-sysext listoutputs all extension-image like objects on the OS, without checking if they are valid or active.systemd-sysext statusonly returns actively merged extensions, which is what we want.root-veritytest, because there is already atrident-overlaymounted at/etc, whensystemd-confextattempts to mount another overlay at/etcthe merging fails. While Trident succeeds, the enabledsystemd-confextservice is not succeeding. This PR removes testing for confexts and updates the confext documentation.