From 3ab95e2036cf975b9ae4a26fda1e0393eda09ffd Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Thu, 11 Oct 2018 17:26:38 +0100
Subject: [PATCH] Remove test-only local git mode
---
gitaly_test.go | 72 -----------------------------------
internal/git/git-http.go | 5 ---
internal/git/git-http_test.go | 9 -----
internal/git/info-refs.go | 36 +-----------------
internal/git/pktline.go | 10 -----
internal/git/upload-pack.go | 33 +---------------
main_test.go | 66 --------------------------------
7 files changed, 3 insertions(+), 228 deletions(-)
diff --git a/gitaly_test.go b/gitaly_test.go
index 4c681e72278c1..714ad34ed0f5d 100644
--- a/gitaly_test.go
+++ b/gitaly_test.go
@@ -327,78 +327,6 @@ func TestPostUploadPackProxiedToGitalyInterrupted(t *testing.T) {
}
}
-func TestGetInfoRefsHandledLocallyDueToEmptyGitalySocketPath(t *testing.T) {
- gitalyServer, _ := startGitalyServer(t, codes.OK)
- defer gitalyServer.Stop()
-
- apiResponse := gitOkBody(t)
- apiResponse.GitalyServer.Address = ""
- ts := testAuthServer(nil, 200, apiResponse)
- defer ts.Close()
-
- ws := startWorkhorseServer(ts.URL)
- defer ws.Close()
-
- resource := "/gitlab-org/gitlab-test.git/info/refs?service=git-upload-pack"
- resp, body := httpGet(t, ws.URL+resource, nil)
-
- assert.Equal(t, 200, resp.StatusCode, "GET %q", resource)
- assert.NotContains(t, string(testhelper.GitalyInfoRefsResponseMock), body, "GET %q: should not have been proxied to Gitaly", resource)
- testhelper.AssertResponseHeader(t, resp, "Content-Type", "application/x-git-upload-pack-advertisement")
-}
-
-func TestPostReceivePackHandledLocallyDueToEmptyGitalySocketPath(t *testing.T) {
- gitalyServer, _ := startGitalyServer(t, codes.OK)
- defer gitalyServer.Stop()
-
- apiResponse := gitOkBody(t)
- apiResponse.GitalyServer.Address = ""
- ts := testAuthServer(nil, 200, apiResponse)
- defer ts.Close()
-
- ws := startWorkhorseServer(ts.URL)
- defer ws.Close()
-
- resource := "/gitlab-org/gitlab-test.git/git-receive-pack"
- payload := []byte("This payload should not reach Gitaly")
- resp, body := httpPost(
- t,
- ws.URL+resource,
- map[string]string{"Content-type": "application/x-git-receive-pack-request"},
- payload,
- )
-
- assert.Equal(t, 200, resp.StatusCode, "POST %q: status code", resource)
- assert.NotContains(t, payload, body, "POST %q: request should not have been proxied to Gitaly", resource)
- testhelper.AssertResponseHeader(t, resp, "Content-Type", "application/x-git-receive-pack-result")
-}
-
-func TestPostUploadPackHandledLocallyDueToEmptyGitalySocketPath(t *testing.T) {
- gitalyServer, _ := startGitalyServer(t, codes.OK)
- defer gitalyServer.Stop()
-
- apiResponse := gitOkBody(t)
- apiResponse.GitalyServer.Address = ""
- ts := testAuthServer(nil, 200, apiResponse)
- defer ts.Close()
-
- ws := startWorkhorseServer(ts.URL)
- defer ws.Close()
-
- resource := "/gitlab-org/gitlab-test.git/git-upload-pack"
- payload := []byte("This payload should not reach Gitaly")
- resp, body := httpPost(
- t,
- ws.URL+resource,
- map[string]string{"Content-type": "application/x-git-upload-pack-request"},
- payload,
- )
-
- assert.Equal(t, 200, resp.StatusCode, "POST %q: status code", resource)
- assert.NotContains(t, payload, body, "POST %q: request should not have been proxied to Gitaly", resource)
- testhelper.AssertResponseHeader(t, resp, "Content-Type", "application/x-git-upload-pack-result")
-}
-
func TestGetBlobProxiedToGitalySuccessfully(t *testing.T) {
gitalyServer, socketPath := startGitalyServer(t, codes.OK)
defer gitalyServer.Stop()
diff --git a/internal/git/git-http.go b/internal/git/git-http.go
index 5d194d4418aa5..5d9ef9fa8a667 100644
--- a/internal/git/git-http.go
+++ b/internal/git/git-http.go
@@ -95,11 +95,6 @@ func getService(r *http.Request) string {
return filepath.Base(r.URL.Path)
}
-func isExitError(err error) bool {
- _, ok := err.(*exec.ExitError)
- return ok
-}
-
func subCommand(rpc string) string {
return strings.TrimPrefix(rpc, "git-")
}
diff --git a/internal/git/git-http_test.go b/internal/git/git-http_test.go
index e324eb8667d5a..88d739a922b56 100644
--- a/internal/git/git-http_test.go
+++ b/internal/git/git-http_test.go
@@ -31,20 +31,11 @@ func createTestPayload() []byte {
return bytes.Repeat([]byte{'0'}, expectedBytes)
}
-func TestHandleUploadPack(t *testing.T) {
- testHandlePostRpc(t, "git-upload-pack", handleUploadPack)
-}
-
func TestHandleReceivePack(t *testing.T) {
testHandlePostRpc(t, "git-receive-pack", handleReceivePack)
}
func testHandlePostRpc(t *testing.T, action string, handler func(*HttpResponseWriter, *http.Request, *api.Response) error) {
- defer func(oldTesting bool) {
- Testing = oldTesting
- }(Testing)
- Testing = true
-
execCommand = fakeExecCommand
defer func() { execCommand = exec.Command }()
diff --git a/internal/git/info-refs.go b/internal/git/info-refs.go
index b3cdebf3eb83b..c6781335b188e 100644
--- a/internal/git/info-refs.go
+++ b/internal/git/info-refs.go
@@ -11,11 +11,6 @@ import (
"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
)
-var (
- // Testing is only set during workhorse testing.
- Testing = false
-)
-
func GetInfoRefsHandler(a *api.API) http.Handler {
return repoPreAuthorizeHandler(a, handleGetInfoRefs)
}
@@ -37,42 +32,13 @@ func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response)
gitProtocol := r.Header.Get("Git-Protocol")
- var err error
- if a.GitalyServer.Address == "" && Testing {
- err = handleGetInfoRefsLocalTesting(w, a, rpc)
- } else {
- err = handleGetInfoRefsWithGitaly(r.Context(), w, a, rpc, gitProtocol)
- }
+ err := handleGetInfoRefsWithGitaly(r.Context(), w, a, rpc, gitProtocol)
if err != nil {
helper.Fail500(w, r, fmt.Errorf("handleGetInfoRefs: %v", err))
}
}
-// This code is not used in production. It is left over from before
-// Gitaly. We left it here to allow local workhorse tests to keep working
-// until we are done migrating Git HTTP to Gitaly.
-func handleGetInfoRefsLocalTesting(w http.ResponseWriter, a *api.Response, rpc string) error {
- if err := pktLine(w, fmt.Sprintf("# service=%s\n", rpc)); err != nil {
- return fmt.Errorf("pktLine: %v", err)
- }
- if err := pktFlush(w); err != nil {
- return fmt.Errorf("pktFlush: %v", err)
- }
-
- cmd, err := startGitCommand(a, nil, w, rpc, "--advertise-refs")
- if err != nil {
- return fmt.Errorf("startGitCommand: %v", err)
- }
- defer helper.CleanUpProcessGroup(cmd) // Ensure brute force subprocess clean-up
-
- if err := cmd.Wait(); err != nil {
- return fmt.Errorf("wait for %v: %v", cmd.Args, err)
- }
-
- return nil
-}
-
func handleGetInfoRefsWithGitaly(ctx context.Context, w http.ResponseWriter, a *api.Response, rpc string, gitProtocol string) error {
smarthttp, err := gitaly.NewSmartHTTPClient(a.GitalyServer)
if err != nil {
diff --git a/internal/git/pktline.go b/internal/git/pktline.go
index 74be623227708..e970f60182d89 100644
--- a/internal/git/pktline.go
+++ b/internal/git/pktline.go
@@ -8,16 +8,6 @@ import (
"strconv"
)
-func pktLine(w io.Writer, s string) error {
- _, err := fmt.Fprintf(w, "%04x%s", len(s)+4, s)
- return err
-}
-
-func pktFlush(w io.Writer) error {
- _, err := fmt.Fprint(w, "0000")
- return err
-}
-
func scanDeepen(body io.Reader) bool {
scanner := bufio.NewScanner(body)
scanner.Split(pktLineSplitter)
diff --git a/internal/git/upload-pack.go b/internal/git/upload-pack.go
index 595cf5a40a78d..02e41a81fba7c 100644
--- a/internal/git/upload-pack.go
+++ b/internal/git/upload-pack.go
@@ -5,7 +5,6 @@ import (
"fmt"
"io"
"net/http"
- "os"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/gitaly"
@@ -28,37 +27,9 @@ func handleUploadPack(w *HttpResponseWriter, r *http.Request, a *api.Response) e
action := getService(r)
writePostRPCHeader(w, action)
- if Testing && a.GitalyServer.Address == "" {
- // This code path is no longer reachable in GitLab 10.0
- err = handleUploadPackLocally(a, r, buffer, w, action)
- } else {
- gitProtocol := r.Header.Get("Git-Protocol")
+ gitProtocol := r.Header.Get("Git-Protocol")
- err = handleUploadPackWithGitaly(r.Context(), a, buffer, w, gitProtocol)
- }
-
- return err
-}
-
-func handleUploadPackLocally(a *api.Response, r *http.Request, stdin *os.File, stdout io.Writer, action string) error {
- isShallowClone := scanDeepen(stdin)
- if _, err := stdin.Seek(0, 0); err != nil {
- return fmt.Errorf("seek tempfile: %v", err)
- }
-
- cmd, err := startGitCommand(a, stdin, stdout, action)
- if err != nil {
- return fmt.Errorf("startGitCommand: %v", err)
- }
- defer helper.CleanUpProcessGroup(cmd)
-
- if err := cmd.Wait(); err != nil && !(isExitError(err) && isShallowClone) {
- helper.LogError(r, fmt.Errorf("wait for %v: %v", cmd.Args, err))
- // Return nil because the response body has been written to already.
- return nil
- }
-
- return nil
+ return handleUploadPackWithGitaly(r.Context(), a, buffer, w, gitProtocol)
}
func handleUploadPackWithGitaly(ctx context.Context, a *api.Response, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) error {
diff --git a/main_test.go b/main_test.go
index 6214d87702def..35f054e9b8a6f 100644
--- a/main_test.go
+++ b/main_test.go
@@ -24,7 +24,6 @@ import (
"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/config"
- "gitlab.com/gitlab-org/gitlab-workhorse/internal/git"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/gitaly"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/testhelper"
@@ -41,8 +40,6 @@ var checkoutDir = path.Join(scratchDir, "test")
var cacheDir = path.Join(scratchDir, "cache")
func TestMain(m *testing.M) {
- git.Testing = true
-
if _, err := os.Stat(path.Join(testRepoRoot, testRepo)); os.IsNotExist(err) {
log.Fatal("cannot find test repository. Please run 'make prepare-tests'")
}
@@ -56,46 +53,6 @@ func TestMain(m *testing.M) {
os.Exit(m.Run())
}
-func TestAllowedClone(t *testing.T) {
- // Prepare clone directory
- require.NoError(t, os.RemoveAll(scratchDir))
-
- // Prepare test server and backend
- ts := testAuthServer(nil, 200, gitOkBody(t))
- defer ts.Close()
- ws := startWorkhorseServer(ts.URL)
- defer ws.Close()
-
- // Do the git clone
- cloneCmd := exec.Command("git", "clone", fmt.Sprintf("%s/%s", ws.URL, testRepo), checkoutDir)
- runOrFail(t, cloneCmd)
-
- // We may have cloned an 'empty' repository, 'git log' will fail in it
- logCmd := exec.Command("git", "log", "-1", "--oneline")
- logCmd.Dir = checkoutDir
- runOrFail(t, logCmd)
-}
-
-func TestAllowedShallowClone(t *testing.T) {
- // Prepare clone directory
- require.NoError(t, os.RemoveAll(scratchDir))
-
- // Prepare test server and backend
- ts := testAuthServer(nil, 200, gitOkBody(t))
- defer ts.Close()
- ws := startWorkhorseServer(ts.URL)
- defer ws.Close()
-
- // Shallow git clone (depth 1)
- cloneCmd := exec.Command("git", "clone", "--depth", "1", fmt.Sprintf("%s/%s", ws.URL, testRepo), checkoutDir)
- runOrFail(t, cloneCmd)
-
- // We may have cloned an 'empty' repository, 'git log' will fail in it
- logCmd := exec.Command("git", "log", "-1", "--oneline")
- logCmd.Dir = checkoutDir
- runOrFail(t, logCmd)
-}
-
func TestDeniedClone(t *testing.T) {
// Prepare clone directory
require.NoError(t, os.RemoveAll(scratchDir))
@@ -113,24 +70,7 @@ func TestDeniedClone(t *testing.T) {
assert.Error(t, err, "git clone should have failed")
}
-func TestAllowedPush(t *testing.T) {
- preparePushRepo(t)
-
- // Prepare the test server and backend
- ts := testAuthServer(nil, 200, gitOkBody(t))
- defer ts.Close()
- ws := startWorkhorseServer(ts.URL)
- defer ws.Close()
-
- // Perform the git push
- pushCmd := exec.Command("git", "push", fmt.Sprintf("%s/%s", ws.URL, testRepo), fmt.Sprintf("master:%s", newBranch()))
- pushCmd.Dir = checkoutDir
- runOrFail(t, pushCmd)
-}
-
func TestDeniedPush(t *testing.T) {
- preparePushRepo(t)
-
// Prepare the test server and backend
ts := testAuthServer(nil, 403, "Access denied")
defer ts.Close()
@@ -573,12 +513,6 @@ func prepareDownloadDir(t *testing.T) {
require.NoError(t, os.MkdirAll(scratchDir, 0755))
}
-func preparePushRepo(t *testing.T) {
- require.NoError(t, os.RemoveAll(scratchDir))
- cloneCmd := exec.Command("git", "clone", path.Join(testRepoRoot, testRepo), checkoutDir)
- runOrFail(t, cloneCmd)
-}
-
func newBranch() string {
return fmt.Sprintf("branch-%d", time.Now().UnixNano())
}
--
GitLab