summaryrefslogtreecommitdiff
path: root/workhorse
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-11-18 13:16:36 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-11-18 13:16:36 +0000
commit311b0269b4eb9839fa63f80c8d7a58f32b8138a0 (patch)
tree07e7870bca8aed6d61fdcc810731c50d2c40af47 /workhorse
parent27909cef6c4170ed9205afa7426b8d3de47cbb0c (diff)
downloadgitlab-ce-311b0269b4eb9839fa63f80c8d7a58f32b8138a0.tar.gz
Add latest changes from gitlab-org/gitlab@14-5-stable-eev14.5.0-rc42
Diffstat (limited to 'workhorse')
-rw-r--r--workhorse/.tool-versions1
-rw-r--r--workhorse/Makefile31
-rw-r--r--workhorse/gitaly_integration_test.go25
-rw-r--r--workhorse/internal/dependencyproxy/dependencyproxy.go41
-rw-r--r--workhorse/internal/dependencyproxy/dependencyproxy_test.go17
-rw-r--r--workhorse/internal/filestore/file_handler.go34
-rw-r--r--workhorse/internal/filestore/file_handler_test.go1
-rw-r--r--workhorse/internal/helper/httptransport/http_transport.go37
-rw-r--r--workhorse/internal/imageresizer/image_resizer.go20
-rw-r--r--workhorse/internal/objectstore/object.go25
-rw-r--r--workhorse/internal/sendurl/sendurl.go22
-rw-r--r--workhorse/internal/upload/uploads_test.go2
-rw-r--r--workhorse/internal/upstream/roundtripper/roundtripper.go12
-rw-r--r--workhorse/internal/upstream/roundtripper/transport.go27
-rw-r--r--workhorse/internal/upstream/routes.go7
-rw-r--r--workhorse/internal/upstream/routes_test.go47
-rw-r--r--workhorse/internal/upstream/upstream_test.go32
-rw-r--r--workhorse/internal/zipartifacts/open_archive.go20
-rw-r--r--workhorse/upload_test.go4
19 files changed, 239 insertions, 166 deletions
diff --git a/workhorse/.tool-versions b/workhorse/.tool-versions
new file mode 100644
index 00000000000..9d3b3b30ff2
--- /dev/null
+++ b/workhorse/.tool-versions
@@ -0,0 +1 @@
+golang 1.16.9
diff --git a/workhorse/Makefile b/workhorse/Makefile
index 0e8c47ae35c..3cf592b0cff 100644
--- a/workhorse/Makefile
+++ b/workhorse/Makefile
@@ -13,6 +13,8 @@ else
BUILD_TIME := $(shell date -u "$(DATE_FMT)")
endif
GOBUILD := go build -ldflags "-X main.Version=$(VERSION_STRING) -X main.BuildTime=$(BUILD_TIME)"
+GITALY := tmp/tests/gitaly/_build/bin/gitaly
+GITALY_PID_FILE := gitaly.pid
EXE_ALL := gitlab-resize-image gitlab-zip-cat gitlab-zip-metadata gitlab-workhorse
INSTALL := install
BUILD_TAGS := tracer_static tracer_static_jaeger continuous_profiler_stackdriver
@@ -63,7 +65,16 @@ install: $(EXE_ALL)
.PHONY: test
test: prepare-tests
$(call message,$@)
- @go test -tags "$(BUILD_TAGS)" ./...
+ go test -tags "$(BUILD_TAGS)" ./... ;\
+ status="$$?" ;\
+ if [ -f "$(GITALY_PID_FILE)" ] ; then \
+ echo "Clean up Gitaly server for workhorse integration test" ;\
+ kill -9 $$(cat $(GITALY_PID_FILE)) ;\
+ rm $(GITALY_PID_FILE) ;\
+ else \
+ echo "Gitaly integration test not running" ;\
+ fi ;\
+ exit "$$status"
@echo SUCCESS
.PHONY: clean
@@ -82,9 +93,27 @@ clean-build:
rm -rf $(TARGET_DIR)
.PHONY: prepare-tests
+prepare-tests: run-gitaly
prepare-tests: testdata/data/group/test.git $(EXE_ALL)
prepare-tests: testdata/scratch
+.PHONY: run-gitaly
+run-gitaly: gitaly.pid
+
+$(GITALY_PID_FILE): gitaly.toml
+ @{ \
+ if [ -z "$${GITALY_ADDRESS+x}" ] ; then \
+ echo "To run gitaly integration tests set GITALY_ADDRESS=tcp://127.0.0.1:8075" ; \
+ else \
+ cd .. ; \
+ GITALY_TESTING_NO_GIT_HOOKS=1 GITALY_PID_FILE=workhorse/$(GITALY_PID_FILE) $(GITALY) workhorse/gitaly.toml ; \
+ fi \
+ } &
+
+gitaly.toml: ../tmp/tests/gitaly/config.toml
+ sed -e 's/^socket_path.*$$/listen_addr = "0.0.0.0:8075"/;s/^\[auth\]$$//;s/^token.*$$//;s/^internal_socket_dir.*$$//' \
+ $< > $@
+
testdata/data/group/test.git:
$(call message,$@)
git clone --quiet --bare https://gitlab.com/gitlab-org/gitlab-test.git $@
diff --git a/workhorse/gitaly_integration_test.go b/workhorse/gitaly_integration_test.go
index 48de45d7935..95e0a03ab6b 100644
--- a/workhorse/gitaly_integration_test.go
+++ b/workhorse/gitaly_integration_test.go
@@ -16,6 +16,7 @@ import (
"strings"
"testing"
+ "github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
@@ -53,6 +54,10 @@ func realGitalyOkBody(t *testing.T) *api.Response {
return realGitalyAuthResponse(gitOkBody(t))
}
+func realGitalyOkBodyWithSidechannel(t *testing.T) *api.Response {
+ return realGitalyAuthResponse(gitOkBodyWithSidechannel(t))
+}
+
func ensureGitalyRepository(t *testing.T, apiResponse *api.Response) error {
ctx, namespace, err := gitaly.NewNamespaceClient(context.Background(), apiResponse.GitalyServer)
if err != nil {
@@ -83,10 +88,18 @@ func ensureGitalyRepository(t *testing.T, apiResponse *api.Response) error {
}
func TestAllowedClone(t *testing.T) {
+ testAllowedClone(t, realGitalyOkBody(t))
+}
+
+func TestAllowedCloneWithSidechannel(t *testing.T) {
+ gitaly.InitializeSidechannelRegistry(logrus.StandardLogger())
+ testAllowedClone(t, realGitalyOkBodyWithSidechannel(t))
+}
+
+func testAllowedClone(t *testing.T, apiResponse *api.Response) {
skipUnlessRealGitaly(t)
// Create the repository in the Gitaly server
- apiResponse := realGitalyOkBody(t)
require.NoError(t, ensureGitalyRepository(t, apiResponse))
// Prepare test server and backend
@@ -107,10 +120,18 @@ func TestAllowedClone(t *testing.T) {
}
func TestAllowedShallowClone(t *testing.T) {
+ testAllowedShallowClone(t, realGitalyOkBody(t))
+}
+
+func TestAllowedShallowCloneWithSidechannel(t *testing.T) {
+ gitaly.InitializeSidechannelRegistry(logrus.StandardLogger())
+ testAllowedShallowClone(t, realGitalyOkBodyWithSidechannel(t))
+}
+
+func testAllowedShallowClone(t *testing.T, apiResponse *api.Response) {
skipUnlessRealGitaly(t)
// Create the repository in the Gitaly server
- apiResponse := realGitalyOkBody(t)
require.NoError(t, ensureGitalyRepository(t, apiResponse))
// Prepare test server and backend
diff --git a/workhorse/internal/dependencyproxy/dependencyproxy.go b/workhorse/internal/dependencyproxy/dependencyproxy.go
index cfb3045544f..90f3042a342 100644
--- a/workhorse/internal/dependencyproxy/dependencyproxy.go
+++ b/workhorse/internal/dependencyproxy/dependencyproxy.go
@@ -4,37 +4,17 @@ import (
"context"
"fmt"
"io"
- "net"
"net/http"
- "time"
- "gitlab.com/gitlab-org/labkit/correlation"
"gitlab.com/gitlab-org/labkit/log"
- "gitlab.com/gitlab-org/labkit/tracing"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/helper"
+ "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata"
)
-// httpTransport defines a http.Transport with values
-// that are more restrictive than for http.DefaultTransport,
-// they define shorter TLS Handshake, and more aggressive connection closing
-// to prevent the connection hanging and reduce FD usage
-var httpTransport = tracing.NewRoundTripper(correlation.NewInstrumentedRoundTripper(&http.Transport{
- Proxy: http.ProxyFromEnvironment,
- DialContext: (&net.Dialer{
- Timeout: 30 * time.Second,
- KeepAlive: 10 * time.Second,
- }).DialContext,
- MaxIdleConns: 2,
- IdleConnTimeout: 30 * time.Second,
- TLSHandshakeTimeout: 10 * time.Second,
- ExpectContinueTimeout: 10 * time.Second,
- ResponseHeaderTimeout: 30 * time.Second,
-}))
-
var httpClient = &http.Client{
- Transport: httpTransport,
+ Transport: httptransport.New(),
}
type Injector struct {
@@ -87,15 +67,28 @@ func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData strin
return
}
+ w.Header().Set("Content-Length", dependencyResponse.Header.Get("Content-Length"))
+
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))
}
saveFileRequest.Header = helper.HeaderClone(r.Header)
- saveFileRequest.ContentLength = dependencyResponse.ContentLength
- w.Header().Del("Content-Length")
+ // forward headers from dependencyResponse to rails and client
+ for key, values := range dependencyResponse.Header {
+ saveFileRequest.Header.Del(key)
+ w.Header().Del(key)
+ for _, value := range values {
+ saveFileRequest.Header.Add(key, value)
+ w.Header().Add(key, value)
+ }
+ }
+
+ // workhorse hijack overwrites the Content-Type header, but we need this header value
+ saveFileRequest.Header.Set("Workhorse-Proxy-Content-Type", dependencyResponse.Header.Get("Content-Type"))
+ saveFileRequest.ContentLength = dependencyResponse.ContentLength
nrw := &nullResponseWriter{header: make(http.Header)}
p.uploadHandler.ServeHTTP(nrw, saveFileRequest)
diff --git a/workhorse/internal/dependencyproxy/dependencyproxy_test.go b/workhorse/internal/dependencyproxy/dependencyproxy_test.go
index 37e54c0b756..d9169b2b4ce 100644
--- a/workhorse/internal/dependencyproxy/dependencyproxy_test.go
+++ b/workhorse/internal/dependencyproxy/dependencyproxy_test.go
@@ -33,7 +33,7 @@ func (f *fakeUploadHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
type errWriter struct{ writes int }
-func (w *errWriter) Header() http.Header { return nil }
+func (w *errWriter) Header() http.Header { return make(http.Header) }
func (w *errWriter) WriteHeader(h int) {}
// First call of Write function succeeds while all the subsequent ones fail
@@ -112,8 +112,15 @@ func TestInject(t *testing.T) {
func TestSuccessfullRequest(t *testing.T) {
content := []byte("result")
+ contentLength := strconv.Itoa(len(content))
+ contentType := "foo"
+ dockerContentDigest := "sha256:asdf1234"
+ overriddenHeader := "originResourceServer"
originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- w.Header().Set("Content-Length", strconv.Itoa(len(content)))
+ w.Header().Set("Content-Length", contentLength)
+ w.Header().Set("Content-Type", contentType)
+ w.Header().Set("Docker-Content-Digest", dockerContentDigest)
+ w.Header().Set("Overridden-Header", overriddenHeader)
w.Write(content)
}))
@@ -130,11 +137,16 @@ func TestSuccessfullRequest(t *testing.T) {
require.Equal(t, "/target/upload", uploadHandler.request.URL.Path)
require.Equal(t, int64(6), uploadHandler.request.ContentLength)
+ require.Equal(t, contentType, uploadHandler.request.Header.Get("Workhorse-Proxy-Content-Type"))
+ require.Equal(t, dockerContentDigest, uploadHandler.request.Header.Get("Docker-Content-Digest"))
+ require.Equal(t, overriddenHeader, uploadHandler.request.Header.Get("Overridden-Header"))
require.Equal(t, content, uploadHandler.body)
require.Equal(t, 200, response.Code)
require.Equal(t, string(content), response.Body.String())
+ require.Equal(t, contentLength, response.Header().Get("Content-Length"))
+ require.Equal(t, dockerContentDigest, response.Header().Get("Docker-Content-Digest"))
}
func TestIncorrectSendData(t *testing.T) {
@@ -175,6 +187,7 @@ func TestFailedOriginServer(t *testing.T) {
func makeRequest(injector *Injector, data string) *httptest.ResponseRecorder {
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/target", nil)
+ r.Header.Set("Overridden-Header", "request")
sendData := base64.StdEncoding.EncodeToString([]byte(data))
injector.Inject(w, r, sendData)
diff --git a/workhorse/internal/filestore/file_handler.go b/workhorse/internal/filestore/file_handler.go
index b4d7250fe0c..dac8d4d6247 100644
--- a/workhorse/internal/filestore/file_handler.go
+++ b/workhorse/internal/filestore/file_handler.go
@@ -43,6 +43,9 @@ type FileHandler struct {
// a map containing different hashes
hashes map[string]string
+
+ // Duration of upload in seconds
+ uploadDuration float64
}
type uploadClaims struct {
@@ -74,11 +77,12 @@ func (fh *FileHandler) GitLabFinalizeFields(prefix string) (map[string]string, e
}
for k, v := range map[string]string{
- "name": fh.Name,
- "path": fh.LocalPath,
- "remote_url": fh.RemoteURL,
- "remote_id": fh.RemoteID,
- "size": strconv.FormatInt(fh.Size, 10),
+ "name": fh.Name,
+ "path": fh.LocalPath,
+ "remote_url": fh.RemoteURL,
+ "remote_id": fh.RemoteID,
+ "size": strconv.FormatInt(fh.Size, 10),
+ "upload_duration": strconv.FormatFloat(fh.uploadDuration, 'f', -1, 64),
} {
data[key(k)] = v
signedData[k] = v
@@ -105,18 +109,20 @@ type consumer interface {
// SaveFileFromReader persists the provided reader content to all the location specified in opts. A cleanup will be performed once ctx is Done
// Make sure the provided context will not expire before finalizing upload with GitLab Rails.
-func SaveFileFromReader(ctx context.Context, reader io.Reader, size int64, opts *SaveFileOpts) (fh *FileHandler, err error) {
- var uploadDestination consumer
- fh = &FileHandler{
+func SaveFileFromReader(ctx context.Context, reader io.Reader, size int64, opts *SaveFileOpts) (*FileHandler, error) {
+ fh := &FileHandler{
Name: opts.TempFilePrefix,
RemoteID: opts.RemoteID,
RemoteURL: opts.RemoteURL,
}
+ uploadStartTime := time.Now()
+ defer func() { fh.uploadDuration = time.Since(uploadStartTime).Seconds() }()
hashes := newMultiHash()
reader = io.TeeReader(reader, hashes.Writer)
var clientMode string
-
+ var uploadDestination consumer
+ var err error
switch {
case opts.IsLocal():
clientMode = "local"
@@ -161,23 +167,19 @@ func SaveFileFromReader(ctx context.Context, reader io.Reader, size int64, opts
return nil, err
}
+ var hlr *hardLimitReader
if opts.MaximumSize > 0 {
if size > opts.MaximumSize {
return nil, SizeError(fmt.Errorf("the upload size %d is over maximum of %d bytes", size, opts.MaximumSize))
}
- hlr := &hardLimitReader{r: reader, n: opts.MaximumSize}
+ hlr = &hardLimitReader{r: reader, n: opts.MaximumSize}
reader = hlr
- defer func() {
- if hlr.n < 0 {
- err = ErrEntityTooLarge
- }
- }()
}
fh.Size, err = uploadDestination.Consume(ctx, reader, opts.Deadline)
if err != nil {
- if err == objectstore.ErrNotEnoughParts {
+ if (err == objectstore.ErrNotEnoughParts) || (hlr != nil && hlr.n < 0) {
err = ErrEntityTooLarge
}
return nil, err
diff --git a/workhorse/internal/filestore/file_handler_test.go b/workhorse/internal/filestore/file_handler_test.go
index 16af56dcf48..f57026a59df 100644
--- a/workhorse/internal/filestore/file_handler_test.go
+++ b/workhorse/internal/filestore/file_handler_test.go
@@ -548,4 +548,5 @@ func checkFileHandlerWithFields(t *testing.T, fh *filestore.FileHandler, fields
require.Equal(t, test.ObjectSHA1, fields[key("sha1")])
require.Equal(t, test.ObjectSHA256, fields[key("sha256")])
require.Equal(t, test.ObjectSHA512, fields[key("sha512")])
+ require.NotEmpty(t, fields[key("upload_duration")])
}
diff --git a/workhorse/internal/helper/httptransport/http_transport.go b/workhorse/internal/helper/httptransport/http_transport.go
new file mode 100644
index 00000000000..c7c3c5283f5
--- /dev/null
+++ b/workhorse/internal/helper/httptransport/http_transport.go
@@ -0,0 +1,37 @@
+package httptransport
+
+import (
+ "net/http"
+ "time"
+
+ "gitlab.com/gitlab-org/labkit/correlation"
+ "gitlab.com/gitlab-org/labkit/tracing"
+)
+
+type Option func(*http.Transport)
+
+// Defines a http.Transport with values
+// that are more restrictive than for http.DefaultTransport,
+// they define shorter TLS Handshake, and more aggressive connection closing
+// to prevent the connection hanging and reduce FD usage
+func New(options ...Option) http.RoundTripper {
+ t := http.DefaultTransport.(*http.Transport).Clone()
+
+ // To avoid keep around TCP connections to http servers we're done with
+ t.MaxIdleConns = 2
+
+ // A stricter timeout for fetching from external sources that can be slow
+ t.ResponseHeaderTimeout = 30 * time.Second
+
+ for _, option := range options {
+ option(t)
+ }
+
+ return tracing.NewRoundTripper(correlation.NewInstrumentedRoundTripper(t))
+}
+
+func WithDisabledCompression() Option {
+ return func(t *http.Transport) {
+ t.DisableCompression = true
+ }
+}
diff --git a/workhorse/internal/imageresizer/image_resizer.go b/workhorse/internal/imageresizer/image_resizer.go
index cd0fa946530..8c3271b6f11 100644
--- a/workhorse/internal/imageresizer/image_resizer.go
+++ b/workhorse/internal/imageresizer/image_resizer.go
@@ -5,7 +5,6 @@ import (
"context"
"fmt"
"io"
- "net"
"net/http"
"os"
"os/exec"
@@ -18,11 +17,11 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
- "gitlab.com/gitlab-org/labkit/correlation"
"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/httptransport"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/log"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata"
)
@@ -69,23 +68,8 @@ const (
var envInjector = tracing.NewEnvInjector()
-// Images might be located remotely in object storage, in which case we need to stream
-// it via http(s)
-var httpTransport = tracing.NewRoundTripper(correlation.NewInstrumentedRoundTripper(&http.Transport{
- Proxy: http.ProxyFromEnvironment,
- DialContext: (&net.Dialer{
- Timeout: 30 * time.Second,
- KeepAlive: 10 * time.Second,
- }).DialContext,
- MaxIdleConns: 2,
- IdleConnTimeout: 30 * time.Second,
- TLSHandshakeTimeout: 10 * time.Second,
- ExpectContinueTimeout: 10 * time.Second,
- ResponseHeaderTimeout: 30 * time.Second,
-}))
-
var httpClient = &http.Client{
- Transport: httpTransport,
+ Transport: httptransport.New(),
}
const (
diff --git a/workhorse/internal/objectstore/object.go b/workhorse/internal/objectstore/object.go
index eaf3bfb2e36..b7c4f12f009 100644
--- a/workhorse/internal/objectstore/object.go
+++ b/workhorse/internal/objectstore/object.go
@@ -5,34 +5,15 @@ import (
"fmt"
"io"
"io/ioutil"
- "net"
"net/http"
- "time"
- "gitlab.com/gitlab-org/labkit/correlation"
"gitlab.com/gitlab-org/labkit/mask"
- "gitlab.com/gitlab-org/labkit/tracing"
-)
-// httpTransport defines a http.Transport with values
-// that are more restrictive than for http.DefaultTransport,
-// they define shorter TLS Handshake, and more aggressive connection closing
-// to prevent the connection hanging and reduce FD usage
-var httpTransport = tracing.NewRoundTripper(correlation.NewInstrumentedRoundTripper(&http.Transport{
- Proxy: http.ProxyFromEnvironment,
- DialContext: (&net.Dialer{
- Timeout: 30 * time.Second,
- KeepAlive: 10 * time.Second,
- }).DialContext,
- MaxIdleConns: 2,
- IdleConnTimeout: 30 * time.Second,
- TLSHandshakeTimeout: 10 * time.Second,
- ExpectContinueTimeout: 10 * time.Second,
- ResponseHeaderTimeout: 30 * time.Second,
-}))
+ "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport"
+)
var httpClient = &http.Client{
- Transport: httpTransport,
+ Transport: httptransport.New(),
}
// Object represents an object on a S3 compatible Object Store service.
diff --git a/workhorse/internal/sendurl/sendurl.go b/workhorse/internal/sendurl/sendurl.go
index ac2e66f95ab..205ec8a0e9f 100644
--- a/workhorse/internal/sendurl/sendurl.go
+++ b/workhorse/internal/sendurl/sendurl.go
@@ -3,18 +3,15 @@ package sendurl
import (
"fmt"
"io"
- "net"
"net/http"
- "time"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
- "gitlab.com/gitlab-org/labkit/correlation"
"gitlab.com/gitlab-org/labkit/mask"
- "gitlab.com/gitlab-org/labkit/tracing"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/helper"
+ "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/log"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata"
)
@@ -47,22 +44,7 @@ var preserveHeaderKeys = map[string]bool{
"Pragma": true, // Support for HTTP 1.0 proxies
}
-// httpTransport defines a http.Transport with values
-// that are more restrictive than for http.DefaultTransport,
-// they define shorter TLS Handshake, and more aggressive connection closing
-// to prevent the connection hanging and reduce FD usage
-var httpTransport = tracing.NewRoundTripper(correlation.NewInstrumentedRoundTripper(&http.Transport{
- Proxy: http.ProxyFromEnvironment,
- DialContext: (&net.Dialer{
- Timeout: 30 * time.Second,
- KeepAlive: 10 * time.Second,
- }).DialContext,
- MaxIdleConns: 2,
- IdleConnTimeout: 30 * time.Second,
- TLSHandshakeTimeout: 10 * time.Second,
- ExpectContinueTimeout: 10 * time.Second,
- ResponseHeaderTimeout: 30 * time.Second,
-}))
+var httpTransport = httptransport.New()
var httpClient = &http.Client{
Transport: httpTransport,
diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go
index e82dcdcbc69..b5db8003d41 100644
--- a/workhorse/internal/upload/uploads_test.go
+++ b/workhorse/internal/upload/uploads_test.go
@@ -114,7 +114,7 @@ func TestUploadHandlerRewritingMultiPartData(t *testing.T) {
require.Equal(t, hash, r.FormValue("file."+algo), "file hash %s", algo)
}
- require.Len(t, r.MultipartForm.Value, 11, "multipart form values")
+ require.Len(t, r.MultipartForm.Value, 12, "multipart form values")
w.WriteHeader(202)
fmt.Fprint(w, "RESPONSE")
diff --git a/workhorse/internal/upstream/roundtripper/roundtripper.go b/workhorse/internal/upstream/roundtripper/roundtripper.go
index fdbca5c0120..fcba50d7975 100644
--- a/workhorse/internal/upstream/roundtripper/roundtripper.go
+++ b/workhorse/internal/upstream/roundtripper/roundtripper.go
@@ -32,19 +32,23 @@ func NewBackendRoundTripper(backend *url.URL, socket string, proxyHeadersTimeout
}
func newBackendRoundTripper(backend *url.URL, socket string, proxyHeadersTimeout time.Duration, developmentMode bool, tlsConf *tls.Config) http.RoundTripper {
- // Copied from the definition of http.DefaultTransport. We can't literally copy http.DefaultTransport because of its hidden internal state.
- transport, dialer := newBackendTransport()
+ transport := http.DefaultTransport.(*http.Transport).Clone()
transport.ResponseHeaderTimeout = proxyHeadersTimeout
transport.TLSClientConfig = tlsConf
+ // Puma does not support http/2, there's no point in reconnecting
+ transport.ForceAttemptHTTP2 = false
+
+ dial := transport.DialContext
+
if backend != nil && socket == "" {
address := mustParseAddress(backend.Host, backend.Scheme)
transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
- return dialer.DialContext(ctx, "tcp", address)
+ return dial(ctx, "tcp", address)
}
} else if socket != "" {
transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
- return dialer.DialContext(ctx, "unix", socket)
+ return dial(ctx, "unix", socket)
}
} else {
panic("backend is nil and socket is empty")
diff --git a/workhorse/internal/upstream/roundtripper/transport.go b/workhorse/internal/upstream/roundtripper/transport.go
deleted file mode 100644
index 84d9623b129..00000000000
--- a/workhorse/internal/upstream/roundtripper/transport.go
+++ /dev/null
@@ -1,27 +0,0 @@
-package roundtripper
-
-import (
- "net"
- "net/http"
- "time"
-)
-
-// newBackendTransport setups the default HTTP transport which Workhorse uses
-// to communicate with the upstream
-func newBackendTransport() (*http.Transport, *net.Dialer) {
- dialler := &net.Dialer{
- Timeout: 30 * time.Second,
- KeepAlive: 30 * time.Second,
- }
-
- transport := &http.Transport{
- Proxy: http.ProxyFromEnvironment,
- DialContext: dialler.DialContext,
- MaxIdleConns: 100,
- IdleConnTimeout: 90 * time.Second,
- TLSHandshakeTimeout: 10 * time.Second,
- ExpectContinueTimeout: 1 * time.Second,
- }
-
- return transport, dialler
-}
diff --git a/workhorse/internal/upstream/routes.go b/workhorse/internal/upstream/routes.go
index d39ba845dc5..22b30fe8a63 100644
--- a/workhorse/internal/upstream/routes.go
+++ b/workhorse/internal/upstream/routes.go
@@ -60,6 +60,7 @@ const (
geoGitProjectPattern = `^/[^-].+\.git/` // Prevent matching routes like /-/push_from_secondary
projectPattern = `^/([^/]+/){1,}[^/]+/`
apiProjectPattern = apiPattern + `v4/projects/[^/]+/` // API: Projects can be encoded via group%2Fsubgroup%2Fproject
+ apiTopicPattern = apiPattern + `v4/topics`
snippetUploadPattern = `^/uploads/personal_snippet`
userUploadPattern = `^/uploads/user`
importPattern = `^/import/`
@@ -295,6 +296,8 @@ func configureRoutes(u *upstream) {
// Overall status can be seen at https://gitlab.com/groups/gitlab-org/-/epics/1802#current-status
u.route("POST", apiProjectPattern+`wikis/attachments\z`, uploadAccelerateProxy),
u.route("POST", apiPattern+`graphql\z`, uploadAccelerateProxy),
+ u.route("POST", apiTopicPattern, uploadAccelerateProxy),
+ u.route("PUT", apiTopicPattern, uploadAccelerateProxy),
u.route("POST", apiPattern+`v4/groups/import`, upload.Accelerate(api, signingProxy, preparers.uploads)),
u.route("POST", apiPattern+`v4/projects/import`, upload.Accelerate(api, signingProxy, preparers.uploads)),
@@ -374,6 +377,10 @@ func configureRoutes(u *upstream) {
// Geo API routes
u.route("", "^/api/v4/geo_nodes", defaultUpstream),
u.route("", "^/api/v4/geo_replication", defaultUpstream),
+ u.route("", "^/api/v4/geo/proxy_git_ssh", defaultUpstream),
+
+ // Internal API routes
+ u.route("", "^/api/v4/internal", defaultUpstream),
// Don't define a catch-all route. If a route does not match, then we know
// the request should be proxied.
diff --git a/workhorse/internal/upstream/routes_test.go b/workhorse/internal/upstream/routes_test.go
new file mode 100644
index 00000000000..f196433f5b4
--- /dev/null
+++ b/workhorse/internal/upstream/routes_test.go
@@ -0,0 +1,47 @@
+package upstream
+
+import (
+ "testing"
+)
+
+func TestProjectNotExistingGitHttpPullWithGeoProxy(t *testing.T) {
+ testCases := []testCase{
+ {"secondary info/refs", "/group/project.git/info/refs", "Local Rails server received request to path /group/project.git/info/refs"},
+ {"primary info/refs", "/-/push_from_secondary/2/group/project.git/info/refs", "Geo primary received request to path /-/push_from_secondary/2/group/project.git/info/refs"},
+ {"primary upload-pack", "/-/push_from_secondary/2/group/project.git/git-upload-pack", "Geo primary received request to path /-/push_from_secondary/2/group/project.git/git-upload-pack"},
+ }
+
+ runTestCasesWithGeoProxyEnabled(t, testCases)
+}
+
+func TestProjectNotExistingGitHttpPushWithGeoProxy(t *testing.T) {
+ testCases := []testCase{
+ {"secondary info/refs", "/group/project.git/info/refs", "Local Rails server received request to path /group/project.git/info/refs"},
+ {"primary info/refs", "/-/push_from_secondary/2/group/project.git/info/refs", "Geo primary received request to path /-/push_from_secondary/2/group/project.git/info/refs"},
+ {"primary receive-pack", "/-/push_from_secondary/2/group/project.git/git-receive-pack", "Geo primary received request to path /-/push_from_secondary/2/group/project.git/git-receive-pack"},
+ }
+
+ runTestCasesWithGeoProxyEnabled(t, testCases)
+}
+
+func TestProjectNotExistingGitSSHPullWithGeoProxy(t *testing.T) {
+ testCases := []testCase{
+ {"GitLab Shell call to authorized-keys", "/api/v4/internal/authorized_keys", "Local Rails server received request to path /api/v4/internal/authorized_keys"},
+ {"GitLab Shell call to allowed", "/api/v4/internal/allowed", "Local Rails server received request to path /api/v4/internal/allowed"},
+ {"GitLab Shell call to info/refs", "/api/v4/geo/proxy_git_ssh/info_refs_receive_pack", "Local Rails server received request to path /api/v4/geo/proxy_git_ssh/info_refs_receive_pack"},
+ {"GitLab Shell call to receive_pack", "/api/v4/geo/proxy_git_ssh/receive_pack", "Local Rails server received request to path /api/v4/geo/proxy_git_ssh/receive_pack"},
+ }
+
+ runTestCasesWithGeoProxyEnabled(t, testCases)
+}
+
+func TestProjectNotExistingGitSSHPushWithGeoProxy(t *testing.T) {
+ testCases := []testCase{
+ {"GitLab Shell call to authorized-keys", "/api/v4/internal/authorized_keys", "Local Rails server received request to path /api/v4/internal/authorized_keys"},
+ {"GitLab Shell call to allowed", "/api/v4/internal/allowed", "Local Rails server received request to path /api/v4/internal/allowed"},
+ {"GitLab Shell call to info/refs", "/api/v4/geo/proxy_git_ssh/info_refs_upload_pack", "Local Rails server received request to path /api/v4/geo/proxy_git_ssh/info_refs_upload_pack"},
+ {"GitLab Shell call to receive_pack", "/api/v4/geo/proxy_git_ssh/upload_pack", "Local Rails server received request to path /api/v4/geo/proxy_git_ssh/upload_pack"},
+ }
+
+ runTestCasesWithGeoProxyEnabled(t, testCases)
+}
diff --git a/workhorse/internal/upstream/upstream_test.go b/workhorse/internal/upstream/upstream_test.go
index 3c942767384..53c15bb7e91 100644
--- a/workhorse/internal/upstream/upstream_test.go
+++ b/workhorse/internal/upstream/upstream_test.go
@@ -88,16 +88,6 @@ func TestGeoProxyFeatureDisabledOnGeoSecondarySite(t *testing.T) {
}
func TestGeoProxyFeatureEnabledOnGeoSecondarySite(t *testing.T) {
- remoteServer, rsDeferredClose := startRemoteServer("Geo primary")
- defer rsDeferredClose()
-
- geoProxyEndpointResponseBody := fmt.Sprintf(`{"geo_proxy_url":"%v"}`, remoteServer.URL)
- railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
- defer deferredClose()
-
- ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, true)
- defer wsDeferredClose()
-
testCases := []testCase{
{"push from secondary is forwarded", "/-/push_from_secondary/foo/bar.git/info/refs", "Geo primary received request to path /-/push_from_secondary/foo/bar.git/info/refs"},
{"LFS files are served locally", "/group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6", "Local Rails server received request to path /group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6"},
@@ -106,7 +96,7 @@ func TestGeoProxyFeatureEnabledOnGeoSecondarySite(t *testing.T) {
{"unknown route is forwarded", "/anything", "Geo primary received request to path /anything"},
}
- runTestCases(t, ws, testCases)
+ runTestCasesWithGeoProxyEnabled(t, testCases)
}
// This test can be removed when the environment variable `GEO_SECONDARY_PROXY` is removed
@@ -227,6 +217,20 @@ func runTestCases(t *testing.T, ws *httptest.Server, testCases []testCase) {
}
}
+func runTestCasesWithGeoProxyEnabled(t *testing.T, testCases []testCase) {
+ remoteServer, rsDeferredClose := startRemoteServer("Geo primary")
+ defer rsDeferredClose()
+
+ geoProxyEndpointResponseBody := fmt.Sprintf(`{"geo_proxy_url":"%v"}`, remoteServer.URL)
+ railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
+ defer deferredClose()
+
+ ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, true)
+ defer wsDeferredClose()
+
+ runTestCases(t, ws, testCases)
+}
+
func newUpstreamConfig(authBackend string) *config.Config {
return &config.Config{
Version: "123",
@@ -284,9 +288,13 @@ func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*h
}
cfg := newUpstreamConfig(railsServerURL)
upstreamHandler := newUpstream(*cfg, logrus.StandardLogger(), myConfigureRoutes)
- ws := httptest.NewServer(upstreamHandler)
+
+ // Secret should be configured before the first Geo API poll happens on server start
+ // to prevent race conditions where the first API call happens without a secret path
testhelper.ConfigureSecret()
+ ws := httptest.NewServer(upstreamHandler)
+
waitForNextApiPoll := func() {}
if enableGeoProxyFeature {
diff --git a/workhorse/internal/zipartifacts/open_archive.go b/workhorse/internal/zipartifacts/open_archive.go
index cf0e38e9ee0..ec2fd691038 100644
--- a/workhorse/internal/zipartifacts/open_archive.go
+++ b/workhorse/internal/zipartifacts/open_archive.go
@@ -5,32 +5,20 @@ import (
"context"
"fmt"
"io"
- "net"
"net/http"
"os"
"strings"
- "time"
+ "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/httptransport"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/httprs"
- "gitlab.com/gitlab-org/labkit/correlation"
"gitlab.com/gitlab-org/labkit/mask"
- "gitlab.com/gitlab-org/labkit/tracing"
)
var httpClient = &http.Client{
- Transport: tracing.NewRoundTripper(correlation.NewInstrumentedRoundTripper(&http.Transport{
- Proxy: http.ProxyFromEnvironment,
- DialContext: (&net.Dialer{
- Timeout: 30 * time.Second,
- KeepAlive: 10 * time.Second,
- }).DialContext,
- IdleConnTimeout: 30 * time.Second,
- TLSHandshakeTimeout: 10 * time.Second,
- ExpectContinueTimeout: 10 * time.Second,
- ResponseHeaderTimeout: 30 * time.Second,
- DisableCompression: true,
- })),
+ Transport: httptransport.New(
+ httptransport.WithDisabledCompression(), // To avoid bugs when serving compressed files from object storage
+ ),
}
type archive struct {
diff --git a/workhorse/upload_test.go b/workhorse/upload_test.go
index 24c14bb12aa..478cbdb1a44 100644
--- a/workhorse/upload_test.go
+++ b/workhorse/upload_test.go
@@ -83,7 +83,7 @@ func uploadTestServer(t *testing.T, authorizeTests func(r *http.Request), extraT
require.NoError(t, r.ParseMultipartForm(100000))
- const nValues = 10 // file name, path, remote_url, remote_id, size, md5, sha1, sha256, sha512, gitlab-workhorse-upload for just the upload (no metadata because we are not POSTing a valid zip file)
+ const nValues = 11 // file name, path, remote_url, remote_id, size, md5, sha1, sha256, sha512, upload_duration, gitlab-workhorse-upload for just the upload (no metadata because we are not POSTing a valid zip file)
require.Len(t, r.MultipartForm.Value, nValues)
require.Empty(t, r.MultipartForm.File, "multipart form files")
@@ -123,6 +123,8 @@ func TestAcceleratedUpload(t *testing.T) {
{"POST", `/api/v4/projects/group%2Fproject/wikis/attachments`, false},
{"POST", `/api/v4/projects/group%2Fsubgroup%2Fproject/wikis/attachments`, false},
{"POST", `/api/graphql`, false},
+ {"POST", `/api/v4/topics`, false},
+ {"PUT", `/api/v4/topics`, false},
{"PUT", "/api/v4/projects/9001/packages/nuget/v1/files", true},
{"PUT", "/api/v4/projects/group%2Fproject/packages/nuget/v1/files", true},
{"PUT", "/api/v4/projects/group%2Fsubgroup%2Fproject/packages/nuget/v1/files", true},