Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-02-16 18:09:50 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2021-02-16 18:09:50 +0300
commitb4e854a900ba9bcbfc3476f88317c59ea048daaf (patch)
tree562b380f7d3587c522d57487465b8df9d0c34746 /workhorse
parent8215fc964a189ae5c876a10f2e7d61933a725e24 (diff)
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'workhorse')
-rw-r--r--workhorse/CHANGELOG10
-rw-r--r--workhorse/VERSION2
-rw-r--r--workhorse/gitaly_test.go175
-rw-r--r--workhorse/go.mod2
-rw-r--r--workhorse/go.sum2
-rw-r--r--workhorse/internal/errortracker/sentry.go60
-rw-r--r--workhorse/internal/helper/helpers.go43
-rw-r--r--workhorse/internal/helper/raven.go58
-rw-r--r--workhorse/internal/imageresizer/image_resizer.go10
-rw-r--r--workhorse/internal/log/logging.go6
-rw-r--r--workhorse/internal/upstream/routes.go2
-rw-r--r--workhorse/internal/upstream/upstream.go3
-rw-r--r--workhorse/main.go5
-rw-r--r--workhorse/main_test.go6
-rw-r--r--workhorse/raven.go40
-rw-r--r--workhorse/upload_test.go68
16 files changed, 349 insertions, 143 deletions
diff --git a/workhorse/CHANGELOG b/workhorse/CHANGELOG
index 510bad4d13f..3142f2601b7 100644
--- a/workhorse/CHANGELOG
+++ b/workhorse/CHANGELOG
@@ -1,5 +1,15 @@
# 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
diff --git a/workhorse/VERSION b/workhorse/VERSION
index 38c47904a63..661bb99fdf9 100644
--- a/workhorse/VERSION
+++ b/workhorse/VERSION
@@ -1 +1 @@
-8.62.0
+8.63.0
diff --git a/workhorse/gitaly_test.go b/workhorse/gitaly_test.go
index 95d6907ac6a..d0e694bf8e7 100644
--- a/workhorse/gitaly_test.go
+++ b/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()
diff --git a/workhorse/go.mod b/workhorse/go.mod
index 6396e419487..20344f0081d 100644
--- a/workhorse/go.mod
+++ b/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
diff --git a/workhorse/go.sum b/workhorse/go.sum
index 4796d40638b..ddb08a1e846 100644
--- a/workhorse/go.sum
+++ b/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=
diff --git a/workhorse/internal/errortracker/sentry.go b/workhorse/internal/errortracker/sentry.go
new file mode 100644
index 00000000000..72a32c8d349
--- /dev/null
+++ b/workhorse/internal/errortracker/sentry.go
@@ -0,0 +1,60 @@
+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),
+ )
+}
diff --git a/workhorse/internal/helper/helpers.go b/workhorse/internal/helper/helpers.go
index f9b46181579..2e23f50b913 100644
--- a/workhorse/internal/helper/helpers.go
+++ b/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)
+
+ for _, cb := range loggerCallbacks {
+ logger = cb(logger)
+ }
-func Fail500(w http.ResponseWriter, r *http.Request, err error) {
- CaptureAndFail(w, r, err, "Internal server error", http.StatusInternalServerError)
+ 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
}
diff --git a/workhorse/internal/helper/raven.go b/workhorse/internal/helper/raven.go
deleted file mode 100644
index 898e8ec85f8..00000000000
--- a/workhorse/internal/helper/raven.go
+++ /dev/null
@@ -1,58 +0,0 @@
-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]")
- }
- }
-}
diff --git a/workhorse/internal/imageresizer/image_resizer.go b/workhorse/internal/imageresizer/image_resizer.go
index 69e9496aec2..7d423b80067 100644
--- a/workhorse/internal/imageresizer/image_resizer.go
+++ b/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)
}
}
diff --git a/workhorse/internal/log/logging.go b/workhorse/internal/log/logging.go
index c65ec07743a..9c19cde1395 100644
--- a/workhorse/internal/log/logging.go
+++ b/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)
}
}
diff --git a/workhorse/internal/upstream/routes.go b/workhorse/internal/upstream/routes.go
index 7b9fef12fc5..edcbfa88a67 100644
--- a/workhorse/internal/upstream/routes.go
+++ b/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`
diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go
index c81a21c0ecd..fd655a07679 100644
--- a/workhorse/internal/upstream/upstream.go
+++ b/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
diff --git a/workhorse/main.go b/workhorse/main.go
index 47ab63a875a..c43ec45240f 100644
--- a/workhorse/main.go
+++ b/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) }()
diff --git a/workhorse/main_test.go b/workhorse/main_test.go
index d15af1d3e4c..b725f36a68a 100644
--- a/workhorse/main_test.go
+++ b/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{
diff --git a/workhorse/raven.go b/workhorse/raven.go
deleted file mode 100644
index f641203f142..00000000000
--- a/workhorse/raven.go
+++ /dev/null
@@ -1,40 +0,0 @@
-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)
- }))
-}
diff --git a/workhorse/upload_test.go b/workhorse/upload_test.go
index 35648ae3994..6d118119dff 100644
--- a/workhorse/upload_test.go
+++ b/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)