Merge branch 'update-workhorse-8-63' into 'master'

Update workhorse to v8.63.0

See merge request gitlab-org/gitlab!54315

GITLAB_WORKHORSE_VERSION

-8.62.0
+8.63.0

changelogs/unreleased/update-workhorse-8-63.yml

+---
+title: Update GitLab Workhorse to v8.63.0
+merge_request: 54315
+author:
+type: other

workhorse/CHANGELOG

 # Changelog for gitlab-workhorse
 
+## v8.63.0
+
+### Added
+- Accept more paths as Git HTTP
+  https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/684
+
+### Other
+- Migrate error tracking from raven to labkit
+  https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/671
+
 ## v8.62.0
 
 ### Added

workhorse/VERSION

-8.62.0
+8.63.0

workhorse/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()

workhorse/go.mod

@@ -8,10 +8,8 @@ require (
 	github.com/FZambia/sentinel v1.0.0
 	github.com/alecthomas/chroma v0.7.3
 	github.com/aws/aws-sdk-go v1.36.1
-	github.com/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054 // indirect
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/disintegration/imaging v1.6.2
-	github.com/getsentry/raven-go v0.2.0
 	github.com/golang/gddo v0.0.0-20190419222130-af0f2af80721
 	github.com/golang/protobuf v1.4.3
 	github.com/gomodule/redigo v2.0.0+incompatible

workhorse/go.sum

@@ -145,8 +145,6 @@ github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QH
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/census-instrumentation/opencensus-proto v0.3.0/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/certifi/gocertifi v0.0.0-20180905225744-ee1a9a0726d2/go.mod h1:GJKEexRPVJrBSOjoqN5VNOIKJ5Q3RViH6eu3puDRwx4=
-github.com/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054 h1:uH66TXeswKn5PW5zdZ39xEwfS9an067BirqA+P4QaLI=
-github.com/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054/go.mod h1:sGbDF6GwGcLpkNXPUTkMRoywsNa/ol15pxFe6ERfguA=
 github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY=
 github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=

workhorse/internal/errortracker/sentry.go

+package errortracker
+
+import (
+	"fmt"
+	"net/http"
+	"os"
+	"runtime/debug"
+
+	"gitlab.com/gitlab-org/labkit/errortracking"
+
+	"gitlab.com/gitlab-org/labkit/log"
+)
+
+// NewHandler allows us to handle panics in upstreams gracefully, by logging them
+// using structured logging and reporting them into Sentry as `error`s with a
+// proper correlation ID attached.
+func NewHandler(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		defer func() {
+			if p := recover(); p != nil {
+				fields := log.ContextFields(r.Context())
+				log.WithFields(fields).Error(p)
+				debug.PrintStack()
+				// A panic isn't always an `error`, so we may have to convert it into one.
+				e, ok := p.(error)
+				if !ok {
+					e = fmt.Errorf("%v", p)
+				}
+				TrackFailedRequest(r, e, fields)
+			}
+		}()
+
+		next.ServeHTTP(w, r)
+	})
+}
+
+func TrackFailedRequest(r *http.Request, err error, fields log.Fields) {
+	captureOpts := []errortracking.CaptureOption{
+		errortracking.WithContext(r.Context()),
+		errortracking.WithRequest(r),
+	}
+	for k, v := range fields {
+		captureOpts = append(captureOpts, errortracking.WithField(k, fmt.Sprintf("%v", v)))
+	}
+
+	errortracking.Capture(err, captureOpts...)
+}
+
+func Initialize(version string) error {
+	// Use a custom environment variable (not SENTRY_DSN) to prevent
+	// clashes with gitlab-rails.
+	sentryDSN := os.Getenv("GITLAB_WORKHORSE_SENTRY_DSN")
+	sentryEnvironment := os.Getenv("GITLAB_WORKHORSE_SENTRY_ENVIRONMENT")
+
+	return errortracking.Initialize(
+		errortracking.WithSentryDSN(sentryDSN),
+		errortracking.WithSentryEnvironment(sentryEnvironment),
+		errortracking.WithVersion(version),
+	)
+}

workhorse/internal/helper/helpers.go

@@ -14,50 +14,31 @@ import (
 	"syscall"
 
 	"github.com/sebest/xff"
-	"gitlab.com/gitlab-org/labkit/log"
-	"gitlab.com/gitlab-org/labkit/mask"
+
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/log"
 )
 
 const NginxResponseBufferHeader = "X-Accel-Buffering"
 
-func logErrorWithFields(r *http.Request, err error, fields log.Fields) {
-	if err != nil {
-		CaptureRavenError(r, err, fields)
-	}
-
-	printError(r, err, fields)
-}
-
-func CaptureAndFail(w http.ResponseWriter, r *http.Request, err error, msg string, code int) {
+func CaptureAndFail(w http.ResponseWriter, r *http.Request, err error, msg string, code int, loggerCallbacks ...log.ConfigureLogger) {
 	http.Error(w, msg, code)
-	logErrorWithFields(r, err, nil)
-}
+	logger := log.WithRequest(r).WithError(err)
 
-func Fail500(w http.ResponseWriter, r *http.Request, err error) {
-	CaptureAndFail(w, r, err, "Internal server error", http.StatusInternalServerError)
+	for _, cb := range loggerCallbacks {
+		logger = cb(logger)
+	}
+
+	logger.Error(msg)
 }
 
-func Fail500WithFields(w http.ResponseWriter, r *http.Request, err error, fields log.Fields) {
-	http.Error(w, "Internal server error", http.StatusInternalServerError)
-	logErrorWithFields(r, err, fields)
+func Fail500(w http.ResponseWriter, r *http.Request, err error, loggerCallbacks ...log.ConfigureLogger) {
+	CaptureAndFail(w, r, err, "Internal server error", http.StatusInternalServerError, loggerCallbacks...)
 }
 
 func RequestEntityTooLarge(w http.ResponseWriter, r *http.Request, err error) {
 	CaptureAndFail(w, r, err, "Request Entity Too Large", http.StatusRequestEntityTooLarge)
 }
 
-func printError(r *http.Request, err error, fields log.Fields) {
-	if r != nil {
-		entry := log.WithContextFields(r.Context(), log.Fields{
-			"method": r.Method,
-			"uri":    mask.URL(r.RequestURI),
-		})
-		entry.WithFields(fields).WithError(err).Error()
-	} else {
-		log.WithFields(fields).WithError(err).Error("unknown error")
-	}
-}
-
 func SetNoCacheHeaders(header http.Header) {
 	header.Set("Cache-Control", "no-cache, no-store, max-age=0, must-revalidate")
 	header.Set("Pragma", "no-cache")
@@ -97,7 +78,7 @@ func OpenFile(path string) (file *os.File, fi os.FileInfo, err error) {
 func URLMustParse(s string) *url.URL {
 	u, err := url.Parse(s)
 	if err != nil {
-		log.WithError(err).WithField("url", s).Fatal("urlMustParse")
+		log.WithError(err).WithFields(log.Fields{"url": s}).Error("urlMustParse")
 	}
 	return u
 }

workhorse/internal/helper/raven.go

-package helper
-
-import (
-	"net/http"
-	"reflect"
-
-	raven "github.com/getsentry/raven-go"
-
-	//lint:ignore SA1019 this was recently deprecated. Update workhorse to use labkit errortracking package.
-	correlation "gitlab.com/gitlab-org/labkit/correlation/raven"
-
-	"gitlab.com/gitlab-org/labkit/log"
-)
-
-var ravenHeaderBlacklist = []string{
-	"Authorization",
-	"Private-Token",
-}
-
-func CaptureRavenError(r *http.Request, err error, fields log.Fields) {
-	client := raven.DefaultClient
-	extra := raven.Extra{}
-
-	for k, v := range fields {
-		extra[k] = v
-	}
-
-	interfaces := []raven.Interface{}
-	if r != nil {
-		CleanHeadersForRaven(r)
-		interfaces = append(interfaces, raven.NewHttp(r))
-
-		//lint:ignore SA1019 this was recently deprecated. Update workhorse to use labkit errortracking package.
-		extra = correlation.SetExtra(r.Context(), extra)
-	}
-
-	exception := &raven.Exception{
-		Stacktrace: raven.NewStacktrace(2, 3, nil),
-		Value:      err.Error(),
-		Type:       reflect.TypeOf(err).String(),
-	}
-	interfaces = append(interfaces, exception)
-
-	packet := raven.NewPacketWithExtra(err.Error(), extra, interfaces...)
-	client.Capture(packet, nil)
-}
-
-func CleanHeadersForRaven(r *http.Request) {
-	if r == nil {
-		return
-	}
-
-	for _, key := range ravenHeaderBlacklist {
-		if r.Header.Get(key) != "" {
-			r.Header.Set(key, "[redacted]")
-		}
-	}
-}

workhorse/internal/imageresizer/image_resizer.go

@@ -428,16 +428,18 @@ func logFields(startTime time.Time, params *resizeParams, outcome *resizeOutcome
 
 func handleOutcome(w http.ResponseWriter, req *http.Request, startTime time.Time, params *resizeParams, outcome *resizeOutcome) {
 	fields := logFields(startTime, params, outcome)
-	log := log.WithRequest(req).WithFields(fields)
+	logger := log.WithRequest(req).WithFields(fields)
 
 	switch outcome.status {
 	case statusRequestFailure:
 		if outcome.bytesWritten <= 0 {
-			helper.Fail500WithFields(w, req, outcome.err, fields)
+			helper.Fail500(w, req, outcome.err, func(b *log.Builder) *log.Builder {
+				return b.WithFields(fields)
+			})
 		} else {
-			log.WithError(outcome.err).Error(outcome.status)
+			logger.WithError(outcome.err).Error(outcome.status)
 		}
 	default:
-		log.Info(outcome.status)
+		logger.Info(outcome.status)
 	}
 }

workhorse/internal/log/logging.go

@@ -8,11 +8,13 @@ import (
 	"gitlab.com/gitlab-org/labkit/mask"
 	"golang.org/x/net/context"
 
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/errortracker"
 )
 
 type Fields = log.Fields
 
+type ConfigureLogger func(*Builder) *Builder
+
 type Builder struct {
 	entry  *logrus.Entry
 	fields log.Fields
@@ -83,6 +85,6 @@ func (b *Builder) Error(args ...interface{}) {
 	b.entry.Error(args...)
 
 	if b.req != nil && b.err != nil {
-		helper.CaptureRavenError(b.req, b.err, b.fields)
+		errortracker.TrackFailedRequest(b.req, b.err, b.fields)
 	}
 }

workhorse/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`

workhorse/internal/upstream/upstream.go

@@ -16,6 +16,7 @@ import (
 	"gitlab.com/gitlab-org/labkit/correlation"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/config"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/errortracker"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/rejectmethods"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/upload"
@@ -63,7 +64,7 @@ func NewUpstream(cfg config.Config, accessLogger *logrus.Logger) http.Handler {
 		correlationOpts = append(correlationOpts, correlation.WithPropagation())
 	}
 
-	handler := correlation.InjectCorrelationID(&up, correlationOpts...)
+	handler := correlation.InjectCorrelationID(errortracker.NewHandler(&up), correlationOpts...)
 	// TODO: move to LabKit https://gitlab.com/gitlab-org/gitlab-workhorse/-/issues/339
 	handler = rejectmethods.NewMiddleware(handler)
 	return handler

workhorse/main.go

@@ -16,6 +16,7 @@ import (
 	"gitlab.com/gitlab-org/labkit/tracing"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/config"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/errortracker"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/queueing"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/redis"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/secret"
@@ -156,6 +157,8 @@ func run(boot bootConfig, cfg config.Config) error {
 	}
 	defer closer.Close()
 
+	errortracker.Initialize(cfg.Version)
+
 	tracing.Initialize(tracing.WithServiceName("gitlab-workhorse"))
 	log.WithField("version", Version).WithField("build_time", BuildTime).Print("Starting")
 
@@ -223,7 +226,7 @@ func run(boot bootConfig, cfg config.Config) error {
 	}
 	defer accessCloser.Close()
 
-	up := wrapRaven(upstream.NewUpstream(cfg, accessLogger))
+	up := upstream.NewUpstream(cfg, accessLogger)
 
 	go func() { finalErrors <- http.Serve(listener, up) }()
 

workhorse/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{

workhorse/raven.go

-package main
-
-import (
-	"net/http"
-	"os"
-
-	raven "github.com/getsentry/raven-go"
-
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
-)
-
-func wrapRaven(h http.Handler) http.Handler {
-	// Use a custom environment variable (not SENTRY_DSN) to prevent
-	// clashes with gitlab-rails.
-	sentryDSN := os.Getenv("GITLAB_WORKHORSE_SENTRY_DSN")
-	sentryEnvironment := os.Getenv("GITLAB_WORKHORSE_SENTRY_ENVIRONMENT")
-	raven.SetDSN(sentryDSN) // sentryDSN may be empty
-
-	if sentryEnvironment != "" {
-		raven.SetEnvironment(sentryEnvironment)
-	}
-
-	if sentryDSN == "" {
-		return h
-	}
-
-	raven.DefaultClient.SetRelease(Version)
-
-	return http.HandlerFunc(raven.RecoveryHandler(
-		func(w http.ResponseWriter, r *http.Request) {
-			defer func() {
-				if p := recover(); p != nil {
-					helper.CleanHeadersForRaven(r)
-					panic(p)
-				}
-			}()
-
-			h.ServeHTTP(w, r)
-		}))
-}

workhorse/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)