Improve errors when loading bundle locally fails #8996
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
steiza
reviewed
Apr 24, 2024
| @@ -13,7 +13,7 @@ import ( | |||
| "github.com/sigstore/sigstore-go/pkg/bundle" | |||
| ) | |||
|
|
|||
| var ErrLocalAttestations = errors.New("failed to load local attestations") | |||
| var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not supported, must be json or jsonl") | |||
There was a problem hiding this comment.
@malancas do we need to care about the file extension?
I'm wondering if GetLocalAttestations() should just always call loadBundlesFromJSONLinesFile(), regardless of the file extension. A .json file is really just a one-line .jsonl file?
There was a problem hiding this comment.
I think the call to loadBundlesFromJSONLinesFile would fail if a user provided a JSON bundle that takes up multiple lines in the file. We could proactively remove whitespace for that case if we wanted and then process it line by line.
There was a problem hiding this comment.
Ah, never mind, a multi-line JSON file makes sense.
steiza
reviewed
Apr 24, 2024
pkg/cmd/attestation/verify/verify.go
Outdated
| @@ -166,7 +166,7 @@ func runVerify(opts *Options) error { | |||
| if ok := errors.Is(err, api.ErrNoAttestations{}); ok { | |||
| return fmt.Errorf("no attestations found for subject: %s", artifact.DigestWithAlg()) | |||
| } | |||
| return fmt.Errorf("failed to fetch attestations for subject: %s", artifact.DigestWithAlg()) | |||
| return fmt.Errorf("failed to fetch attestations for subject: %s: %v", artifact.DigestWithAlg(), err) | |||
There was a problem hiding this comment.
Regardless of whether we drop the the file extension requirement or not, we should definitely keep this part!
steiza
approved these changes
Apr 25, 2024
| @@ -13,7 +13,7 @@ import ( | |||
| "github.com/sigstore/sigstore-go/pkg/bundle" | |||
| ) | |||
|
|
|||
| var ErrLocalAttestations = errors.New("failed to load local attestations") | |||
| var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not supported, must be json or jsonl") | |||
There was a problem hiding this comment.
Ah, never mind, a multi-line JSON file makes sense.
williammartin
force-pushed
the
wm/improve-verify-error-messages
branch
from
dde3d2d
to
054b306
Compare
|
@steiza it looks like the original error issue was addressed here: 63640b1#diff-84cf161d364effa25914785d22d193f33133a448d02e5f1559cd41ef6b48eff4R174-R180. This now logs a warning and then returns the error which will be printed by the app i.e.: 
|
steiza
approved these changes
Apr 26, 2024
alexcb
pushed a commit
to earthly/earthly
that referenced
this pull request
May 7, 2024
[](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://togithub.com/cli/cli) | minor | `v2.48.0` -> `v2.49.0` | --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.49.0`](https://togithub.com/cli/cli/releases/tag/v2.49.0): GitHub CLI 2.49.0 [Compare Source](https://togithub.com/cli/cli/compare/v2.48.0...v2.49.0) #### Support for GitHub Artifact Attestations `v2.49.0` release introduces the `attestation` command set for downloading and verifying attestations about artifacts built in GitHub Actions! This is part of the larger Artifact Attestations initiative. An artifact attestation is a piece of cryptographically signed metadata that is generated as part of your artifact build process. These attestations bind artifacts to the details of the workflow run that produced them, and allow you to guarantee the integrity and provenance of any artifact built in GitHub Actions. ```shell ### Verify a local artifact gh attestation verify artifact.bin -o <your org> ### Verify a local artifact against a local artifact attestation gh attestation verify artifact.bin -b ./artifact-v0.0.1-bundle.json -o <your org> ### Verify an OCI image gh attestation verify oci://ghcr.io/foo/bar:latest -o <your org> ### Download artifact attestations gh attestation download artifact.bin -o <your org> ``` To get started, check out gh help attestation. You can also use the `gh at <command>` alias for short. #### What's Changed - Improve gh run rerun docs by [@​sochotnicky](https://togithub.com/sochotnicky) in [cli/cli#8969 - build(deps): bump golang.org/x/net from 0.21.0 to 0.23.0 by [@​dependabot](https://togithub.com/dependabot) in [cli/cli#8981 - Update `sigstore-go` dependency to v0.3.0 by [@​malancas](https://togithub.com/malancas) in [cli/cli#8977 - `gh attestation tuf-root-verify` offline test fix by [@​malancas](https://togithub.com/malancas) in [cli/cli#8975 - Update `gh attestation verify` output by [@​malancas](https://togithub.com/malancas) in [cli/cli#8991 - build(deps): bump google.golang.org/grpc from 1.62.1 to 1.62.2 by [@​dependabot](https://togithub.com/dependabot) in [cli/cli#8989 - Remove `Hidden` flag from `gh attestation` command by [@​malancas](https://togithub.com/malancas) in [cli/cli#8998 - Add colon for `gh secret set` by [@​NeroBlackstone](https://togithub.com/NeroBlackstone) in [cli/cli#9004 - Improve errors when loading bundle locally fails by [@​williammartin](https://togithub.com/williammartin) in [cli/cli#8996 - Support offline mode for `gh attestation verify` by [@​steiza](https://togithub.com/steiza) in [cli/cli#8997 - Add `projectsV2` to JSON fields of `gh repo` commands by [@​babakks](https://togithub.com/babakks) in [cli/cli#9007 - Support long URLs in `gh repo clone` by [@​babakks](https://togithub.com/babakks) in [cli/cli#9008 - Fix issue with closing pager stream by [@​babakks](https://togithub.com/babakks) in [cli/cli#9020 - proof of concept for flag-level disable auth check by [@​andyfeller](https://togithub.com/andyfeller) in [cli/cli#9000 - Be more general with attestation host checks by [@​williammartin](https://togithub.com/williammartin) in [cli/cli#9019 - Add beta designation on attestation command set by [@​andyfeller](https://togithub.com/andyfeller) in [cli/cli#9022 - Tweaked gh attestation help strings to generate nicer cli manual site. by [@​phillmv](https://togithub.com/phillmv) in [cli/cli#9025 - Update cli/go-gh to v2.9.0 by [@​andyfeller](https://togithub.com/andyfeller) in [cli/cli#9023 - Document repo clone protocol behaviour by [@​williammartin](https://togithub.com/williammartin) in [cli/cli#9030 #### New Contributors - [@​sochotnicky](https://togithub.com/sochotnicky) made their first contribution in [cli/cli#8969 - [@​NeroBlackstone](https://togithub.com/NeroBlackstone) made their first contribution in [cli/cli#9004 - [@​phillmv](https://togithub.com/phillmv) made their first contribution in [cli/cli#9025 **Full Changelog**: cli/cli@v2.48.0...v2.49.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 4pm on monday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/earthly/earthly). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNDAuMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4zNDAuMTAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbInJlbm92YXRlIl19--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
](https://renovatebot.com){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is the difference between:
and
or