diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-02-16 18:09:50 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-02-16 18:09:50 +0300 |
commit | b4e854a900ba9bcbfc3476f88317c59ea048daaf (patch) | |
tree | 562b380f7d3587c522d57487465b8df9d0c34746 /workhorse | |
parent | 8215fc964a189ae5c876a10f2e7d61933a725e24 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'workhorse')
-rw-r--r-- | workhorse/CHANGELOG | 10 | ||||
-rw-r--r-- | workhorse/VERSION | 2 | ||||
-rw-r--r-- | workhorse/gitaly_test.go | 175 | ||||
-rw-r--r-- | workhorse/go.mod | 2 | ||||
-rw-r--r-- | workhorse/go.sum | 2 | ||||
-rw-r--r-- | workhorse/internal/errortracker/sentry.go | 60 | ||||
-rw-r--r-- | workhorse/internal/helper/helpers.go | 43 | ||||
-rw-r--r-- | workhorse/internal/helper/raven.go | 58 | ||||
-rw-r--r-- | workhorse/internal/imageresizer/image_resizer.go | 10 | ||||
-rw-r--r-- | workhorse/internal/log/logging.go | 6 | ||||
-rw-r--r-- | workhorse/internal/upstream/routes.go | 2 | ||||
-rw-r--r-- | workhorse/internal/upstream/upstream.go | 3 | ||||
-rw-r--r-- | workhorse/main.go | 5 | ||||
-rw-r--r-- | workhorse/main_test.go | 6 | ||||
-rw-r--r-- | workhorse/raven.go | 40 | ||||
-rw-r--r-- | workhorse/upload_test.go | 68 |
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) |