o/snapstate: respect validation sets when refreshing and installing components with snaps #14783
Conversation
github-actions
bot
added
the
Needs Documentation -auto-
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14783 +/- ##
==========================================
- Coverage 78.95% 78.20% -0.75%
==========================================
Files 1084 1148 +64
Lines 146638 151351 +4713
==========================================
+ Hits 115773 118359 +2586
- Misses 23667 25659 +1992
- Partials 7198 7333 +135
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…onents with snaps
andrewphelpsj
force-pushed
the
component-vsets-on-refresh-install
branch
from
905260a
to
bc97387
Compare
github-actions
bot
removed
the
Needs Documentation -auto-
overlord/snapstate/target.go
Outdated
| return fmt.Errorf( | ||
| "cannot %s snap %q due to enforcing rules of validation set %s", | ||
| verb, | ||
| instanceName, | ||
| constraints.Sets.CommaSeparated(), | ||
| ) |
There was a problem hiding this comment.
Sorry to be nitpicking on style but to me this more verbose and actually less readable than just splitting it in two lines
| return fmt.Errorf( | |
| "cannot %s snap %q due to enforcing rules of validation set %s", | |
| verb, | |
| instanceName, | |
| constraints.Sets.CommaSeparated(), | |
| ) | |
| return fmt.Errorf("cannot %s snap %q due to enforcing rules of validation set %s", | |
| verb, instanceName, constraints.Sets.CommaSeparated()) |
IMO it makes it easier to match the format specifiers to the variables
| constraints snapasserts.SnapPresenceConstraints, | ||
| action string, | ||
| ) error { | ||
| if constraints.Presence == asserts.PresenceInvalid { |
There was a problem hiding this comment.
I think we're missing a test for this case
There was a problem hiding this comment.
Fixed, thanks. There was an identical check right before calling this function, preventing the test that should have hit it from going in there.
overlord/snapstate/target.go
Outdated
| // should something like this maybe be a method on snapasserts.ValidationSets? | ||
| // the error strings are the main thing that don't really fit well in there |
There was a problem hiding this comment.
It seems fine as a standalone function, a method doesn't feel particularly more natural than this
This change makes it so that we respect validation sets when installing/refreshing components alongside snaps.
Note that the store doesn't yet fully support introspecting the validation sets that are sent during a refresh, so they cannot yet change which component revisions are actually returned with the actions. We double check the actions that are returned from the store to ensure that we always follow the correct constraints.
Based on #14769 for now.