feat: add implementation for `notation inspect` by byronchien · Pull Request #528 · notaryproject/notation

byronchien · 2023-02-02T01:23:53Z

Adds support for notation inspect (spec here)

Example output:

chienb@a07817b52895 notation % ./bin/notation inspect $IMAGE
Resolved artifact tag `v1` to digest `sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b` before inspect.
Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.
Inspecting all signatures for signed artifact
localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b
└── application/vnd.cncf.notary.signature
    └── sha256:34e5843a1a8b1607d2ba7da9b61c6a5b3953f5680751c160fb944df87b01b2b2
        ├── signature algorithm : RSASSA-PSS-SHA-256
        ├── signed attributes
        │   ├── expiry : 0001-01-01 00:00:00 +0000 UTC
        │   ├── signingScheme : notary.x509
        │   └── signingTime : 2023-01-27 17:02:22 -0800 PST
        ├── user defined attributes
        │   └── io.wabbit-networks.buildId : 123
        ├── unsigned attributes
        │   └── signingAgent : Notation/1.0.0
        ├── certificates
        │   └── SHA1 fingerprint e1ef7b0f984d1f8222d6bf297e1ad10047997b54
        │       ├── issued to : CN=byron.test,O=Notary,L=Seattle,ST=WA,C=US
        │       ├── issued by : CN=byron.test,O=Notary,L=Seattle,ST=WA,C=US
        │       └── expiry : 2023-01-29 01:02:13 +0000 UTC
        └── signed artifact
            ├── media type : application/vnd.docker.distribution.manifest.v2+json
            ├── digest : sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b
            └── size : 942
chienb@a07817b52895 notation % ./bin/notation inspect $IMAGE --output json
{
    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
    "Signatures": [
        {
            "digest": "sha256:34e5843a1a8b1607d2ba7da9b61c6a5b3953f5680751c160fb944df87b01b2b2",
            "signatureAlgorithm": "RSASSA-PSS-SHA-256",
            "signedAttributes": {
                "expiry": "0001-01-01 00:00:00 +0000 UTC",
                "signingScheme": "notary.x509",
                "signingTime": "2023-01-27 17:02:22 -0800 PST"
            },
            "userDefinedAttributes": {
                "io.wabbit-networks.buildId": "123"
            },
            "unsignedAttributes": {
                "signingAgent": "Notation/1.0.0"
            },
            "certificates": [
                {
                    "SHA1Fingerprint": "e1ef7b0f984d1f8222d6bf297e1ad10047997b54",
                    "issuedTo": "CN=byron.test,O=Notary,L=Seattle,ST=WA,C=US",
                    "issuedBy": "CN=byron.test,O=Notary,L=Seattle,ST=WA,C=US",
                    "expiry": "2023-01-29 01:02:13 +0000 UTC"
                }
            ],
            "signedArtifact": {
                "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
                "digest": "sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b",
                "size": 942
            }
        }
    ]
}

NOTE: this PR is on top of the changes for user-metadata verifictation and json output from there PRs:

suggested order for review: notation-go #261 => notation-core-go #111 => notation #527 =>notation #528 (this one)

Signed-off-by: Byron Chien chienb@amazon.com

byronchien · 2023-02-02T01:31:50Z

order to review:
#507
#527
#528

byronchien · 2023-02-02T01:32:26Z

depends on
notaryproject/notation-go#261
notaryproject/notation-core-go#111

cmd/notation/inspect.go

internal/envelope/envelope.go

codecov-commenter · 2023-02-09T08:29:10Z

Codecov Report

Merging #528 (00896e4) into main (54b42cb) will decrease coverage by 1.98%.
The diff coverage is 21.16%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #528      +/-   ##
==========================================
- Coverage   36.34%   34.36%   -1.98%     
==========================================
  Files          30       32       +2     
  Lines        1607     1848     +241     
==========================================
+ Hits          584      635      +51     
- Misses       1002     1192     +190     
  Partials       21       21

Impacted Files	Coverage Δ
cmd/notation/main.go	`0.00% <0.00%> (ø)`
internal/envelope/envelope.go	`25.92% <0.00%> (-74.08%)`	⬇️
cmd/notation/inspect.go	`14.21% <14.21%> (ø)`
internal/tree/tree.go	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

priteshbandi · 2023-02-10T00:32:29Z

cmd/notation/inspect.go

+const (
+	treeItemPrefix     = "├──"
+	treeItemPrefixLast = "└──"
+	subTreePrefix      = "│  "
+	subTreePrefixLast  = "   "
+)


Can we use some external library to create this tree structure? If not, can we create one?

opened issue to track this #545

+1 on having a new package for tree printing.

It should be done in this PR instead of refactoring in a later PR for code quality considerations.

Otherwise, the notation inspect code is not maintainable, especially for new contributors.

If you don't consider printing tree while loading signatures pages, you may consider https://github.com/need-being/go-tree as it is MIT.

IMO adding not a well known library such as need-being/go-tree is risky and since the amount of code we are taking about is less than 100 lines I would prefer that we we write our own.

priteshbandi · 2023-02-10T00:36:43Z

test/e2e/suite/scenario/quickstart.go

 	It("Verify the container image with jws format", func() {
 		notation.Exec("verify", artifact.ReferenceWithDigest()).
-			MatchContent(fmt.Sprintf("Successfully verified signature for %s\n", artifact.ReferenceWithDigest()))
+			MatchKeyWords(fmt.Sprintf("Successfully verified signature for %s\n", artifact.ReferenceWithDigest()))


Why are we changing this?

Same question here.

when running the E2E tests, sometimes the tests with user-metadata would run before this one, causing the output to mismatch since the signature with user-metadata would be verified first and cause the output to not match exactly. Changing it so that the test just checks that the verification passes successfully regardless of whether tags are present or not.

cmd/notation/inspect.go

internal/envelope/envelope.go

cmd/notation/inspect.go

internal/envelope/envelope.go

priteshbandi

LGTM

shizhMSFT · 2023-02-10T04:24:05Z

test/e2e/suite/scenario/quickstart.go

 	It("Verify the container image with jws format", func() {
 		notation.Exec("verify", artifact.ReferenceWithDigest()).
-			MatchContent(fmt.Sprintf("Successfully verified signature for %s\n", artifact.ReferenceWithDigest()))
+			MatchKeyWords(fmt.Sprintf("Successfully verified signature for %s\n", artifact.ReferenceWithDigest()))


Same question here.

shizhMSFT · 2023-02-10T04:30:32Z

cmd/notation/inspect.go

+const (
+	treeItemPrefix     = "├──"
+	treeItemPrefixLast = "└──"
+	subTreePrefix      = "│  "
+	subTreePrefixLast  = "   "
+)


+1 on having a new package for tree printing.

shizhMSFT · 2023-02-10T04:32:04Z

cmd/notation/inspect.go

+const (
+	treeItemPrefix     = "├──"
+	treeItemPrefixLast = "└──"
+	subTreePrefix      = "│  "
+	subTreePrefixLast  = "   "
+)


It should be done in this PR instead of refactoring in a later PR for code quality considerations.

Otherwise, the notation inspect code is not maintainable, especially for new contributors.

cmd/notation/inspect.go

shizhMSFT · 2023-02-10T05:25:04Z

cmd/notation/inspect.go

+const (
+	treeItemPrefix     = "├──"
+	treeItemPrefixLast = "└──"
+	subTreePrefix      = "│  "
+	subTreePrefixLast  = "   "
+)


If you don't consider printing tree while loading signatures pages, you may consider https://github.com/need-being/go-tree as it is MIT.

cmd/notation/inspect.go

shizhMSFT · 2023-02-10T11:57:00Z

cmd/notation/inspect.go

+const (
+	NotarySignatureMediaType = "application/vnd.cncf.notary.signature"
+)


You can directly reference registry.ArtifactTypeNotation from notation-go.

cmd/notation/inspect.go

Needs re-review

priteshbandi · 2023-02-10T18:10:48Z

go.mod


 require (
 	github.com/docker/docker-credential-helpers v0.7.0
+	github.com/need-being/go-tree v0.1.0


Adding not a well known library such as need-being/go-tree is risky and since the amount of code we are taking about is less than 100 lines I would prefer that we we write our own.

Since it is MIT, we can borrow their code.

internal/ioutil/tree.go

shizhMSFT · 2023-02-13T15:17:28Z

go.mod


 require (
 	github.com/docker/docker-credential-helpers v0.7.0
+	github.com/need-being/go-tree v0.1.0


Since it is MIT, we can borrow their code.

internal/ioutil/tree.go

shizhMSFT

LGTM with suggestions

cmd/notation/inspect.go

priteshbandi

LGTM

patrickzheng200

LGTM with all comments addressed.

shizhMSFT

LGTM

priteshbandi

LGTM

Signed-off-by: Byron Chien <byronc@ucla.edu>

shizhMSFT

LGTM

patrickzheng200

LGTM

patrickzheng200

LGTM

shizhMSFT

LGTM

priteshbandi

LGTM

Adds support for `notation inspect` ([spechere](https://github.com/notaryproject/notation/blob/main/specs/commandline/inspect.md)) Example output: ``` chienb@a07817b52895 notation % ./bin/notation inspect $IMAGE localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b └── application/vnd.cncf.notary.signature └── sha256:34e5843a1a8b1607d2ba7da9b61c6a5b3953f5680751c160fb944df87b01b2b2 ├── signature algorithm : RSASSA-PSS-SHA-256 ├── signed attributes │ ├── expiry : 0001-01-01 00:00:00 +0000 UTC │ ├── signingScheme : notary.x509 │ └── signingTime : 2023-01-27 17:02:22 -0800 PST ├── user defined attributes │ └── io.wabbit-networks.buildId : 123 ├── unsigned attributes │ └── signingAgent : Notation/1.0.0 ├── certificates │ └── SHA1 fingerprint e1ef7b0f984d1f8222d6bf297e1ad10047997b54 │ ├── issued to : CN=byron.test,O=Notary,L=Seattle,ST=WA,C=US │ ├── issued by : CN=byron.test,O=Notary,L=Seattle,ST=WA,C=US │ └── expiry : 2023-01-29 01:02:13 +0000 UTC └── signed artifact ├── media type : application/vnd.docker.distribution.manifest.v2+json ├── digest : sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b └── size : 942 ``` Signed-off-by: Byron Chien <chienb@amazon.com> Signed-off-by: Josh Duffney <jduffney@microsoft.com>

Adds support for `notation inspect` ([spechere](https://github.com/notaryproject/notation/blob/main/specs/commandline/inspect.md)) Example output: ``` chienb@a07817b52895 notation % ./bin/notation inspect $IMAGE localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b └── application/vnd.cncf.notary.signature └── sha256:34e5843a1a8b1607d2ba7da9b61c6a5b3953f5680751c160fb944df87b01b2b2 ├── signature algorithm : RSASSA-PSS-SHA-256 ├── signed attributes │ ├── expiry : 0001-01-01 00:00:00 +0000 UTC │ ├── signingScheme : notary.x509 │ └── signingTime : 2023-01-27 17:02:22 -0800 PST ├── user defined attributes │ └── io.wabbit-networks.buildId : 123 ├── unsigned attributes │ └── signingAgent : Notation/1.0.0 ├── certificates │ └── SHA1 fingerprint e1ef7b0f984d1f8222d6bf297e1ad10047997b54 │ ├── issued to : CN=byron.test,O=Notary,L=Seattle,ST=WA,C=US │ ├── issued by : CN=byron.test,O=Notary,L=Seattle,ST=WA,C=US │ └── expiry : 2023-01-29 01:02:13 +0000 UTC └── signed artifact ├── media type : application/vnd.docker.distribution.manifest.v2+json ├── digest : sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b └── size : 942 ``` Signed-off-by: Byron Chien <chienb@amazon.com>

byronchien mentioned this pull request Feb 2, 2023

feat: add support for json output for notation verify #527

Merged

byronchien force-pushed the inspect-impl branch from d1c1baa to 784408e Compare February 2, 2023 17:31

This was referenced Feb 2, 2023

feat: add a method for parsing the descriptor from a payload notaryproject/notation-go#261

Closed

Adds a string conversion for algorithms and some formatting notaryproject/notation-core-go#111

Closed

vaninrao10 added this to the RC-2 milestone Feb 2, 2023

priteshbandi requested review from patrickzheng200, priteshbandi and shizhMSFT February 3, 2023 01:22

patrickzheng200 reviewed Feb 3, 2023

View reviewed changes

patrickzheng200 reviewed Feb 7, 2023

View reviewed changes

cmd/notation/inspect.go Outdated Show resolved Hide resolved

shizhMSFT changed the title ~~Adds implementation for notation inspect~~ feat: add implementation for notation inspect Feb 8, 2023

byronchien force-pushed the inspect-impl branch 3 times, most recently from 555dc3f to 5f4afdf Compare February 8, 2023 19:13

patrickzheng200 reviewed Feb 9, 2023

View reviewed changes

cmd/notation/inspect.go Show resolved Hide resolved

cmd/notation/inspect.go Outdated Show resolved Hide resolved

internal/envelope/envelope.go Outdated Show resolved Hide resolved

internal/envelope/envelope.go Outdated Show resolved Hide resolved

byronchien force-pushed the inspect-impl branch 3 times, most recently from 15bb78b to e675c75 Compare February 10, 2023 01:09

priteshbandi reviewed Feb 10, 2023

View reviewed changes

patrickzheng200 reviewed Feb 10, 2023

View reviewed changes

internal/envelope/envelope.go Outdated Show resolved Hide resolved

cmd/notation/inspect.go Show resolved Hide resolved

internal/envelope/envelope.go Show resolved Hide resolved

priteshbandi previously approved these changes Feb 10, 2023

View reviewed changes

shizhMSFT reviewed Feb 10, 2023

View reviewed changes

byronchien force-pushed the inspect-impl branch from 4ab2a4d to a8e2dc0 Compare February 10, 2023 07:51

patrickzheng200 reviewed Feb 10, 2023

View reviewed changes

cmd/notation/inspect.go Outdated Show resolved Hide resolved

shizhMSFT reviewed Feb 10, 2023

View reviewed changes

priteshbandi reviewed Feb 10, 2023

View reviewed changes

shizhMSFT reviewed Feb 13, 2023

View reviewed changes

shizhMSFT previously approved these changes Feb 15, 2023

View reviewed changes

cmd/notation/inspect.go Outdated Show resolved Hide resolved

priteshbandi previously approved these changes Feb 15, 2023

View reviewed changes

byronchien dismissed stale reviews from priteshbandi and shizhMSFT via 5025773 February 15, 2023 00:57

priteshbandi previously approved these changes Feb 15, 2023

View reviewed changes

patrickzheng200 previously approved these changes Feb 15, 2023

View reviewed changes

priteshbandi requested a review from shizhMSFT February 15, 2023 07:55

shizhMSFT previously approved these changes Feb 15, 2023

View reviewed changes

byronchien dismissed stale reviews from shizhMSFT, patrickzheng200, and priteshbandi via 84b6753 February 15, 2023 18:43

byronchien force-pushed the inspect-impl branch from 84b6753 to d30f9bf Compare February 15, 2023 18:56

priteshbandi previously approved these changes Feb 15, 2023

View reviewed changes

Add support for notation inspect

00896e4

Signed-off-by: Byron Chien <byronc@ucla.edu>

byronchien mentioned this pull request Feb 16, 2023

feat: add support for notation inspect #556

Closed

shizhMSFT previously approved these changes Feb 16, 2023

View reviewed changes

patrickzheng200 previously approved these changes Feb 16, 2023

View reviewed changes

byronchien dismissed stale reviews from patrickzheng200, shizhMSFT, and priteshbandi via 00896e4 February 16, 2023 01:08

byronchien force-pushed the inspect-impl branch from d30f9bf to 00896e4 Compare February 16, 2023 01:08

patrickzheng200 approved these changes Feb 16, 2023

View reviewed changes

shizhMSFT approved these changes Feb 16, 2023

View reviewed changes

priteshbandi approved these changes Feb 16, 2023

View reviewed changes

priteshbandi merged commit d62fd58 into notaryproject:main Feb 16, 2023

yizha1 linked an issue Feb 17, 2023 that may be closed by this pull request

Implementing inspect command #219

Closed

Conversation

byronchien commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

byronchien commented Feb 2, 2023

Uh oh!

byronchien commented Feb 2, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

priteshbandi Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

priteshbandi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shizhMSFT Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

byronchien commented Feb 2, 2023 •

edited

Loading

codecov-commenter commented Feb 9, 2023 •

edited

Loading

priteshbandi Feb 10, 2023 •

edited

Loading

shizhMSFT Feb 10, 2023 •

edited

Loading