Populate Git Protocol

gitaly_test.go

@@ -65,9 +65,19 @@ func TestGetInfoRefsProxiedToGitalySuccessfully(t *testing.T) {
 	apiResponse := gitOkBody(t)
 	apiResponse.GitalyServer.Address = gitalyAddress
 
-	for _, showAllRefs := range []bool{true, false} {
-		t.Run(fmt.Sprintf("ShowAllRefs=%v", showAllRefs), func(t *testing.T) {
-			apiResponse.ShowAllRefs = showAllRefs
+	testCases := []struct {
+		showAllRefs bool
+		gitRpc      string
+	}{
+		{showAllRefs: false, gitRpc: "git-upload-pack"},
+		{showAllRefs: true, gitRpc: "git-upload-pack"},
+		{showAllRefs: false, gitRpc: "git-receive-pack"},
+		{showAllRefs: true, gitRpc: "git-receive-pack"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("ShowAllRefs=%v,gitRpc=%v", tc.showAllRefs, tc.gitRpc), func(t *testing.T) {
+			apiResponse.ShowAllRefs = tc.showAllRefs
 
 			ts := testAuthServer(nil, 200, apiResponse)
 			defer ts.Close()
@@ -75,19 +85,18 @@ func TestGetInfoRefsProxiedToGitalySuccessfully(t *testing.T) {
 			ws := startWorkhorseServer(ts.URL)
 			defer ws.Close()
 
-			resource := "/gitlab-org/gitlab-test.git/info/refs?service=git-upload-pack"
-			_, body := httpGet(t, ws.URL+resource)
+			gitProtocol := "fake git protocol"
+			resource := "/gitlab-org/gitlab-test.git/info/refs?service=" + tc.gitRpc
+			_, body := httpGet(t, ws.URL+resource, map[string]string{"Git-Protocol": gitProtocol})
 
-			expectedContent := "\n\000" + string(testhelper.GitalyInfoRefsResponseMock) + "\000"
-			if showAllRefs {
+			expectedContent := fmt.Sprintf("\n\000%s\000%s\000%s\000", gitProtocol, tc.gitRpc, testhelper.GitalyInfoRefsResponseMock)
+			if tc.showAllRefs {
 				expectedContent = git.GitConfigShowAllRefs + expectedContent
 			}
 
-			assert.Equal(t, expectedContent, body, "GET %q: response body", resource)
-
+			require.Equal(t, expectedContent, body, "GET %q: response body", resource)
 		})
 	}
-
 }
 
 func TestGetInfoRefsProxiedToGitalyInterruptedStream(t *testing.T) {
@@ -138,11 +147,15 @@ func TestPostReceivePackProxiedToGitalySuccessfully(t *testing.T) {
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
 
+	gitProtocol := "fake Git protocol"
 	resource := "/gitlab-org/gitlab-test.git/git-receive-pack"
 	resp, body := httpPost(
 		t,
 		ws.URL+resource,
-		"application/x-git-receive-pack-request",
+		map[string]string{
+			"Content-Type": "application/x-git-receive-pack-request",
+			"Git-Protocol": "fake Git protocol",
+		},
 		testhelper.GitalyReceivePackResponseMock,
 	)
 
@@ -151,6 +164,7 @@ func TestPostReceivePackProxiedToGitalySuccessfully(t *testing.T) {
 		apiResponse.Repository.RelativePath,
 		apiResponse.GL_ID,
 		apiResponse.GL_USERNAME,
+		gitProtocol,
 		string(testhelper.GitalyReceivePackResponseMock),
 	}, "\000")
 
@@ -222,17 +236,22 @@ func TestPostUploadPackProxiedToGitalySuccessfully(t *testing.T) {
 			ws := startWorkhorseServer(ts.URL)
 			defer ws.Close()
 
+			gitProtocol := "fake git protocol"
 			resource := "/gitlab-org/gitlab-test.git/git-upload-pack"
 			resp, body := httpPost(
 				t,
 				ws.URL+resource,
-				"application/x-git-upload-pack-request",
+				map[string]string{
+					"Content-Type": "application/x-git-upload-pack-request",
+					"Git-Protocol": gitProtocol,
+				},
 				testhelper.GitalyUploadPackResponseMock,
 			)
 
 			expectedBodyParts := []string{
 				apiResponse.Repository.StorageName,
 				apiResponse.Repository.RelativePath,
+				gitProtocol,
 			}
 			if tc.showAllRefs {
 				expectedBodyParts = append(expectedBodyParts, git.GitConfigShowAllRefs+"\n")
@@ -302,7 +321,7 @@ func TestGetInfoRefsHandledLocallyDueToEmptyGitalySocketPath(t *testing.T) {
 	defer ws.Close()
 
 	resource := "/gitlab-org/gitlab-test.git/info/refs?service=git-upload-pack"
-	resp, body := httpGet(t, ws.URL+resource)
+	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)
@@ -323,7 +342,12 @@ func TestPostReceivePackHandledLocallyDueToEmptyGitalySocketPath(t *testing.T) {
 
 	resource := "/gitlab-org/gitlab-test.git/git-receive-pack"
 	payload := []byte("This payload should not reach Gitaly")
-	resp, body := httpPost(t, ws.URL+resource, "application/x-git-receive-pack-request", payload)
+	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)
@@ -344,7 +368,12 @@ func TestPostUploadPackHandledLocallyDueToEmptyGitalySocketPath(t *testing.T) {
 
 	resource := "/gitlab-org/gitlab-test.git/git-upload-pack"
 	payload := []byte("This payload should not reach Gitaly")
-	resp, body := httpPost(t, ws.URL+resource, "application/x-git-upload-pack-request", payload)
+	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)

internal/git/info-refs.go

@@ -35,11 +35,13 @@ func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response)
 	w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-advertisement", rpc))
 	w.Header().Set("Cache-Control", "no-cache")
 
+	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)
+		err = handleGetInfoRefsWithGitaly(r.Context(), w, a, rpc, gitProtocol)
 	}
 
 	if err != nil {
@@ -71,13 +73,13 @@ func handleGetInfoRefsLocalTesting(w http.ResponseWriter, a *api.Response, rpc s
 	return nil
 }
 
-func handleGetInfoRefsWithGitaly(ctx context.Context, w http.ResponseWriter, a *api.Response, rpc string) error {
+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 {
 		return fmt.Errorf("GetInfoRefsHandler: %v", err)
 	}
 
-	infoRefsResponseReader, err := smarthttp.InfoRefsResponseReader(ctx, &a.Repository, rpc, gitConfigOptions(a))
+	infoRefsResponseReader, err := smarthttp.InfoRefsResponseReader(ctx, &a.Repository, rpc, gitConfigOptions(a), gitProtocol)
 	if err != nil {
 		return fmt.Errorf("GetInfoRefsHandler: %v", err)
 	}

internal/git/receive-pack.go

@@ -24,7 +24,9 @@ func handleReceivePack(w *GitHttpResponseWriter, r *http.Request, a *api.Respons
 	if a.GitalyServer.Address == "" {
 		err = handleReceivePackLocally(a, r, cr, cw, action)
 	} else {
-		err = handleReceivePackWithGitaly(r.Context(), a, cr, cw)
+		gitProtocol := r.Header.Get("Git-Protocol")
+
+		err = handleReceivePackWithGitaly(r.Context(), a, cr, cw, gitProtocol)
 	}
 
 	return err
@@ -46,13 +48,13 @@ func handleReceivePackLocally(a *api.Response, r *http.Request, stdin io.Reader,
 	return nil
 }
 
-func handleReceivePackWithGitaly(ctx context.Context, a *api.Response, clientRequest io.Reader, clientResponse io.Writer) error {
+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, clientRequest, clientResponse); err != nil {
+	if err := smarthttp.ReceivePack(ctx, &a.Repository, a.GL_ID, a.GL_USERNAME, a.GL_REPOSITORY, clientRequest, clientResponse, gitProtocol); err != nil {
 		return fmt.Errorf("smarthttp.ReceivePack: %v", err)
 	}
 

internal/git/upload-pack.go

@@ -32,7 +32,9 @@ func handleUploadPack(w *GitHttpResponseWriter, r *http.Request, a *api.Response
 		// This code path is no longer reachable in GitLab 10.0
 		err = handleUploadPackLocally(a, r, buffer, w, action)
 	} else {
-		err = handleUploadPackWithGitaly(r.Context(), a, buffer, w)
+		gitProtocol := r.Header.Get("Git-Protocol")
+
+		err = handleUploadPackWithGitaly(r.Context(), a, buffer, w, gitProtocol)
 	}
 
 	return err
@@ -59,13 +61,13 @@ func handleUploadPackLocally(a *api.Response, r *http.Request, stdin *os.File, s
 	return nil
 }
 
-func handleUploadPackWithGitaly(ctx context.Context, a *api.Response, clientRequest io.Reader, clientResponse io.Writer) error {
+func handleUploadPackWithGitaly(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.UploadPack: %v", err)
 	}
 
-	if err := smarthttp.UploadPack(ctx, &a.Repository, clientRequest, clientResponse, gitConfigOptions(a)); err != nil {
+	if err := smarthttp.UploadPack(ctx, &a.Repository, clientRequest, clientResponse, gitConfigOptions(a), gitProtocol); err != nil {
 		return fmt.Errorf("smarthttp.UploadPack: %v", err)
 	}
 

internal/gitaly/smarthttp.go

@@ -13,8 +13,12 @@ type SmartHTTPClient struct {
 	pb.SmartHTTPServiceClient
 }
 
-func (client *SmartHTTPClient) InfoRefsResponseReader(ctx context.Context, repo *pb.Repository, rpc string, gitConfigOptions []string) (io.Reader, error) {
-	rpcRequest := &pb.InfoRefsRequest{Repository: repo, GitConfigOptions: gitConfigOptions}
+func (client *SmartHTTPClient) InfoRefsResponseReader(ctx context.Context, repo *pb.Repository, rpc string, gitConfigOptions []string, gitProtocol string) (io.Reader, error) {
+	rpcRequest := &pb.InfoRefsRequest{
+		Repository:       repo,
+		GitConfigOptions: gitConfigOptions,
+		GitProtocol:      gitProtocol,
+	}
 
 	switch rpc {
 	case "git-upload-pack":
@@ -39,7 +43,7 @@ func infoRefsReader(stream infoRefsClient) io.Reader {
 	})
 }
 
-func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *pb.Repository, glId string, glUsername string, glRepository string, clientRequest io.Reader, clientResponse io.Writer) error {
+func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *pb.Repository, glId string, glUsername string, glRepository string, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) error {
 	stream, err := client.PostReceivePack(ctx)
 	if err != nil {
 		return err
@@ -50,6 +54,7 @@ func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *pb.Reposit
 		GlId:         glId,
 		GlUsername:   glUsername,
 		GlRepository: glRepository,
+		GitProtocol:  gitProtocol,
 	}
 
 	if err := stream.Send(rpcRequest); err != nil {
@@ -86,7 +91,7 @@ func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *pb.Reposit
 	return nil
 }
 
-func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *pb.Repository, clientRequest io.Reader, clientResponse io.Writer, gitConfigOptions []string) error {
+func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *pb.Repository, clientRequest io.Reader, clientResponse io.Writer, gitConfigOptions []string, gitProtocol string) error {
 	stream, err := client.PostUploadPack(ctx)
 	if err != nil {
 		return err
@@ -95,6 +100,7 @@ func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *pb.Reposito
 	rpcRequest := &pb.PostUploadPackRequest{
 		Repository:       repo,
 		GitConfigOptions: gitConfigOptions,
+		GitProtocol:      gitProtocol,
 	}
 
 	if err := stream.Send(rpcRequest); err != nil {

internal/testhelper/gitaly.go

@@ -57,10 +57,12 @@ func (s *GitalyTestServer) InfoRefsUploadPack(in *pb.InfoRefsRequest, stream pb.
 		return err
 	}
 
-	fmt.Printf("Result: %+v", in)
+	fmt.Printf("Result: %+v\n", in)
 
 	data := []byte(strings.Join([]string{
 		strings.Join(in.GitConfigOptions, "\n") + "\n",
+		in.GitProtocol,
+		"git-upload-pack",
 		GitalyInfoRefsResponseMock,
 	}, "\000") + "\000")
 
@@ -85,12 +87,24 @@ func (s *GitalyTestServer) InfoRefsReceivePack(in *pb.InfoRefsRequest, stream pb
 		return err
 	}
 
-	response := &pb.InfoRefsResponse{
-		Data: []byte(GitalyInfoRefsResponseMock),
-	}
-	if err := stream.Send(response); err != nil {
+	fmt.Printf("Result: %+v\n", in)
+
+	data := []byte(strings.Join([]string{
+		strings.Join(in.GitConfigOptions, "\n") + "\n",
+		in.GitProtocol,
+		"git-receive-pack",
+		GitalyInfoRefsResponseMock,
+	}, "\000") + "\000")
+
+	nSends, err := sendBytes(data, 100, func(p []byte) error {
+		return stream.Send(&pb.InfoRefsResponse{Data: p})
+	})
+	if err != nil {
 		return err
 	}
+	if nSends <= 1 {
+		panic("should have sent more than one message")
+	}
 
 	return s.finalError()
 }
@@ -114,6 +128,7 @@ func (s *GitalyTestServer) PostReceivePack(stream pb.SmartHTTPService_PostReceiv
 		repo.GetRelativePath(),
 		req.GlId,
 		req.GlUsername,
+		req.GitProtocol,
 	}, "\000") + "\000")
 
 	// The body of the request starts in the second message
@@ -158,6 +173,7 @@ func (s *GitalyTestServer) PostUploadPack(stream pb.SmartHTTPService_PostUploadP
 	data := []byte(strings.Join([]string{
 		repo.GetStorageName(),
 		repo.GetRelativePath(),
+		req.GitProtocol,
 		strings.Join(req.GitConfigOptions, "\n") + "\n",
 	}, "\000") + "\000")
 

main_test.go

@@ -167,7 +167,7 @@ func TestRegularProjectsAPI(t *testing.T) {
 		"/api/v3/projects/foo%2Fbar%2Fbaz/repository/not/special",
 		"/api/v3/projects/foo%2Fbar%2Fbaz%2Fqux/repository/not/special",
 	} {
-		resp, body := httpGet(t, ws.URL+resource)
+		resp, body := httpGet(t, ws.URL+resource, nil)
 
 		assert.Equal(t, 200, resp.StatusCode, "GET %q: status code", resource)
 		assert.Equal(t, apiResponse, body, "GET %q: response body", resource)
@@ -206,7 +206,7 @@ func TestAllowedStaticFile(t *testing.T) {
 		"/static%20file.txt",
 		"/static file.txt",
 	} {
-		resp, body := httpGet(t, ws.URL+resource)
+		resp, body := httpGet(t, ws.URL+resource, nil)
 
 		assert.Equal(t, 200, resp.StatusCode, "GET %q: status code", resource)
 		assert.Equal(t, content, body, "GET %q: response body", resource)
@@ -227,7 +227,7 @@ func TestStaticFileRelativeURL(t *testing.T) {
 	defer ws.Close()
 
 	resource := "/my-relative-url/static.txt"
-	resp, body := httpGet(t, ws.URL+resource)
+	resp, body := httpGet(t, ws.URL+resource, nil)
 
 	assert.Equal(t, 200, resp.StatusCode, "GET %q: status code", resource)
 	assert.Equal(t, content, body, "GET %q: response body", resource)
@@ -251,7 +251,7 @@ func TestAllowedPublicUploadsFile(t *testing.T) {
 		"/uploads/static%20file.txt",
 		"/uploads/static file.txt",
 	} {
-		resp, body := httpGet(t, ws.URL+resource)
+		resp, body := httpGet(t, ws.URL+resource, nil)
 
 		assert.Equal(t, 200, resp.StatusCode, "GET %q: status code", resource)
 		assert.Equal(t, content, body, "GET %q: response body", resource)
@@ -276,7 +276,7 @@ func TestDeniedPublicUploadsFile(t *testing.T) {
 		"/uploads/static.txt",
 		"/uploads%2Fstatic.txt",
 	} {
-		resp, body := httpGet(t, ws.URL+resource)
+		resp, body := httpGet(t, ws.URL+resource, nil)
 
 		assert.Equal(t, 404, resp.StatusCode, "GET %q: status code", resource)
 		assert.Equal(t, "", body, "GET %q: response body", resource)
@@ -307,7 +307,7 @@ This is a static error page for code 499
 	defer ws.Close()
 
 	resourcePath := "/error-499"
-	resp, body := httpGet(t, ws.URL+resourcePath)
+	resp, body := httpGet(t, ws.URL+resourcePath, nil)
 
 	assert.Equal(t, 499, resp.StatusCode, "GET %q: status code", resourcePath)
 	assert.Equal(t, string(errorPageBody), body, "GET %q: response body", resourcePath)
@@ -505,7 +505,7 @@ func TestApiContentTypeBlock(t *testing.T) {
 	defer ws.Close()
 
 	resourcePath := "/something"
-	resp, body := httpGet(t, ws.URL+resourcePath)
+	resp, body := httpGet(t, ws.URL+resourcePath, nil)
 
 	assert.Equal(t, 500, resp.StatusCode, "GET %q: status code", resourcePath)
 	assert.NotContains(t, wrongResponse, body, "GET %q: response body", resourcePath)
@@ -683,8 +683,15 @@ func repoPath(t *testing.T) string {
 	return path.Join(cwd, testRepoRoot, testRepo)
 }
 
-func httpGet(t *testing.T, url string) (*http.Response, string) {
-	resp, err := http.Get(url)
+func httpGet(t *testing.T, url string, headers map[string]string) (*http.Response, string) {
+	req, err := http.NewRequest("GET", url, nil)
+	require.NoError(t, err)
+
+	for k, v := range headers {
+		req.Header.Set(k, v)
+	}
+
+	resp, err := http.DefaultClient.Do(req)
 	require.NoError(t, err)
 	defer resp.Body.Close()
 
@@ -694,8 +701,15 @@ func httpGet(t *testing.T, url string) (*http.Response, string) {
 	return resp, string(b)
 }
 
-func httpPost(t *testing.T, url, contentType string, reqBody []byte) (*http.Response, string) {
-	resp, err := http.Post(url, contentType, bytes.NewReader(reqBody))
+func httpPost(t *testing.T, url string, headers map[string]string, reqBody []byte) (*http.Response, string) {
+	req, err := http.NewRequest("POST", url, bytes.NewReader(reqBody))
+	require.NoError(t, err)
+
+	for k, v := range headers {
+		req.Header.Set(k, v)
+	}
+
+	resp, err := http.DefaultClient.Do(req)
 	require.NoError(t, err)
 	defer resp.Body.Close()