Skip to content

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

Closed
dmitris wants to merge 1 commit intosigstore:mainfrom
dmitris:jfrog-403
Closed

DNM: treat 403 as non-fatal when checking for manifests#2972
dmitris wants to merge 1 commit intosigstore:mainfrom
dmitris:jfrog-403

Conversation

@dmitris
Copy link
Copy Markdown
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
Copy Markdown

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 <[email protected]>
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

3 participants