Skip to content

fix(cas): return 404 error if artifact does not exist - #227

Merged
migmartri merged 3 commits into
chainloop-dev:mainfrom
migmartri:fix-cas
Jul 10, 2023
Merged

fix(cas): return 404 error if artifact does not exist#227
migmartri merged 3 commits into
chainloop-dev:mainfrom
migmartri:fix-cas

Conversation

@migmartri

Copy link
Copy Markdown
Member

Handles the fact that an image doesn't exist in the CAS instead of returning a generic error and notifying the exception manager.

This patch also includes some makefile refactoring to check that some of the new tools required are in place.

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri
migmartri requested a review from danlishka July 10, 2023 10:33
res, err := backend.Describe(ctx, req.Digest)
if err != nil {
res, err := b.Describe(ctx, req.Digest)
if err != nil && backend.IsNotFound(err) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we now check if the error is not found

img, err := remote.Image(ref, remote.WithAuthFromKeychain(b.keychain))
if err != nil {
var e *transport.Error
if errors.As(err, &e) && e.StatusCode == http.StatusNotFound {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is the way we bubble up the not found error

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>

@danlishka danlishka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@migmartri
migmartri merged commit 53efa31 into chainloop-dev:main Jul 10, 2023
@migmartri
migmartri deleted the fix-cas branch July 10, 2023 11:01
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.

2 participants