a/snapasserts, o/assertstate: implement validate-component task handler #13964
Conversation
andrewphelpsj
requested review from
pedronis,
alfonsosanchezbeato and
MiguelPires
andrewphelpsj
force-pushed
the
validate-component
branch
2 times, most recently
from
17449b0
to
472604e
Compare
overlord/assertstate/assertmgr.go
Outdated
| return err | ||
| } | ||
|
|
||
| // TODO: do we need to do something like snapasserts.CheckProvenanceWithVerifiedRevision for components? |
There was a problem hiding this comment.
This seems like its already included in the checks above, but maybe I'm misunderstanding
There was a problem hiding this comment.
@pedronis can you speak to the necessity of this check and the one in https://github.com/snapcore/snapd/pull/13964/files#r1604780776? They're modeled after the ones from doValidateSnap
There was a problem hiding this comment.
this is about checking provenance value from inside the blob, we need to check with snapcraft whether it's being stored atm or not, it might not yet be but it should though
There was a problem hiding this comment.
Per @mr-cal, the snapcraft side of this isn't implemented at the moment
There was a problem hiding this comment.
Snapcraft takes the provenance keyword from a snapcraft.yaml and writes it into meta/snap.yaml.
If I understand this correctly, snapcraft should always write the same provenance keyword in meta/component.yaml file?
There was a problem hiding this comment.
Yes, I believe that is correct.
There was a problem hiding this comment.
From discussions with @pedronis, we're gonna leave the TODO for now and come back to this when the snapcraft side is implemented
andrewphelpsj
force-pushed
the
validate-component
branch
from
0d5a13a
to
2c08d8a
Compare
overlord/assertstate/assertmgr.go
Outdated
| return err | ||
| } | ||
|
|
||
| // TODO: do we need to do something like snapasserts.CheckProvenanceWithVerifiedRevision for components? |
There was a problem hiding this comment.
this is about checking provenance value from inside the blob, we need to check with snapcraft whether it's being stored atm or not, it might not yet be but it should though
| provInf := "" | ||
| if provenance != "" { | ||
| provInf = fmt.Sprintf(" provenance: %s", provenance) | ||
| } | ||
| return fmt.Errorf("internal error: cannot find pre-populated snap-resource-revision assertion for %q: %s%s", name, hash, provInf) |
There was a problem hiding this comment.
this bit of code doesn't seem reached by tests
There was a problem hiding this comment.
Added in 3ad8649
| digest := makeDigest(12) | ||
| const size = uint64(1024) | ||
| const resourceName = "test-component" | ||
| const snapID = "snap-id-1" |
There was a problem hiding this comment.
This isn't being used below. Also, as a minor preference, not having these externalised unless it's necessary makes it easier to follow the test IMO
andrewphelpsj
force-pushed
the
validate-component
branch
from
3ad8649
to
3591bce
Compare
This PR adds a task handler for performing the validity checks on a component from the store. This is mostly fairly consistent with how
validate-snapworks, with the addition of checking that there is asnap-resource-pairthat exists for the component and snap revision.