Merge branch 'toplevel-git-http-routes' into 'master'

Accept more paths as Git HTTP

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

changelogs/unreleased/toplevel-git-http-routes.yml

+---
+title: Accept more paths as Git HTTP
+merge_request: 684
+author:
+type: added

gitaly_test.go

@@ -9,6 +9,7 @@ import (
 	"math/rand"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"os/exec"
 	"path"
@@ -169,6 +170,62 @@ func TestGetInfoRefsProxiedToGitalyInterruptedStream(t *testing.T) {
 	waitDone(t, done)
 }
 
+func TestGetInfoRefsRouting(t *testing.T) {
+	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
+	defer gitalyServer.GracefulStop()
+
+	apiResponse := gitOkBody(t)
+	apiResponse.GitalyServer.Address = "unix:" + socketPath
+	ts := testAuthServer(t, nil, url.Values{"service": {"git-receive-pack"}}, 200, apiResponse)
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method string
+		path   string
+		status int
+	}{
+		// valid requests
+		{"GET", "/toplevel.git/info/refs?service=git-receive-pack", 200},
+		{"GET", "/toplevel.wiki.git/info/refs?service=git-receive-pack", 200},
+		{"GET", "/toplevel/child/project.git/info/refs?service=git-receive-pack", 200},
+		{"GET", "/toplevel/child/project.wiki.git/info/refs?service=git-receive-pack", 200},
+		{"GET", "/toplevel/child/project/snippets/123.git/info/refs?service=git-receive-pack", 200},
+		{"GET", "/snippets/123.git/info/refs?service=git-receive-pack", 200},
+		// failing due to missing service parameter
+		{"GET", "/foo/bar.git/info/refs", 403},
+		// failing due to invalid service parameter
+		{"GET", "/foo/bar.git/info/refs?service=git-zzz-pack", 403},
+		// failing due to invalid repository path
+		{"GET", "/.git/info/refs?service=git-receive-pack", 204},
+		// failing due to invalid request method
+		{"POST", "/toplevel.git/info/refs?service=git-receive-pack", 204},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			req, err := http.NewRequest(tc.method, ws.URL+tc.path, nil)
+			require.NoError(t, err)
+
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			body := string(testhelper.ReadAll(t, resp.Body))
+
+			if tc.status == 200 {
+				require.Equal(t, 200, resp.StatusCode)
+				require.Contains(t, body, "\x00", "expect response generated by test gitaly server")
+			} else {
+				require.Equal(t, tc.status, resp.StatusCode)
+				require.Empty(t, body, "normal request has empty response body")
+			}
+		})
+	}
+}
+
 func waitDone(t *testing.T, done chan struct{}) {
 	t.Helper()
 	select {
@@ -259,6 +316,65 @@ func TestPostReceivePackProxiedToGitalyInterrupted(t *testing.T) {
 	waitDone(t, done)
 }
 
+func TestPostReceivePackRouting(t *testing.T) {
+	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
+	defer gitalyServer.GracefulStop()
+
+	apiResponse := gitOkBody(t)
+	apiResponse.GitalyServer.Address = "unix:" + socketPath
+	ts := testAuthServer(t, nil, nil, 200, apiResponse)
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method      string
+		path        string
+		contentType string
+		match       bool
+	}{
+		{"POST", "/toplevel.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/toplevel.wiki.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/toplevel/child/project.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/toplevel/child/project.wiki.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/toplevel/child/project/snippets/123.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/snippets/123.git/git-receive-pack", "application/x-git-receive-pack-request", true},
+		{"POST", "/foo/bar/git-receive-pack", "application/x-git-receive-pack-request", false},
+		{"POST", "/foo/bar.git/git-zzz-pack", "application/x-git-receive-pack-request", false},
+		{"POST", "/.git/git-receive-pack", "application/x-git-receive-pack-request", false},
+		{"POST", "/toplevel.git/git-receive-pack", "application/x-git-upload-pack-request", false},
+		{"GET", "/toplevel.git/git-receive-pack", "application/x-git-receive-pack-request", false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			req, err := http.NewRequest(
+				tc.method,
+				ws.URL+tc.path,
+				bytes.NewReader(testhelper.GitalyReceivePackResponseMock),
+			)
+			require.NoError(t, err)
+
+			req.Header.Set("Content-Type", tc.contentType)
+
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			body := string(testhelper.ReadAll(t, resp.Body))
+
+			if tc.match {
+				require.Equal(t, 200, resp.StatusCode)
+				require.Contains(t, body, "\x00", "expect response generated by test gitaly server")
+			} else {
+				require.Equal(t, 204, resp.StatusCode)
+				require.Empty(t, body, "normal request has empty response body")
+			}
+		})
+	}
+}
+
 // ReaderFunc is an adapter to turn a conforming function into an io.Reader.
 type ReaderFunc func(b []byte) (int, error)
 
@@ -376,6 +492,65 @@ func TestPostUploadPackProxiedToGitalyInterrupted(t *testing.T) {
 	waitDone(t, done)
 }
 
+func TestPostUploadPackRouting(t *testing.T) {
+	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
+	defer gitalyServer.GracefulStop()
+
+	apiResponse := gitOkBody(t)
+	apiResponse.GitalyServer.Address = "unix:" + socketPath
+	ts := testAuthServer(t, nil, nil, 200, apiResponse)
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method      string
+		path        string
+		contentType string
+		match       bool
+	}{
+		{"POST", "/toplevel.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/toplevel.wiki.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/toplevel/child/project.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/toplevel/child/project.wiki.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/toplevel/child/project/snippets/123.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/snippets/123.git/git-upload-pack", "application/x-git-upload-pack-request", true},
+		{"POST", "/foo/bar/git-upload-pack", "application/x-git-upload-pack-request", false},
+		{"POST", "/foo/bar.git/git-zzz-pack", "application/x-git-upload-pack-request", false},
+		{"POST", "/.git/git-upload-pack", "application/x-git-upload-pack-request", false},
+		{"POST", "/toplevel.git/git-upload-pack", "application/x-git-receive-pack-request", false},
+		{"GET", "/toplevel.git/git-upload-pack", "application/x-git-upload-pack-request", false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			req, err := http.NewRequest(
+				tc.method,
+				ws.URL+tc.path,
+				bytes.NewReader(testhelper.GitalyReceivePackResponseMock),
+			)
+			require.NoError(t, err)
+
+			req.Header.Set("Content-Type", tc.contentType)
+
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			body := string(testhelper.ReadAll(t, resp.Body))
+
+			if tc.match {
+				require.Equal(t, 200, resp.StatusCode)
+				require.Contains(t, body, "\x00", "expect response generated by test gitaly server")
+			} else {
+				require.Equal(t, 204, resp.StatusCode)
+				require.Empty(t, body, "normal request has empty response body")
+			}
+		})
+	}
+}
+
 func TestGetDiffProxiedToGitalySuccessfully(t *testing.T) {
 	gitalyServer, socketPath := startGitalyServer(t, codes.OK)
 	defer gitalyServer.GracefulStop()

internal/upstream/routes.go

@@ -55,7 +55,7 @@ type uploadPreparers struct {
 const (
 	apiPattern           = `^/api/`
 	ciAPIPattern         = `^/ci/api/`
-	gitProjectPattern    = `^/([^/]+/){1,}[^/]+\.git/`
+	gitProjectPattern    = `^/.+\.git/`
 	projectPattern       = `^/([^/]+/){1,}[^/]+/`
 	snippetUploadPattern = `^/uploads/personal_snippet`
 	userUploadPattern    = `^/uploads/user`

main_test.go

@@ -694,6 +694,12 @@ func testAuthServer(t *testing.T, url *regexp.Regexp, params url.Values, code in
 	return testhelper.TestServerWithHandler(url, func(w http.ResponseWriter, r *http.Request) {
 		require.NotEmpty(t, r.Header.Get("X-Request-Id"))
 
+		// return a 204 No Content response if we don't receive the JWT header
+		if r.Header.Get(secret.RequestHeader) == "" {
+			w.WriteHeader(204)
+			return
+		}
+
 		w.Header().Set("Content-Type", api.ResponseContentType)
 
 		logEntry := log.WithFields(log.Fields{

upload_test.go

@@ -292,6 +292,74 @@ func TestLfsUpload(t *testing.T) {
 	require.Equal(t, rspBody, string(rspData))
 }
 
+func TestLfsUploadRouting(t *testing.T) {
+	reqBody := "test data"
+	rspBody := "test success"
+	oid := "916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9"
+
+	ts := testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get(secret.RequestHeader) == "" {
+			w.WriteHeader(204)
+		} else {
+			fmt.Fprint(w, rspBody)
+		}
+	})
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method      string
+		path        string
+		contentType string
+		match       bool
+	}{
+		{"PUT", "/toplevel.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/toplevel.wiki.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/toplevel/child/project.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/toplevel/child/project.wiki.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/toplevel/child/project/snippets/123.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/snippets/123.git/gitlab-lfs/objects", "application/octet-stream", true},
+		{"PUT", "/foo/bar/gitlab-lfs/objects", "application/octet-stream", false},
+		{"PUT", "/foo/bar.git/gitlab-lfs/objects/zzz", "application/octet-stream", false},
+		{"PUT", "/.git/gitlab-lfs/objects", "application/octet-stream", false},
+		{"PUT", "/toplevel.git/gitlab-lfs/objects", "application/zzz", false},
+		{"POST", "/toplevel.git/gitlab-lfs/objects", "application/octet-stream", false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.path, func(t *testing.T) {
+			resource := fmt.Sprintf(tc.path+"/%s/%d", oid, len(reqBody))
+
+			req, err := http.NewRequest(
+				tc.method,
+				ws.URL+resource,
+				strings.NewReader(reqBody),
+			)
+			require.NoError(t, err)
+
+			req.Header.Set("Content-Type", tc.contentType)
+			req.ContentLength = int64(len(reqBody))
+
+			resp, err := http.DefaultClient.Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			rspData, err := ioutil.ReadAll(resp.Body)
+			require.NoError(t, err)
+
+			if tc.match {
+				require.Equal(t, 200, resp.StatusCode)
+				require.Equal(t, rspBody, string(rspData), "expect response generated by test upstream server")
+			} else {
+				require.Equal(t, 204, resp.StatusCode)
+				require.Empty(t, rspData, "normal request has empty response body")
+			}
+		})
+	}
+}
+
 func packageUploadTestServer(t *testing.T, method string, resource string, reqBody string, rspBody string) *httptest.Server {
 	return testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) {
 		require.Equal(t, r.Method, method)