image,seed: fail on --validation=ignore if model has enforced validation-sets #16376
Conversation
github-actions
bot
added
the
Run only one system
There was a problem hiding this comment.
Pull request overview
This PR modifies the snap prepare-image command to enforce validation sets that have mode "enforce" even when the --validation=ignore flag is used. Previously, --validation=ignore would skip all validation set checks, but this change ensures that model-enforced validation sets are always validated regardless of this flag.
Key Changes:
- Modified
CheckValidationSetsto accept amodelEnforcedOnlyparameter that filters validation sets based on their enforcement mode - Updated the prepare-image workflow to pass
modelEnforcedOnly=truewhen--validation=ignoreis specified - Added comprehensive integration test to verify the new behavior
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
seed/seedwriter/writer.go |
Added modelEnforcedOnly parameter to validation set checking functions to filter by enforcement mode |
image/image_linux.go |
Modified to check enforced validation sets even when --validation=ignore is used |
tests/main/prepare-image-validation-sets-fails-on-ignore/task.yaml |
Integration test verifying prepare-image fails with enforced validation sets despite --validation=ignore |
tests/main/prepare-image-validation-sets-fails-on-ignore/asserts/vs-bar.json |
Test assertion defining a validation set requiring test-snapd-sh at revision 13 |
tests/main/prepare-image-validation-sets-fails-on-ignore/asserts/core-20.json |
Test model assertion with enforced validation set reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // specified validation set. Info for all seed snaps must have been derived prior | ||
| // to this call. | ||
| func (w *Writer) CheckValidationSets() error { | ||
| func (w *Writer) CheckValidationSets(modelEnforcedOnly bool) error { |
There was a problem hiding this comment.
The function signature of CheckValidationSets has been changed to accept a modelEnforcedOnly parameter, but the existing test calls in seed/seedwriter/writer_test.go (lines 4376, 4479, 4588, 4658, 5003) have not been updated to pass this new parameter. This will cause compilation errors. All existing calls should be updated to pass false as the parameter value to maintain the current test behavior.
| fakestore new-snap-revision --dir "${STORE_DIR}" "$SNAP_PATH" --snap-rev-json snap-"$SNAP_NAME"-rev.json | ||
| } | ||
| echo Download snaps needed for the the model |
There was a problem hiding this comment.
Typo: "the the model" should be "the model"
| echo Download snaps needed for the the model | |
| echo Download snaps needed for the model |
| // run validation-set checks, this is also done by store but | ||
| // we double-check for the seed. |
There was a problem hiding this comment.
The comment should be updated to clarify that when --validation=ignore is used, only model-enforced validation sets are checked, while all validation sets are checked otherwise. This is important to document the new behavior introduced by this change.
| // run validation-set checks, this is also done by store but | |
| // we double-check for the seed. | |
| // Run validation-set checks; this is also done by the store but we | |
| // double-check for the seed. When --validation=ignore is used, only | |
| // model-enforced validation sets are checked; otherwise all validation | |
| // sets are checked. |
|
|
||
| // CheckValidationSets validates all snaps that are to be seeded against any | ||
| // specified validation set. Info for all seed snaps must have been derived prior | ||
| // to this call. |
There was a problem hiding this comment.
The function documentation should be updated to describe the new modelEnforcedOnly parameter. It should explain that when true, only validation sets with enforce mode are checked, while when false, all validation sets are checked.
| // to this call. | |
| // to this call. When modelEnforcedOnly is true, only validation sets with | |
| // enforce mode are checked; when false, all validation sets are checked. |
|
Wed Dec 17 10:49:40 UTC 2025 No spread failures reported |
1 participant
No description provided.