Return an informative error message when using 'api/v1' for S3 GW end…

…point (#7828)

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

* Update const and comment

* Add system test

* Fix tests

* Update error message

* Fix lint

* Update test

* Fix test

* Clarify test

* Fix test

* Fix test

* Refactor tests

* Improve code style

esti/catalog_export_test.go

@@ -177,7 +177,11 @@ func testSymlinkS3Exporter(t *testing.T, ctx context.Context, repo string, tmplD
 	storageURL, err := url.Parse(symlinksPrefix)
 	require.NoError(t, err, "failed extracting bucket name")
 
-	s3Client, err := testutil.SetupTestS3Client("https://s3.amazonaws.com", testData.AWSAccessKeyID, testData.AWSSecretAccessKey)
+	key := testData.AWSAccessKeyID
+	secret := testData.AWSSecretAccessKey
+	endpoint := "https://s3.amazonaws.com"
+	forcePathStyle := viper.GetBool("force_path_style")
+	s3Client, err := testutil.SetupTestS3Client(endpoint, key, secret, forcePathStyle)
 
 	require.NoError(t, err, "failed creating s3 client")
 

esti/delete_objects_test.go

@@ -87,7 +87,8 @@ func TestDeleteObjects_Viewer(t *testing.T) {
 	key := resCreateCreds.JSON201.AccessKeyId
 	secret := resCreateCreds.JSON201.SecretAccessKey
 	s3Endpoint := viper.GetString("s3_endpoint")
-	s3Client, err := testutil.SetupTestS3Client(s3Endpoint, key, secret)
+	forcePathStyle := viper.GetBool("force_path_style")
+	s3Client, err := testutil.SetupTestS3Client(s3Endpoint, key, secret, forcePathStyle)
 	require.NoError(t, err)
 
 	// delete objects using viewer

esti/s3_gateway_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"io"
 	"math/rand"
 	"net/http"
@@ -655,3 +657,28 @@ func TestS3ReadObjectRedirect(t *testing.T) {
 		require.Contains(t, err.Error(), "307 Temporary Redirect")
 	})
 }
+
+func createS3Client(endpoint string, t *testing.T) *s3.Client {
+	accessKeyID := viper.GetString("access_key_id")
+	secretAccessKey := viper.GetString("secret_access_key")
+	s3Client, err := testutil.SetupTestS3Client(endpoint, accessKeyID, secretAccessKey, true)
+	require.NoError(t, err, "failed creating s3 client")
+	return s3Client
+}
+
+func TestPossibleAPIEndpointError(t *testing.T) {
+	ctx, _, repo := setupTest(t)
+	defer tearDownTest(repo)
+
+	t.Run("use_open_api_for_client_endpoint", func(t *testing.T) {
+		s3Client := createS3Client(endpointURL+apiutil.BaseURL, t)
+		_, listErr := s3Client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: aws.String("not-exists")})
+		require.ErrorContains(t, listErr, gtwerrors.ErrNoSuchBucketPossibleAPIEndpoint.Error())
+	})
+
+	t.Run("use_proper_client_endpoint", func(t *testing.T) {
+		s3Client := createS3Client(endpointURL, t)
+		_, listErr := s3Client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: aws.String("not-exists")})
+		require.ErrorContains(t, listErr, gtwerrors.ErrNoSuchBucket.Error())
+	})
+}

pkg/gateway/errors/errors.go

@@ -3,6 +3,8 @@ package errors
 import (
 	"encoding/xml"
 	"net/http"
+
+	"github.com/treeverse/lakefs/pkg/api/apiutil"
 )
 
 /*
@@ -80,6 +82,7 @@ const (
 	ErrNoSuchBucket
 	ErrNoSuchBucketPolicy
 	ErrNoSuchBucketLifecycle
+	ErrNoSuchBucketPossibleAPIEndpoint
 	ErrNoSuchKey
 	ErrNoSuchUpload
 	ErrNoSuchVersion
@@ -341,6 +344,11 @@ var Codes = errorCodeMap{
 		Description:    "The bucket lifecycle configuration does not exist",
 		HTTPStatusCode: http.StatusNotFound,
 	},
+	ErrNoSuchBucketPossibleAPIEndpoint: {
+		Code:           "ErrNoSuchBucketPossibleAPIEndpoint",
+		Description:    `Repository "api" not found; this can happen if your endpoint URL mistakenly ends in "` + apiutil.BaseURL + `".`,
+		HTTPStatusCode: http.StatusNotFound,
+	},
 	ErrNoSuchKey: {
 		Code:           "NoSuchKey",
 		Description:    "The specified key does not exist.",

pkg/gateway/middleware.go

@@ -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"
@@ -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
 		}

pkg/testutil/setup.go

@@ -128,14 +128,15 @@ func SetupTestingEnv(params *SetupTestingEnvParams) (logging.Logger, apigen.Clie
 	}
 
 	s3Endpoint := viper.GetString("s3_endpoint")
-	svc, err := SetupTestS3Client(s3Endpoint, key, secret)
+	forcePathStyle := viper.GetBool("force_path_style")
+	svc, err := SetupTestS3Client(s3Endpoint, key, secret, forcePathStyle)
 	if err != nil {
 		logger.WithError(err).Fatal("could not initialize S3 client")
 	}
 	return logger, client, svc, endpointURL
 }
 
-func SetupTestS3Client(endpoint, key, secret string) (*s3.Client, error) {
+func SetupTestS3Client(endpoint, key, secret string, forcePathStyle bool) (*s3.Client, error) {
 	if !strings.HasPrefix(endpoint, "http") {
 		endpoint = "http://" + endpoint
 	}
@@ -146,10 +147,9 @@ func SetupTestS3Client(endpoint, key, secret string) (*s3.Client, error) {
 	if err != nil {
 		return nil, err
 	}
-	forcePathStyleS3Client := viper.GetBool("force_path_style")
 	svc := s3.NewFromConfig(cfg, func(options *s3.Options) {
 		options.BaseEndpoint = aws.String(endpoint)
-		options.UsePathStyle = forcePathStyleS3Client
+		options.UsePathStyle = forcePathStyle
 	})
 	return svc, nil
 }