diff --git a/changelogs/unreleased/add-git-suffix-when-cloning-from-url.yml b/changelogs/unreleased/add-git-suffix-when-cloning-from-url.yml new file mode 100644 index 0000000000000000000000000000000000000000..b36ada7ffa2a4f93a1390bbc4d0ea18dd209aebe --- /dev/null +++ b/changelogs/unreleased/add-git-suffix-when-cloning-from-url.yml @@ -0,0 +1,5 @@ +--- +title: Try adding .git suffix to the URL when cloning from remote fails +merge_request: 1467 +author: +type: changed diff --git a/internal/service/repository/create_from_url.go b/internal/service/repository/create_from_url.go index 7bfa59a2f520f198cd0978abbb6024ca7e41e4a4..4b149275db2a9eb6969c2decc8cd26f6e675fdd4 100644 --- a/internal/service/repository/create_from_url.go +++ b/internal/service/repository/create_from_url.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "strings" "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -28,22 +29,14 @@ func (s *server) CreateRepositoryFromURL(ctx context.Context, req *gitalypb.Crea return nil, status.Errorf(codes.InvalidArgument, "CreateRepositoryFromURL: dest dir exists") } - args := []string{ - "-c", - "http.followRedirects=false", - "clone", - "--bare", - "--", - req.Url, - repositoryFullPath, - } - cmd, err := git.CommandWithoutRepo(ctx, args...) - if err != nil { - return nil, status.Errorf(codes.Internal, "CreateRepositoryFromURL: clone cmd start: %v", err) - } - if err := cmd.Wait(); err != nil { - os.RemoveAll(repositoryFullPath) - return nil, status.Errorf(codes.Internal, "CreateRepositoryFromURL: clone cmd wait: %v", err) + if _, err = cloneRepositoryFromURL(ctx, req.Url, repositoryFullPath); err != nil { + if !strings.HasSuffix(strings.ToLower(req.Url), ".git") { + if _, err = cloneRepositoryFromURL(ctx, req.Url+".git", repositoryFullPath); err != nil { + return cloneErrorMessage(err) + } + } else { + return cloneErrorMessage(err) + } } // CreateRepository is harmless on existing repositories with the side effect that it creates the hook symlink. @@ -69,3 +62,30 @@ func validateCreateRepositoryFromURLRequest(req *gitalypb.CreateRepositoryFromUR return nil } + +func cloneRepositoryFromURL(ctx context.Context, url string, repositoryFullPath string) (*gitalypb.CreateRepositoryFromURLResponse, error) { + args := []string{ + "-c", + "http.followRedirects=false", + "clone", + "--bare", + "--", + url, + repositoryFullPath, + } + + cmd, err := git.CommandWithoutRepo(ctx, args...) + if err != nil { + return nil, status.Errorf(codes.Internal, "CreateRepositoryFromURL: clone cmd start: %v", err) + } + + if err := cmd.Wait(); err != nil { + os.RemoveAll(repositoryFullPath) + return nil, status.Errorf(codes.Internal, "CreateRepositoryFromURL: clone cmd wait: %v", err) + } + return &gitalypb.CreateRepositoryFromURLResponse{}, nil +} + +func cloneErrorMessage(err error) (*gitalypb.CreateRepositoryFromURLResponse, error) { + return nil, status.Errorf(codes.Internal, "CreateRepositoryFromURL: clone failed: %v", err) +} diff --git a/internal/service/repository/create_from_url_test.go b/internal/service/repository/create_from_url_test.go index b760926e6bfc366a942c4714f7c0ddbcaee9219d..ffe909a40b12e8f6ef7819125856a7b1d0a569a9 100644 --- a/internal/service/repository/create_from_url_test.go +++ b/internal/service/repository/create_from_url_test.go @@ -135,3 +135,32 @@ func TestPreventingRedirect(t *testing.T) { require.Error(t, err) } + +func TestAddingGitSuffix(t *testing.T) { + server, serverSocketPath := runRepoServer(t) + defer server.Stop() + + client, conn := newRepositoryClient(t, serverSocketPath) + defer conn.Close() + + ctx, cancel := testhelper.Context() + defer cancel() + + importedRepo := &gitalypb.Repository{ + RelativePath: "imports/test-repo-import", + StorageName: testhelper.DefaultStorageName, + } + + httpServerState, redirectingServer := StartRedirectingTestServer() + defer redirectingServer.Close() + + req := &gitalypb.CreateRepositoryFromURLRequest{ + Repository: importedRepo, + Url: redirectingServer.URL + "/repo", + } + + _, err := client.CreateRepositoryFromURL(ctx, req) + + require.Error(t, err) + require.Equal(t, httpServerState.requestedPaths, []string{"/repo/info/refs", "/repo.git/info/refs"}) +} diff --git a/internal/service/repository/redirecting_test_server_test.go b/internal/service/repository/redirecting_test_server_test.go index 8369b835d2428d44d7c2e9ca1272a9731de23772..2acc56bebb02042206c71db1e28302bab294c829 100644 --- a/internal/service/repository/redirecting_test_server_test.go +++ b/internal/service/repository/redirecting_test_server_test.go @@ -18,6 +18,7 @@ const redirectURL = "/redirect_url" type RedirectingTestServerState struct { serverVisited bool serverVisitedAfterRedirect bool + requestedPaths []string } // StartRedirectingTestServer starts the test server with initial state @@ -25,6 +26,7 @@ func StartRedirectingTestServer() (*RedirectingTestServerState, *httptest.Server state := &RedirectingTestServerState{} server := httptest.NewServer( http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + state.requestedPaths = append(state.requestedPaths, r.URL.Path) if r.URL.Path == redirectURL { state.serverVisitedAfterRedirect = true } else {