Merge branch 'remove-local-git-blob' into 'master'

Remove local git blob handling code

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

gitaly_integration_test.go

@@ -7,13 +7,16 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"strconv"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/gitaly"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/testhelper"
 )
 
 var (
@@ -128,7 +131,7 @@ func TestAllowedPush(t *testing.T) {
 	require.NoError(t, ensureGitalyRepository(t, apiResponse))
 
 	// Prepare the test server and backend
-	ts := testAuthServer(nil, 200, realGitalyOkBody(t))
+	ts := testAuthServer(nil, 200, apiResponse)
 	defer ts.Close()
 	ws := startWorkhorseServer(ts.URL)
 	defer ws.Close()
@@ -138,3 +141,37 @@ func TestAllowedPush(t *testing.T) {
 	pushCmd.Dir = checkoutDir
 	runOrFail(t, pushCmd)
 }
+
+func TestAllowedGetGitBlob(t *testing.T) {
+	skipUnlessRealGitaly(t)
+
+	// Create the repository in the Gitaly server
+	apiResponse := realGitalyOkBody(t)
+	require.NoError(t, ensureGitalyRepository(t, apiResponse))
+
+	// the LICENSE file in the test repository
+	oid := "50b27c6518be44c42c4d87966ae2481ce895624c"
+	expectedBody := "The MIT License (MIT)"
+	bodyLen := 1075
+
+	jsonParams := fmt.Sprintf(
+		`{
+			"GitalyServer":{"Address":"%s", "Token":""},
+			"GetBlobRequest":{
+				"repository":{"storage_name":"%s", "relative_path":"%s"},
+				"oid":"%s",
+				"limit":-1
+			}
+		}`,
+		gitalyAddress, apiResponse.Repository.StorageName, apiResponse.Repository.RelativePath, oid,
+	)
+
+	resp, body, err := doSendDataRequest("/something", "git-blob", jsonParams)
+	require.NoError(t, err)
+	shortBody := string(body[:len(expectedBody)])
+
+	assert.Equal(t, 200, resp.StatusCode, "GET %q: status code", resp.Request.URL)
+	assert.Equal(t, expectedBody, shortBody, "GET %q: response body", resp.Request.URL)
+	testhelper.AssertResponseHeader(t, resp, "Content-Length", strconv.Itoa(bodyLen))
+	assertNginxResponseBuffering(t, "no", resp, "GET %q: nginx response buffering", resp.Request.URL)
+}

gitaly_test.go

@@ -11,7 +11,6 @@ import (
 	"os"
 	"os/exec"
 	"path"
-	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -327,28 +326,6 @@ func TestPostUploadPackProxiedToGitalyInterrupted(t *testing.T) {
 	}
 }
 
-func TestGetBlobProxiedToGitalySuccessfully(t *testing.T) {
-	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
-	defer gitalyServer.Stop()
-
-	gitalyAddress := "unix://" + socketPath
-	repoStorage := "default"
-	oid := "54fcc214b94e78d7a41a9a8fe6d87a5e59500e51"
-	repoRelativePath := "foo/bar.git"
-	jsonParams := fmt.Sprintf(`{"GitalyServer":{"Address":"%s","Token":""},"GetBlobRequest":{"repository":{"storage_name":"%s","relative_path":"%s"},"oid":"%s","limit":-1}}`,
-		gitalyAddress, repoStorage, repoRelativePath, oid)
-	expectedBody := testhelper.GitalyGetBlobResponseMock
-	blobLength := len(expectedBody)
-
-	resp, body, err := doSendDataRequest("/something", "git-blob", jsonParams)
-	require.NoError(t, err)
-
-	assert.Equal(t, 200, resp.StatusCode, "GET %q: status code", resp.Request.URL)
-	assert.Equal(t, expectedBody, string(body), "GET %q: response body", resp.Request.URL)
-	assert.Equal(t, blobLength, len(body), "GET %q: body size", resp.Request.URL)
-	testhelper.AssertResponseHeader(t, resp, "Content-Length", strconv.Itoa(blobLength))
-}
-
 func TestGetDiffProxiedToGitalySuccessfully(t *testing.T) {
 	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
 	defer gitalyServer.Stop()

internal/git/blob.go

@@ -2,22 +2,17 @@ package git
 
 import (
 	"fmt"
-	"io"
 	"net/http"
-	"strings"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/gitaly"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/log"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/senddata"
 )
 
 type blob struct{ senddata.Prefix }
 type blobParams struct {
-	RepoPath       string
-	BlobId         string
 	GitalyServer   gitaly.Server
 	GetBlobRequest pb.GetBlobRequest
 }
@@ -31,55 +26,14 @@ func (b *blob) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 		return
 	}
 
-	if params.GitalyServer.Address != "" {
-		handleSendBlobWithGitaly(w, r, &params)
-	} else {
-		handleSendBlobLocally(w, r, &params)
-	}
-}
-
-func handleSendBlobWithGitaly(w http.ResponseWriter, r *http.Request, params *blobParams) {
 	blobClient, err := gitaly.NewBlobClient(params.GitalyServer)
 	if err != nil {
 		helper.Fail500(w, r, fmt.Errorf("blob.GetBlob: %v", err))
+		return
 	}
 
 	if err := blobClient.SendBlob(r.Context(), w, &params.GetBlobRequest); err != nil {
 		helper.Fail500(w, r, fmt.Errorf("blob.GetBlob: %v", err))
-	}
-}
-
-func handleSendBlobLocally(w http.ResponseWriter, r *http.Request, params *blobParams) {
-	log.WithFields(r.Context(), log.Fields{
-		"blobId": params.BlobId,
-		"path":   r.URL.Path,
-	}).Print("SendBlob: sending")
-
-	sizeOutput, err := gitCommand("git", "--git-dir="+params.RepoPath, "cat-file", "-s", params.BlobId).Output()
-	if err != nil {
-		helper.Fail500(w, r, fmt.Errorf("SendBlob: get blob size: %v", err))
-		return
-	}
-
-	gitShowCmd := gitCommand("git", "--git-dir="+params.RepoPath, "cat-file", "blob", params.BlobId)
-	stdout, err := gitShowCmd.StdoutPipe()
-	if err != nil {
-		helper.Fail500(w, r, fmt.Errorf("SendBlob: git cat-file stdout: %v", err))
-		return
-	}
-	if err := gitShowCmd.Start(); err != nil {
-		helper.Fail500(w, r, fmt.Errorf("SendBlob: start %v: %v", gitShowCmd, err))
-		return
-	}
-	defer helper.CleanUpProcessGroup(gitShowCmd)
-
-	w.Header().Set("Content-Length", strings.TrimSpace(string(sizeOutput)))
-	if _, err := io.Copy(w, stdout); err != nil {
-		helper.LogError(r, &copyError{fmt.Errorf("SendBlob: copy git cat-file stdout: %v", err)})
-		return
-	}
-	if err := gitShowCmd.Wait(); err != nil {
-		helper.LogError(r, fmt.Errorf("SendBlob: wait for git cat-file: %v", err))
 		return
 	}
 }

main_test.go

@@ -13,7 +13,6 @@ import (
 	"os/exec"
 	"path"
 	"regexp"
-	"strconv"
 	"testing"
 	"time"
 
@@ -381,22 +380,6 @@ func TestSendURLForArtifacts(t *testing.T) {
 	assert.Equal(t, fileContents, string(body), "GET %q: response body", resourcePath)
 }
 
-func TestGetGitBlob(t *testing.T) {
-	blobID := "50b27c6518be44c42c4d87966ae2481ce895624c" // the LICENSE file in the test repository
-	blobLength := 1075
-	jsonParams := fmt.Sprintf(`{"RepoPath":"%s","BlobId":"%s"}`, path.Join(testRepoRoot, testRepo), blobID)
-	expectedBody := "The MIT License (MIT)"
-
-	resp, body, err := doSendDataRequest("/something", "git-blob", jsonParams)
-	require.NoError(t, err)
-
-	assert.Equal(t, 200, resp.StatusCode, "GET %q: status code", resp.Request.URL)
-	assert.Equal(t, expectedBody, string(body[:len(expectedBody)]), "GET %q: response body", resp.Request.URL)
-	assert.Equal(t, blobLength, len(body), "GET %q: body size", resp.Request.URL)
-	testhelper.AssertResponseHeader(t, resp, "Content-Length", strconv.Itoa(blobLength))
-	assertNginxResponseBuffering(t, "no", resp, "GET %q: nginx response buffering", resp.Request.URL)
-}
-
 func TestGetGitDiff(t *testing.T) {
 	fromSha := "be93687618e4b132087f430a4d8fc3a609c9b77c"
 	toSha := "54fcc214b94e78d7a41a9a8fe6d87a5e59500e51"