lakectl endpoint configure and endpoint path reuse (#6609)

* API endpoint use same constant and guide not to configure /api/v1

* Build url and warn on configure lakectl with path

* lakectl config validate endpoint doesn't include path

* apply code review changes

cmd/lakectl/cmd/config.go

@@ -30,15 +30,33 @@ var configCmd = &cobra.Command{
 		// get user input
 		questions := []struct {
 			Key    string
-			Prompt *promptui.Prompt
+			Prompt promptui.Prompt
 		}{
-			{Key: "credentials.access_key_id", Prompt: &promptui.Prompt{Label: "Access key ID"}},
-			{Key: "credentials.secret_access_key", Prompt: &promptui.Prompt{Label: "Secret access key", Mask: '*'}},
-			{Key: "server.endpoint_url", Prompt: &promptui.Prompt{Label: "Server endpoint URL", Validate: func(rawURL string) error {
-				_, err := url.ParseRequestURI(rawURL)
-				return err
-			}}},
+			{
+				Key: "credentials.access_key_id",
+				Prompt: promptui.Prompt{
+					Label: "Access key ID",
+				},
+			},
+			{
+				Key: "credentials.secret_access_key",
+				Prompt: promptui.Prompt{
+					Label: "Secret access key",
+					Mask:  '*',
+				},
+			},
+			{
+				Key: "server.endpoint_url",
+				Prompt: promptui.Prompt{
+					Label: "Server endpoint URL (e.g. http://localhost:8000)",
+					Validate: func(rawURL string) error {
+						_, err := url.ParseRequestURI(rawURL)
+						return err
+					},
+				},
+			},
 		}
+
 		for _, question := range questions {
 			question.Prompt.Default = viper.GetString(question.Key)
 			val, err := question.Prompt.Run()

cmd/lakectl/cmd/docs.go

@@ -53,7 +53,7 @@ lakectl config
 # Config file /home/janedoe/.lakectl.yaml will be used
 # Access key ID: AKIAIOSFODNN7EXAMPLE
 # Secret access key: ****************************************
-# Server endpoint URL: http://localhost:8000/api/v1
+# Server endpoint URL: http://localhost:8000
 ` + "```" + `
 
 This will setup a ` + "`$HOME/.lakectl.yaml`" + ` file with the credentials and API endpoint you've supplied.

cmd/lakectl/cmd/log.go

@@ -56,7 +56,7 @@ func (d *dotWriter) Write(commits []apigen.Commit) {
 			label = fmt.Sprintf("<b>%s</b>", label)
 		}
 		baseURL := strings.TrimSuffix(strings.TrimSuffix(
-			string(cfg.Server.EndpointURL), "/api/v1"), "/")
+			string(cfg.Server.EndpointURL), apiutil.BaseURL), "/")
 		_, _ = fmt.Fprintf(d.w, "\n\t\"%s\" [shape=note target=\"_blank\" href=\"%s/repositories/%s/commits/%s\" label=< %s >]\n",
 			commit.Id, baseURL, repoID, commit.Id, label)
 		for _, parent := range commit.Parents {

docs/reference/cli.md

@@ -41,7 +41,7 @@ lakectl config
 # Config file /home/janedoe/.lakectl.yaml will be used
 # Access key ID: AKIAIOSFODNN7EXAMPLE
 # Secret access key: ****************************************
-# Server endpoint URL: http://localhost:8000/api/v1
+# Server endpoint URL: http://localhost:8000
 ```
 
 This will setup a `$HOME/.lakectl.yaml` file with the credentials and API endpoint you've supplied.

esti/gc_utils_test.go

@@ -8,22 +8,24 @@ import (
 	"os/exec"
 	"strings"
 
+	"github.com/treeverse/lakefs/pkg/api/apiutil"
 	"github.com/treeverse/lakefs/pkg/logging"
 )
 
 func getSparkSubmitArgs(entryPoint string) []string {
 	return []string{
 		"--master", "spark://localhost:7077",
 		"--conf", "spark.driver.extraJavaOptions=-Divy.cache.dir=/tmp -Divy.home=/tmp",
-		"--conf", "spark.hadoop.lakefs.api.url=http://lakefs:8000/api/v1",
+		"--conf", "spark.hadoop.lakefs.api.url=http://lakefs:8000" + apiutil.BaseURL,
 		"--conf", "spark.hadoop.lakefs.api.access_key=AKIAIOSFDNN7EXAMPLEQ",
 		"--conf", "spark.hadoop.lakefs.api.secret_key=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
 		"--class", entryPoint,
 	}
 }
 
 func getDockerArgs(workingDirectory string, localJar string) []string {
-	return []string{"run", "--network", "host", "--add-host", "lakefs:127.0.0.1",
+	return []string{
+		"run", "--network", "host", "--add-host", "lakefs:127.0.0.1",
 		"-v", fmt.Sprintf("%s/ivy:/opt/bitnami/spark/.ivy2", workingDirectory),
 		"-v", fmt.Sprintf("%s:/opt/metaclient/client.jar", localJar),
 		"--rm",

esti/lakectl_doctor_test.go

@@ -6,12 +6,13 @@ import (
 
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/require"
+	"github.com/treeverse/lakefs/pkg/api/apiutil"
 )
 
 func TestLakectlDoctor(t *testing.T) {
 	accessKeyID := viper.GetString("access_key_id")
 	secretAccessKey := viper.GetString("secret_access_key")
-	endPointURL := viper.GetString("endpoint_url") + "/api/v1"
+	endPointURL := viper.GetString("endpoint_url") + apiutil.BaseURL
 	u, err := url.Parse(endpointURL)
 	require.NoError(t, err)
 	vars := map[string]string{

pkg/actions/lua.go

@@ -14,6 +14,7 @@ import (
 	lualibs "github.com/treeverse/lakefs/pkg/actions/lua"
 	"github.com/treeverse/lakefs/pkg/actions/lua/lakefs"
 	luautil "github.com/treeverse/lakefs/pkg/actions/lua/util"
+	"github.com/treeverse/lakefs/pkg/api/apiutil"
 	"github.com/treeverse/lakefs/pkg/auth"
 	"github.com/treeverse/lakefs/pkg/auth/model"
 	"github.com/treeverse/lakefs/pkg/graveler"
@@ -95,8 +96,11 @@ func (h *LuaHook) Run(ctx context.Context, record graveler.HookRecord, buf *byte
 		if h.Endpoint == nil {
 			return fmt.Errorf("no endpoint configured, cannot request object: %s: %w", h.ScriptPath, ErrInvalidAction)
 		}
-		reqURL := fmt.Sprintf("/api/v1/repositories/%s/refs/%s/objects",
-			url.PathEscape(string(record.RepositoryID)), url.PathEscape(string(record.SourceRef)))
+		reqURL, err := url.JoinPath(apiutil.BaseURL,
+			"repositories", string(record.RepositoryID), "refs", string(record.SourceRef), "objects")
+		if err != nil {
+			return err
+		}
 		req, err := http.NewRequest(http.MethodGet, reqURL, nil)
 		if err != nil {
 			return err

pkg/actions/lua/lakefs/client.go

@@ -14,6 +14,7 @@ import (
 	"github.com/Shopify/go-lua"
 	"github.com/go-chi/chi/v5"
 	"github.com/treeverse/lakefs/pkg/actions/lua/util"
+	"github.com/treeverse/lakefs/pkg/api/apiutil"
 	"github.com/treeverse/lakefs/pkg/auth"
 	"github.com/treeverse/lakefs/pkg/auth/model"
 	"github.com/treeverse/lakefs/pkg/version"
@@ -29,14 +30,15 @@ func check(l *lua.State, err error) {
 	}
 }
 
-func newLakeFSRequest(ctx context.Context, user *model.User, method, url string, data []byte) (*http.Request, error) {
-	if !strings.HasPrefix(url, "/api/") {
-		if strings.HasPrefix(url, "/") {
-			url = fmt.Sprintf("/api/v1%s", url)
-		} else {
-			url = fmt.Sprintf("/api/v1/%s", url)
+func newLakeFSRequest(ctx context.Context, user *model.User, method, reqURL string, data []byte) (*http.Request, error) {
+	if !strings.HasPrefix(reqURL, "/api/") {
+		var err error
+		reqURL, err = url.JoinPath(apiutil.BaseURL, reqURL)
+		if err != nil {
+			return nil, err
 		}
 	}
+
 	var body io.Reader
 	if data == nil {
 		body = bytes.NewReader(data)
@@ -47,16 +49,16 @@ func newLakeFSRequest(ctx context.Context, user *model.User, method, url string,
 	ctx = context.WithValue(ctx, chi.RouteCtxKey, nil)
 	// Add user to the request context
 	ctx = auth.WithUser(ctx, user)
-	req, err := http.NewRequestWithContext(ctx, method, url, body)
+	req, err := http.NewRequestWithContext(ctx, method, reqURL, body)
 	if err != nil {
 		return nil, err
 	}
 	req.Header.Set("User-Agent", LuaClientUserAgent)
 	return req, nil
 }
 
-func newLakeFSJSONRequest(ctx context.Context, user *model.User, method, url string, data []byte) (*http.Request, error) {
-	req, err := newLakeFSRequest(ctx, user, method, url, data)
+func newLakeFSJSONRequest(ctx context.Context, user *model.User, method, reqURL string, data []byte) (*http.Request, error) {
+	req, err := newLakeFSRequest(ctx, user, method, reqURL, data)
 	if err != nil {
 		return nil, err
 	}
@@ -86,9 +88,11 @@ func OpenClient(l *lua.State, ctx context.Context, user *model.User, server *htt
 				if err != nil {
 					check(l, err)
 				}
-
-				path := fmt.Sprintf("/repositories/%s/tags", url.PathEscape(repo))
-				req, err := newLakeFSJSONRequest(ctx, user, http.MethodPost, path, data)
+				reqURL, err := url.JoinPath("/repositories", repo, "tags")
+				if err != nil {
+					check(l, err)
+				}
+				req, err := newLakeFSJSONRequest(ctx, user, http.MethodPost, reqURL, data)
 				if err != nil {
 					check(l, err)
 				}
@@ -98,7 +102,10 @@ func OpenClient(l *lua.State, ctx context.Context, user *model.User, server *htt
 				repo := lua.CheckString(l, 1)
 				leftRef := lua.CheckString(l, 2)
 				rightRef := lua.CheckString(l, 3)
-				reqURL := fmt.Sprintf("/repositories/%s/refs/%s/diff/%s", url.PathEscape(repo), url.PathEscape(leftRef), url.PathEscape(rightRef))
+				reqURL, err := url.JoinPath("/repositories", repo, "refs", leftRef, "diff", rightRef)
+				if err != nil {
+					check(l, err)
+				}
 				req, err := newLakeFSJSONRequest(ctx, user, http.MethodGet, reqURL, nil)
 				if err != nil {
 					check(l, err)
@@ -123,7 +130,10 @@ func OpenClient(l *lua.State, ctx context.Context, user *model.User, server *htt
 			{Name: "list_objects", Function: func(state *lua.State) int {
 				repo := lua.CheckString(l, 1)
 				ref := lua.CheckString(l, 2)
-				reqURL := fmt.Sprintf("/repositories/%s/refs/%s/objects/ls", url.PathEscape(repo), url.PathEscape(ref))
+				reqURL, err := url.JoinPath("/repositories", repo, "refs", ref, "objects/ls")
+				if err != nil {
+					check(l, err)
+				}
 				req, err := newLakeFSJSONRequest(ctx, user, http.MethodGet, reqURL, nil)
 				if err != nil {
 					check(l, err)
@@ -155,7 +165,10 @@ func OpenClient(l *lua.State, ctx context.Context, user *model.User, server *htt
 			{Name: "get_object", Function: func(state *lua.State) int {
 				repo := lua.CheckString(l, 1)
 				ref := lua.CheckString(l, 2)
-				reqURL := fmt.Sprintf("/repositories/%s/refs/%s/objects", url.PathEscape(repo), url.PathEscape(ref))
+				reqURL, err := url.JoinPath("/repositories", repo, "refs", ref, "objects")
+				if err != nil {
+					check(l, err)
+				}
 				req, err := newLakeFSJSONRequest(ctx, user, http.MethodGet, reqURL, nil)
 				if err != nil {
 					check(l, err)
@@ -173,7 +186,10 @@ func OpenClient(l *lua.State, ctx context.Context, user *model.User, server *htt
 			{Name: "diff_branch", Function: func(state *lua.State) int {
 				repo := lua.CheckString(l, 1)
 				branch := lua.CheckString(l, 2)
-				reqURL := fmt.Sprintf("/repositories/%s/branches/%s/diff", url.PathEscape(repo), url.PathEscape(branch))
+				reqURL, err := url.JoinPath("/repositories", repo, "branches", branch, "diff")
+				if err != nil {
+					check(l, err)
+				}
 				req, err := newLakeFSJSONRequest(ctx, user, http.MethodGet, reqURL, nil)
 				if err != nil {
 					check(l, err)