diff options
Diffstat (limited to 'workhorse')
58 files changed, 685 insertions, 779 deletions
diff --git a/workhorse/gitaly_integration_test.go b/workhorse/gitaly_integration_test.go index a2826c3edc4..ed44aaddbc3 100644 --- a/workhorse/gitaly_integration_test.go +++ b/workhorse/gitaly_integration_test.go @@ -58,7 +58,6 @@ func ensureGitalyRepository(t *testing.T, apiResponse *api.Response) error { ctx, namespace, err := gitaly.NewNamespaceClient( context.Background(), apiResponse.GitalyServer, - gitaly.WithFeatures(apiResponse.GitalyServer.Features), ) if err != nil { diff --git a/workhorse/gitaly_test.go b/workhorse/gitaly_test.go index 234a11e5dc9..2d7f727003f 100644 --- a/workhorse/gitaly_test.go +++ b/workhorse/gitaly_test.go @@ -78,7 +78,7 @@ func TestGetInfoRefsProxiedToGitalySuccessfully(t *testing.T) { for k, v := range badMetadata { features[k] = v } - apiResponse.GitalyServer.Features = features + apiResponse.GitalyServer.CallMetadata = features testCases := []struct { showAllRefs bool diff --git a/workhorse/go.mod b/workhorse/go.mod index 51adab831c2..80c017ad1cb 100644 --- a/workhorse/go.mod +++ b/workhorse/go.mod @@ -6,18 +6,18 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v0.4.1 github.com/BurntSushi/toml v1.2.1 github.com/FZambia/sentinel v1.1.1 - github.com/alecthomas/chroma/v2 v2.3.0 - github.com/aws/aws-sdk-go v1.44.136 + github.com/alecthomas/chroma/v2 v2.4.0 + github.com/aws/aws-sdk-go v1.44.157 github.com/disintegration/imaging v1.6.2 github.com/getsentry/raven-go v0.2.0 - github.com/golang-jwt/jwt/v4 v4.4.2 + github.com/golang-jwt/jwt/v4 v4.4.3 github.com/golang/gddo v0.0.0-20210115222349-20d68f94ee1f github.com/golang/protobuf v1.5.2 github.com/gomodule/redigo v2.0.0+incompatible github.com/gorilla/websocket v1.5.0 github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 - github.com/johannesboyne/gofakes3 v0.0.0-20221110173912-32fb85c5aed6 + github.com/johannesboyne/gofakes3 v0.0.0-20221128113635-c2f5cc6b5294 github.com/jpillora/backoff v1.0.0 github.com/mitchellh/copystructure v1.2.0 github.com/prometheus/client_golang v1.14.0 @@ -26,7 +26,7 @@ require ( github.com/sirupsen/logrus v1.9.0 github.com/smartystreets/goconvey v1.7.2 github.com/stretchr/testify v1.8.1 - gitlab.com/gitlab-org/gitaly/v15 v15.5.1 + gitlab.com/gitlab-org/gitaly/v15 v15.6.2 gitlab.com/gitlab-org/golang-archive-zip v0.1.1 gitlab.com/gitlab-org/labkit v1.16.1 gocloud.dev v0.27.0 @@ -35,7 +35,7 @@ require ( golang.org/x/net v0.1.0 golang.org/x/oauth2 v0.0.0-20220722155238-128564f6959c golang.org/x/tools v0.1.12 - google.golang.org/grpc v1.50.1 + google.golang.org/grpc v1.51.0 google.golang.org/protobuf v1.28.1 honnef.co/go/tools v0.3.3 ) @@ -69,7 +69,7 @@ require ( github.com/dlclark/regexp2 v1.4.0 // indirect github.com/go-ole/go-ole v1.2.4 // indirect github.com/gogo/protobuf v1.3.2 // indirect - github.com/golang-jwt/jwt v3.2.1+incompatible // indirect + github.com/golang-jwt/jwt v3.2.2+incompatible // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/google/go-cmp v0.5.9 // indirect github.com/google/pprof v0.0.0-20220608213341-c488b8fa1db3 // indirect @@ -107,13 +107,13 @@ require ( github.com/uber/jaeger-lib v2.4.1+incompatible // indirect go.opencensus.io v0.23.0 // indirect go.uber.org/atomic v1.9.0 // indirect - golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa // indirect + golang.org/x/crypto v0.0.0-20220926161630-eccd6366d1be // indirect golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e // indirect golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect - golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 // indirect + golang.org/x/sync v0.1.0 // indirect golang.org/x/sys v0.1.0 // indirect golang.org/x/text v0.4.0 // indirect - golang.org/x/time v0.0.0-20220722155302-e5dcc9cfc0b9 // indirect + golang.org/x/time v0.2.0 // indirect golang.org/x/xerrors v0.0.0-20220609144429-65e65417b02f // indirect google.golang.org/api v0.91.0 // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/workhorse/go.sum b/workhorse/go.sum index bf0c7df390d..5e095f5b417 100644 --- a/workhorse/go.sum +++ b/workhorse/go.sum @@ -194,8 +194,9 @@ github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d h1:G0m3OIz70MZUW github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg= github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g= github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5/go.mod h1:SkGFH1ia65gfNATL8TAiHDNxPzPdmEL5uirI2Uyuz6c= -github.com/alecthomas/chroma/v2 v2.3.0 h1:83xfxrnjv8eK+Cf8qZDzNo3PPF9IbTWHs7z28GY6D0U= -github.com/alecthomas/chroma/v2 v2.3.0/go.mod h1:mZxeWZlxP2Dy+/8cBob2PYd8O2DwNAzave5AY7A2eQw= +github.com/alecthomas/assert/v2 v2.2.0 h1:f6L/b7KE2bfA+9O4FL3CM/xJccDEwPVYd5fALBiuwvw= +github.com/alecthomas/chroma/v2 v2.4.0 h1:Loe2ZjT5x3q1bcWwemqyqEi8p11/IV/ncFCeLYDpWC4= +github.com/alecthomas/chroma/v2 v2.4.0/go.mod h1:6kHzqF5O6FUSJzBXW7fXELjb+e+7OXW4UpoPqMO7IBQ= github.com/alecthomas/repr v0.1.0 h1:ENn2e1+J3k09gyj2shc0dHr/yjaWSHRlrJ4DPMevDqE= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= @@ -227,8 +228,8 @@ github.com/aws/aws-sdk-go v1.43.11/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4 github.com/aws/aws-sdk-go v1.43.31/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/aws/aws-sdk-go v1.44.45/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/aws/aws-sdk-go v1.44.68/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= -github.com/aws/aws-sdk-go v1.44.136 h1:J1KJJssa8pjU8jETYUxwRS37KTcxjACfKd9GK8t+5ZU= -github.com/aws/aws-sdk-go v1.44.136/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI= +github.com/aws/aws-sdk-go v1.44.157 h1:JVBPpEWC8+yA7CbfAuTl/ZFFlHS3yoqWFqxFyTCISwg= +github.com/aws/aws-sdk-go v1.44.157/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI= github.com/aws/aws-sdk-go-v2 v0.18.0/go.mod h1:JWVYvqSMppoMJC0x5wdwiImzgXTI9FuZwxzkQq9wy+g= github.com/aws/aws-sdk-go-v2 v1.16.8 h1:gOe9UPR98XSf7oEJCcojYg+N2/jCRm4DdeIsP85pIyQ= github.com/aws/aws-sdk-go-v2 v1.16.8/go.mod h1:6CpKuLXg2w7If3ABZCl/qZ6rEgwtjZTn4eAf4RcEyuw= @@ -682,12 +683,14 @@ github.com/gogo/protobuf v1.3.0/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXP github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= -github.com/golang-jwt/jwt v3.2.1+incompatible h1:73Z+4BJcrTC+KczS6WvTPvRGOp1WmfEP4Q1lOd9Z/+c= github.com/golang-jwt/jwt v3.2.1+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I= +github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY= +github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I= github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= github.com/golang-jwt/jwt/v4 v4.2.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= -github.com/golang-jwt/jwt/v4 v4.4.2 h1:rcc4lwaZgFMCZ5jxF9ABolDcIHdBytAFgqFPbSJQAYs= github.com/golang-jwt/jwt/v4 v4.4.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang-jwt/jwt/v4 v4.4.3 h1:Hxl6lhQFj4AnOX6MLrsCb/+7tCj7DxP7VA+2rDIq5AU= +github.com/golang-jwt/jwt/v4 v4.4.3/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0= github.com/golang-sql/sqlexp v0.1.0/go.mod h1:J4ad9Vo8ZCWQ2GMrC4UCQy1JpCbwU9m3EOqtpKwwwHI= github.com/golang/gddo v0.0.0-20210115222349-20d68f94ee1f h1:16RtHeWGkJMc80Etb8RPCcKevXGldr57+LOyZt8zOlg= @@ -909,6 +912,7 @@ github.com/hashicorp/yamux v0.1.1 h1:yrQxtgseBDrq9Y652vSRDvsKCJKOUD+GzTS4Y0Y8pvE github.com/hashicorp/yamux v0.1.1/go.mod h1:CtWFDAQgb7dxtzFs4tWbplKIe2jSi3+5vKbgIO0SLnQ= github.com/hetznercloud/hcloud-go v1.33.1/go.mod h1:XX/TQub3ge0yWR2yHWmnDVIrB+MQbda1pHxkUmDlUME= github.com/hetznercloud/hcloud-go v1.35.0/go.mod h1:mepQwR6va27S3UQthaEPGS86jtzSY9xWL1e9dyxXpgA= +github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/hudl/fargo v1.3.0/go.mod h1:y3CKSmjA+wD2gak7sUSXTAoopbhU08POFhmITJgmKTg= github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho= @@ -975,8 +979,8 @@ github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHW github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/joefitzgerald/rainbow-reporter v0.1.0/go.mod h1:481CNgqmVHQZzdIbN52CupLJyoVwB10FQ/IQlF1pdL8= -github.com/johannesboyne/gofakes3 v0.0.0-20221110173912-32fb85c5aed6 h1:eQGUsj2LcsLzfrHY1noKDSU7h+c9/rw9pQPwbQ9g1jQ= -github.com/johannesboyne/gofakes3 v0.0.0-20221110173912-32fb85c5aed6/go.mod h1:LIAXxPvcUXwOcTIj9LSNSUpE9/eMHalTWxsP/kmWxQI= +github.com/johannesboyne/gofakes3 v0.0.0-20221128113635-c2f5cc6b5294 h1:AJISYN7tPo3lGqwYmEYQdlftcQz48i8LNk/BRUKCTig= +github.com/johannesboyne/gofakes3 v0.0.0-20221128113635-c2f5cc6b5294/go.mod h1:LIAXxPvcUXwOcTIj9LSNSUpE9/eMHalTWxsP/kmWxQI= github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqxOKXbg= github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/jonboulle/clockwork v0.2.2/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8= @@ -1482,8 +1486,8 @@ github.com/yvasiyarov/go-metrics v0.0.0-20140926110328-57bccd1ccd43/go.mod h1:aX github.com/yvasiyarov/gorelic v0.0.0-20141212073537-a9bba5b9ab50/go.mod h1:NUSPSUX/bi6SeDMUh6brw0nXpxHnc96TguQh0+r/ssA= github.com/yvasiyarov/newrelic_platform_go v0.0.0-20140908184405-b21fdbd4370f/go.mod h1:GlGEuHIJweS1mbCqG+7vt2nvWLzLLnRHbXz5JKd/Qbg= github.com/zenazn/goji v0.9.0/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q= -gitlab.com/gitlab-org/gitaly/v15 v15.5.1 h1:EbkAYAeTLllJzX3N3Sy3ZcmKtBzI5OovT5c5MWI16Bo= -gitlab.com/gitlab-org/gitaly/v15 v15.5.1/go.mod h1:G5q5H6OYMSEDnKXsQoYTzI+ysCTfM4Of2z0v6xeHtRY= +gitlab.com/gitlab-org/gitaly/v15 v15.6.2 h1:ivbMoXWgkDSJebuIFtPYGAIQ9/2P5ShxJoHt0cflwfo= +gitlab.com/gitlab-org/gitaly/v15 v15.6.2/go.mod h1:RKa+3ADKfTonDb1pe8AtppdNHNeOM+ChtMmB7T0QWhY= gitlab.com/gitlab-org/golang-archive-zip v0.1.1 h1:35k9giivbxwF03+8A05Cm8YoxoakU8FBCj5gysjCTCE= gitlab.com/gitlab-org/golang-archive-zip v0.1.1/go.mod h1:ZDtqpWPGPB9qBuZnZDrKQjIdJtkN7ZAoVwhT6H2o2kE= gitlab.com/gitlab-org/labkit v1.16.1 h1:J+HmNVR5bvPfrv9/fgKICFis2nmEugRXHMeRPvsVZUg= @@ -1616,8 +1620,9 @@ golang.org/x/crypto v0.0.0-20211202192323-5770296d904e/go.mod h1:IxCIyHEi3zRg3s0 golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220214200702-86341886e292/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220511200225-c6db032c6c88/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa h1:zuSxTR4o9y82ebqCUJYNGJbGPo6sKVl54f/TVDObg1c= golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= +golang.org/x/crypto v0.0.0-20220926161630-eccd6366d1be h1:fmw3UbQh+nxngCAHrDCCztao/kbYFnWjoqop8dHx05A= +golang.org/x/crypto v0.0.0-20220926161630-eccd6366d1be/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -1785,8 +1790,9 @@ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220601150217-0de741cfad7f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 h1:uVc8UZUe6tr40fFVnUP5Oj+veunVezqYl9z7DYw9xzw= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= +golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -1960,8 +1966,9 @@ golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac/go.mod h1:tRJNPiyCQ0inRvYxb golang.org/x/time v0.0.0-20220210224613-90d013bbcef8/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20220224211638-0e9765cccd65/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20220609170525-579cf78fd858/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= -golang.org/x/time v0.0.0-20220722155302-e5dcc9cfc0b9 h1:ftMN5LMiBFjbzleLqtoBZk7KdJwhuybIU+FckUHgoyQ= golang.org/x/time v0.0.0-20220722155302-e5dcc9cfc0b9/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= +golang.org/x/time v0.2.0 h1:52I/1L54xyEQAYdtcSuxtiT84KGYTBGXwayxmIpNJhE= +golang.org/x/time v0.2.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20180828015842-6cd1fcedba52/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -2260,8 +2267,8 @@ google.golang.org/grpc v1.46.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACu google.golang.org/grpc v1.46.2/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk= google.golang.org/grpc v1.47.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk= google.golang.org/grpc v1.48.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk= -google.golang.org/grpc v1.50.1 h1:DS/BukOZWp8s6p4Dt/tOaJaTQyPyOoCcrjroHuCeLzY= -google.golang.org/grpc v1.50.1/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI= +google.golang.org/grpc v1.51.0 h1:E1eGv1FTqoLIdnBCZufiSHgKjlqG6fKFf6pPWtMTh8U= +google.golang.org/grpc v1.51.0/go.mod h1:wgNDFcnuBGmxLKI/qn4T+m5BtEBYXJPvibbUPsAIPww= google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go index 6a6a51b27bb..1758bb5a6a8 100644 --- a/workhorse/internal/api/api.go +++ b/workhorse/internal/api/api.go @@ -18,6 +18,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/secret" ) @@ -128,6 +129,7 @@ type Response struct { // GL_REPOSITORY is an environment variable used by gitlab-shell hooks during // 'git push' and 'git pull' GL_REPOSITORY string + // GitConfigOptions holds the custom options that we want to pass to the git command GitConfigOptions []string // StoreLFSPath is provided by the GitLab Rails application to mark where the tmp file should be placed. @@ -162,9 +164,9 @@ type Response struct { } type GitalyServer struct { - Address string `json:"address"` - Token string `json:"token"` - Features map[string]string `json:"features"` + Address string `json:"address"` + Token string `json:"token"` + CallMetadata map[string]string `json:"call_metadata"` } // singleJoiningSlash is taken from reverseproxy.go:singleJoiningSlash @@ -225,7 +227,7 @@ func (api *API) newRequest(r *http.Request, suffix string) (*http.Request, error authReq := &http.Request{ Method: r.Method, URL: rebaseUrl(r.URL, api.URL, suffix), - Header: helper.HeaderClone(r.Header), + Header: r.Header.Clone(), } authReq = authReq.WithContext(r.Context()) @@ -306,7 +308,7 @@ func (api *API) PreAuthorizeFixedPath(r *http.Request, method string, path strin if err != nil { return nil, fmt.Errorf("construct auth request: %w", err) } - authReq.Header = helper.HeaderClone(r.Header) + authReq.Header = r.Header.Clone() authReq.URL.RawQuery = r.URL.RawQuery failureResponse, apiResponse, err := api.PreAuthorize(path, authReq) @@ -334,7 +336,7 @@ func (api *API) PreAuthorizeHandler(next HandleFunc, suffix string) http.Handler } if err != nil { - helper.Fail500(w, r, err) + fail.Request(w, r, err) return } @@ -360,7 +362,7 @@ func (api *API) doRequestWithoutRedirects(authReq *http.Request) (*http.Response } // removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h. -// See https://tools.ietf.org/html/rfc7230#section-6.1 +// See https://www.rfc-editor.org/rfc/rfc7230#section-6.1 func removeConnectionHeaders(h http.Header) { for _, f := range h["Connection"] { for _, sf := range strings.Split(f, ",") { @@ -389,7 +391,7 @@ func passResponseBack(httpResponse *http.Response, w http.ResponseWriter, r *htt // the entire response body in memory before sending it on. responseBody, err := bufferResponse(httpResponse.Body) if err != nil { - helper.Fail500(w, r, err) + fail.Request(w, r, err) return } httpResponse.Body.Close() // Free up the Puma thread diff --git a/workhorse/internal/api/block.go b/workhorse/internal/api/block.go index 43763fc2b13..aac43f8cf77 100644 --- a/workhorse/internal/api/block.go +++ b/workhorse/internal/api/block.go @@ -5,6 +5,7 @@ import ( "net/http" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" ) // Prevent internal API responses intended for gitlab-workhorse from @@ -48,7 +49,7 @@ func (b *blocker) WriteHeader(status int) { b.status = 500 b.Header().Del("Content-Length") b.hijacked = true - helper.Fail500(b.rw, b.r, fmt.Errorf("api.blocker: forbidden content-type: %q", ResponseContentType)) + fail.Request(b.rw, b.r, fmt.Errorf("api.blocker: forbidden content-type: %q", ResponseContentType)) return } diff --git a/workhorse/internal/api/block_test.go b/workhorse/internal/api/block_test.go index 0beb401d2f5..c1ffe93dfb8 100644 --- a/workhorse/internal/api/block_test.go +++ b/workhorse/internal/api/block_test.go @@ -20,7 +20,7 @@ func TestBlocker(t *testing.T) { { desc: "blocked", contentType: ResponseContentType, - out: "Internal server error\n", + out: "Internal Server Error\n", }, { desc: "pass", diff --git a/workhorse/internal/api/channel_settings.go b/workhorse/internal/api/channel_settings.go index 91798334a03..ed03b04a69b 100644 --- a/workhorse/internal/api/channel_settings.go +++ b/workhorse/internal/api/channel_settings.go @@ -8,8 +8,6 @@ import ( "net/url" "github.com/gorilla/websocket" - - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" ) type ChannelSettings struct { @@ -53,7 +51,10 @@ func (t *ChannelSettings) Dialer() *websocket.Dialer { func (t *ChannelSettings) Clone() *ChannelSettings { // Doesn't clone the strings, but that's OK as strings are immutable in go cloned := *t - cloned.Header = helper.HeaderClone(t.Header) + cloned.Header = t.Header.Clone() + if cloned.Header == nil { + cloned.Header = make(http.Header) + } return &cloned } diff --git a/workhorse/internal/artifacts/entry.go b/workhorse/internal/artifacts/entry.go index d5b3dfb672c..e2eef174989 100644 --- a/workhorse/internal/artifacts/entry.go +++ b/workhorse/internal/artifacts/entry.go @@ -16,7 +16,8 @@ import ( "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/labkit/mask" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/command" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" "gitlab.com/gitlab-org/gitlab/workhorse/internal/zipartifacts" ) @@ -30,7 +31,7 @@ var SendEntry = &entry{"artifacts-entry:"} func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) { var params entryParams if err := e.Unpack(¶ms, sendData); err != nil { - helper.Fail500(w, r, fmt.Errorf("SendEntry: unpack sendData: %v", err)) + fail.Request(w, r, fmt.Errorf("SendEntry: unpack sendData: %v", err)) return } @@ -41,7 +42,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) }).Print("SendEntry: sending") if params.Archive == "" || params.Entry == "" { - helper.Fail500(w, r, fmt.Errorf("SendEntry: Archive or Entry is empty")) + fail.Request(w, r, fmt.Errorf("SendEntry: Archive or Entry is empty")) return } @@ -50,7 +51,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) if os.IsNotExist(err) { http.NotFound(w, r) } else if err != nil { - helper.Fail500(w, r, fmt.Errorf("SendEntry: %v", err)) + fail.Request(w, r, fmt.Errorf("SendEntry: %v", err)) } } @@ -83,7 +84,7 @@ func unpackFileFromZip(ctx context.Context, archivePath, encodedFilename string, if err := catFile.Start(); err != nil { return fmt.Errorf("start %v: %v", catFile.Args, err) } - defer helper.CleanUpProcessGroup(catFile) + defer command.KillProcessGroup(catFile) basename := filepath.Base(fileName) reader := bufio.NewReader(stdout) @@ -114,7 +115,7 @@ func waitCatFile(cmd *exec.Cmd) error { return nil } - st, ok := helper.ExitStatus(err) + st, ok := command.ExitStatus(err) if ok && (st == zipartifacts.CodeArchiveNotFound || st == zipartifacts.CodeEntryNotFound) { return os.ErrNotExist diff --git a/workhorse/internal/builds/register.go b/workhorse/internal/builds/register.go index f28ad75e1d8..0a2fe47ed7e 100644 --- a/workhorse/internal/builds/register.go +++ b/workhorse/internal/builds/register.go @@ -1,8 +1,10 @@ package builds import ( + "bytes" "encoding/json" "errors" + "io" "net/http" "time" @@ -10,6 +12,7 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/redis" ) @@ -63,11 +66,18 @@ func readRunnerBody(w http.ResponseWriter, r *http.Request) ([]byte, error) { registerHandlerOpenAtReading.Inc() defer registerHandlerOpenAtReading.Dec() - return helper.ReadRequestBody(w, r, maxRegisterBodySize) + return readRequestBody(w, r, maxRegisterBodySize) +} + +func readRequestBody(w http.ResponseWriter, r *http.Request, maxBodySize int64) ([]byte, error) { + limitedBody := http.MaxBytesReader(w, r.Body, maxBodySize) + defer limitedBody.Close() + + return io.ReadAll(limitedBody) } func readRunnerRequest(r *http.Request, body []byte) (*runnerRequest, error) { - if !helper.IsApplicationJson(r) { + if !isApplicationJson(r) { return nil, errors.New("invalid content-type received") } @@ -80,6 +90,11 @@ func readRunnerRequest(r *http.Request, body []byte) (*runnerRequest, error) { return &runnerRequest, nil } +func isApplicationJson(r *http.Request) bool { + contentType := r.Header.Get("Content-Type") + return helper.IsContentType("application/json", contentType) +} + func proxyRegisterRequest(h http.Handler, w http.ResponseWriter, r *http.Request) { registerHandlerOpenAtProxying.Inc() defer registerHandlerOpenAtProxying.Dec() @@ -105,11 +120,12 @@ func RegisterHandler(h http.Handler, watchHandler WatchKeyHandler, pollingDurati requestBody, err := readRunnerBody(w, r) if err != nil { registerHandlerBodyReadErrors.Inc() - helper.RequestEntityTooLarge(w, r, &largeBodyError{err}) + fail.Request(w, r, &largeBodyError{err}, + fail.WithStatus(http.StatusRequestEntityTooLarge)) return } - newRequest := helper.CloneRequestWithNewBody(r, requestBody) + newRequest := cloneRequestWithNewBody(r, requestBody) runnerRequest, err := readRunnerRequest(r, requestBody) if err != nil { @@ -161,3 +177,10 @@ func RegisterHandler(h http.Handler, watchHandler WatchKeyHandler, pollingDurati } }) } + +func cloneRequestWithNewBody(r *http.Request, body []byte) *http.Request { + newReq := r.Clone(r.Context()) + newReq.Body = io.NopCloser(bytes.NewReader(body)) + newReq.ContentLength = int64(len(body)) + return newReq +} diff --git a/workhorse/internal/builds/register_test.go b/workhorse/internal/builds/register_test.go index 3c975f61003..d5cbebd500b 100644 --- a/workhorse/internal/builds/register_test.go +++ b/workhorse/internal/builds/register_test.go @@ -106,3 +106,50 @@ func TestRegisterHandlerWatcherNoChange(t *testing.T) { expectWatcherToBeExecuted(t, redis.WatchKeyStatusNoChange, nil, http.StatusNoContent) } + +func TestReadRequestBody(t *testing.T) { + data := []byte("123456") + rw := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/test", bytes.NewBuffer(data)) + + result, err := readRequestBody(rw, req, 1000) + require.NoError(t, err) + require.Equal(t, data, result) +} + +func TestReadRequestBodyLimit(t *testing.T) { + data := []byte("123456") + rw := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/test", bytes.NewBuffer(data)) + + _, err := readRequestBody(rw, req, 2) + require.Error(t, err) +} + +func TestApplicationJson(t *testing.T) { + req, _ := http.NewRequest("POST", "/test", nil) + req.Header.Set("Content-Type", "application/json") + + require.True(t, isApplicationJson(req), "expected to match 'application/json' as 'application/json'") + + req.Header.Set("Content-Type", "application/json; charset=utf-8") + require.True(t, isApplicationJson(req), "expected to match 'application/json; charset=utf-8' as 'application/json'") + + req.Header.Set("Content-Type", "text/plain") + require.False(t, isApplicationJson(req), "expected not to match 'text/plain' as 'application/json'") +} + +func TestCloneRequestWithBody(t *testing.T) { + input := []byte("test") + newInput := []byte("new body") + req, _ := http.NewRequest("POST", "/test", bytes.NewBuffer(input)) + newReq := cloneRequestWithNewBody(req, newInput) + + require.NotEqual(t, req, newReq) + require.NotEqual(t, req.Body, newReq.Body) + require.NotEqual(t, len(newInput), newReq.ContentLength) + + var buffer bytes.Buffer + io.Copy(&buffer, newReq.Body) + require.Equal(t, newInput, buffer.Bytes()) +} diff --git a/workhorse/internal/channel/channel.go b/workhorse/internal/channel/channel.go index e740015d54a..f8228620a83 100644 --- a/workhorse/internal/channel/channel.go +++ b/workhorse/internal/channel/channel.go @@ -2,7 +2,9 @@ package channel import ( "fmt" + "net" "net/http" + "strings" "time" "github.com/gorilla/websocket" @@ -10,7 +12,7 @@ import ( "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" ) var ( @@ -24,7 +26,7 @@ var ( func Handler(myAPI *api.API) http.Handler { return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) { if err := a.Channel.Validate(); err != nil { - helper.Fail500(w, r, err) + fail.Request(w, r, err) return } @@ -45,7 +47,7 @@ func Handler(myAPI *api.API) http.Handler { func ProxyChannel(w http.ResponseWriter, r *http.Request, settings *api.ChannelSettings, proxy *Proxy) { server, err := connectToServer(settings, r) if err != nil { - helper.Fail500(w, r, err) + fail.Request(w, r, err) log.ContextLogger(r.Context()).WithError(err).Print("Channel: connecting to server failed") return } @@ -109,7 +111,7 @@ func pingLoop(conn Connection) { func connectToServer(settings *api.ChannelSettings, r *http.Request) (Connection, error) { settings = settings.Clone() - helper.SetForwardedFor(&settings.Header, r) + setForwardedFor(&settings.Header, r) conn, _, err := settings.Dial() if err != nil { @@ -130,3 +132,19 @@ func closeAfterMaxTime(proxy *Proxy, maxSessionTime int) { maxSessionTime, ) } + +func setForwardedFor(newHeaders *http.Header, originalRequest *http.Request) { + if clientIP, _, err := net.SplitHostPort(originalRequest.RemoteAddr); err == nil { + var header string + + // If we aren't the first proxy retain prior + // X-Forwarded-For information as a comma+space + // separated list and fold multiple headers into one. + if prior, ok := originalRequest.Header["X-Forwarded-For"]; ok { + header = strings.Join(prior, ", ") + ", " + clientIP + } else { + header = clientIP + } + newHeaders.Set("X-Forwarded-For", header) + } +} diff --git a/workhorse/internal/channel/channel_test.go b/workhorse/internal/channel/channel_test.go new file mode 100644 index 00000000000..fade6e42c27 --- /dev/null +++ b/workhorse/internal/channel/channel_test.go @@ -0,0 +1,49 @@ +package channel + +import ( + "net/http" + "testing" +) + +func TestSetForwardedForGeneratesHeader(t *testing.T) { + testCases := []struct { + remoteAddr string + previousForwardedFor []string + expected string + }{ + { + "8.8.8.8:3000", + nil, + "8.8.8.8", + }, + { + "8.8.8.8:3000", + []string{"138.124.33.63, 151.146.211.237"}, + "138.124.33.63, 151.146.211.237, 8.8.8.8", + }, + { + "8.8.8.8:3000", + []string{"8.154.76.107", "115.206.118.179"}, + "8.154.76.107, 115.206.118.179, 8.8.8.8", + }, + } + for _, tc := range testCases { + headers := http.Header{} + originalRequest := http.Request{ + RemoteAddr: tc.remoteAddr, + } + + if tc.previousForwardedFor != nil { + originalRequest.Header = http.Header{ + "X-Forwarded-For": tc.previousForwardedFor, + } + } + + setForwardedFor(&headers, &originalRequest) + + result := headers.Get("X-Forwarded-For") + if result != tc.expected { + t.Fatalf("Expected %v, got %v", tc.expected, result) + } + } +} diff --git a/workhorse/internal/dependencyproxy/dependencyproxy.go b/workhorse/internal/dependencyproxy/dependencyproxy.go index 6651b5aee84..e170b001806 100644 --- a/workhorse/internal/dependencyproxy/dependencyproxy.go +++ b/workhorse/internal/dependencyproxy/dependencyproxy.go @@ -8,7 +8,7 @@ import ( "gitlab.com/gitlab-org/labkit/log" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" ) @@ -57,7 +57,7 @@ func (p *Injector) SetUploadHandler(uploadHandler http.Handler) { func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData string) { dependencyResponse, err := p.fetchUrl(r.Context(), sendData) if err != nil { - helper.Fail500(w, r, err) + fail.Request(w, r, err) return } defer dependencyResponse.Body.Close() @@ -72,9 +72,9 @@ func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData strin teeReader := io.TeeReader(dependencyResponse.Body, w) saveFileRequest, err := http.NewRequestWithContext(r.Context(), "POST", r.URL.String()+"/upload", teeReader) if err != nil { - helper.Fail500(w, r, fmt.Errorf("dependency proxy: failed to create request: %w", err)) + fail.Request(w, r, fmt.Errorf("dependency proxy: failed to create request: %w", err)) } - saveFileRequest.Header = helper.HeaderClone(r.Header) + saveFileRequest.Header = r.Header.Clone() // forward headers from dependencyResponse to rails and client for key, values := range dependencyResponse.Header { @@ -96,7 +96,7 @@ func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData strin if nrw.status != http.StatusOK { fields := log.Fields{"code": nrw.status} - helper.Fail500WithFields(nrw, r, fmt.Errorf("dependency proxy: failed to upload file"), fields) + fail.Request(nrw, r, fmt.Errorf("dependency proxy: failed to upload file"), fail.WithFields(fields)) } } diff --git a/workhorse/internal/dependencyproxy/dependencyproxy_test.go b/workhorse/internal/dependencyproxy/dependencyproxy_test.go index 6056433f3b1..d893ddc500f 100644 --- a/workhorse/internal/dependencyproxy/dependencyproxy_test.go +++ b/workhorse/internal/dependencyproxy/dependencyproxy_test.go @@ -153,14 +153,14 @@ func TestIncorrectSendData(t *testing.T) { response := makeRequest(NewInjector(), "") require.Equal(t, 500, response.Code) - require.Equal(t, "Internal server error\n", response.Body.String()) + require.Equal(t, "Internal Server Error\n", response.Body.String()) } func TestIncorrectSendDataUrl(t *testing.T) { response := makeRequest(NewInjector(), `{"Token": "token", "Url": "url"}`) require.Equal(t, 500, response.Code) - require.Equal(t, "Internal server error\n", response.Body.String()) + require.Equal(t, "Internal Server Error\n", response.Body.String()) } func TestFailedOriginServer(t *testing.T) { diff --git a/workhorse/internal/git/archive.go b/workhorse/internal/git/archive.go index 4c7b519310f..3361a8bed44 100644 --- a/workhorse/internal/git/archive.go +++ b/workhorse/internal/git/archive.go @@ -23,7 +23,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" ) @@ -53,14 +53,14 @@ var ( func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string) { var params archiveParams if err := a.Unpack(¶ms, sendData); err != nil { - helper.Fail500(w, r, fmt.Errorf("SendArchive: unpack sendData: %v", err)) + fail.Request(w, r, fmt.Errorf("SendArchive: unpack sendData: %v", err)) return } urlPath := r.URL.Path format, ok := parseBasename(filepath.Base(urlPath)) if !ok { - helper.Fail500(w, r, fmt.Errorf("SendArchive: invalid format: %s", urlPath)) + fail.Request(w, r, fmt.Errorf("SendArchive: invalid format: %s", urlPath)) return } @@ -93,7 +93,7 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string // to finalize the cached archive. tempFile, err = prepareArchiveTempfile(path.Dir(params.ArchivePath), archiveFilename) if err != nil { - helper.Fail500(w, r, fmt.Errorf("SendArchive: create tempfile: %v", err)) + fail.Request(w, r, fmt.Errorf("SendArchive: create tempfile: %v", err)) return } defer tempFile.Close() @@ -104,7 +104,7 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string archiveReader, err = handleArchiveWithGitaly(r, ¶ms, format) if err != nil { - helper.Fail500(w, r, fmt.Errorf("operations.GetArchive: %v", err)) + fail.Request(w, r, fmt.Errorf("operations.GetArchive: %v", err)) return } @@ -132,11 +132,7 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string func handleArchiveWithGitaly(r *http.Request, params *archiveParams, format gitalypb.GetArchiveRequest_Format) (io.Reader, error) { var request *gitalypb.GetArchiveRequest - ctx, c, err := gitaly.NewRepositoryClient( - r.Context(), - params.GitalyServer, - gitaly.WithFeatures(params.GitalyServer.Features), - ) + ctx, c, err := gitaly.NewRepositoryClient(r.Context(), params.GitalyServer) if err != nil { return nil, err diff --git a/workhorse/internal/git/blob.go b/workhorse/internal/git/blob.go index 39bd4490e66..06b0eb08228 100644 --- a/workhorse/internal/git/blob.go +++ b/workhorse/internal/git/blob.go @@ -8,7 +8,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" ) @@ -23,24 +23,20 @@ var SendBlob = &blob{"git-blob:"} func (b *blob) Inject(w http.ResponseWriter, r *http.Request, sendData string) { var params blobParams if err := b.Unpack(¶ms, sendData); err != nil { - helper.Fail500(w, r, fmt.Errorf("SendBlob: unpack sendData: %v", err)) + fail.Request(w, r, fmt.Errorf("SendBlob: unpack sendData: %v", err)) return } - ctx, blobClient, err := gitaly.NewBlobClient( - r.Context(), - params.GitalyServer, - gitaly.WithFeatures(params.GitalyServer.Features), - ) + ctx, blobClient, err := gitaly.NewBlobClient(r.Context(), params.GitalyServer) if err != nil { - helper.Fail500(w, r, fmt.Errorf("blob.GetBlob: %v", err)) + fail.Request(w, r, fmt.Errorf("blob.GetBlob: %v", err)) return } setBlobHeaders(w) if err := blobClient.SendBlob(ctx, w, ¶ms.GetBlobRequest); err != nil { - helper.Fail500(w, r, fmt.Errorf("blob.GetBlob: %v", err)) + fail.Request(w, r, fmt.Errorf("blob.GetBlob: %v", err)) return } } diff --git a/workhorse/internal/git/diff.go b/workhorse/internal/git/diff.go index b4878384e2b..d450d1b9034 100644 --- a/workhorse/internal/git/diff.go +++ b/workhorse/internal/git/diff.go @@ -8,7 +8,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" ) @@ -24,23 +24,19 @@ var SendDiff = &diff{"git-diff:"} func (d *diff) Inject(w http.ResponseWriter, r *http.Request, sendData string) { var params diffParams if err := d.Unpack(¶ms, sendData); err != nil { - helper.Fail500(w, r, fmt.Errorf("SendDiff: unpack sendData: %v", err)) + fail.Request(w, r, fmt.Errorf("SendDiff: unpack sendData: %v", err)) return } request := &gitalypb.RawDiffRequest{} if err := gitaly.UnmarshalJSON(params.RawDiffRequest, request); err != nil { - helper.Fail500(w, r, fmt.Errorf("diff.RawDiff: %v", err)) + fail.Request(w, r, fmt.Errorf("diff.RawDiff: %v", err)) return } - ctx, diffClient, err := gitaly.NewDiffClient( - r.Context(), - params.GitalyServer, - gitaly.WithFeatures(params.GitalyServer.Features), - ) + ctx, diffClient, err := gitaly.NewDiffClient(r.Context(), params.GitalyServer) if err != nil { - helper.Fail500(w, r, fmt.Errorf("diff.RawDiff: %v", err)) + fail.Request(w, r, fmt.Errorf("diff.RawDiff: %v", err)) return } diff --git a/workhorse/internal/git/format-patch.go b/workhorse/internal/git/format-patch.go index 264a4001232..a4306474aa5 100644 --- a/workhorse/internal/git/format-patch.go +++ b/workhorse/internal/git/format-patch.go @@ -8,7 +8,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" ) @@ -24,24 +24,20 @@ var SendPatch = &patch{"git-format-patch:"} func (p *patch) Inject(w http.ResponseWriter, r *http.Request, sendData string) { var params patchParams if err := p.Unpack(¶ms, sendData); err != nil { - helper.Fail500(w, r, fmt.Errorf("SendPatch: unpack sendData: %v", err)) + fail.Request(w, r, fmt.Errorf("SendPatch: unpack sendData: %v", err)) return } request := &gitalypb.RawPatchRequest{} if err := gitaly.UnmarshalJSON(params.RawPatchRequest, request); err != nil { - helper.Fail500(w, r, fmt.Errorf("diff.RawPatch: %v", err)) + fail.Request(w, r, fmt.Errorf("diff.RawPatch: %v", err)) return } - ctx, diffClient, err := gitaly.NewDiffClient( - r.Context(), - params.GitalyServer, - gitaly.WithFeatures(params.GitalyServer.Features), - ) + ctx, diffClient, err := gitaly.NewDiffClient(r.Context(), params.GitalyServer) if err != nil { - helper.Fail500(w, r, fmt.Errorf("diff.RawPatch: %v", err)) + fail.Request(w, r, fmt.Errorf("diff.RawPatch: %v", err)) return } diff --git a/workhorse/internal/git/info-refs.go b/workhorse/internal/git/info-refs.go index 2eaed388f60..3e0e4dcb3e5 100644 --- a/workhorse/internal/git/info-refs.go +++ b/workhorse/internal/git/info-refs.go @@ -14,7 +14,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" ) func GetInfoRefsHandler(a *api.API) http.Handler { @@ -47,21 +47,16 @@ func handleGetInfoRefs(rw http.ResponseWriter, r *http.Request, a *api.Response) err = fmt.Errorf("handleGetInfoRefs: %v", err) if status != nil && status.Code() == grpccodes.Unavailable { - helper.CaptureAndFail(responseWriter, r, err, "The git server, Gitaly, is not available at this time. Please contact your administrator.", http.StatusServiceUnavailable) + fail.Request(responseWriter, r, err, fail.WithStatus(http.StatusServiceUnavailable), + fail.WithBody("The git server, Gitaly, is not available at this time. Please contact your administrator.")) } else { - helper.Fail500(responseWriter, r, err) + fail.Request(responseWriter, r, err) } } } func handleGetInfoRefsWithGitaly(ctx context.Context, responseWriter *HttpResponseWriter, a *api.Response, rpc, gitProtocol, encoding string) error { - ctx, smarthttp, err := gitaly.NewSmartHTTPClient( - ctx, - a.GitalyServer, - gitaly.WithFeatures(a.GitalyServer.Features), - gitaly.WithUserID(a.GL_ID), - gitaly.WithUsername(a.GL_USERNAME), - ) + ctx, smarthttp, err := gitaly.NewSmartHTTPClient(ctx, a.GitalyServer) if err != nil { return err } diff --git a/workhorse/internal/helper/writeafterreader.go b/workhorse/internal/git/io.go index 3626d70e493..7b62b04395c 100644 --- a/workhorse/internal/helper/writeafterreader.go +++ b/workhorse/internal/git/io.go @@ -1,13 +1,48 @@ -package helper +package git import ( + "context" "fmt" "io" "os" "sync" ) -type WriteFlusher interface { +type contextReader struct { + ctx context.Context + underlyingReader io.Reader +} + +func newContextReader(ctx context.Context, underlyingReader io.Reader) *contextReader { + return &contextReader{ + ctx: ctx, + underlyingReader: underlyingReader, + } +} + +func (r *contextReader) Read(b []byte) (int, error) { + if r.canceled() { + return 0, r.err() + } + + n, err := r.underlyingReader.Read(b) + + if r.canceled() { + err = r.err() + } + + return n, err +} + +func (r *contextReader) canceled() bool { + return r.err() != nil +} + +func (r *contextReader) err() error { + return r.ctx.Err() +} + +type writeFlusher interface { io.Writer Flush() error } @@ -16,7 +51,7 @@ type WriteFlusher interface { // returned some error), all writes to w are sent to a tempfile first. // The caller must call Flush() on the returned WriteFlusher to ensure // all data is propagated to w. -func NewWriteAfterReader(r io.Reader, w io.Writer) (io.Reader, WriteFlusher) { +func newWriteAfterReader(r io.Reader, w io.Writer) (io.Reader, writeFlusher) { br := &busyReader{Reader: r} return br, &coupledWriter{Writer: w, busyReader: br} } diff --git a/workhorse/internal/helper/writeafterreader_test.go b/workhorse/internal/git/io_test.go index c3da428184b..f283c20c23c 100644 --- a/workhorse/internal/helper/writeafterreader_test.go +++ b/workhorse/internal/git/io_test.go @@ -1,17 +1,94 @@ -package helper +package git import ( "bytes" + "context" "fmt" "io" "testing" "testing/iotest" + "time" + + "github.com/stretchr/testify/require" ) +type fakeReader struct { + n int + err error +} + +func (f *fakeReader) Read(b []byte) (int, error) { + return f.n, f.err +} + +type fakeContextWithTimeout struct { + n int + threshold int +} + +func (*fakeContextWithTimeout) Deadline() (deadline time.Time, ok bool) { + return +} + +func (*fakeContextWithTimeout) Done() <-chan struct{} { + return nil +} + +func (*fakeContextWithTimeout) Value(key interface{}) interface{} { + return nil +} + +func (f *fakeContextWithTimeout) Err() error { + f.n++ + if f.n > f.threshold { + return context.DeadlineExceeded + } + + return nil +} + +func TestContextReaderRead(t *testing.T) { + underlyingReader := &fakeReader{n: 1, err: io.EOF} + + for _, tc := range []struct { + desc string + ctx *fakeContextWithTimeout + expectedN int + expectedErr error + }{ + { + desc: "Before and after read deadline checks are fine", + ctx: &fakeContextWithTimeout{n: 0, threshold: 2}, + expectedN: underlyingReader.n, + expectedErr: underlyingReader.err, + }, + { + desc: "Before read deadline check fails", + ctx: &fakeContextWithTimeout{n: 0, threshold: 0}, + expectedN: 0, + expectedErr: context.DeadlineExceeded, + }, + { + desc: "After read deadline check fails", + ctx: &fakeContextWithTimeout{n: 0, threshold: 1}, + expectedN: underlyingReader.n, + expectedErr: context.DeadlineExceeded, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + cr := newContextReader(tc.ctx, underlyingReader) + + n, err := cr.Read(nil) + require.Equal(t, tc.expectedN, n) + require.Equal(t, tc.expectedErr, err) + }) + } +} + func TestBusyReader(t *testing.T) { testData := "test data" r := testReader(testData) - br, _ := NewWriteAfterReader(r, &bytes.Buffer{}) + br, _ := newWriteAfterReader(r, &bytes.Buffer{}) result, err := io.ReadAll(br) if err != nil { @@ -25,7 +102,7 @@ func TestBusyReader(t *testing.T) { func TestFirstWriteAfterReadDone(t *testing.T) { writeRecorder := &bytes.Buffer{} - br, cw := NewWriteAfterReader(&bytes.Buffer{}, writeRecorder) + br, cw := newWriteAfterReader(&bytes.Buffer{}, writeRecorder) if _, err := io.Copy(io.Discard, br); err != nil { t.Fatalf("copy from busyreader: %v", err) } @@ -44,7 +121,7 @@ func TestFirstWriteAfterReadDone(t *testing.T) { func TestWriteDelay(t *testing.T) { writeRecorder := &bytes.Buffer{} w := &complainingWriter{Writer: writeRecorder} - br, cw := NewWriteAfterReader(&bytes.Buffer{}, w) + br, cw := newWriteAfterReader(&bytes.Buffer{}, w) testData1 := "1 test" if _, err := io.Copy(cw, testReader(testData1)); err != nil { diff --git a/workhorse/internal/git/receive-pack.go b/workhorse/internal/git/receive-pack.go index a85f0edccac..5e93c0f36d1 100644 --- a/workhorse/internal/git/receive-pack.go +++ b/workhorse/internal/git/receive-pack.go @@ -6,7 +6,6 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" ) // Will not return a non-nil error after the response body has been @@ -15,18 +14,12 @@ func handleReceivePack(w *HttpResponseWriter, r *http.Request, a *api.Response) action := getService(r) writePostRPCHeader(w, action) - cr, cw := helper.NewWriteAfterReader(r.Body, w) + cr, cw := newWriteAfterReader(r.Body, w) defer cw.Flush() gitProtocol := r.Header.Get("Git-Protocol") - ctx, smarthttp, err := gitaly.NewSmartHTTPClient( - r.Context(), - a.GitalyServer, - gitaly.WithFeatures(a.GitalyServer.Features), - gitaly.WithUserID(a.GL_ID), - gitaly.WithUsername(a.GL_USERNAME), - ) + ctx, smarthttp, err := gitaly.NewSmartHTTPClient(r.Context(), a.GitalyServer) if err != nil { return fmt.Errorf("smarthttp.ReceivePack: %v", err) } diff --git a/workhorse/internal/git/snapshot.go b/workhorse/internal/git/snapshot.go index 70832ec9211..777ecd144a8 100644 --- a/workhorse/internal/git/snapshot.go +++ b/workhorse/internal/git/snapshot.go @@ -9,7 +9,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" ) @@ -31,30 +31,26 @@ func (s *snapshot) Inject(w http.ResponseWriter, r *http.Request, sendData strin var params snapshotParams if err := s.Unpack(¶ms, sendData); err != nil { - helper.Fail500(w, r, fmt.Errorf("SendSnapshot: unpack sendData: %v", err)) + fail.Request(w, r, fmt.Errorf("SendSnapshot: unpack sendData: %v", err)) return } request := &gitalypb.GetSnapshotRequest{} if err := gitaly.UnmarshalJSON(params.GetSnapshotRequest, request); err != nil { - helper.Fail500(w, r, fmt.Errorf("SendSnapshot: unmarshal GetSnapshotRequest: %v", err)) + fail.Request(w, r, fmt.Errorf("SendSnapshot: unmarshal GetSnapshotRequest: %v", err)) return } - ctx, c, err := gitaly.NewRepositoryClient( - r.Context(), - params.GitalyServer, - gitaly.WithFeatures(params.GitalyServer.Features), - ) + ctx, c, err := gitaly.NewRepositoryClient(r.Context(), params.GitalyServer) if err != nil { - helper.Fail500(w, r, fmt.Errorf("SendSnapshot: gitaly.NewRepositoryClient: %v", err)) + fail.Request(w, r, fmt.Errorf("SendSnapshot: gitaly.NewRepositoryClient: %v", err)) return } reader, err := c.SnapshotReader(ctx, request) if err != nil { - helper.Fail500(w, r, fmt.Errorf("SendSnapshot: client.SnapshotReader: %v", err)) + fail.Request(w, r, fmt.Errorf("SendSnapshot: client.SnapshotReader: %v", err)) return } diff --git a/workhorse/internal/git/upload-pack.go b/workhorse/internal/git/upload-pack.go index bbed5224b2d..ef2a00bf3ac 100644 --- a/workhorse/internal/git/upload-pack.go +++ b/workhorse/internal/git/upload-pack.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" ) var ( @@ -31,8 +30,8 @@ func handleUploadPack(w *HttpResponseWriter, r *http.Request, a *api.Response) e readerCtx, cancel := context.WithTimeout(ctx, uploadPackTimeout) defer cancel() - limited := helper.NewContextReader(readerCtx, r.Body) - cr, cw := helper.NewWriteAfterReader(limited, w) + limited := newContextReader(readerCtx, r.Body) + cr, cw := newWriteAfterReader(limited, w) defer cw.Flush() action := getService(r) @@ -44,13 +43,7 @@ func handleUploadPack(w *HttpResponseWriter, r *http.Request, a *api.Response) e } func handleUploadPackWithGitaly(ctx context.Context, a *api.Response, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) error { - ctx, smarthttp, err := gitaly.NewSmartHTTPClient( - ctx, - a.GitalyServer, - gitaly.WithFeatures(a.GitalyServer.Features), - gitaly.WithUserID(a.GL_ID), - gitaly.WithUsername(a.GL_USERNAME), - ) + ctx, smarthttp, err := gitaly.NewSmartHTTPClient(ctx, a.GitalyServer) if err != nil { return fmt.Errorf("get gitaly client: %w", err) } diff --git a/workhorse/internal/gitaly/gitaly.go b/workhorse/internal/gitaly/gitaly.go index b695acbb688..af7425be1cf 100644 --- a/workhorse/internal/gitaly/gitaly.go +++ b/workhorse/internal/gitaly/gitaly.go @@ -67,42 +67,23 @@ func InitializeSidechannelRegistry(logger *logrus.Logger) { } } -type MetadataFunc func(metadata.MD) - -func WithUserID(userID string) MetadataFunc { - return func(md metadata.MD) { - md.Append("user_id", userID) - } +var allowedMetadataKeys = map[string]bool{ + "user_id": true, + "username": true, + "remote_ip": true, } -func WithUsername(username string) MetadataFunc { - return func(md metadata.MD) { - md.Append("username", username) - } -} - -func WithFeatures(features map[string]string) MetadataFunc { - return func(md metadata.MD) { - for k, v := range features { - if !strings.HasPrefix(k, "gitaly-feature-") { - continue - } - md.Append(k, v) - } - } -} - -func withOutgoingMetadata(ctx context.Context, addMetadataFuncs ...MetadataFunc) context.Context { +func withOutgoingMetadata(ctx context.Context, gs api.GitalyServer) context.Context { md := metadata.New(nil) - - for _, f := range addMetadataFuncs { - f(md) + for k, v := range gs.CallMetadata { + if strings.HasPrefix(k, "gitaly-feature-") || allowedMetadataKeys[k] { + md.Set(k, v) + } } - return metadata.NewOutgoingContext(ctx, md) } -func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer, metadataFuncs ...MetadataFunc) (context.Context, *SmartHTTPClient, error) { +func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer) (context.Context, *SmartHTTPClient, error) { conn, err := getOrCreateConnection(server) if err != nil { return nil, nil, err @@ -112,48 +93,44 @@ func NewSmartHTTPClient(ctx context.Context, server api.GitalyServer, metadataFu SmartHTTPServiceClient: grpcClient, sidechannelRegistry: sidechannelRegistry, } - - return withOutgoingMetadata( - ctx, - metadataFuncs..., - ), smartHTTPClient, nil + return withOutgoingMetadata(ctx, server), smartHTTPClient, nil } -func NewBlobClient(ctx context.Context, server api.GitalyServer, addMetadataFuncs ...MetadataFunc) (context.Context, *BlobClient, error) { +func NewBlobClient(ctx context.Context, server api.GitalyServer) (context.Context, *BlobClient, error) { conn, err := getOrCreateConnection(server) if err != nil { return nil, nil, err } grpcClient := gitalypb.NewBlobServiceClient(conn) - return withOutgoingMetadata(ctx, addMetadataFuncs...), &BlobClient{grpcClient}, nil + return withOutgoingMetadata(ctx, server), &BlobClient{grpcClient}, nil } -func NewRepositoryClient(ctx context.Context, server api.GitalyServer, addMetadataFuncs ...MetadataFunc) (context.Context, *RepositoryClient, error) { +func NewRepositoryClient(ctx context.Context, server api.GitalyServer) (context.Context, *RepositoryClient, error) { conn, err := getOrCreateConnection(server) if err != nil { return nil, nil, err } grpcClient := gitalypb.NewRepositoryServiceClient(conn) - return withOutgoingMetadata(ctx, addMetadataFuncs...), &RepositoryClient{grpcClient}, nil + return withOutgoingMetadata(ctx, server), &RepositoryClient{grpcClient}, nil } // NewNamespaceClient is only used by the Gitaly integration tests at present -func NewNamespaceClient(ctx context.Context, server api.GitalyServer, addMetadataFuncs ...MetadataFunc) (context.Context, *NamespaceClient, error) { +func NewNamespaceClient(ctx context.Context, server api.GitalyServer) (context.Context, *NamespaceClient, error) { conn, err := getOrCreateConnection(server) if err != nil { return nil, nil, err } grpcClient := gitalypb.NewNamespaceServiceClient(conn) - return withOutgoingMetadata(ctx, addMetadataFuncs...), &NamespaceClient{grpcClient}, nil + return withOutgoingMetadata(ctx, server), &NamespaceClient{grpcClient}, nil } -func NewDiffClient(ctx context.Context, server api.GitalyServer, addMetadataFuncs ...MetadataFunc) (context.Context, *DiffClient, error) { +func NewDiffClient(ctx context.Context, server api.GitalyServer) (context.Context, *DiffClient, error) { conn, err := getOrCreateConnection(server) if err != nil { return nil, nil, err } grpcClient := gitalypb.NewDiffServiceClient(conn) - return withOutgoingMetadata(ctx, addMetadataFuncs...), &DiffClient{grpcClient}, nil + return withOutgoingMetadata(ctx, server), &DiffClient{grpcClient}, nil } func getOrCreateConnection(server api.GitalyServer) (*grpc.ClientConn, error) { diff --git a/workhorse/internal/gitaly/gitaly_test.go b/workhorse/internal/gitaly/gitaly_test.go index f693f102447..0ea5da20da3 100644 --- a/workhorse/internal/gitaly/gitaly_test.go +++ b/workhorse/internal/gitaly/gitaly_test.go @@ -21,13 +21,9 @@ func TestNewSmartHTTPClient(t *testing.T) { ctx, client, err := NewSmartHTTPClient( context.Background(), serverFixture(), - WithFeatures(features()), - WithUsername("gl_username"), - WithUserID("gl_id"), ) require.NoError(t, err) testOutgoingMetadata(t, ctx) - testOutgoingIDAndUsername(t, ctx) require.NotNil(t, client.sidechannelRegistry) } @@ -35,7 +31,6 @@ func TestNewBlobClient(t *testing.T) { ctx, _, err := NewBlobClient( context.Background(), serverFixture(), - WithFeatures(features()), ) require.NoError(t, err) testOutgoingMetadata(t, ctx) @@ -45,7 +40,6 @@ func TestNewRepositoryClient(t *testing.T) { ctx, _, err := NewRepositoryClient( context.Background(), serverFixture(), - WithFeatures(features()), ) require.NoError(t, err) @@ -56,7 +50,6 @@ func TestNewNamespaceClient(t *testing.T) { ctx, _, err := NewNamespaceClient( context.Background(), serverFixture(), - WithFeatures(features()), ) require.NoError(t, err) testOutgoingMetadata(t, ctx) @@ -66,62 +59,45 @@ func TestNewDiffClient(t *testing.T) { ctx, _, err := NewDiffClient( context.Background(), serverFixture(), - WithFeatures(features()), ) require.NoError(t, err) testOutgoingMetadata(t, ctx) } func testOutgoingMetadata(t *testing.T, ctx context.Context) { + t.Helper() md, ok := metadata.FromOutgoingContext(ctx) require.True(t, ok, "get metadata from context") - for k, v := range allowedFeatures() { - actual := md[k] - require.Len(t, actual, 1, "expect one value for %v", k) - require.Equal(t, v, actual[0], "value for %v", k) - } - - for k := range badFeatureMetadata() { - require.Empty(t, md[k], "value for bad key %v", k) - } -} - -func testOutgoingIDAndUsername(t *testing.T, ctx context.Context) { - md, ok := metadata.FromOutgoingContext(ctx) - require.True(t, ok, "get metadata from context") - - require.Equal(t, md["user_id"], []string{"gl_id"}) - require.Equal(t, md["username"], []string{"gl_username"}) -} - -func features() map[string]string { - features := make(map[string]string) - for k, v := range allowedFeatures() { - features[k] = v - } - - for k, v := range badFeatureMetadata() { - features[k] = v - } - - return features + require.Equal(t, metadata.MD{"username": {"janedoe"}}, md) } func serverFixture() api.GitalyServer { - return api.GitalyServer{Address: "tcp://localhost:123"} -} - -func allowedFeatures() map[string]string { - return map[string]string{ - "gitaly-feature-foo": "bar", - "gitaly-feature-qux": "baz", + return api.GitalyServer{ + Address: "tcp://localhost:123", + CallMetadata: map[string]string{"username": "janedoe"}, } } -func badFeatureMetadata() map[string]string { - return map[string]string{ - "bad-metadata-1": "bad-value-1", - "bad-metadata-2": "bad-value-2", - } +func TestWithOutgoingMetadata(t *testing.T) { + ctx := withOutgoingMetadata(context.Background(), api.GitalyServer{ + CallMetadata: map[string]string{ + "gitaly-feature-abc": "true", + "gitaly-featuregarbage": "blocked", + "bad-header": "blocked", + "user_id": "234", + "username": "janedoe", + "remote_ip": "1.2.3.4", + }, + }) + + md, ok := metadata.FromOutgoingContext(ctx) + require.True(t, ok) + + require.Equal(t, metadata.MD{ + "gitaly-feature-abc": {"true"}, + "user_id": {"234"}, + "username": {"janedoe"}, + "remote_ip": {"1.2.3.4"}, + }, md) } diff --git a/workhorse/internal/helper/command/command.go b/workhorse/internal/helper/command/command.go new file mode 100644 index 00000000000..59c8c9a3db2 --- /dev/null +++ b/workhorse/internal/helper/command/command.go @@ -0,0 +1,30 @@ +package command + +import ( + "os/exec" + "syscall" +) + +func ExitStatus(err error) (int, bool) { + if v, ok := err.(interface{ ExitCode() int }); ok { + return v.ExitCode(), true + } else if err != nil { + return -1, false + } else { + return 0, false + } +} + +func KillProcessGroup(cmd *exec.Cmd) { + if cmd == nil { + return + } + + if p := cmd.Process; p != nil && p.Pid > 0 { + // Send SIGTERM to the process group of cmd + syscall.Kill(-p.Pid, syscall.SIGTERM) + } + + // reap our child process + cmd.Wait() +} diff --git a/workhorse/internal/helper/context_reader.go b/workhorse/internal/helper/context_reader.go deleted file mode 100644 index a4764043147..00000000000 --- a/workhorse/internal/helper/context_reader.go +++ /dev/null @@ -1,40 +0,0 @@ -package helper - -import ( - "context" - "io" -) - -type ContextReader struct { - ctx context.Context - underlyingReader io.Reader -} - -func NewContextReader(ctx context.Context, underlyingReader io.Reader) *ContextReader { - return &ContextReader{ - ctx: ctx, - underlyingReader: underlyingReader, - } -} - -func (r *ContextReader) Read(b []byte) (int, error) { - if r.canceled() { - return 0, r.err() - } - - n, err := r.underlyingReader.Read(b) - - if r.canceled() { - err = r.err() - } - - return n, err -} - -func (r *ContextReader) canceled() bool { - return r.err() != nil -} - -func (r *ContextReader) err() error { - return r.ctx.Err() -} diff --git a/workhorse/internal/helper/context_reader_test.go b/workhorse/internal/helper/context_reader_test.go deleted file mode 100644 index 257ec4e35f2..00000000000 --- a/workhorse/internal/helper/context_reader_test.go +++ /dev/null @@ -1,83 +0,0 @@ -package helper - -import ( - "context" - "io" - "testing" - "time" - - "github.com/stretchr/testify/require" -) - -type fakeReader struct { - n int - err error -} - -func (f *fakeReader) Read(b []byte) (int, error) { - return f.n, f.err -} - -type fakeContextWithTimeout struct { - n int - threshold int -} - -func (*fakeContextWithTimeout) Deadline() (deadline time.Time, ok bool) { - return -} - -func (*fakeContextWithTimeout) Done() <-chan struct{} { - return nil -} - -func (*fakeContextWithTimeout) Value(key interface{}) interface{} { - return nil -} - -func (f *fakeContextWithTimeout) Err() error { - f.n++ - if f.n > f.threshold { - return context.DeadlineExceeded - } - - return nil -} - -func TestContextReaderRead(t *testing.T) { - underlyingReader := &fakeReader{n: 1, err: io.EOF} - - for _, tc := range []struct { - desc string - ctx *fakeContextWithTimeout - expectedN int - expectedErr error - }{ - { - desc: "Before and after read deadline checks are fine", - ctx: &fakeContextWithTimeout{n: 0, threshold: 2}, - expectedN: underlyingReader.n, - expectedErr: underlyingReader.err, - }, - { - desc: "Before read deadline check fails", - ctx: &fakeContextWithTimeout{n: 0, threshold: 0}, - expectedN: 0, - expectedErr: context.DeadlineExceeded, - }, - { - desc: "After read deadline check fails", - ctx: &fakeContextWithTimeout{n: 0, threshold: 1}, - expectedN: underlyingReader.n, - expectedErr: context.DeadlineExceeded, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - cr := NewContextReader(tc.ctx, underlyingReader) - - n, err := cr.Read(nil) - require.Equal(t, tc.expectedN, n) - require.Equal(t, tc.expectedErr, err) - }) - } -} diff --git a/workhorse/internal/helper/raven.go b/workhorse/internal/helper/exception/exception.go index 898e8ec85f8..9b1628ffecb 100644 --- a/workhorse/internal/helper/raven.go +++ b/workhorse/internal/helper/exception/exception.go @@ -1,4 +1,4 @@ -package helper +package exception import ( "net/http" @@ -17,7 +17,7 @@ var ravenHeaderBlacklist = []string{ "Private-Token", } -func CaptureRavenError(r *http.Request, err error, fields log.Fields) { +func Track(r *http.Request, err error, fields log.Fields) { client := raven.DefaultClient extra := raven.Extra{} @@ -27,7 +27,7 @@ func CaptureRavenError(r *http.Request, err error, fields log.Fields) { interfaces := []raven.Interface{} if r != nil { - CleanHeadersForRaven(r) + CleanHeaders(r) interfaces = append(interfaces, raven.NewHttp(r)) //lint:ignore SA1019 this was recently deprecated. Update workhorse to use labkit errortracking package. @@ -45,7 +45,7 @@ func CaptureRavenError(r *http.Request, err error, fields log.Fields) { client.Capture(packet, nil) } -func CleanHeadersForRaven(r *http.Request) { +func CleanHeaders(r *http.Request) { if r == nil { return } diff --git a/workhorse/internal/helper/fail/fail.go b/workhorse/internal/helper/fail/fail.go new file mode 100644 index 00000000000..32c2940a0cc --- /dev/null +++ b/workhorse/internal/helper/fail/fail.go @@ -0,0 +1,45 @@ +package fail + +import ( + "net/http" + + "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" +) + +type failure struct { + status int + body string + fields log.Fields +} + +type Option func(*failure) + +// WithStatus sets the HTTP status and body text of the failure response. +func WithStatus(status int) Option { + return func(f *failure) { + f.status = status + f.body = http.StatusText(status) + } +} + +// WithBody sets the body text of the failure response. Note that +// subsequent applications of WithStatus will override the response body. +func WithBody(body string) Option { return func(f *failure) { f.body = body } } + +// WithFields adds log fields to the failure log message. +func WithFields(fields log.Fields) Option { return func(f *failure) { f.fields = fields } } + +// Request combines error handling actions for a failed HTTP request. By +// default it writes a generic HTTP 500 response to w. The status code +// and response body can be modified by passing options. The value of +// err, if non nil, is logged and reported to Sentry. +func Request(w http.ResponseWriter, r *http.Request, err error, options ...Option) { + f := &failure{} + WithStatus(http.StatusInternalServerError)(f) + for _, opt := range options { + opt(f) + } + + http.Error(w, f.body, f.status) + log.WithRequest(r).WithFields(f.fields).WithError(err).Error() +} diff --git a/workhorse/internal/helper/fail/fail_test.go b/workhorse/internal/helper/fail/fail_test.go new file mode 100644 index 00000000000..ceb037d2da7 --- /dev/null +++ b/workhorse/internal/helper/fail/fail_test.go @@ -0,0 +1,21 @@ +package fail + +import ( + "bytes" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRequestWorksWithNils(t *testing.T) { + body := bytes.NewBuffer(nil) + w := httptest.NewRecorder() + w.Body = body + + Request(w, nil, nil) + + require.Equal(t, http.StatusInternalServerError, w.Code) + require.Equal(t, "Internal Server Error\n", body.String()) +} diff --git a/workhorse/internal/helper/helpers.go b/workhorse/internal/helper/helpers.go index 33318407f88..a4a91901ea9 100644 --- a/workhorse/internal/helper/helpers.go +++ b/workhorse/internal/helper/helpers.go @@ -1,69 +1,14 @@ package helper import ( - "bytes" "errors" - "io" "mime" - "net" - "net/http" "net/url" "os" - "os/exec" - "strings" - "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) { - 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 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") - header.Set("Expires", "Fri, 01 Jan 1990 00:00:00 GMT") -} - func OpenFile(path string) (file *os.File, fi os.FileInfo, err error) { file, err = os.Open(path) if err != nil { @@ -97,113 +42,12 @@ 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}).Fatal("urlMustParse") } return u } -func HTTPError(w http.ResponseWriter, r *http.Request, error string, code int) { - if r.ProtoAtLeast(1, 1) { - // Force client to disconnect if we render request error - w.Header().Set("Connection", "close") - } - - http.Error(w, error, code) -} - -func HeaderClone(h http.Header) http.Header { - h2 := make(http.Header, len(h)) - for k, vv := range h { - vv2 := make([]string, len(vv)) - copy(vv2, vv) - h2[k] = vv2 - } - return h2 -} - -func CleanUpProcessGroup(cmd *exec.Cmd) { - if cmd == nil { - return - } - - process := cmd.Process - if process != nil && process.Pid > 0 { - // Send SIGTERM to the process group of cmd - syscall.Kill(-process.Pid, syscall.SIGTERM) - } - - // reap our child process - cmd.Wait() -} - -func ExitStatus(err error) (int, bool) { - exitError, ok := err.(*exec.ExitError) - if !ok { - return 0, false - } - - waitStatus, ok := exitError.Sys().(syscall.WaitStatus) - if !ok { - return 0, false - } - - return waitStatus.ExitStatus(), true -} - -func DisableResponseBuffering(w http.ResponseWriter) { - w.Header().Set(NginxResponseBufferHeader, "no") -} - -func AllowResponseBuffering(w http.ResponseWriter) { - w.Header().Del(NginxResponseBufferHeader) -} - -func FixRemoteAddr(r *http.Request) { - // Unix domain sockets have a remote addr of @. This will make the - // xff package lookup the X-Forwarded-For address if available. - if r.RemoteAddr == "@" { - r.RemoteAddr = "127.0.0.1:0" - } - r.RemoteAddr = xff.GetRemoteAddr(r) -} - -func SetForwardedFor(newHeaders *http.Header, originalRequest *http.Request) { - if clientIP, _, err := net.SplitHostPort(originalRequest.RemoteAddr); err == nil { - var header string - - // If we aren't the first proxy retain prior - // X-Forwarded-For information as a comma+space - // separated list and fold multiple headers into one. - if prior, ok := originalRequest.Header["X-Forwarded-For"]; ok { - header = strings.Join(prior, ", ") + ", " + clientIP - } else { - header = clientIP - } - newHeaders.Set("X-Forwarded-For", header) - } -} - func IsContentType(expected, actual string) bool { parsed, _, err := mime.ParseMediaType(actual) return err == nil && parsed == expected } - -func IsApplicationJson(r *http.Request) bool { - contentType := r.Header.Get("Content-Type") - return IsContentType("application/json", contentType) -} - -func ReadRequestBody(w http.ResponseWriter, r *http.Request, maxBodySize int64) ([]byte, error) { - limitedBody := http.MaxBytesReader(w, r.Body, maxBodySize) - defer limitedBody.Close() - - return io.ReadAll(limitedBody) -} - -func CloneRequestWithNewBody(r *http.Request, body []byte) *http.Request { - newReq := *r - newReq.Body = io.NopCloser(bytes.NewReader(body)) - newReq.Header = HeaderClone(r.Header) - newReq.ContentLength = int64(len(body)) - return &newReq -} diff --git a/workhorse/internal/helper/helpers_test.go b/workhorse/internal/helper/helpers_test.go deleted file mode 100644 index 93d1ee33d59..00000000000 --- a/workhorse/internal/helper/helpers_test.go +++ /dev/null @@ -1,142 +0,0 @@ -package helper - -import ( - "bytes" - "io" - "net/http" - "net/http/httptest" - "testing" - - "github.com/stretchr/testify/require" -) - -func TestFixRemoteAddr(t *testing.T) { - testCases := []struct { - initial string - forwarded string - expected string - }{ - {initial: "@", forwarded: "", expected: "127.0.0.1:0"}, - {initial: "@", forwarded: "18.245.0.1", expected: "18.245.0.1:0"}, - {initial: "@", forwarded: "127.0.0.1", expected: "127.0.0.1:0"}, - {initial: "@", forwarded: "192.168.0.1", expected: "127.0.0.1:0"}, - {initial: "192.168.1.1:0", forwarded: "", expected: "192.168.1.1:0"}, - {initial: "192.168.1.1:0", forwarded: "18.245.0.1", expected: "18.245.0.1:0"}, - } - - for _, tc := range testCases { - req, err := http.NewRequest("POST", "unix:///tmp/test.socket/info/refs", nil) - require.NoError(t, err) - - req.RemoteAddr = tc.initial - - if tc.forwarded != "" { - req.Header.Add("X-Forwarded-For", tc.forwarded) - } - - FixRemoteAddr(req) - - require.Equal(t, tc.expected, req.RemoteAddr) - } -} - -func TestSetForwardedForGeneratesHeader(t *testing.T) { - testCases := []struct { - remoteAddr string - previousForwardedFor []string - expected string - }{ - { - "8.8.8.8:3000", - nil, - "8.8.8.8", - }, - { - "8.8.8.8:3000", - []string{"138.124.33.63, 151.146.211.237"}, - "138.124.33.63, 151.146.211.237, 8.8.8.8", - }, - { - "8.8.8.8:3000", - []string{"8.154.76.107", "115.206.118.179"}, - "8.154.76.107, 115.206.118.179, 8.8.8.8", - }, - } - for _, tc := range testCases { - headers := http.Header{} - originalRequest := http.Request{ - RemoteAddr: tc.remoteAddr, - } - - if tc.previousForwardedFor != nil { - originalRequest.Header = http.Header{ - "X-Forwarded-For": tc.previousForwardedFor, - } - } - - SetForwardedFor(&headers, &originalRequest) - - result := headers.Get("X-Forwarded-For") - if result != tc.expected { - t.Fatalf("Expected %v, got %v", tc.expected, result) - } - } -} - -func TestReadRequestBody(t *testing.T) { - data := []byte("123456") - rw := httptest.NewRecorder() - req, _ := http.NewRequest("POST", "/test", bytes.NewBuffer(data)) - - result, err := ReadRequestBody(rw, req, 1000) - require.NoError(t, err) - require.Equal(t, data, result) -} - -func TestReadRequestBodyLimit(t *testing.T) { - data := []byte("123456") - rw := httptest.NewRecorder() - req, _ := http.NewRequest("POST", "/test", bytes.NewBuffer(data)) - - _, err := ReadRequestBody(rw, req, 2) - require.Error(t, err) -} - -func TestCloneRequestWithBody(t *testing.T) { - input := []byte("test") - newInput := []byte("new body") - req, _ := http.NewRequest("POST", "/test", bytes.NewBuffer(input)) - newReq := CloneRequestWithNewBody(req, newInput) - - require.NotEqual(t, req, newReq) - require.NotEqual(t, req.Body, newReq.Body) - require.NotEqual(t, len(newInput), newReq.ContentLength) - - var buffer bytes.Buffer - io.Copy(&buffer, newReq.Body) - require.Equal(t, newInput, buffer.Bytes()) -} - -func TestApplicationJson(t *testing.T) { - req, _ := http.NewRequest("POST", "/test", nil) - req.Header.Set("Content-Type", "application/json") - - require.True(t, IsApplicationJson(req), "expected to match 'application/json' as 'application/json'") - - req.Header.Set("Content-Type", "application/json; charset=utf-8") - require.True(t, IsApplicationJson(req), "expected to match 'application/json; charset=utf-8' as 'application/json'") - - req.Header.Set("Content-Type", "text/plain") - require.False(t, IsApplicationJson(req), "expected not to match 'text/plain' as 'application/json'") -} - -func TestFail500WorksWithNils(t *testing.T) { - body := bytes.NewBuffer(nil) - w := httptest.NewRecorder() - w.Body = body - - Fail500(w, nil, nil) - - require.Equal(t, http.StatusInternalServerError, w.Code) - require.Equal(t, "Internal server error\n", body.String()) -} diff --git a/workhorse/internal/helper/nginx/nginx.go b/workhorse/internal/helper/nginx/nginx.go new file mode 100644 index 00000000000..ca7c8543f75 --- /dev/null +++ b/workhorse/internal/helper/nginx/nginx.go @@ -0,0 +1,13 @@ +package nginx + +import "net/http" + +const ResponseBufferHeader = "X-Accel-Buffering" + +func DisableResponseBuffering(w http.ResponseWriter) { + w.Header().Set(ResponseBufferHeader, "no") +} + +func AllowResponseBuffering(w http.ResponseWriter) { + w.Header().Del(ResponseBufferHeader) +} diff --git a/workhorse/internal/helper/tempfile.go b/workhorse/internal/helper/tempfile.go deleted file mode 100644 index f5864f549d0..00000000000 --- a/workhorse/internal/helper/tempfile.go +++ /dev/null @@ -1,34 +0,0 @@ -package helper - -import ( - "io" - "os" -) - -func ReadAllTempfile(r io.Reader) (tempfile *os.File, err error) { - tempfile, err = os.CreateTemp("", "gitlab-workhorse-read-all-tempfile") - if err != nil { - return nil, err - } - - defer func() { - // Avoid leaking an open file if the function returns with an error - if err != nil { - tempfile.Close() - } - }() - - if err := os.Remove(tempfile.Name()); err != nil { - return nil, err - } - - if _, err := io.Copy(tempfile, r); err != nil { - return nil, err - } - - if _, err := tempfile.Seek(0, 0); err != nil { - return nil, err - } - - return tempfile, nil -} diff --git a/workhorse/internal/imageresizer/image_resizer.go b/workhorse/internal/imageresizer/image_resizer.go index 092369cd2af..72f345239a6 100644 --- a/workhorse/internal/imageresizer/image_resizer.go +++ b/workhorse/internal/imageresizer/image_resizer.go @@ -20,7 +20,8 @@ import ( "gitlab.com/gitlab-org/labkit/tracing" "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/command" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" @@ -191,7 +192,7 @@ func (r *Resizer) Inject(w http.ResponseWriter, req *http.Request, paramsData st // We need to log this separately since the subsequent steps might add other failures. log.WithRequest(req).WithFields(logFields(start, params, &outcome)).WithError(err).Error() } - defer helper.CleanUpProcessGroup(resizeCmd) + defer command.KillProcessGroup(resizeCmd) w.Header().Del("Content-Length") outcome.bytesWritten, err = serveImage(imageReader, w, resizeCmd) @@ -419,7 +420,7 @@ func handleOutcome(w http.ResponseWriter, req *http.Request, startTime time.Time switch outcome.status { case statusRequestFailure: if outcome.bytesWritten <= 0 { - helper.Fail500WithFields(w, req, outcome.err, fields) + fail.Request(w, req, outcome.err, fail.WithFields(fields)) } else { log.WithError(outcome.err).Error(outcome.status) } diff --git a/workhorse/internal/log/logging.go b/workhorse/internal/log/logging.go index 80c09c1bf02..004ae8a8604 100644 --- a/workhorse/internal/log/logging.go +++ b/workhorse/internal/log/logging.go @@ -8,7 +8,7 @@ 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/helper/exception" ) type Fields = log.Fields @@ -79,10 +79,18 @@ func Error(args ...interface{}) { NewBuilder().Error(args...) } +func (b *Builder) trackException() { + if b.err != nil { + exception.Track(b.req, b.err, b.fields) + } +} + func (b *Builder) Error(args ...interface{}) { + b.trackException() b.entry.Error(args...) +} - if b.req != nil && b.err != nil { - helper.CaptureRavenError(b.req, b.err, b.fields) - } +func (b *Builder) Fatal(args ...interface{}) { + b.trackException() + b.entry.Fatal(args...) } diff --git a/workhorse/internal/proxy/proxy.go b/workhorse/internal/proxy/proxy.go index 06e2c65a6a8..a7c3b322da7 100644 --- a/workhorse/internal/proxy/proxy.go +++ b/workhorse/internal/proxy/proxy.go @@ -8,6 +8,7 @@ import ( "time" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/nginx" ) var ( @@ -45,6 +46,14 @@ func NewProxy(myURL *url.URL, version string, roundTripper http.RoundTripper, op u.Path = "" p.reverseProxy = httputil.NewSingleHostReverseProxy(&u) p.reverseProxy.Transport = roundTripper + chainDirector(p.reverseProxy, func(r *http.Request) { + r.Header.Set("Gitlab-Workhorse", p.Version) + r.Header.Set("Gitlab-Workhorse-Proxy-Start", fmt.Sprintf("%d", time.Now().UnixNano())) + + for k, v := range p.customHeaders { + r.Header.Set(k, v) + } + }) for _, option := range options { option(&p) @@ -54,10 +63,7 @@ func NewProxy(myURL *url.URL, version string, roundTripper http.RoundTripper, op // because of https://github.com/golang/go/issues/28168, the // upstream won't receive the expected Host header unless this // is forced in the Director func here - previousDirector := p.reverseProxy.Director - p.reverseProxy.Director = func(request *http.Request) { - previousDirector(request) - + chainDirector(p.reverseProxy, func(request *http.Request) { // send original host along for the upstream // to know it's being proxied under a different Host // (for redirects and other stuff that depends on this) @@ -66,27 +72,23 @@ func NewProxy(myURL *url.URL, version string, roundTripper http.RoundTripper, op // override the Host with the target request.Host = request.URL.Host - } + }) } return &p } -func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // Clone request - req := *r - req.Header = helper.HeaderClone(r.Header) - - // Set Workhorse version - req.Header.Set("Gitlab-Workhorse", p.Version) - req.Header.Set("Gitlab-Workhorse-Proxy-Start", fmt.Sprintf("%d", time.Now().UnixNano())) - - for k, v := range p.customHeaders { - req.Header.Set(k, v) +func chainDirector(rp *httputil.ReverseProxy, nextDirector func(*http.Request)) { + previous := rp.Director + rp.Director = func(r *http.Request) { + previous(r) + nextDirector(r) } +} +func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { if p.AllowResponseBuffering { - helper.AllowResponseBuffering(w) + nginx.AllowResponseBuffering(w) } // If the ultimate client disconnects when the response isn't fully written @@ -100,5 +102,5 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } }() - p.reverseProxy.ServeHTTP(w, &req) + p.reverseProxy.ServeHTTP(w, r) } diff --git a/workhorse/internal/queueing/requests.go b/workhorse/internal/queueing/requests.go index 34d4c985f53..c3df614de41 100644 --- a/workhorse/internal/queueing/requests.go +++ b/workhorse/internal/queueing/requests.go @@ -6,7 +6,7 @@ import ( "github.com/prometheus/client_golang/prometheus" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" ) const ( @@ -46,7 +46,7 @@ func QueueRequests(name string, h http.Handler, limit, queueLimit uint, queueTim http.Error(w, "Service Unavailable", http.StatusServiceUnavailable) default: - helper.Fail500(w, r, err) + fail.Request(w, r, err) } }) diff --git a/workhorse/internal/senddata/senddata.go b/workhorse/internal/senddata/senddata.go index 190a37c1a15..4cb96890ee2 100644 --- a/workhorse/internal/senddata/senddata.go +++ b/workhorse/internal/senddata/senddata.go @@ -5,6 +5,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/headers" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/nginx" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata/contentprocessor" "github.com/prometheus/client_golang/prometheus" @@ -88,7 +89,7 @@ func (s *sendDataResponseWriter) tryInject() bool { for _, injecter := range s.injecters { if injecter.Match(header) { s.hijacked = true - helper.DisableResponseBuffering(s.rw) + nginx.DisableResponseBuffering(s.rw) crw := helper.NewCountingResponseWriter(s.rw) injecter.Inject(crw, s.req, header) sendDataResponses.WithLabelValues(injecter.Name()).Inc() diff --git a/workhorse/internal/sendfile/sendfile.go b/workhorse/internal/sendfile/sendfile.go index 07b1789445a..70d93f1109c 100644 --- a/workhorse/internal/sendfile/sendfile.go +++ b/workhorse/internal/sendfile/sendfile.go @@ -20,6 +20,8 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/headers" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/nginx" ) var ( @@ -93,7 +95,7 @@ func (s *sendFileResponseWriter) WriteHeader(status int) { s.hijacked = true // Serve the file - helper.DisableResponseBuffering(s.rw) + nginx.DisableResponseBuffering(s.rw) sendFileFromDisk(s.rw, s.req, file) return } @@ -129,7 +131,7 @@ func sendFileFromDisk(w http.ResponseWriter, r *http.Request, file string) { if contentTypeHeaderPresent { data, err := io.ReadAll(io.LimitReader(content, headers.MaxDetectSize)) if err != nil { - helper.Fail500(w, r, fmt.Errorf("content type detection: %v", err)) + fail.Request(w, r, fmt.Errorf("content type detection: %v", err)) return } diff --git a/workhorse/internal/sendurl/sendurl.go b/workhorse/internal/sendurl/sendurl.go index 8e679c6b475..e689fc84a0f 100644 --- a/workhorse/internal/sendurl/sendurl.go +++ b/workhorse/internal/sendurl/sendurl.go @@ -10,7 +10,7 @@ import ( "gitlab.com/gitlab-org/labkit/mask" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" @@ -83,7 +83,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) defer sendURLOpenRequests.Dec() if err := e.Unpack(¶ms, sendData); err != nil { - helper.Fail500(w, r, fmt.Errorf("SendURL: unpack sendData: %v", err)) + fail.Request(w, r, fmt.Errorf("SendURL: unpack sendData: %v", err)) return } @@ -94,7 +94,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) if params.URL == "" { sendURLRequestsInvalidData.Inc() - helper.Fail500(w, r, fmt.Errorf("SendURL: URL is empty")) + fail.Request(w, r, fmt.Errorf("SendURL: URL is empty")) return } @@ -102,7 +102,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) newReq, err := http.NewRequest("GET", params.URL, nil) if err != nil { sendURLRequestsInvalidData.Inc() - helper.Fail500(w, r, fmt.Errorf("SendURL: NewRequest: %v", err)) + fail.Request(w, r, fmt.Errorf("SendURL: NewRequest: %v", err)) return } newReq = newReq.WithContext(r.Context()) @@ -120,7 +120,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) } if err != nil { sendURLRequestsRequestFailed.Inc() - helper.Fail500(w, r, fmt.Errorf("SendURL: Do request: %v", err)) + fail.Request(w, r, fmt.Errorf("SendURL: Do request: %v", err)) return } diff --git a/workhorse/internal/staticpages/deploy_page.go b/workhorse/internal/staticpages/deploy_page.go index 3dc2d982981..ca0931addd0 100644 --- a/workhorse/internal/staticpages/deploy_page.go +++ b/workhorse/internal/staticpages/deploy_page.go @@ -4,8 +4,6 @@ import ( "net/http" "os" "path/filepath" - - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" ) func (s *Static) DeployPage(handler http.Handler) http.Handler { @@ -18,7 +16,7 @@ func (s *Static) DeployPage(handler http.Handler) http.Handler { return } - helper.SetNoCacheHeaders(w.Header()) + setNoCacheHeaders(w.Header()) w.Header().Set("Content-Type", "text/html; charset=utf-8") w.WriteHeader(http.StatusOK) w.Write(data) diff --git a/workhorse/internal/staticpages/error_pages.go b/workhorse/internal/staticpages/error_pages.go index e0ba7a5ceef..d1aa7603658 100644 --- a/workhorse/internal/staticpages/error_pages.go +++ b/workhorse/internal/staticpages/error_pages.go @@ -9,8 +9,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" ) var ( @@ -84,7 +82,7 @@ func (s *errorPageResponseWriter) WriteHeader(status int) { s.hijacked = true staticErrorResponses.WithLabelValues(fmt.Sprintf("%d", s.status)).Inc() - helper.SetNoCacheHeaders(s.rw.Header()) + setNoCacheHeaders(s.rw.Header()) s.rw.Header().Set("Content-Type", contentType) s.rw.Header().Set("Content-Length", fmt.Sprintf("%d", len(data))) s.rw.Header().Del("Transfer-Encoding") diff --git a/workhorse/internal/staticpages/static.go b/workhorse/internal/staticpages/static.go index 5b804e4d644..c5c0573090b 100644 --- a/workhorse/internal/staticpages/static.go +++ b/workhorse/internal/staticpages/static.go @@ -1,6 +1,14 @@ package staticpages +import "net/http" + type Static struct { DocumentRoot string Exclude []string } + +func setNoCacheHeaders(header http.Header) { + header.Set("Cache-Control", "no-cache, no-store, max-age=0, must-revalidate") + header.Set("Pragma", "no-cache") + header.Set("Expires", "Fri, 01 Jan 1990 00:00:00 GMT") +} diff --git a/workhorse/internal/upload/artifacts_uploader.go b/workhorse/internal/upload/artifacts_uploader.go index a8c944a1d33..c83874e7293 100644 --- a/workhorse/internal/upload/artifacts_uploader.go +++ b/workhorse/internal/upload/artifacts_uploader.go @@ -16,7 +16,7 @@ import ( "gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/command" "gitlab.com/gitlab-org/gitlab/workhorse/internal/lsif_transformer/parser" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination" "gitlab.com/gitlab-org/gitlab/workhorse/internal/zipartifacts" @@ -83,7 +83,7 @@ func (a *artifactsUploadProcessor) generateMetadataFromZip(ctx context.Context, if err := zipMd.Start(); err != nil { return nil, err } - defer helper.CleanUpProcessGroup(zipMd) + defer command.KillProcessGroup(zipMd) fh, err := destination.Upload(ctx, zipMdOut, -1, "metadata.gz", metaOpts) if err != nil { @@ -91,7 +91,7 @@ func (a *artifactsUploadProcessor) generateMetadataFromZip(ctx context.Context, } if err := zipMd.Wait(); err != nil { - st, ok := helper.ExitStatus(err) + st, ok := command.ExitStatus(err) if !ok { return nil, err diff --git a/workhorse/internal/upload/body_uploader.go b/workhorse/internal/upload/body_uploader.go index 4b5152c283c..4733415c2c1 100644 --- a/workhorse/internal/upload/body_uploader.go +++ b/workhorse/internal/upload/body_uploader.go @@ -8,7 +8,7 @@ import ( "strings" "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination" ) @@ -19,20 +19,20 @@ func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler { return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) { opts, err := p.Prepare(a) if err != nil { - helper.Fail500(w, r, fmt.Errorf("RequestBody: preparation failed: %v", err)) + fail.Request(w, r, fmt.Errorf("RequestBody: preparation failed: %v", err)) return } fh, err := destination.Upload(r.Context(), r.Body, r.ContentLength, "upload", opts) if err != nil { - helper.Fail500(w, r, fmt.Errorf("RequestBody: upload failed: %v", err)) + fail.Request(w, r, fmt.Errorf("RequestBody: upload failed: %v", err)) return } data := url.Values{} fields, err := fh.GitLabFinalizeFields("file") if err != nil { - helper.Fail500(w, r, fmt.Errorf("RequestBody: finalize fields failed: %v", err)) + fail.Request(w, r, fmt.Errorf("RequestBody: finalize fields failed: %v", err)) return } @@ -49,7 +49,7 @@ func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler { sft := SavedFileTracker{Request: r} sft.Track("file", fh.LocalPath) if err := sft.Finalize(r.Context()); err != nil { - helper.Fail500(w, r, fmt.Errorf("RequestBody: finalize failed: %v", err)) + fail.Request(w, r, fmt.Errorf("RequestBody: finalize failed: %v", err)) return } diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go index 7b9ac6b996e..ad9623f569c 100644 --- a/workhorse/internal/upload/rewrite.go +++ b/workhorse/internal/upload/rewrite.go @@ -67,11 +67,8 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, fi // Create multipart reader reader, err := r.MultipartReader() if err != nil { - if err == http.ErrNotMultipart { - // We want to be able to recognize http.ErrNotMultipart elsewhere so no fmt.Errorf - return http.ErrNotMultipart - } - return fmt.Errorf("get multipart reader: %v", err) + // We want to be able to recognize these errors elsewhere so no fmt.Errorf + return err } multipartUploadRequests.WithLabelValues(filter.Name()).Inc() diff --git a/workhorse/internal/upload/uploads.go b/workhorse/internal/upload/uploads.go index f214e1ac297..a3072aa5d00 100644 --- a/workhorse/internal/upload/uploads.go +++ b/workhorse/internal/upload/uploads.go @@ -12,7 +12,7 @@ import ( "github.com/golang-jwt/jwt/v4" "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/exif" "gitlab.com/gitlab-org/gitlab/workhorse/internal/zipartifacts" @@ -51,23 +51,23 @@ func interceptMultipartFiles(w http.ResponseWriter, r *http.Request, h http.Hand err := rewriteFormFilesFromMultipart(r, writer, filter, fa, p) if err != nil { switch err { - case ErrInjectedClientParam: - helper.CaptureAndFail(w, r, err, "Bad Request", http.StatusBadRequest) - case ErrTooManyFilesUploaded: - helper.CaptureAndFail(w, r, err, err.Error(), http.StatusBadRequest) case http.ErrNotMultipart: h.ServeHTTP(w, r) - case destination.ErrEntityTooLarge: - helper.RequestEntityTooLarge(w, r, err) - case zipartifacts.ErrBadMetadata: - helper.RequestEntityTooLarge(w, r, err) + case ErrInjectedClientParam, http.ErrMissingBoundary: + fail.Request(w, r, err, fail.WithStatus(http.StatusBadRequest)) + case ErrTooManyFilesUploaded: + fail.Request(w, r, err, fail.WithStatus(http.StatusBadRequest), fail.WithBody(err.Error())) + case destination.ErrEntityTooLarge, zipartifacts.ErrBadMetadata: + fail.Request(w, r, err, fail.WithStatus(http.StatusRequestEntityTooLarge)) case exif.ErrRemovingExif: - helper.CaptureAndFail(w, r, err, "Failed to process image", http.StatusUnprocessableEntity) + fail.Request(w, r, err, fail.WithStatus(http.StatusUnprocessableEntity), + fail.WithBody("Failed to process image")) default: if errors.Is(err, context.DeadlineExceeded) { - helper.CaptureAndFail(w, r, err, "deadline exceeded", http.StatusGatewayTimeout) + fail.Request(w, r, err, fail.WithStatus(http.StatusGatewayTimeout), + fail.WithBody("deadline exceeded")) } else { - helper.Fail500(w, r, fmt.Errorf("handleFileUploads: extract files from multipart: %v", err)) + fail.Request(w, r, fmt.Errorf("handleFileUploads: extract files from multipart: %v", err)) } } return @@ -82,7 +82,7 @@ func interceptMultipartFiles(w http.ResponseWriter, r *http.Request, h http.Hand r.Header.Set("Content-Type", writer.FormDataContentType()) if err := filter.Finalize(r.Context()); err != nil { - helper.Fail500(w, r, fmt.Errorf("handleFileUploads: Finalize: %v", err)) + fail.Request(w, r, fmt.Errorf("handleFileUploads: Finalize: %v", err)) return } diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go index 3655e9fc8c9..cc786079e36 100644 --- a/workhorse/internal/upload/uploads_test.go +++ b/workhorse/internal/upload/uploads_test.go @@ -352,6 +352,18 @@ func TestInvalidFileNames(t *testing.T) { } } +func TestBadMultipartHeader(t *testing.T) { + httpRequest, err := http.NewRequest("POST", "/example", bytes.NewReader(nil)) + require.NoError(t, err) + + // Invalid header: missing boundary + httpRequest.Header.Set("Content-Type", "multipart/form-data") + + response := httptest.NewRecorder() + testInterceptMultipartFiles(t, response, httpRequest, nilHandler, &SavedFileTracker{Request: httpRequest}) + require.Equal(t, 400, response.Code) +} + func TestContentDispositionRewrite(t *testing.T) { testhelper.ConfigureSecret() diff --git a/workhorse/internal/upstream/handlers.go b/workhorse/internal/upstream/handlers.go index 5974170e172..85fee0bf7e2 100644 --- a/workhorse/internal/upstream/handlers.go +++ b/workhorse/internal/upstream/handlers.go @@ -6,7 +6,7 @@ import ( "io" "net/http" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" ) func contentEncodingHandler(h http.Handler) http.Handler { @@ -26,7 +26,7 @@ func contentEncodingHandler(h http.Handler) http.Handler { } if err != nil { - helper.Fail500(w, r, fmt.Errorf("contentEncodingHandler: %v", err)) + fail.Request(w, r, fmt.Errorf("contentEncodingHandler: %v", err)) return } defer body.Close() diff --git a/workhorse/internal/upstream/routes.go b/workhorse/internal/upstream/routes.go index c47053ad682..982f3a5b5f8 100644 --- a/workhorse/internal/upstream/routes.go +++ b/workhorse/internal/upstream/routes.go @@ -425,7 +425,7 @@ func configureRoutes(u *upstream) { func denyWebsocket(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if websocket.IsWebSocketUpgrade(r) { - helper.HTTPError(w, r, "websocket upgrade not allowed", http.StatusBadRequest) + httpError(w, r, "websocket upgrade not allowed", http.StatusBadRequest) return } diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go index 248f190e316..34fe300192f 100644 --- a/workhorse/internal/upstream/upstream.go +++ b/workhorse/internal/upstream/upstream.go @@ -16,6 +16,7 @@ import ( "net/url" "strings" + "github.com/sebest/xff" "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/labkit/correlation" @@ -24,6 +25,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/builds" "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/nginx" proxypkg "gitlab.com/gitlab-org/gitlab/workhorse/internal/proxy" "gitlab.com/gitlab-org/gitlab/workhorse/internal/rejectmethods" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload" @@ -124,19 +126,19 @@ func (u *upstream) configureURLPrefix() { } func (u *upstream) ServeHTTP(w http.ResponseWriter, r *http.Request) { - helper.FixRemoteAddr(r) + fixRemoteAddr(r) - helper.DisableResponseBuffering(w) + nginx.DisableResponseBuffering(w) // Drop RequestURI == "*" (FIXME: why?) if r.RequestURI == "*" { - helper.HTTPError(w, r, "Connection upgrade not allowed", http.StatusBadRequest) + httpError(w, r, "Connection upgrade not allowed", http.StatusBadRequest) return } // Disallow connect if r.Method == "CONNECT" { - helper.HTTPError(w, r, "CONNECT not allowed", http.StatusBadRequest) + httpError(w, r, "CONNECT not allowed", http.StatusBadRequest) return } @@ -144,7 +146,7 @@ func (u *upstream) ServeHTTP(w http.ResponseWriter, r *http.Request) { URIPath := urlprefix.CleanURIPath(r.URL.EscapedPath()) prefix := u.URLPrefix if !prefix.Match(URIPath) { - helper.HTTPError(w, r, fmt.Sprintf("Not found %q", URIPath), http.StatusNotFound) + httpError(w, r, fmt.Sprintf("Not found %q", URIPath), http.StatusNotFound) return } @@ -155,7 +157,7 @@ func (u *upstream) ServeHTTP(w http.ResponseWriter, r *http.Request) { if route == nil { // The protocol spec in git/Documentation/technical/http-protocol.txt // says we must return 403 if no matching service is found. - helper.HTTPError(w, r, "Forbidden", http.StatusForbidden) + httpError(w, r, "Forbidden", http.StatusForbidden) return } @@ -275,3 +277,21 @@ func (u *upstream) updateGeoProxyFieldsFromData(geoProxyData *apipkg.GeoProxyDat u.geoProxyCableRoute = u.wsRoute(`^/-/cable\z`, geoProxyUpstream) u.geoProxyRoute = u.route("", "", geoProxyUpstream, withGeoProxy()) } + +func httpError(w http.ResponseWriter, r *http.Request, error string, code int) { + if r.ProtoAtLeast(1, 1) { + // Force client to disconnect if we render request error + w.Header().Set("Connection", "close") + } + + http.Error(w, error, code) +} + +func fixRemoteAddr(r *http.Request) { + // Unix domain sockets have a remote addr of @. This will make the + // xff package lookup the X-Forwarded-For address if available. + if r.RemoteAddr == "@" { + r.RemoteAddr = "127.0.0.1:0" + } + r.RemoteAddr = xff.GetRemoteAddr(r) +} diff --git a/workhorse/internal/upstream/upstream_test.go b/workhorse/internal/upstream/upstream_test.go index 7ab3e67116f..705e40c74d5 100644 --- a/workhorse/internal/upstream/upstream_test.go +++ b/workhorse/internal/upstream/upstream_test.go @@ -435,3 +435,33 @@ func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*h return ws, ws.Close, waitForNextApiPoll } + +func TestFixRemoteAddr(t *testing.T) { + testCases := []struct { + initial string + forwarded string + expected string + }{ + {initial: "@", forwarded: "", expected: "127.0.0.1:0"}, + {initial: "@", forwarded: "18.245.0.1", expected: "18.245.0.1:0"}, + {initial: "@", forwarded: "127.0.0.1", expected: "127.0.0.1:0"}, + {initial: "@", forwarded: "192.168.0.1", expected: "127.0.0.1:0"}, + {initial: "192.168.1.1:0", forwarded: "", expected: "192.168.1.1:0"}, + {initial: "192.168.1.1:0", forwarded: "18.245.0.1", expected: "18.245.0.1:0"}, + } + + for _, tc := range testCases { + req, err := http.NewRequest("POST", "unix:///tmp/test.socket/info/refs", nil) + require.NoError(t, err) + + req.RemoteAddr = tc.initial + + if tc.forwarded != "" { + req.Header.Add("X-Forwarded-For", tc.forwarded) + } + + fixRemoteAddr(req) + + require.Equal(t, tc.expected, req.RemoteAddr) + } +} diff --git a/workhorse/main_test.go b/workhorse/main_test.go index 5ebc26c7ac7..382fb16a16c 100644 --- a/workhorse/main_test.go +++ b/workhorse/main_test.go @@ -29,6 +29,7 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/nginx" "gitlab.com/gitlab-org/gitlab/workhorse/internal/secret" "gitlab.com/gitlab-org/gitlab/workhorse/internal/testhelper" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upstream" @@ -491,9 +492,9 @@ func TestSendURLForArtifacts(t *testing.T) { transferEncoding []string contentLength int }{ - {"No content-length, chunked TE", chunkedHandler, []string{"chunked"}, -1}, // Case 3 in https://tools.ietf.org/html/rfc7230#section-3.3.2 - {"Known content-length, identity TE", regularHandler, nil, len(expectedBody)}, // Case 5 in https://tools.ietf.org/html/rfc7230#section-3.3.2 - {"No content-length, identity TE", rawHandler, []string{"chunked"}, -1}, // Case 7 in https://tools.ietf.org/html/rfc7230#section-3.3.2 + {"No content-length, chunked TE", chunkedHandler, []string{"chunked"}, -1}, // Case 3 in https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 + {"Known content-length, identity TE", regularHandler, nil, len(expectedBody)}, // Case 5 in https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 + {"No content-length, identity TE", rawHandler, []string{"chunked"}, -1}, // Case 7 in https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2 } { t.Run(tc.name, func(t *testing.T) { server := httptest.NewServer(tc.handler) @@ -853,7 +854,7 @@ func httpPost(t *testing.T, url string, headers map[string]string, reqBody io.Re } func requireNginxResponseBuffering(t *testing.T, expected string, resp *http.Response, msgAndArgs ...interface{}) { - actual := resp.Header.Get(helper.NginxResponseBufferHeader) + actual := resp.Header.Get(nginx.ResponseBufferHeader) require.Equal(t, expected, actual, msgAndArgs...) } diff --git a/workhorse/raven.go b/workhorse/raven.go index 2db24b0b3d4..582900b15f4 100644 --- a/workhorse/raven.go +++ b/workhorse/raven.go @@ -6,7 +6,7 @@ import ( raven "github.com/getsentry/raven-go" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/exception" ) func wrapRaven(h http.Handler) http.Handler { @@ -30,7 +30,7 @@ func wrapRaven(h http.Handler) http.Handler { func(w http.ResponseWriter, r *http.Request) { defer func() { if p := recover(); p != nil { - helper.CleanHeadersForRaven(r) + exception.CleanHeaders(r) panic(p) } }() |