Skip to content

doc: notation Inspect Command line Spec#500

Merged
priteshbandi merged 9 commits intonotaryproject:mainfrom
vaninrao10:vani-branch-2
Jan 13, 2023
Merged

doc: notation Inspect Command line Spec#500
priteshbandi merged 9 commits intonotaryproject:mainfrom
vaninrao10:vani-branch-2

Conversation

@vaninrao10
Copy link
Contributor

Use Case / Scenarios:

  • Troubleshooting
    
  • Auditing
    
  • If verification of the image digest / signature digest fails due to wrong certificate configuration, user will execute the inspect command to extract the certificate thumb print, a hash of a certificate which is a unique identifier for certificates.
    

@vaninrao10 vaninrao10 requested review from shizhMSFT and yizha1 January 5, 2023 21:29
@shizhMSFT
Copy link
Contributor

@vaninrao10 What's the difference between this PR and #490? BTW, could you revise the PR title to conform the conventional commit style?

@iamsamirzon
Copy link
Contributor

@vaninrao10 What's the difference between this PR and #490? BTW, could you revise the PR title to conform the conventional commit style?

@shizhMSFT - No change. 490 was abandoned as there was an unexpected error with committing its change. Vani just moved the same files/edits into this new PR. Could you remind me the "conventional commit style". I could perhaps make the changes for Vani, so you can approve and she can merge the PR in the morning.

@shizhMSFT
Copy link
Contributor

Could you remind me the "conventional commit style".

Basically, changing the PR title to doc: notation Inspect Command line Spec. Here are the details.

@vaninrao10 vaninrao10 changed the title Notation Inspect Command line Spec doc: notation Inspect Command line Spec Jan 6, 2023
@shizhMSFT shizhMSFT added this to the RC-2 milestone Jan 9, 2023
@shizhMSFT shizhMSFT added the documentation Improvements or additions to documentation label Jan 9, 2023
Copy link
Member

@FeynmanZhou FeynmanZhou left a comment

Choose a reason for hiding this comment

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

Only a couple of comments left above

@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2023

Codecov Report

Merging #500 (a7901f1) into main (c736231) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #500   +/-   ##
=======================================
  Coverage   29.57%   29.57%           
=======================================
  Files          26       26           
  Lines        1515     1515           
=======================================
  Hits          448      448           
  Misses       1050     1050           
  Partials       17       17           

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

Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

LGTM with following feedback

Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

LGTM

@vaninrao10
Copy link
Contributor Author

LGTM

For Shiwei's comment - With "notation inspect < repo/individual-signature-digest >" how do we know whether it is a manifest of a signature or a signed artifact? Since there is an ambiguity, will need the right customer experience approach.

  • Included a message output as follows for the signed artifact "Inspecting all signatures for signed artifact"
  • Include a message output as follows for the signature "Inspecting signature".
  • Additional "mediaType" field in JSON which will depict if it is a signed artifact or signature.
    This spec only talks about signed artifact hence signature level inspect is not defined yet.

@yizha1
Copy link
Contributor

yizha1 commented Jan 13, 2023

notation inspect individual-signature-digest

Maybe it is more intuitive to introduce a flag later for single signature

notation inspect --signature <individual-signature-digest>

@vaninrao10
Copy link
Contributor Author

notation inspect individual-signature-digest

Maybe it is more intuitive to introduce a flag later for single signature

notation inspect --signature <individual-signature-digest>

Added the comment in the new issue so we dont lose this comment.

Changes to the review - Signed-off-by: Vani Rao <vaninrao@amazon.com>

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
to the new branch - Signed-off-by: Vani Rao <vaninrao@amazon.com>

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
…azon.com>and Feynman

Update the comments from Shiwei. Signed-off-by: Vani Rao <vaninrao@amazon.com>and Feynman

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
Cert Chain added - Signed-off-by: Vani Rao <vaninrao@amazon.com>

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
Vertical lines added - Signed-off-by: Vani Rao <vaninrao@amazon.com>

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
….com>

Vertical lines format change Signed-off-by: Vani Rao <vaninrao@amazon.com>

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
…igned artifact passed in the inspect command - Signed-off-by: Vani Rao <vaninrao@amazon.com>

Resolved the ambiguity whether it is a manifest of a signature or a signed artifact passed in the inspect command - Signed-off-by: Vani Rao <vaninrao@amazon.com>

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
…@amazon.com>

Shiweis small changes w.r.t space - Signed-off-by: Vani Rao <vaninrao@amazon.com>

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
Caption change Signed-off-by: Vani Rao <vaninrao@amazon.com>

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
Copy link
Member

@FeynmanZhou FeynmanZhou left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@priteshbandi priteshbandi merged commit 1281179 into notaryproject:main Jan 13, 2023
Copy link
Contributor

@yizha1 yizha1 left a comment

Choose a reason for hiding this comment

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

LGTM, a little bit later, but I want to share my opinions. @vaninrao10 Thanks Vani, you did a great job.

priteshbandi pushed a commit to priteshbandi/notation that referenced this pull request Feb 1, 2023
Use Case / Scenarios:
- Troubleshooting
- Auditing
- If verification of the image digest / signature digest fails due towrong certificate configuration, user will execute the inspect commandto extract the certificate thumb print, a hash of a certificate which is a unique identifier for certificates.

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
7h3-3mp7y-m4n pushed a commit to 7h3-3mp7y-m4n/notation that referenced this pull request Mar 29, 2025
Use Case / Scenarios:
- Troubleshooting
- Auditing
- If verification of the image digest / signature digest fails due towrong certificate configuration, user will execute the inspect commandto extract the certificate thumb print, a hash of a certificate which is a unique identifier for certificates.

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
FeynmanZhou pushed a commit to FeynmanZhou/notation that referenced this pull request May 15, 2025
Use Case / Scenarios:
- Troubleshooting
- Auditing
- If verification of the image digest / signature digest fails due towrong certificate configuration, user will execute the inspect commandto extract the certificate thumb print, a hash of a certificate which is a unique identifier for certificates.

Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants

Comments