diff options
Diffstat (limited to 'workhorse')
| -rw-r--r-- | workhorse/CHANGELOG | 16 | ||||
| -rw-r--r-- | workhorse/PROCESS.md | 4 | ||||
| -rw-r--r-- | workhorse/README.md | 14 | ||||
| l---------[-rw-r--r--] | workhorse/VERSION | 2 | ||||
| -rw-r--r-- | workhorse/config_test.go | 2 | ||||
| -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/upstream.go | 3 | ||||
| -rw-r--r-- | workhorse/main.go | 7 | ||||
| -rw-r--r-- | workhorse/raven.go | 40 |
15 files changed, 162 insertions, 107 deletions
diff --git a/workhorse/CHANGELOG b/workhorse/CHANGELOG index 0d29061ccaf..b742affae07 100644 --- a/workhorse/CHANGELOG +++ b/workhorse/CHANGELOG @@ -1,17 +1,21 @@ # Changelog for gitlab-workhorse -## v8.63.2 +## v8.65.0 -### Security -- Stop logging when path is excluded - https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/ - -## v8.63.1 +### Fixed +- Fix long polling to default to 50 s instead of 50 ns + https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/687 ### Security - Use URL.EscapePath() in upstream router https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/ +## v8.64.0 + +### Other +- Revert "Migrate to labkit error tracking" + https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/685 + ## v8.63.0 ### Added diff --git a/workhorse/PROCESS.md b/workhorse/PROCESS.md index 797de59d47e..cf29b23b2c0 100644 --- a/workhorse/PROCESS.md +++ b/workhorse/PROCESS.md @@ -31,6 +31,10 @@ The final merge must be performed by a maintainer. ## Releases +> Below we describe the legacy release process, from when Workhorse +> had its own repository. These instructions are still useful for +> security backports. + New versions of Workhorse can be released by one of the Workhorse maintainers. The release process is: diff --git a/workhorse/README.md b/workhorse/README.md index c1ff104cda8..c7617645b34 100644 --- a/workhorse/README.md +++ b/workhorse/README.md @@ -9,17 +9,11 @@ GitLab](doc/architecture/gitlab_features.md) that would not work efficiently wit ## Canonical source -The canonical source for Workhorse is currently -[gitlab-org/gitlab-workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse). -As explained in https://gitlab.com/groups/gitlab-org/-/epics/4826, we -are in the process of moving the canonical source to +The canonical source for Workhorse is [gitlab-org/gitlab/workhorse](https://gitlab.com/gitlab-org/gitlab/tree/master/workhorse). - -Until that transition is complete, changes (Merge Requests) for -Workhorse should be submitted at -[gitlab-org/gitlab-workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse). -Once merged, they will propagate to gitlab-org/gitlab/workhorse via -the usual Workhorse release process. +Prior to https://gitlab.com/groups/gitlab-org/-/epics/4826, it was +[gitlab-org/gitlab-workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse/tree/master), +but that repository is no longer used for development. ## Documentation diff --git a/workhorse/VERSION b/workhorse/VERSION index 48309c07a55..6ff19de4b80 100644..120000 --- a/workhorse/VERSION +++ b/workhorse/VERSION @@ -1 +1 @@ -8.63.2 +../VERSION
\ No newline at end of file diff --git a/workhorse/config_test.go b/workhorse/config_test.go index f9d12bd5e2b..cf33e5bb7ca 100644 --- a/workhorse/config_test.go +++ b/workhorse/config_test.go @@ -85,7 +85,7 @@ func TestConfigDefaults(t *testing.T) { DocumentRoot: "public", ProxyHeadersTimeout: 5 * time.Minute, APIQueueTimeout: queueing.DefaultTimeout, - APICILongPollingDuration: 50 * time.Nanosecond, // TODO this is meant to be 50*time.Second but it has been wrong for ages + APICILongPollingDuration: 50 * time.Second, ImageResizerConfig: config.DefaultImageResizerConfig, } diff --git a/workhorse/go.mod b/workhorse/go.mod index 20344f0081d..6396e419487 100644 --- a/workhorse/go.mod +++ b/workhorse/go.mod @@ -8,8 +8,10 @@ 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 ddb08a1e846..4796d40638b 100644 --- a/workhorse/go.sum +++ b/workhorse/go.sum @@ -145,6 +145,8 @@ 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 deleted file mode 100644 index 72a32c8d349..00000000000 --- a/workhorse/internal/errortracker/sentry.go +++ /dev/null @@ -1,60 +0,0 @@ -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 2e23f50b913..f9b46181579 100644 --- a/workhorse/internal/helper/helpers.go +++ b/workhorse/internal/helper/helpers.go @@ -14,31 +14,50 @@ import ( "syscall" "github.com/sebest/xff" - - "gitlab.com/gitlab-org/gitlab-workhorse/internal/log" + "gitlab.com/gitlab-org/labkit/log" + "gitlab.com/gitlab-org/labkit/mask" ) const NginxResponseBufferHeader = "X-Accel-Buffering" -func CaptureAndFail(w http.ResponseWriter, r *http.Request, err error, msg string, code int, loggerCallbacks ...log.ConfigureLogger) { - http.Error(w, msg, code) - logger := log.WithRequest(r).WithError(err) - - for _, cb := range loggerCallbacks { - logger = cb(logger) +func logErrorWithFields(r *http.Request, err error, fields log.Fields) { + if err != nil { + CaptureRavenError(r, err, fields) } - logger.Error(msg) + printError(r, err, fields) +} + +func CaptureAndFail(w http.ResponseWriter, r *http.Request, err error, msg string, code int) { + http.Error(w, msg, code) + logErrorWithFields(r, err, nil) +} + +func Fail500(w http.ResponseWriter, r *http.Request, err error) { + CaptureAndFail(w, r, err, "Internal server error", http.StatusInternalServerError) } -func Fail500(w http.ResponseWriter, r *http.Request, err error, loggerCallbacks ...log.ConfigureLogger) { - CaptureAndFail(w, r, err, "Internal server error", http.StatusInternalServerError, loggerCallbacks...) +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 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") @@ -78,7 +97,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).WithFields(log.Fields{"url": s}).Error("urlMustParse") + log.WithError(err).WithField("url", s).Fatal("urlMustParse") } return u } diff --git a/workhorse/internal/helper/raven.go b/workhorse/internal/helper/raven.go new file mode 100644 index 00000000000..898e8ec85f8 --- /dev/null +++ b/workhorse/internal/helper/raven.go @@ -0,0 +1,58 @@ +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 7d423b80067..69e9496aec2 100644 --- a/workhorse/internal/imageresizer/image_resizer.go +++ b/workhorse/internal/imageresizer/image_resizer.go @@ -428,18 +428,16 @@ 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) - logger := log.WithRequest(req).WithFields(fields) + log := log.WithRequest(req).WithFields(fields) switch outcome.status { case statusRequestFailure: if outcome.bytesWritten <= 0 { - helper.Fail500(w, req, outcome.err, func(b *log.Builder) *log.Builder { - return b.WithFields(fields) - }) + helper.Fail500WithFields(w, req, outcome.err, fields) } else { - logger.WithError(outcome.err).Error(outcome.status) + log.WithError(outcome.err).Error(outcome.status) } default: - logger.Info(outcome.status) + log.Info(outcome.status) } } diff --git a/workhorse/internal/log/logging.go b/workhorse/internal/log/logging.go index 9c19cde1395..c65ec07743a 100644 --- a/workhorse/internal/log/logging.go +++ b/workhorse/internal/log/logging.go @@ -8,13 +8,11 @@ import ( "gitlab.com/gitlab-org/labkit/mask" "golang.org/x/net/context" - "gitlab.com/gitlab-org/gitlab-workhorse/internal/errortracker" + "gitlab.com/gitlab-org/gitlab-workhorse/internal/helper" ) type Fields = log.Fields -type ConfigureLogger func(*Builder) *Builder - type Builder struct { entry *logrus.Entry fields log.Fields @@ -85,6 +83,6 @@ func (b *Builder) Error(args ...interface{}) { b.entry.Error(args...) if b.req != nil && b.err != nil { - errortracker.TrackFailedRequest(b.req, b.err, b.fields) + helper.CaptureRavenError(b.req, b.err, b.fields) } } diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go index b834f155185..80e7d4056b6 100644 --- a/workhorse/internal/upstream/upstream.go +++ b/workhorse/internal/upstream/upstream.go @@ -16,7 +16,6 @@ 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" @@ -68,7 +67,7 @@ func newUpstream(cfg config.Config, accessLogger *logrus.Logger, routesCallback correlationOpts = append(correlationOpts, correlation.WithPropagation()) } - handler := correlation.InjectCorrelationID(errortracker.NewHandler(&up), correlationOpts...) + handler := correlation.InjectCorrelationID(&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 c43ec45240f..28162a00fae 100644 --- a/workhorse/main.go +++ b/workhorse/main.go @@ -16,7 +16,6 @@ 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" @@ -103,7 +102,7 @@ func buildConfig(arg0 string, args []string) (*bootConfig, *config.Config, error fset.UintVar(&cfg.APILimit, "apiLimit", 0, "Number of API requests allowed at single time") fset.UintVar(&cfg.APIQueueLimit, "apiQueueLimit", 0, "Number of API requests allowed to be queued") fset.DurationVar(&cfg.APIQueueTimeout, "apiQueueDuration", queueing.DefaultTimeout, "Maximum queueing duration of requests") - fset.DurationVar(&cfg.APICILongPollingDuration, "apiCiLongPollingDuration", 50, "Long polling duration for job requesting for runners (default 50s - enabled)") + fset.DurationVar(&cfg.APICILongPollingDuration, "apiCiLongPollingDuration", 50*time.Second, "Long polling duration for job requesting for runners (default 50s - enabled)") fset.BoolVar(&cfg.PropagateCorrelationID, "propagateCorrelationID", false, "Reuse existing Correlation-ID from the incoming request header `X-Request-ID` if present") if err := fset.Parse(args); err != nil { @@ -157,8 +156,6 @@ 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") @@ -226,7 +223,7 @@ func run(boot bootConfig, cfg config.Config) error { } defer accessCloser.Close() - up := upstream.NewUpstream(cfg, accessLogger) + up := wrapRaven(upstream.NewUpstream(cfg, accessLogger)) go func() { finalErrors <- http.Serve(listener, up) }() diff --git a/workhorse/raven.go b/workhorse/raven.go new file mode 100644 index 00000000000..f641203f142 --- /dev/null +++ b/workhorse/raven.go @@ -0,0 +1,40 @@ +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) + })) +} |