Respect the ShowAllRefs flag for upload-pack operations

gitaly_test.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/git"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/gitaly"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/testhelper"
 
@@ -60,17 +61,29 @@ func TestGetInfoRefsProxiedToGitalySuccessfully(t *testing.T) {
 	apiResponse := gitOkBody(t)
 	apiResponse.GitalyServer.Address = gitalyAddress
 
-	ts := testAuthServer(nil, 200, apiResponse)
-	defer ts.Close()
+	for _, showAllRefs := range []bool{true, false} {
+		t.Run(fmt.Sprintf("ShowAllRefs=%v", showAllRefs), func(t *testing.T) {
+			apiResponse.ShowAllRefs = showAllRefs
 
-	ws := startWorkhorseServer(ts.URL)
-	defer ws.Close()
+			ts := testAuthServer(nil, 200, apiResponse)
+			defer ts.Close()
 
-	resource := "/gitlab-org/gitlab-test.git/info/refs?service=git-upload-pack"
-	_, body := httpGet(t, ws.URL+resource)
+			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)
+
+			expectedContent := "\n\000" + string(testhelper.GitalyInfoRefsResponseMock) + "\000"
+			if showAllRefs {
+				expectedContent = git.GitConfigShowAllRefs + expectedContent
+			}
+
+			assert.Equal(t, expectedContent, body, "GET %q: response body", resource)
+
+		})
+	}
 
-	expectedContent := string(testhelper.GitalyInfoRefsResponseMock)
-	assert.Equal(t, expectedContent, body, "GET %q: response body", resource)
 }
 
 func TestGetInfoRefsProxiedToGitalyInterruptedStream(t *testing.T) {
@@ -182,11 +195,20 @@ func TestPostReceivePackProxiedToGitalyInterrupted(t *testing.T) {
 }
 
 func TestPostUploadPackProxiedToGitalySuccessfully(t *testing.T) {
-	apiResponse := gitOkBody(t)
-
-	for _, code := range []codes.Code{codes.OK, codes.Unavailable} {
-		func() {
-			gitalyServer, socketPath := startGitalyServer(t, code)
+	for i, tc := range []struct {
+		showAllRefs bool
+		code        codes.Code
+	}{
+		{true, codes.OK},
+		{true, codes.Unavailable},
+		{false, codes.OK},
+		{false, codes.Unavailable},
+	} {
+		t.Run(fmt.Sprintf("Case %d", i), func(t *testing.T) {
+			apiResponse := gitOkBody(t)
+			apiResponse.ShowAllRefs = tc.showAllRefs
+
+			gitalyServer, socketPath := startGitalyServer(t, tc.code)
 			defer gitalyServer.Stop()
 
 			apiResponse.GitalyServer.Address = "unix://" + socketPath
@@ -204,16 +226,23 @@ func TestPostUploadPackProxiedToGitalySuccessfully(t *testing.T) {
 				testhelper.GitalyUploadPackResponseMock,
 			)
 
-			expectedBody := strings.Join([]string{
+			expectedBodyParts := []string{
 				apiResponse.Repository.StorageName,
 				apiResponse.Repository.RelativePath,
-				string(testhelper.GitalyUploadPackResponseMock),
-			}, "\000")
+			}
+			if tc.showAllRefs {
+				expectedBodyParts = append(expectedBodyParts, git.GitConfigShowAllRefs+"\n")
+			} else {
+				expectedBodyParts = append(expectedBodyParts, "\n")
+			}
+
+			expectedBodyParts = append(expectedBodyParts, string(testhelper.GitalyUploadPackResponseMock))
+			expectedBody := strings.Join(expectedBodyParts, "\000")
 
 			assert.Equal(t, 200, resp.StatusCode, "POST %q", resource)
 			assert.Equal(t, expectedBody, body, "POST %q: response body", resource)
 			testhelper.AssertResponseHeader(t, resp, "Content-Type", "application/x-git-upload-pack-result")
-		}()
+		})
 	}
 }
 

internal/api/api.go

@@ -114,6 +114,8 @@ type Response struct {
 	// Repository object for making gRPC requests to Gitaly. This will
 	// eventually replace the RepoPath field.
 	Repository pb.Repository
+	// For git-http, does the requestor have the right to view all refs?
+	ShowAllRefs bool
 }
 
 // singleJoiningSlash is taken from reverseproxy.go:NewSingleHostReverseProxy

internal/git/git-http.go

@@ -20,6 +20,12 @@ import (
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
 )
 
+const (
+	// We have to use a negative transfer.hideRefs since this is the only way
+	// to undo an already set parameter: https://www.spinics.net/lists/git/msg256772.html
+	GitConfigShowAllRefs = "transfer.hideRefs=!refs"
+)
+
 func ReceivePack(a *api.API) http.Handler {
 	return postRPCHandler(a, "handleReceivePack", handleReceivePack)
 }
@@ -28,6 +34,16 @@ func UploadPack(a *api.API) http.Handler {
 	return postRPCHandler(a, "handleUploadPack", handleUploadPack)
 }
 
+func gitConfigOptions(a *api.Response) []string {
+	var out []string
+
+	if a.ShowAllRefs {
+		out = append(out, GitConfigShowAllRefs)
+	}
+
+	return out
+}
+
 func postRPCHandler(a *api.API, name string, handler func(*GitHttpResponseWriter, *http.Request, *api.Response) error) http.Handler {
 	return repoPreAuthorizeHandler(a, func(rw http.ResponseWriter, r *http.Request, ar *api.Response) {
 		cr := &countReadCloser{ReadCloser: r.Body}

internal/git/info-refs.go

@@ -77,7 +77,7 @@ func handleGetInfoRefsWithGitaly(ctx context.Context, w http.ResponseWriter, a *
 		return fmt.Errorf("GetInfoRefsHandler: %v", err)
 	}
 
-	infoRefsResponseReader, err := smarthttp.InfoRefsResponseReader(ctx, &a.Repository, rpc)
+	infoRefsResponseReader, err := smarthttp.InfoRefsResponseReader(ctx, &a.Repository, rpc, gitConfigOptions(a))
 	if err != nil {
 		return fmt.Errorf("GetInfoRefsHandler: %v", err)
 	}

internal/git/upload-pack.go

@@ -65,7 +65,7 @@ func handleUploadPackWithGitaly(ctx context.Context, a *api.Response, clientRequ
 		return fmt.Errorf("smarthttp.UploadPack: %v", err)
 	}
 
-	if err := smarthttp.UploadPack(ctx, &a.Repository, clientRequest, clientResponse); err != nil {
+	if err := smarthttp.UploadPack(ctx, &a.Repository, clientRequest, clientResponse, gitConfigOptions(a)); err != nil {
 		return fmt.Errorf("smarthttp.UploadPack: %v", err)
 	}
 

internal/gitaly/smarthttp.go

@@ -13,8 +13,8 @@ type SmartHTTPClient struct {
 	pb.SmartHTTPServiceClient
 }
 
-func (client *SmartHTTPClient) InfoRefsResponseReader(ctx context.Context, repo *pb.Repository, rpc string) (io.Reader, error) {
-	rpcRequest := &pb.InfoRefsRequest{Repository: repo}
+func (client *SmartHTTPClient) InfoRefsResponseReader(ctx context.Context, repo *pb.Repository, rpc string, gitConfigOptions []string) (io.Reader, error) {
+	rpcRequest := &pb.InfoRefsRequest{Repository: repo, GitConfigOptions: gitConfigOptions}
 
 	switch rpc {
 	case "git-upload-pack":
@@ -86,14 +86,15 @@ 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) error {
+func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *pb.Repository, clientRequest io.Reader, clientResponse io.Writer, gitConfigOptions []string) error {
 	stream, err := client.PostUploadPack(ctx)
 	if err != nil {
 		return err
 	}
 
 	rpcRequest := &pb.PostUploadPackRequest{
-		Repository: repo,
+		Repository:       repo,
+		GitConfigOptions: gitConfigOptions,
 	}
 
 	if err := stream.Send(rpcRequest); err != nil {

internal/testhelper/gitaly.go

@@ -53,7 +53,14 @@ func (s *GitalyTestServer) InfoRefsUploadPack(in *pb.InfoRefsRequest, stream pb.
 		return err
 	}
 
-	nSends, err := sendBytes([]byte(GitalyInfoRefsResponseMock), 100, func(p []byte) error {
+	fmt.Printf("Result: %+v", in)
+
+	data := []byte(strings.Join([]string{
+		strings.Join(in.GitConfigOptions, "\n") + "\n",
+		GitalyInfoRefsResponseMock,
+	}, "\000") + "\000")
+
+	nSends, err := sendBytes(data, 100, func(p []byte) error {
 		return stream.Send(&pb.InfoRefsResponse{Data: p})
 	})
 	if err != nil {
@@ -147,6 +154,7 @@ func (s *GitalyTestServer) PostUploadPack(stream pb.SmartHTTPService_PostUploadP
 	data := []byte(strings.Join([]string{
 		repo.GetStorageName(),
 		repo.GetRelativePath(),
+		strings.Join(req.GitConfigOptions, "\n") + "\n",
 	}, "\000") + "\000")
 
 	// The body of the request starts in the second message
@@ -307,6 +315,10 @@ func (s *GitalyTestServer) Exists(context.Context, *pb.RepositoryExistsRequest)
 	return nil, nil
 }
 
+func (s *GitalyTestServer) HasLocalBranches(ctx context.Context, in *pb.HasLocalBranchesRequest) (*pb.HasLocalBranchesResponse, error) {
+	return nil, nil
+}
+
 func (s *GitalyTestServer) CommitDelta(in *pb.CommitDeltaRequest, stream pb.DiffService_CommitDeltaServer) error {
 	return nil
 }
@@ -319,6 +331,10 @@ func (s *GitalyTestServer) CommitPatch(in *pb.CommitPatchRequest, stream pb.Diff
 	return nil
 }
 
+func (s *GitalyTestServer) GetBlobs(in *pb.GetBlobsRequest, stream pb.BlobService_GetBlobsServer) error {
+	return nil
+}
+
 // sendBytes returns the number of times the 'sender' function was called and an error.
 func sendBytes(data []byte, chunkSize int, sender func([]byte) error) (int, error) {
 	i := 0