Merge branch 'remove-local-git-receive-pack' into 'master'

Remove local git receive-pack implementation

See merge request gitlab-org/gitlab-workhorse!326

internal/git/git-http.go

@@ -8,9 +8,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"os/exec"
 	"path/filepath"
-	"strings"
 	"sync"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
@@ -67,22 +65,6 @@ func repoPreAuthorizeHandler(myAPI *api.API, handleFunc api.HandleFunc) http.Han
 	}, "")
 }
 
-func startGitCommand(a *api.Response, stdin io.Reader, stdout io.Writer, action string, options ...string) (cmd *exec.Cmd, err error) {
-	// Prepare our Git subprocess
-	args := []string{subCommand(action), "--stateless-rpc"}
-	args = append(args, options...)
-	args = append(args, a.RepoPath)
-	cmd = gitCommandApi(a, "git", args...)
-	cmd.Stdin = stdin
-	cmd.Stdout = stdout
-
-	if err = cmd.Start(); err != nil {
-		return nil, fmt.Errorf("start %v: %v", cmd.Args, err)
-	}
-
-	return cmd, nil
-}
-
 func writePostRPCHeader(w http.ResponseWriter, action string) {
 	w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-result", action))
 	w.Header().Set("Cache-Control", "no-cache")
@@ -95,10 +77,6 @@ func getService(r *http.Request) string {
 	return filepath.Base(r.URL.Path)
 }
 
-func subCommand(rpc string) string {
-	return strings.TrimPrefix(rpc, "git-")
-}
-
 type countReadCloser struct {
 	n int64
 	io.ReadCloser

internal/git/git-http_test.go

-package git
-
-import (
-	"bytes"
-	"fmt"
-	"io"
-	"io/ioutil"
-	"net/http"
-	"net/http/httptest"
-	"os"
-	"os/exec"
-	"testing"
-
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
-)
-
-const (
-	expectedBytes = 102400
-	GL_ID         = "test-user"
-)
-
-// From https://npf.io/2015/06/testing-exec-command/
-func fakeExecCommand(command string, args ...string) *exec.Cmd {
-	cs := []string{"-test.run=TestGitCommandProcess", "--", command}
-	cs = append(cs, args...)
-	cmd := exec.Command(os.Args[0], cs...)
-	return cmd
-}
-
-func createTestPayload() []byte {
-	return bytes.Repeat([]byte{'0'}, expectedBytes)
-}
-
-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) {
-	execCommand = fakeExecCommand
-	defer func() { execCommand = exec.Command }()
-
-	testInput := createTestPayload()
-	body := bytes.NewReader([]byte(testInput))
-	url := fmt.Sprintf("/gitlab/gitlab-ce.git/?service=%s", action)
-	req, err := http.NewRequest("GET", url, body)
-
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	resp := &api.Response{GL_ID: GL_ID}
-
-	rr := httptest.NewRecorder()
-	handler(NewHttpResponseWriter(rr), req, resp)
-
-	// Check HTTP status code
-	if status := rr.Code; status != http.StatusOK {
-		t.Errorf("handler returned wrong status code: expected: %v, got %v",
-			http.StatusOK, status)
-	}
-
-	ct := fmt.Sprintf("application/x-%s-result", action)
-	headers := []struct {
-		key   string
-		value string
-	}{
-		{"Content-Type", ct},
-		{"Cache-Control", "no-cache"},
-	}
-
-	// Check HTTP headers
-	for _, h := range headers {
-		if value := rr.Header().Get(h.key); value != h.value {
-			t.Errorf("HTTP header %v does not match: expected: %v, got %v",
-				h.key, h.value, value)
-		}
-	}
-
-	if rr.Body.String() != string(testInput) {
-		t.Errorf("handler did not receive expected data: got %d, expected %d bytes",
-			len(rr.Body.String()), len(testInput))
-	}
-}
-
-func stringInSlice(a string, list []string) bool {
-	for _, b := range list {
-		if b == a {
-			return true
-		}
-	}
-	return false
-}
-
-func TestGitCommandProcess(t *testing.T) {
-	if os.Getenv("GL_ID") != GL_ID {
-		return
-	}
-
-	defer os.Exit(0)
-
-	uploadPack := stringInSlice("upload-pack", os.Args)
-
-	if uploadPack {
-		// First, send a large payload to stdout so that this executable will be blocked
-		// until the reader consumes the data
-		testInput := createTestPayload()
-		body := bytes.NewReader([]byte(testInput))
-		io.Copy(os.Stdout, body)
-
-		// Now consume all the data to unblock the sender
-		ioutil.ReadAll(os.Stdin)
-	} else {
-		io.Copy(os.Stdout, os.Stdin)
-	}
-}

internal/git/receive-pack.go

 package git
 
 import (
-	"context"
 	"fmt"
-	"io"
 	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
@@ -20,41 +18,14 @@ func handleReceivePack(w *HttpResponseWriter, r *http.Request, a *api.Response)
 	cr, cw := helper.NewWriteAfterReader(r.Body, w)
 	defer cw.Flush()
 
-	var err error
-	if a.GitalyServer.Address == "" {
-		err = handleReceivePackLocally(a, r, cr, cw, action)
-	} else {
 	gitProtocol := r.Header.Get("Git-Protocol")
 
-		err = handleReceivePackWithGitaly(r.Context(), a, cr, cw, gitProtocol)
-	}
-
-	return err
-}
-
-func handleReceivePackLocally(a *api.Response, r *http.Request, stdin io.Reader, stdout io.Writer, action string) error {
-	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 {
-		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
-}
-
-func handleReceivePackWithGitaly(ctx context.Context, a *api.Response, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) error {
 	smarthttp, err := gitaly.NewSmartHTTPClient(a.GitalyServer)
 	if err != nil {
 		return fmt.Errorf("smarthttp.ReceivePack: %v", err)
 	}
 
-	if err := smarthttp.ReceivePack(ctx, &a.Repository, a.GL_ID, a.GL_USERNAME, a.GL_REPOSITORY, a.GitConfigOptions, clientRequest, clientResponse, gitProtocol); err != nil {
+	if err := smarthttp.ReceivePack(r.Context(), &a.Repository, a.GL_ID, a.GL_USERNAME, a.GL_REPOSITORY, a.GitConfigOptions, cr, cw, gitProtocol); err != nil {
 		return fmt.Errorf("smarthttp.ReceivePack: %v", err)
 	}