Skip to content

Add output flag for notation verify - #1

Open
byronchien wants to merge 26 commits into
attestations-implfrom
metadata-output
Open

Add output flag for notation verify#1
byronchien wants to merge 26 commits into
attestations-implfrom
metadata-output

Conversation

@byronchien

Copy link
Copy Markdown
Owner

Adds output flag for notation verify to format output as json. Also updates the notation verify spec for the new flag.

Signed-off-by: Byron Chien [email protected]

Comment thread internal/ioutil/print.go Outdated
Comment thread internal/cmd/flags.go
Comment thread cmd/notation/verify.go Outdated
Comment thread internal/ioutil/print.go
Comment thread specs/commandline/verify.md Outdated

@priteshbandi priteshbandi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few nitpicks, else LGTM

Comment thread cmd/notation/verify.go Outdated
}

func printMetadataIfPresent(outcome *notation.VerificationOutcome) {
func printResult(outputFormat string, reference string, outcome *notation.VerificationOutcome) error {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func printResult(outputFormat string, reference string, outcome *notation.VerificationOutcome) error {
func printResult(outputFormat, reference string, outcome *notation.VerificationOutcome) error {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huh, didn't realize it wasn't necessary, is this a go thing?

Comment thread specs/commandline/verify.md Outdated
Comment thread specs/commandline/verify.md Outdated
"reference": "localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9",
"userMetadata": {
"io.wabbit-networks.buildId": "123"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we missing result?

Comment thread cmd/notation/verify.go Outdated
Comment on lines +88 to +90
fmt.Printf("Resolved artifact tag `%s` to digest `%s` before verification.\n", ref.Reference, manifestDesc.Digest.String())
fmt.Println("Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this should go into std.err instead of std.out. its not part of your change but can we open an issue to track this?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread cmd/notation/verify.go Outdated
Comment on lines +193 to +195
if outputFormat == ioutil.OutputJson {
output.UserMetadata = metadata
return ioutil.PrintObjectAsJson(output)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we throwing away outputFormat object if output != json ?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, can make it cleaner and only setup the object if output format is json

Comment thread cmd/notation/verify.go Outdated

func printMetadataIfPresent(outcome *notation.VerificationOutcome) {
func printResult(outputFormat, reference string, outcome *notation.VerificationOutcome) error {
output := verifyOutput{Reference: reference, Result: "Success", UserMetadata: map[string]string{}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: usually its best practice to start with failure mode and add then override data, unless you have all the data you need to call success.

Byron Chien added 3 commits February 6, 2023 15:05
Signed-off-by: Byron Chien <[email protected]>
Signed-off-by: Byron Chien <[email protected]>
@byronchien
byronchien force-pushed the attestations-impl branch 2 times, most recently from 5f040e0 to 1c3cddf Compare February 7, 2023 00:29
Byron Chien and others added 2 commits February 7, 2023 16:08
* Added error handling and unit tests.
* WIP for notaryproject#516 
* Resolves notaryproject#525 

Signed-off-by: Junjie Gao <[email protected]>
Byron Chien and others added 4 commits February 8, 2023 09:13
Signed-off-by: Byron Chien <[email protected]>
…y cmds (notaryproject#507)

Adds support for signed user metadata in `notation sign` and `notation verify`. [Relevant spec](notaryproject#498)

example sign usage:
chienb@a07817b52895 notation % notation sign $IMAGE --user-metadata io.wabbit-networks.buildId=123 --user-metadata io.wabbit-networks.buildTime=123
Successfully signed localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b
---------------
example verification:
chienb@a07817b52895 notation % notation verify $IMAGE --user-metadata io.wabbit-networks.buildTime=123
Resolved artifact tag `v1` to digest `sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b` before verification.
Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.
Successfully verified signature for localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b

The artifact was signed with the following user metadata.
KEY                            VALUE
io.wabbit-networks.buildTime   123
io.wabbit-networks.buildId     123
-----

Signed-off-by: Byron Chien <[email protected]>
priteshbandi and others added 6 commits February 8, 2023 10:52
Don't access value of default pointer if it is nil. This is actually a bug(unable to delete key if defualt key is not present) fix.

Signed-off-by: Pritesh Bandi <[email protected]>
- fail fast on unknown output format
- print warnings to stderr for both output formats
- omit empty metadata from json response

Signed-off-by: Byron Chien <[email protected]>
- rename PrintObjectAsJson => PrintObjectAsJSON
- move output format constants to flags.go
- use switch for verify output behavior
- add documentation output methods
- call out failure behavior in spec

Signed-off-by: Byron Chien <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants