Upgrade cosign to 2.0.0 in tests #2109
Conversation
| @@ -46,6 +46,7 @@ func SignCosign(rawRef string, keyRef string) error { | |||
| cosignCmd.Env = append(cosignCmd.Env, "COSIGN_EXPERIMENTAL=true") | |||
| } | |||
|
|
|||
| cosignCmd.Args = append(cosignCmd.Args, "--yes") | |||
There was a problem hiding this comment.
Hi @ningziwen , Is it relevant to expose the prompt feature as nerdctl options ? And make it silent within integration tests .
There was a problem hiding this comment.
This is not to expose --yes, but to add --yes to make it silent by default. So it can be silent within integration tests and in the downstream software that is comsuming nerdctl.
The alternative option is to expose --cosign-yes in nerdctl. But I feel as Cosign is treated as the dependency of nerdctl feature, nerdctl can be kind of opininated to say yes to it, and make users have the expection of saying yes to this. (we can also add some documentation if needed)
There was a problem hiding this comment.
LGTM but please introduce some documentation for users regarding this default --yes
There was a problem hiding this comment.
FYI there is also a password prompt that can be disablied by exporting COSIGN_PASSWORD
source
Would you please reflect the prompt that you are targeting here in the PR comment and as I said before add some comments to the cosign doc. Thanks
There was a problem hiding this comment.
I think COSIGN_PASSWORD only disables the prompt of inputting cosign key passphrase. The prompt in this PR is only for private statement, which can't be disabled by COSIGN_PASSWORD.
PrivacyStatement = `
Note that there may be personally identifiable information associated with this signed artifact.
This may include the email address associated with the account with which you authenticate.
This information will be used for signing this artifact and will be stored in public transparency logs and cannot be removed later.`
PrivacyStatementConfirmation = " By typing 'y', you attest that you grant (or have permission to grant) and agree to have this information stored permanently in transparency logs."
--yes is also specifically for privacy statement.
There was a problem hiding this comment.
Added documentation.
There was a problem hiding this comment.
From a privacy point of view, I think it is better to make it explicit for nerdctl users imo. We should make our users opt-in. It can be extended into the nerdctl config rather than having a cmd arg everytime.
There was a problem hiding this comment.
agree regarding privacy point of view purpose, but I think the idea here is to unblock the upgrade to 2.0.0, so let's accept it as default value, and make new PR to make it explicit for nerdctl users
Signed-off-by: Ziwen Ning <[email protected]>
@fahedouch Fixed |
|
Thanks |
) #295 *Description of changes:* Add COSIGN_PASSWORD env pass-through to allow users use Cosign. The feature should be experimental as it is experimental in Nerdctl. As Finch points to Nerdctl documentation today, users could see Cosign feature is experimental in Nerdctl documentation so will not mention experimental in Finch explicitly. *Testing done:* The tests only covers signing by push and verification by pull and run with the keys as MVP. The tests won't pass until the [changed](containerd/nerdctl#2109) is integrated to Finch. Tested locally that it can work with this change in Nerdctl. - [ X ] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Ziwen Ning <[email protected]>
Upgrade cosign to 2.0.0 in tests.
Need to add --yes to skip prompt because the prompt was added in 1.9.0.