diff --git a/README.md b/README.md index d007e8d5384dbd4de04f9cd0e107971cd1e3ca92..273ee49a2d602170dcf5ac382c4b2e80d474c629 100644 --- a/README.md +++ b/README.md @@ -1,12 +1,9 @@ # gitlab-workhorse -gitlab-workhorse was designed to unload Git HTTP traffic from -the GitLab Rails app (Unicorn) to a separate daemon. It also serves -'git archive' downloads for GitLab. All authentication and -authorization logic is still handled by the GitLab Rails app. +Gitlab-workhorse is a smart reverse proxy for GitLab. It handles +"large" HTTP requests such as file downloads, file uploads, Git +push/pull and Git archive downloads. -Architecture: Git client -> NGINX -> gitlab-workhorse (makes -auth request to GitLab Rails app) -> git-upload-pack ## Usage @@ -18,6 +15,10 @@ Options: Authentication/authorization backend (default "http://localhost:8080") -authSocket string Optional: Unix domain socket to dial authBackend at + -developmentMode + Allow to serve assets from Rails app + -documentRoot string + Path to static files content (default "public") -listenAddr string Listen address for HTTP server (default "localhost:8181") -listenNetwork string @@ -26,19 +27,17 @@ Options: Umask for Unix socket, default: 022 (default 18) -pprofListenAddr string pprof listening address, e.g. 'localhost:6060' + -proxyHeadersTimeout duration + How long to wait for response headers when proxying the request (default 1m0s) -version Print version and exit ``` -gitlab-workhorse allows Git HTTP clients to push and pull to -and from Git repositories. Each incoming request is first replayed -(with an empty request body) to an external authentication/authorization -HTTP server: the 'auth backend'. The auth backend is expected to -be a GitLab Unicorn process. The 'auth response' is a JSON message -which tells gitlab-workhorse the path of the Git repository -to read from/write to. +The 'auth backend' refers to the GitLab Rails applicatoin. The name is +a holdover from when gitlab-workhorse only handled Git push/pull over +HTTP. -gitlab-workhorse can listen on either a TCP or a Unix domain socket. It +Gitlab-workhorse can listen on either a TCP or a Unix domain socket. It can also open a second listening TCP listening socket with the Go [net/http/pprof profiler server](http://golang.org/pkg/net/http/pprof/). diff --git a/internal/git/archive.go b/internal/git/archive.go index 4665f80326e35097f4003fdd40722bc68884d4b7..ba4a978db7263aaf8eb9307a8eae839e6c815ed8 100644 --- a/internal/git/archive.go +++ b/internal/git/archive.go @@ -16,6 +16,7 @@ import ( "os/exec" "path" "path/filepath" + "syscall" "time" ) @@ -84,6 +85,7 @@ func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) { stdout = archiveStdout } else { compressCmd.Stdin = archiveStdout + compressCmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} stdout, err = compressCmd.StdoutPipe() if err != nil { @@ -96,7 +98,7 @@ func handleGetArchive(w http.ResponseWriter, r *http.Request, a *api.Response) { helper.Fail500(w, fmt.Errorf("handleGetArchive: start %v: %v", compressCmd.Args, err)) return } - defer compressCmd.Wait() + defer cleanUpProcessGroup(compressCmd) archiveStdout.Close() } diff --git a/internal/staticpages/error_pages.go b/internal/staticpages/error_pages.go index bac05634d1bf26f206e62b87ce4a0dc018141422..31774bdc469354bc9b1fae7347e5e634b6cb3dd2 100644 --- a/internal/staticpages/error_pages.go +++ b/internal/staticpages/error_pages.go @@ -60,7 +60,10 @@ func (s *errorPageResponseWriter) Flush() { s.WriteHeader(http.StatusOK) } -func (st *Static) ErrorPages(handler http.Handler) http.Handler { +func (st *Static) ErrorPages(enabled bool, handler http.Handler) http.Handler { + if !enabled { + return handler + } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { rw := errorPageResponseWriter{ rw: w, diff --git a/internal/staticpages/error_pages_test.go b/internal/staticpages/error_pages_test.go index b78eae7dc5627b32fb22b2c3227ddbc7c18c5f87..3695a65b97b471df8c6c1e8b2b969edffc45874c 100644 --- a/internal/staticpages/error_pages_test.go +++ b/internal/staticpages/error_pages_test.go @@ -27,7 +27,7 @@ func TestIfErrorPageIsPresented(t *testing.T) { fmt.Fprint(w, "Not Found") }) st := &Static{dir} - st.ErrorPages(h).ServeHTTP(w, nil) + st.ErrorPages(true, h).ServeHTTP(w, nil) w.Flush() helper.AssertResponseCode(t, w, 404) @@ -48,9 +48,32 @@ func TestIfErrorPassedIfNoErrorPageIsFound(t *testing.T) { fmt.Fprint(w, errorResponse) }) st := &Static{dir} - st.ErrorPages(h).ServeHTTP(w, nil) + st.ErrorPages(true, h).ServeHTTP(w, nil) w.Flush() helper.AssertResponseCode(t, w, 404) helper.AssertResponseBody(t, w, errorResponse) } + +func TestIfErrorPageIsIgnoredInDevelopment(t *testing.T) { + dir, err := ioutil.TempDir("", "error_page") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + errorPage := "ERROR" + ioutil.WriteFile(filepath.Join(dir, "500.html"), []byte(errorPage), 0600) + + w := httptest.NewRecorder() + serverError := "Interesting Server Error" + h := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(500) + fmt.Fprint(w, serverError) + }) + st := &Static{dir} + st.ErrorPages(false, h).ServeHTTP(w, nil) + w.Flush() + helper.AssertResponseCode(t, w, 500) + helper.AssertResponseBody(t, w, serverError) +} diff --git a/internal/upstream/routes.go b/internal/upstream/routes.go index 3c152b47ecd718023867407aa87a551ec103031c..2c0681574490fd6d50c71fb0e99bd6556463a706 100644 --- a/internal/upstream/routes.go +++ b/internal/upstream/routes.go @@ -88,7 +88,7 @@ func (u *Upstream) configureRoutes() { route{"", nil, static.ServeExisting(u.URLPrefix(), staticpages.CacheDisabled, static.DeployPage( - static.ErrorPages( + static.ErrorPages(u.DevelopmentMode, proxy, ), ), diff --git a/internal/upstream/upstream.go b/internal/upstream/upstream.go index f778c51029c3e81402f146454ee231bd1df9847c..f2f3754b7a4502e6de358eeb244b010ca39c01f6 100644 --- a/internal/upstream/upstream.go +++ b/internal/upstream/upstream.go @@ -81,7 +81,7 @@ func (u *Upstream) ServeHTTP(ow http.ResponseWriter, r *http.Request) { } // Check URL Root - URIPath := urlprefix.CleanURIPath(r.URL.Path) + URIPath := urlprefix.CleanURIPath(r.URL.EscapedPath()) prefix := u.URLPrefix() if !prefix.Match(URIPath) { httpError(&w, r, fmt.Sprintf("Not found %q", URIPath), http.StatusNotFound) diff --git a/main_test.go b/main_test.go index a1ce9640db8d65ab5b9b86628c2dfa391f3081bf..6f402a31dd7f9c55d6bd89dd25f9a1ebb7588cb3 100644 --- a/main_test.go +++ b/main_test.go @@ -15,6 +15,7 @@ import ( "os/exec" "path" "regexp" + "strings" "testing" "time" ) @@ -199,6 +200,29 @@ func TestAllowedApiDownloadZip(t *testing.T) { runOrFail(t, extractCmd) } +func TestAllowedApiDownloadZipWithSlash(t *testing.T) { + prepareDownloadDir(t) + + // Prepare test server and backend + archiveName := "foobar.zip" + ts := testAuthServer(nil, 200, archiveOkBody(t, archiveName)) + defer ts.Close() + ws := startWorkhorseServer(ts.URL) + defer ws.Close() + + // Use foo%2Fbar instead of a numeric ID + downloadCmd := exec.Command("curl", "-J", "-O", fmt.Sprintf("%s/api/v3/projects/foo%%2Fbar/repository/archive.zip", ws.URL)) + if !strings.Contains(downloadCmd.Args[3], `projects/foo%2Fbar/repository`) { + t.Fatalf("Cannot find percent-2F: %v", downloadCmd.Args) + } + downloadCmd.Dir = scratchDir + runOrFail(t, downloadCmd) + + extractCmd := exec.Command("unzip", archiveName) + extractCmd.Dir = scratchDir + runOrFail(t, extractCmd) +} + func TestDownloadCacheHit(t *testing.T) { prepareDownloadDir(t)