Fix pkg/cosign/errors

[jonjohnsonjr]

1. Remove ThrowError

This function is the identity function, so it doesn't appear to actually
do anything? Tests pass after deleting it.

2. Unwrap

Errors should implement Unwrap when they wrap another error so that
errors.Is/As will work with it.

3. Don't break policy-controller

This adds back some public symbols that were removed that prevent
policy-controller from bumping its cosign dependency. They are marked
deprecated.

https://github.com/sigstore/policy-controller/blob/b59a2bf7e4f95e2f9126ce8b4d9dd5bcfbd11c9f/pkg/webhook/validator.go#L518
https://github.com/sigstore/policy-controller/blob/b59a2bf7e4f95e2f9126ce8b4d9dd5bcfbd11c9f/pkg/webhook/validator.go#L836
https://github.com/sigstore/policy-controller/blob/b59a2bf7e4f95e2f9126ce8b4d9dd5bcfbd11c9f/pkg/webhook/validator.go#L912

[codecov]

Codecov Report

Merging #3050 (5416f97) into main (1660de7) will decrease coverage by 0.14%.
The diff coverage is 33.84%.

@@            Coverage Diff             @@
##             main    #3050      +/-   ##
==========================================
- Coverage   31.08%   30.94%   -0.14%     
==========================================
  Files         155      155              
  Lines        9739     9747       +8     
==========================================
- Hits         3027     3016      -11     
- Misses       6250     6269      +19     
  Partials      462      462              

Impacted Files	Coverage Δ
pkg/cosign/errors.go	6.66% <0.00%> (-21.91%)	⬇️
pkg/policy/errors.go	50.00% <0.00%> (-50.00%)	⬇️
pkg/cosign/verify.go	36.80% <43.90%> (-0.42%)	⬇️
pkg/policy/eval.go	78.37% <100.00%> (ø)

[jonjohnsonjr]

@ChrisJBurns I think I'm missing some context for some of this stuff, do these changes seem ~reasonable to you?

[ChrisJBurns]

Hi @jonjohnsonjr , changes seem good with me! 👍

The ThrowError was mainly to have a common way of throwing errors with a bit of syntactic sugar, there was no functional benefit for it being there. The unwrapping functions seem good to me also. On the Policy Controller side of things, I didn't actually realise that we were using functions in Cosign to throw errors in other applications, so for backwards compatibility that seems ok with me to put it back in. Although I'd definitely raise the question of whether we should be using Cosign as a library for throwing errors in other applications? As we seem to be coupling the two a bit? Would it be easier to copy the VerificationFailure error code we're putting back in this PR and just put it in policy controller? It doesn't really seem to be anything Cosign specific? Feel free to correct me as I've not got any experience with the policy controller thus far!

[jonjohnsonjr]

Although I'd definitely raise the question of whether we should be using Cosign as a library for throwing errors in other applications? As we seem to be coupling the two a bit? Would it be easier to copy the VerificationFailure error code we're putting back in this PR and just put it in policy controller? It doesn't really seem to be anything Cosign specific? Feel free to correct me as I've not got any experience with the policy controller thus far!

I think that would have been better to do originally, but for the sake of the dependency graph, I think it's easier to just add these back (as deprecated) and then have policy-controller deal with that problem when it bumps cosign.

[ChrisJBurns]

@jonjohnsonjr Yep that's fair enough. Out of curiosity, do you know where else Cosign is used like it is in the policy controller? Just so I can bare it in mind when doing future changes? As from here onwards I'll be adding a bunch of exit codes for when certain errors happen (and encourage others to do the same - perfect for good first issues)

[jonjohnsonjr]

Out of curiosity, do you know where else Cosign is used like it is in the policy controller?

I only happened upon this because I tried to bump a cosign dependency and it broke a policy-controller dep :P

I think a lot of these are forks, but if you wanted to dig into it, I'd start here: https://pkg.go.dev/github.com/sigstore/cosign/pkg/cosign?tab=importedby

[cpanato]

lgtm, thanks sorry for taking to long to review

[dmitris]

👍