Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
asserts/tool,cmd/snap: introduce hidden "snap sign" #1573
Conversation
pedronis
added some commits
May 27, 2016
|
|
pedronis
added
the
Blocked
label
Jul 21, 2016
chenhan1218
commented
Jul 25, 2016
|
Do we have plan to sign device-serial assertion via this "snap sign" command? Or is any alternative tool for signing device-serial? |
niemeyer
added
the
Decaying
label
Aug 26, 2016
pedronis
added some commits
Aug 28, 2016
pedronis
removed
Blocked
Decaying
labels
Aug 29, 2016
mvo5
reviewed
Aug 29, 2016
| +// Sign produces the text of a signed assertion as specified by req. | ||
| +func Sign(req *SignRequest, keypairMgr asserts.KeypairManager) ([]byte, error) { | ||
| + var headers map[string]interface{} | ||
| + err := yaml.Unmarshal(req.Statement, &headers) |
mvo5
Aug 29, 2016
Collaborator
(nitpick) This could be a single line, i.e. if err := yaml.Unmarshal(...); err != nil {
mvo5
reviewed
Aug 29, 2016
| + "gadget": "brand-gadget", | ||
| + "kernel": "baz-linux", | ||
| + "store": "brand-store", | ||
| + "required-snaps": []interface{}{"foo", "bar"}, |
pedronis
Aug 29, 2016
Contributor
because that's what the parsing produces (because it could also be a list of lists), and the two are not comparable for free
mvo5
reviewed
Aug 29, 2016
| +} | ||
| + | ||
| +// Sign produces the text of a signed assertion as specified by req. | ||
| +func Sign(req *SignRequest, keypairMgr asserts.KeypairManager) ([]byte, error) { |
mvo5
Aug 29, 2016
Collaborator
Will SignRequest grow more struct members? It seems like for just two we might as well have sign(key, statement, keypairmgr). Of course if it will grow or if there are other good reasons thats totally fine, just wondering a bit :)
pedronis
Aug 29, 2016
Contributor
@mvo5 it started with a lot of members, and now is down to two otoh exactly what members has changed as well quite a bit so I'm on a fence of simplifying to just args, anyway I expect it two grow a couple more (if we want to kill some duplication in a similar tool used for service admin)
mvo5
reviewed
Aug 29, 2016
| + req := tool.SignRequest{ | ||
| + KeyID: s.testKeyID, | ||
| + | ||
| + Statement: []byte(modelYaml + `body: "BODY" |
mvo5
Aug 29, 2016
Collaborator
I wonder if it makes sense to test a multi line body here. But given that its just yaml maybe I'm overthinking it.
mvo5
reviewed
Aug 29, 2016
| + }, | ||
| + } | ||
| + | ||
| + for _, t := range tests { |
|
Looks very nice, some nitpick/question, but certainly no blockers. |
|
Thanks for your replies! |
|
discussed with @niemeyer, this should move to take only JSON |
niemeyer
reviewed
Aug 29, 2016
| +`) | ||
| + | ||
| +type cmdSign struct { | ||
| + KeyName string `long:"key-name" description:"name of the key to use, otherwise use the default key" default:"default"` |
niemeyer
reviewed
Aug 29, 2016
| +import ( | ||
| + "fmt" | ||
| + | ||
| + "gopkg.in/yaml.v2" |
niemeyer
Aug 29, 2016
Contributor
JSON per conversation today (rationale for the record: this is about inter-tooling communication rather than human, and JSON has a much simpler syntax which is harder to get wrong - better for security concerns).
niemeyer
reviewed
Aug 29, 2016
| + */ | ||
| + | ||
| +// Package tool offers tooling to sign assertions. | ||
| +package tool |
niemeyer
Aug 29, 2016
Contributor
Can we call this something like signtool instead? Will be nicer when using and when navigating the FS.
niemeyer
reviewed
Aug 29, 2016
| +) | ||
| + | ||
| +// SignRequest specifies the complete input for signing an assertion. | ||
| +type SignRequest struct { |
|
Only trivials. LGTM, and thanks! |
pedronis commentedJul 20, 2016
No description provided.