summaryrefslogtreecommitdiff
path: root/workhorse
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-03-16 18:18:33 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-03-16 18:18:33 +0000
commitf64a639bcfa1fc2bc89ca7db268f594306edfd7c (patch)
treea2c3c2ebcc3b45e596949db485d6ed18ffaacfa1 /workhorse
parentbfbc3e0d6583ea1a91f627528bedc3d65ba4b10f (diff)
downloadgitlab-ce-f64a639bcfa1fc2bc89ca7db268f594306edfd7c.tar.gz
Add latest changes from gitlab-org/gitlab@13-10-stable-eev13.10.0-rc40
Diffstat (limited to 'workhorse')
-rw-r--r--workhorse/CHANGELOG16
-rw-r--r--workhorse/PROCESS.md4
-rw-r--r--workhorse/README.md14
l---------[-rw-r--r--]workhorse/VERSION2
-rw-r--r--workhorse/config_test.go2
-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/upstream.go3
-rw-r--r--workhorse/main.go7
-rw-r--r--workhorse/raven.go40
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)
+ }))
+}