Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define experience for notation sign command #91

Closed
Tracked by #40
gokarnm opened this issue Sep 13, 2021 · 11 comments · Fixed by #341
Closed
Tracked by #40

Define experience for notation sign command #91

gokarnm opened this issue Sep 13, 2021 · 11 comments · Fixed by #341
Assignees
Labels
spec Specifications to define the product requirements
Milestone

Comments

@gokarnm
Copy link
Contributor

gokarnm commented Sep 13, 2021

Things to consider

  • Should we use a default key or need the user to explicitly specify the key
  • Does signing automatically push signature to repo? Which repo?
  • How does one filter out private signatures from being automatically pushed to public repos?
  • Make params like --trust more intuitive.

Related to #78 #88

@iamsamirzon iamsamirzon added this to To do in Notation Project Board via automation Oct 11, 2021
@dtzar dtzar added this to the RC-1 milestone Jun 29, 2022
@dtzar
Copy link
Contributor

dtzar commented Jun 29, 2022

Spec needs to be finalized to start on this.

@dtzar dtzar added the spec Specifications to define the product requirements label Jul 11, 2022
@dtzar
Copy link
Contributor

dtzar commented Jul 11, 2022

Per some feedback, I'd like to see something implemented such as a --dry-run switch on notation sign which returns back information about what is going to be signed. This way someone can make sure they are going to be signing the right artifact.

@iamsamirzon
Copy link
Contributor

Per some feedback, I'd like to see something implemented such as a --dry-run switch on notation sign which returns back information about what is going to be signed. This way someone can make sure they are going to be signing the right artifact.

@dtzar - Extending the above, @gokarnm pointed out that we should have a similar debug capability for all CLI commands, that can be used for "diagnostic logging" to find problems and validate workflows. Do you think that is a separate user story, or should it be a requirement for all CLI command we implement.

@yizha1
Copy link
Contributor

yizha1 commented Aug 11, 2022

Here is a summary. I suggest changing the list in the issue to task-list, and check the one which is resolved.

  • Should we use a default key or need the user to explicitly specify the key

User can specify the key or use the default key without specifying any key

  • Does signing automatically push signature to repo? Which repo?

After successful signing, sigature by default is pushed to the same repo as container image

  • How does one filter out private signatures from being automatically pushed to public repos?

User can disable the auto-push using --push false, and generate signatures to specify path using --output flag, @shizhMSFT please comment

  • Make params like --trust more intuitive.

@gokarnm could you elaborate this one?

@gokarnm
Copy link
Contributor Author

gokarnm commented Sep 8, 2022

--trust may have been a parameter supported in early alpha releases, I'm not able to find a reference to it. The overall reason this issue was created was to track documenting the CLI experience (spec) for Sign (and Verify) command so that we can gather feedback from the community and finalize the CLI experience that can be then implemented.

@yizha1
Copy link
Contributor

yizha1 commented Sep 8, 2022

--trust may have been a parameter supported in early alpha releases, I'm not able to find a reference to it. The overall reason this issue was created was to track documenting the CLI experience (spec) for Sign (and Verify) command so that we can gather feedback from the community and finalize the CLI experience that can be then implemented.

I will start to update the sign command in notation_cli.md (like verifying existing options, adding examples) and then issue a PR for a review. This PR can also be used to align rc.1 sign behavior. What do you think? If we agree with this initiative, we can later do the same thing for other commands.

@gokarnm
Copy link
Contributor Author

gokarnm commented Sep 8, 2022

Yes, that sounds great. We want to go through each command that we want to stabilize in RC1 and define/review the experience.

@yizha1
Copy link
Contributor

yizha1 commented Sep 9, 2022

@gokarnm @SteveLasker @dtzar Any thoughts on the following proposal, if you all agree, I can issue a PR to update the sign CLI document to fix the following issues and add more examples as well.

  • Since TSA based timestamping will not be available in rc.1, I propose to remove the following option from notation sign command:

    -e, --expiry duration         expire duration
    -t, --timestamp string        timestamp the signed signature via the remote TSA
    
  • Currently after a successful sign, the signature is pushed to the remote registry by default, but the signature is also stored as a file in directory $HOME/cache/notation. Do we have a valid use case to store the signature locally? If not, I suggest to remove signature cache from rc.1, and removing notation cache command which is used to manage the local signatures

  • The same question on option --push-reference different remote to store signature. If there is no valid use case to push a signature to a different remote for storage, I suggest removing this option as well. We can use ORAS CLI/API to copy container images and signatures from one registry to another registry, which is more secure.

@iamsamirzon
Copy link
Contributor

as discussed in the meeting on 9/19

  • "-e" option should stay for RC-1. "-t" option can be removed for the RC-1
  • Lets remove "notation cache".. , if "cache" is removed, there is no need for "notation push/pull" commands?
  • The notation push and pull were created to push and pull signatures from and to cache. Additionally, there are no near term use cases for "Cache" in RC-1, so we can remove it as well.

@dtzar
Copy link
Contributor

dtzar commented Sep 26, 2022

Agree with Samir's three bullets. expiry wouldn't really change as it is just a user-defined timestamp.
As far as --push-reference I'd also agree to remove.

With cache - we should remove the notation cache command for RC.1. That said - my guess is that when we enable local signing post RC.1, we would want/use the actual signature local so we could use that code. I'd check with the developers on their thoughts there. If we would use that code, then I'd leave notation code there which leaves the signature itself on local storage.

@yizha1 yizha1 linked a pull request Oct 8, 2022 that will close this issue
@yizha1
Copy link
Contributor

yizha1 commented Oct 18, 2022

Closed since PR #341 merged

@yizha1 yizha1 closed this as completed Oct 18, 2022
Notation Project Board automation moved this from To do to Done Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec Specifications to define the product requirements
Development

Successfully merging a pull request may close this issue.

6 participants