fix(aggregator): ancillary signing on evolving files #2425
Merged
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
Alenar
requested review from
sfauvel,
jpraynaud,
dlachaume,
turmelclem and
Copilot
There was a problem hiding this comment.
Pull Request Overview
This PR fixes ancillary file signing during snapshotting by copying evolving files to a temporary directory and updates test assertions to use an enhanced directory structure comparison macro.
- Introduces a new module (dir_eq) and macro (assert_dir_eq!) to compare directory structures in tests.
- Updates tests across multiple modules to use the new assert_dir_eq! macro and replaces legacy temporary directory utilities with temp_dir_create!.
- Adds functionality in the snapshotter to copy files to a temporary folder ensuring file consistency during ancillary signing.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| mithril-common/src/test_utils/mod.rs | Added dir_eq module and re-export. |
| mithril-common/src/test_utils/dir_eq.rs | Introduced DirStructure and assert_dir_eq! macro implementation. |
| mithril-client/* | Replaced filesystem assertions with assert_dir_eq! in various tests. |
| mithril-aggregator/* | Updated file archiver/appender and snapshotter tests to use assert_dir_eq! and refactored ancillary snapshot handling. |
Comments suppressed due to low confidence (1)
mithril-aggregator/src/services/snapshotter/test_doubles.rs:320
- The function name 'test_fake_snasphotter' appears to contain a typo. Consider renaming it to 'test_fake_snapshotter' for clarity.
async fn test_fake_snasphotter() {
mithril-aggregator/src/services/snapshotter/compressed_archive_snapshotter.rs
Outdated
Show resolved
Hide resolved
Alenar
force-pushed
the
djo/2362/fix-ancillary-signing-on-evolving-files
branch
from
49f8e90 to
45d73ba
Compare
Alenar
force-pushed
the
djo/2362/fix-ancillary-signing-on-evolving-files
branch
from
45d73ba to
6e70001
Compare
Alenar
force-pushed
the
djo/2362/fix-ancillary-signing-on-evolving-files
branch
from
6e70001 to
c504bad
Compare
dlachaume
approved these changes
Apr 18, 2025
mithril-aggregator/src/services/snapshotter/compressed_archive_snapshotter.rs
Outdated
Show resolved
Hide resolved
mithril-aggregator/src/services/snapshotter/compressed_archive_snapshotter.rs
Outdated
Show resolved
Hide resolved
Alenar
force-pushed
the
djo/2362/fix-ancillary-signing-on-evolving-files
branch
3 times, most recently
from
13b87bd to
f872829
Compare
- use `temp_dir_create!` when applicable - remove uneeded intermediate variables
To avoid signing a manifest with files hashes that may be different from the one in the archived files since they may be evolving between the two operations.
Can be used against another directory or against a string that express the expected structure using `*` to tell the depth of each items.
* mithril-aggregator from `0.7.34` to `0.7.35` * mithril-client from `0.11.20` to `0.11.21` * mithril-common from `0.5.22` to `0.5.23`
Alenar
force-pushed
the
djo/2362/fix-ancillary-signing-on-evolving-files
branch
from
f872829 to
733e8bb
Compare
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.
Content
This PR make the aggregator copy the files that are included in an ancillary archive into a temporary folder so those files doesn't evolve while the snapshotting of ancillary is in progress, avoiding mismatch between sha of files in the ancillary manifest and the sha of files in the archive.
This PR also add a new test macro:
assert_dir_eq!that allow comparison of two directories together or, more interestingly, against a string that codify the expected dir structure, i.e:Pre-submit checklist
Issue(s)
Relates to #2362