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.

Sign up for GitHub

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

Return an informative error message when using 'api/v1' for S3 GW endpoint #7828

Merged
merged 13 commits into from
Jun 6, 2024
Merged

Return an informative error message when using 'api/v1' for S3 GW endpoint #7828

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a9cd490
Return an informative error message when using 'api/v1' for S3 GW end…
itaigilo Jun 3, 2024
a33c847
Update const and comment
itaigilo Jun 3, 2024
b6f532b
Add system test
itaigilo Jun 3, 2024
d48f4fc
Fix tests
itaigilo Jun 3, 2024
a13940c
Update error message
itaigilo Jun 3, 2024
13af4df
Fix lint
itaigilo Jun 3, 2024
7a18b48
Update test
itaigilo Jun 5, 2024
d0a58f4
Fix test
itaigilo Jun 5, 2024
140164c
Clarify test
itaigilo Jun 5, 2024
0f0fdde
Fix test
itaigilo Jun 5, 2024
57a5be2
Fix test
itaigilo Jun 5, 2024
1547bcf
Refactor tests
itaigilo Jun 5, 2024
52f7251
Improve code style
itaigilo Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/gateway/errors/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const (
ErrNoSuchBucket
ErrNoSuchBucketPolicy
ErrNoSuchBucketLifecycle
ErrNoSuchBucketPossibleAPIEndpoint
ErrNoSuchKey
ErrNoSuchUpload
ErrNoSuchVersion
Expand Down Expand Up @@ -341,6 +342,11 @@ var Codes = errorCodeMap{
Description: "The bucket lifecycle configuration does not exist",
HTTPStatusCode: http.StatusNotFound,
},
ErrNoSuchBucketPossibleAPIEndpoint: {
Code: "ErrNoSuchBucketPossibleAPIEndpoint",
Description: "Using lakeFS API URI as the endpoint. Try removing the 'api/v1/' part from the endpoint.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps afford people an explanation? I'm thinking of the poor user who has a repo named "api" and manages to mitype a branch name "v1"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you squeeze more into an error message?
If so - any suggestions on how to improve this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm... good thing they don't deduct bandwidth for errors from my paycheck. I'd go with a definitive error message, followed by a suggestion:

Suggested change
Description: "Using lakeFS API URI as the endpoint. Try removing the 'api/v1/' part from the endpoint.",
Description: `Repository "api" not found; this can happen if your endpoint URL mistakenly ends in "` + apiutil.BaseURL + `".`,

(the ` is an alternative quotation mark that reduces the number of backslashes needed here.)

HTTPStatusCode: http.StatusNotFound,
},
ErrNoSuchKey: {
Code: "NoSuchKey",
Description: "The specified key does not exist.",
Expand Down
9 changes: 9 additions & 0 deletions pkg/gateway/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"
"time"

"github.com/treeverse/lakefs/pkg/api/apiutil"
"github.com/treeverse/lakefs/pkg/auth"
"github.com/treeverse/lakefs/pkg/catalog"
gatewayerrors "github.com/treeverse/lakefs/pkg/gateway/errors"
Expand Down Expand Up @@ -184,6 +185,14 @@ func EnrichWithRepositoryOrFallback(c *catalog.Catalog, authService auth.Gateway
fallbackProxy.ServeHTTP(w, req)
return
}

// users often set the gateway endpoint in the clients with /api/v1/ which is the openAPI endpoint.
// returning a more informative error in such case.
if strings.HasPrefix(req.RequestURI, apiutil.BaseURL) {
_ = o.EncodeError(w, req, err, gatewayerrors.ErrNoSuchBucketPossibleAPIEndpoint.ToAPIErr())
return
}

_ = o.EncodeError(w, req, err, gatewayerrors.ErrNoSuchBucket.ToAPIErr())
return
}
Expand Down
Loading