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

DNM: treat 403 as non-fatal when checking for manifests #2972

Closed
wants to merge 1 commit into from

Conversation

dmitris
Copy link
Contributor

@dmitris dmitris commented May 11, 2023

Update

NB - please Do Not Merge = PR on-hold, likely will close once google/go-containerregistry#1701 is merged and released.

Summary

As described in #2973, some Docker registries (ex.JFrog Artifactory) return 403 instead of 404 for a non-existent tag if that tag starts with 'sha256' - which results in a fatal error and inability to use cosign sign. This is a recent change - the breakage started after #2929 ("last known good" commit bdf1c76) and is related to or caused by upgrade of the github.com/google/go-containerregistry dependency.

Details

This change treats 403 the same way as 404 to overcome this.

It is similar and related to
google/go-containerregistry#1691 "Make 403 non-fatal for manifest existence checks".

Testing

  • nothing should change for the "normal" well-behaving registries
  • cosign sign works again with the JFrog Artifactory registry

TODO / Followup

  • change go.mod to refer to the upcoming release of github.com/google/go-containerregistry; can be done in a follow-up PR

Release Note

  • Bug fixes and fixes of previous known issues
  • Treat 403 Forbidden as non-fatal error when checking whether manifests exist

Documentation

no changes

@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #2972 (3d8cf65) into main (95ae338) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2972   +/-   ##
=======================================
  Coverage   30.26%   30.27%           
=======================================
  Files         151      151           
  Lines        9469     9470    +1     
=======================================
+ Hits         2866     2867    +1     
  Misses       6158     6158           
  Partials      445      445           
Impacted Files Coverage Δ
pkg/oci/remote/signatures.go 79.31% <100.00%> (+0.73%) ⬆️

Some Docker registries (ex.JFrog Artifactory) return
403 instead of 404 for a non-existent tag if that tag
starts with 'sha256' - which results in a fatal error
and inability to use `cosign sign`.
This change treats 403 the same way as 404 to overcome this.

It is similar and related to
google/go-containerregistry#1691
"Make 403 non-fatal for manifest existence checks".

Closes sigstore#2973.

Signed-off-by: Dmitry S <dsavints@gmail.com>
Copy link
Member

@cpanato cpanato 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
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

lgtm

@cpanato cpanato requested a review from znewman01 May 15, 2023 09:18
@dmitris dmitris changed the title treat 403 as non-fatal when checking for manifests DNM: treat 403 as non-fatal when checking for manifests May 16, 2023
@dmitris dmitris marked this pull request as draft May 16, 2023 20:57
@dmitris
Copy link
Contributor Author

dmitris commented May 17, 2023

done in #2990

@dmitris dmitris closed this May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants