-
Notifications
You must be signed in to change notification settings - Fork 556
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
ci: add a OCI registry test for referrers support #3253
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3253 +/- ##
==========================================
- Coverage 30.59% 30.59% -0.01%
==========================================
Files 155 155
Lines 9858 9859 +1
==========================================
Hits 3016 3016
- Misses 6392 6393 +1
Partials 450 450
|
export COSIGN_PASSWORD=$pass | ||
export COSIGN_YES="true" | ||
export COSIGN_OCI_EXPERIMENTAL=1 | ||
export COSIGN_EXPERIMENTAL=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this flag does not exist anymore, you can drop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear, this env var still exists because ...
$ ./cosign sign --allow-insecure-registry --registry-referrers-mode=oci-1-1
Error: invalid argument "oci-1-1" for "--registry-referrers-mode" flag: in order to use mode "oci-1-1", you must set COSIGN_EXPERIMENTAL=1
main.go:74: error during command execution: invalid argument "oci-1-1" for "--registry-referrers-mode" flag: in order to use mode "oci-1-1", you must set COSIGN_EXPERIMENTAL=1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
humm, interesting, this should be all removed. ok, thanks!
-e REGISTRY_HTTP_TLS_CERTIFICATE=/insecure-certs/domain.crt \ | ||
-e REGISTRY_HTTP_TLS_KEY=/insecure-certs/domain.key \ | ||
-p $INSECURE_REGISTRY_PORT:$INSECURE_REGISTRY_PORT \ | ||
ghcr.io/project-zot/zot-minimal-linux-amd64:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to use latest here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can pin a version instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a ENV var to control this instead.
653b75b
to
7b0bafb
Compare
AFAICT CI issues related to this PR should be resolved. |
Looks like the CI tests have passed. |
cc: @jdolitsky also since `commit 2b3ff73
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nits
thanks!
@@ -0,0 +1,48 @@ | |||
#!/usr/bin/env bash | |||
# | |||
# Copyright 2021 The Sigstore Authors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Copyright 2021 The Sigstore Authors. | |
# Copyright 2023 The Sigstore Authors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
set -ex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make that explicit
like
set -o errexit
set -o nounset
set -o pipefail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
export COSIGN_PASSWORD=$pass | ||
export COSIGN_YES="true" | ||
export COSIGN_OCI_EXPERIMENTAL=1 | ||
export COSIGN_EXPERIMENTAL=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
humm, interesting, this should be all removed. ok, thanks!
@github-actions ^ appears to be a GH/setup issue - not related to the PR, pls re-run. |
zot is a strongly OCI conformant registry with referrers support. --- Note the new --registry-referrers-mode flag which must be set to “oci-1-1”. References: [1] https://www.chainguard.dev/unchained/building-towards-oci-v1-1-support-in-cosign [2] https://zotregistry.io Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
5b2d30a added a CLI option that was never parsed. Setting --experimental-oci11=true has no effect. Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
^ broke OCI referrers path. Also a case for a e2e test wrt this path. https://github.com/sigstore/cosign/actions/runs/6539098529/job/17756433097 cc: @cpanato @vaikas @jdolitsky Fixed now. |
Looks fine to me, I'll let @cpanato approve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
Thanks for merging this PR. If you run into issues, pls reach out at https://github.com/project-zot/zot |
zot is a strongly OCI conformant registry with referrers support.
Note the new --registry-referrers-mode flag which must be set to “oci-1-1”.
References:
https://www.chainguard.dev/unchained/building-towards-oci-v1-1-support-in-cosign
Summary
Release Note
Documentation