Merge branch 'jv-git-http-routing' into 'master'

Accept more paths as Git HTTP

Closes gitlab#294007

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

changelogs/unreleased/jv-git-http-routing.yml

+---
+title: Support Git HTTP on toplevel repositories
+merge_request: 670
+author:
+type: added

gitaly_test.go

@@ -169,6 +169,57 @@ 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, nil, 200, apiResponse)
+	defer ts.Close()
+
+	ws := startWorkhorseServer(ts.URL)
+	defer ws.Close()
+
+	testCases := []struct {
+		method string
+		path   string
+		match  bool
+	}{
+		{"GET", "/toplevel.git/info/refs?service=git-receive-pack", true},
+		{"GET", "/toplevel.wiki.git/info/refs?service=git-upload-pack", true},
+		{"GET", "/toplevel/child/project.git/info/refs?service=git-receive-pack", true},
+		{"GET", "/toplevel/child/project.wiki.git/info/refs?service=git-upload-pack", true},
+		{"GET", "/toplevel/child/project/snippets/123.git/info/refs?service=git-receive-pack", true},
+		{"GET", "/snippets/123.git/info/refs?service=git-upload-pack", true},
+		{"GET", "/foo/bar.git/info/refs", false},
+		{"GET", "/foo/bar.git/info/refs?service=git-zzz-pack", false},
+		{"GET", "/.git/info/refs?service=git-upload-pack", false},
+		{"POST", "/toplevel.git/info/refs?service=git-receive-pack", false},
+	}
+
+	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.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 waitDone(t *testing.T, done chan struct{}) {
 	t.Helper()
 	select {
@@ -259,6 +310,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 +486,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/git/info-refs.go

@@ -20,6 +20,11 @@ func GetInfoRefsHandler(a *api.API) http.Handler {
 	return repoPreAuthorizeHandler(a, handleGetInfoRefs)
 }
 
+func IsSmartInfoRefs(r *http.Request) bool {
+	service := r.URL.Query().Get("service")
+	return r.Method == "GET" && (service == "git-upload-pack" || service == "git-receive-pack")
+}
+
 func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response) {
 	responseWriter := NewHttpResponseWriter(rw)
 	// Log 0 bytes in because we ignore the request body (and there usually is none anyway).

internal/git/info-refs_test.go

+package git
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsSmartInfoRefs(t *testing.T) {
+	testCases := []struct {
+		method string
+		url    string
+		match  bool
+	}{
+		{"GET", "?service=git-upload-pack", true},
+		{"GET", "?service=git-receive-pack", true},
+		{"GET", "", false},
+		{"GET", "?service=", false},
+		{"GET", "?service=foo", false},
+		{"POST", "?service=git-upload-pack", false},
+		{"POST", "?service=git-receive-pack", false},
+	}
+
+	for _, tc := range testCases {
+		url, err := url.Parse(tc.url)
+		require.NoError(t, err)
+
+		r := http.Request{Method: tc.method, URL: url}
+		require.Equal(t, tc.match, IsSmartInfoRefs(&r))
+	}
+}

internal/upstream/routes.go

@@ -53,13 +53,13 @@ type uploadPreparers struct {
 }
 
 const (
-	apiPattern           = `^/api/`
-	ciAPIPattern         = `^/ci/api/`
-	gitProjectPattern    = `^/([^/]+/){1,}[^/]+\.git/`
-	projectPattern       = `^/([^/]+/){1,}[^/]+/`
-	snippetUploadPattern = `^/uploads/personal_snippet`
-	userUploadPattern    = `^/uploads/user`
-	importPattern        = `^/import/`
+	apiPattern           = `\A/api/`
+	ciAPIPattern         = `\A/ci/api/`
+	gitRepositoryPattern = `\A/.+\.git/`
+	projectPattern       = `\A/([^/]+/){1,}[^/]+/`
+	snippetUploadPattern = `\A/uploads/personal_snippet`
+	userUploadPattern    = `\A/uploads/user`
+	importPattern        = `\A/import/`
 )
 
 func compileRegexp(regexpStr string) *regexp.Regexp {
@@ -222,10 +222,10 @@ func (u *upstream) configureRoutes() {
 
 	u.Routes = []routeEntry{
 		// Git Clone
-		u.route("GET", gitProjectPattern+`info/refs\z`, git.GetInfoRefsHandler(api)),
-		u.route("POST", gitProjectPattern+`git-upload-pack\z`, contentEncodingHandler(git.UploadPack(api)), withMatcher(isContentType("application/x-git-upload-pack-request"))),
-		u.route("POST", gitProjectPattern+`git-receive-pack\z`, contentEncodingHandler(git.ReceivePack(api)), withMatcher(isContentType("application/x-git-receive-pack-request"))),
-		u.route("PUT", gitProjectPattern+`gitlab-lfs/objects/([0-9a-f]{64})/([0-9]+)\z`, lfs.PutStore(api, signingProxy, preparers.lfs), withMatcher(isContentType("application/octet-stream"))),
+		u.route("GET", gitRepositoryPattern+`info/refs\z`, git.GetInfoRefsHandler(api), withMatcher(git.IsSmartInfoRefs)),
+		u.route("POST", gitRepositoryPattern+`git-upload-pack\z`, contentEncodingHandler(git.UploadPack(api)), withMatcher(isContentType("application/x-git-upload-pack-request"))),
+		u.route("POST", gitRepositoryPattern+`git-receive-pack\z`, contentEncodingHandler(git.ReceivePack(api)), withMatcher(isContentType("application/x-git-receive-pack-request"))),
+		u.route("PUT", gitRepositoryPattern+`gitlab-lfs/objects/([0-9a-f]{64})/([0-9]+)\z`, lfs.PutStore(api, signingProxy, preparers.lfs), withMatcher(isContentType("application/octet-stream"))),
 
 		// CI Artifacts
 		u.route("POST", apiPattern+`v4/jobs/[0-9]+/artifacts\z`, contentEncodingHandler(artifacts.UploadArtifacts(api, signingProxy, preparers.artifacts))),

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

@@ -234,7 +234,7 @@ func TestLfsUpload(t *testing.T) {
 	reqBody := "test data"
 	rspBody := "test success"
 	oid := "916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9"
-	resource := fmt.Sprintf("/%s/gitlab-lfs/objects/%s/%d", testRepo, oid, len(reqBody))
+	resource := fmt.Sprintf("/gitlab-org/gitlab-test.git/gitlab-lfs/objects/%s/%d", oid, len(reqBody))
 
 	lfsApiResponse := fmt.Sprintf(
 		`{"TempPath":%q, "LfsOid":%q, "LfsSize": %d}`,
@@ -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, 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, "PUT")