Improve workhorse dependencyproxy logic
🔥 Problem
In Investigate dependency proxy for packages perfo... (#451242 - closed), we noticed that the workhorse dependency proxy logic will do these interactions (simplified):
sequenceDiagram
autonumber
Client ($ mvn) ->> Workhorse: Hey, give me `foobar-1.2.3.jar`
Workhorse ->> Rails: Hey, given me `foobar-1.2.3.jar`
Rails ->> Workhorse: I don't have it but get it from this upstream and upload it here.
Workhorse ->> Upstream: Hey, give me `foobar-1.2.3.jar`
Upstream ->> Workhorse: Certainly, here is `foobar-1.2.3.jar`
Workhorse ->> Rails: I have a upload to do, where do you want me to put this file?
Rails ->> Workhorse: Certainly, put it here on object storage.
Workhorse ->> Object Storage: Put `foobar-1.2.3.jar`
Workhorse ->> Client ($ mvn): Here is `foobar-1.2.3.jar`
Workhorse ->> Rails: I put `foobar-1.2.3.jar` on object storage.
Rails ->> Workhorse: Thanks, I created the dependency proxy cache for `foobar-1.2.3.jar`Things to note:
- Request
3is how rails gives the instructions on what to do to workhorse. -
8and9doesn't happen in sequence but in parallel. I don't want to go into technical details but it is very similar to plumbing. The data flows from the upstream and workhorse will implement aTjunction so that it flows into the client and the upload destination (object storage) at the same time. - We can see that the dependency proxy uses the workhorse assisted uploads. I don't want into details but the main thing is that workhorse will ask Rails where to put the file, upload it and then "confirm" the upload to Rails.
- The Client will receive the file even though workhorse will still confirm the upload to rails (interactions
10and11) - From the previous point, we can see that for a single Client interaction, we have
3requests done to the rails backend.
This works nicely in general. The problem is its scalability. In Investigate dependency proxy for packages perfo... (#451242 - closed), we used a dummy project that pulled over 1200 files. Under specific conditions (no response cached from upstreams), the above interactions will happen for every file. This will snowball into an increased latency by a noticeable factor.
🚒 Solution
One solution explored for this is to cut down the amount of interactions. Basically, when the rails backend gives the dependency proxy instructions to workhorse, it also gives the upload authorization payload (instructs about where to upload the file). This way, workhorse doesn't need to ask for that authorization (interactions 6 and 7), it already has it.
Here is the updated schema:
sequenceDiagram
autonumber
Client ($ mvn) ->> Workhorse: Hey, give me `foobar-1.2.3.jar`
Workhorse ->> Rails: Hey, given me `foobar-1.2.3.jar`
Rails ->> Workhorse: I don't have it but get it from this upstream and upload it here.
Workhorse ->> Upstream: Hey, give me `foobar-1.2.3.jar`
Upstream ->> Workhorse: Certainly, here is `foobar-1.2.3.jar`
Workhorse ->> Object Storage: Put `foobar-1.2.3.jar`
Workhorse ->> Client ($ mvn): Here is `foobar-1.2.3.jar`
Workhorse ->> Rails: I put `foobar-1.2.3.jar` on object storage.
Rails ->> Workhorse: Thanks, I created the dependency proxy cache for `foobar-1.2.3.jar`From the early findings in the analysis, this could lead to a 47% improvement.
⚙ Technical details
Here is a quick and dirty (eg. not a production-ready grade) change that we done to test the solution.
diff
diff --git a/ee/lib/api/concerns/dependency_proxy/packages_helpers.rb b/ee/lib/api/concerns/dependency_proxy/packages_helpers.rb
index e2dfdada1a82..c95ef9b13144 100644
--- a/ee/lib/api/concerns/dependency_proxy/packages_helpers.rb
+++ b/ee/lib/api/concerns/dependency_proxy/packages_helpers.rb
@@ -99,7 +99,8 @@ def send_and_upload_remote_url
upload_config = {
method: upload_method,
url: upload_url,
- headers: upload_headers
+ headers: upload_headers,
+ authorize_response: ::Packages::PackageFileUploader.workhorse_authorize(has_length: true)
}
send_workhorse_headers(
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index d66f070a7ebe..12f7b3266563 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -215,7 +215,8 @@ def send_dependency(headers, url, upload_config: {})
'UploadConfig' => {
'Method' => upload_config[:method],
'Url' => upload_config[:url],
- 'Headers' => (upload_config[:headers] || {}).transform_values { |v| Array.wrap(v) }
+ 'Headers' => (upload_config[:headers] || {}).transform_values { |v| Array.wrap(v) },
+ 'AuthorizeResponse' => upload_config[:authorize_response] || {}
}.compact_blank!
}
params.compact_blank!
diff --git a/workhorse/internal/dependencyproxy/dependencyproxy.go b/workhorse/internal/dependencyproxy/dependencyproxy.go
index a28d108b431b..79fd62ae76f2 100644
--- a/workhorse/internal/dependencyproxy/dependencyproxy.go
+++ b/workhorse/internal/dependencyproxy/dependencyproxy.go
@@ -11,9 +11,11 @@ import (
"gitlab.com/gitlab-org/labkit/log"
+ "gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
"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"
+ "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload"
)
const dialTimeout = 10 * time.Second
@@ -26,7 +28,7 @@ var httpClient = &http.Client{
type Injector struct {
senddata.Prefix
- uploadHandler http.Handler
+ uploadHandler upload.WithoutAuthorizeHandler
}
type entryParams struct {
@@ -36,9 +38,10 @@ type entryParams struct {
}
type uploadConfig struct {
- Headers http.Header
- Method string
- Url string
+ Headers http.Header
+ Method string
+ Url string
+ AuthorizeResponse api.Response
}
type nullResponseWriter struct {
@@ -64,7 +67,7 @@ func NewInjector() *Injector {
return &Injector{Prefix: "send-dependency:"}
}
-func (p *Injector) SetUploadHandler(uploadHandler http.Handler) {
+func (p *Injector) SetUploadHandler(uploadHandler upload.WithoutAuthorizeHandler) {
p.uploadHandler = uploadHandler
}
@@ -116,7 +119,7 @@ func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData strin
saveFileRequest.ContentLength = dependencyResponse.ContentLength
nrw := &nullResponseWriter{header: make(http.Header)}
- p.uploadHandler.ServeHTTP(nrw, saveFileRequest)
+ p.uploadHandler.WithoutAuthorizeServeHTTP(nrw, saveFileRequest, ¶ms.UploadConfig.AuthorizeResponse)
if nrw.status != http.StatusOK {
fields := log.Fields{"code": nrw.status}
diff --git a/workhorse/internal/dependencyproxy/dependencyproxy_test.go b/workhorse/internal/dependencyproxy/dependencyproxy_test.go
index b028fbcf355c..a1e580db0bfe 100644
--- a/workhorse/internal/dependencyproxy/dependencyproxy_test.go
+++ b/workhorse/internal/dependencyproxy/dependencyproxy_test.go
@@ -7,17 +7,13 @@ import (
"io"
"net/http"
"net/http/httptest"
- "strconv"
- "strings"
"testing"
"time"
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
- "gitlab.com/gitlab-org/gitlab/workhorse/internal/testhelper"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/transport"
- "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload"
)
type fakeUploadHandler struct {
@@ -58,198 +54,198 @@ func (f *fakePreAuthHandler) PreAuthorizeHandler(handler api.HandleFunc, _ strin
})
}
-func TestInject(t *testing.T) {
- contentLength := 32768 + 1
- content := strings.Repeat("p", contentLength)
-
- testCases := []struct {
- desc string
- responseWriter http.ResponseWriter
- contentLength int
- handlerMustBeCalled bool
- }{
- {
- desc: "the uploading successfully finalized",
- responseWriter: httptest.NewRecorder(),
- contentLength: contentLength,
- handlerMustBeCalled: true,
- }, {
- desc: "a user failed to receive the response",
- responseWriter: &errWriter{},
- contentLength: contentLength,
- handlerMustBeCalled: false,
- }, {
- desc: "the origin resource server returns partial response",
- responseWriter: httptest.NewRecorder(),
- contentLength: contentLength + 1,
- handlerMustBeCalled: false,
- },
- }
- testhelper.ConfigureSecret()
-
- for _, tc := range testCases {
- originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- w.Header().Set("Content-Length", strconv.Itoa(tc.contentLength))
- w.Write([]byte(content))
- }))
- defer originResourceServer.Close()
-
- // RequestBody expects http.Handler as its second param, we can create a stub function and verify that
- // it's only called for successful requests
- handlerIsCalled := false
- handlerFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { handlerIsCalled = true })
-
- bodyUploader := upload.RequestBody(&fakePreAuthHandler{}, handlerFunc, &upload.DefaultPreparer{})
-
- injector := NewInjector()
- injector.SetUploadHandler(bodyUploader)
-
- r := httptest.NewRequest("GET", "/target", nil)
- sendData := base64.StdEncoding.EncodeToString([]byte(`{"Token": "token", "Url": "` + originResourceServer.URL + `/url"}`))
-
- injector.Inject(tc.responseWriter, r, sendData)
-
- require.Equal(t, tc.handlerMustBeCalled, handlerIsCalled, "a partial file must not be saved")
- }
-}
-
-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", contentLength)
- w.Header().Set("Content-Type", contentType)
- w.Header().Set("Docker-Content-Digest", dockerContentDigest)
- w.Header().Set("Overridden-Header", overriddenHeader)
- w.Write(content)
- }))
- defer originResourceServer.Close()
-
- uploadHandler := &fakeUploadHandler{
- handler: func(w http.ResponseWriter, r *http.Request) {
- w.WriteHeader(200)
- },
- }
-
- injector := NewInjector()
- injector.SetUploadHandler(uploadHandler)
-
- response := makeRequest(injector, `{"Token": "token", "Url": "`+originResourceServer.URL+`/url"}`)
-
- 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 TestValidUploadConfiguration(t *testing.T) {
- content := []byte("content")
- contentLength := strconv.Itoa(len(content))
- contentType := "text/plain"
- testHeader := "test-received-url"
- originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- w.Header().Set(testHeader, r.URL.Path)
- w.Header().Set("Content-Length", contentLength)
- w.Header().Set("Content-Type", contentType)
- w.Write(content)
- }))
- defer originResourceServer.Close()
-
- testCases := []struct {
- desc string
- uploadConfig *uploadConfig
- expectedConfig uploadConfig
- }{
- {
- desc: "with the default values",
- expectedConfig: uploadConfig{
- Method: http.MethodPost,
- Url: "/target/upload",
- },
- }, {
- desc: "with overriden method",
- uploadConfig: &uploadConfig{
- Method: http.MethodPut,
- },
- expectedConfig: uploadConfig{
- Method: http.MethodPut,
- Url: "/target/upload",
- },
- }, {
- desc: "with overriden url",
- uploadConfig: &uploadConfig{
- Url: "http://test.org/overriden/upload",
- },
- expectedConfig: uploadConfig{
- Method: http.MethodPost,
- Url: "http://test.org/overriden/upload",
- },
- }, {
- desc: "with overriden headers",
- uploadConfig: &uploadConfig{
- Headers: map[string][]string{"Private-Token": {"123456789"}},
- },
- expectedConfig: uploadConfig{
- Headers: map[string][]string{"Private-Token": {"123456789"}},
- Method: http.MethodPost,
- Url: "/target/upload",
- },
- },
- }
-
- for _, tc := range testCases {
- t.Run(tc.desc, func(t *testing.T) {
- uploadHandler := &fakeUploadHandler{
- handler: func(w http.ResponseWriter, r *http.Request) {
- require.Equal(t, tc.expectedConfig.Url, r.URL.String())
- require.Equal(t, tc.expectedConfig.Method, r.Method)
-
- if tc.expectedConfig.Headers != nil {
- for k, v := range tc.expectedConfig.Headers {
- require.Equal(t, v, r.Header[k])
- }
- }
-
- w.WriteHeader(200)
- },
- }
-
- injector := NewInjector()
- injector.SetUploadHandler(uploadHandler)
-
- sendData := map[string]interface{}{
- "Token": "token",
- "Url": originResourceServer.URL + `/remote/file`,
- }
-
- if tc.uploadConfig != nil {
- sendData["UploadConfig"] = tc.uploadConfig
- }
-
- sendDataJsonString, err := json.Marshal(sendData)
- require.NoError(t, err)
-
- response := makeRequest(injector, string(sendDataJsonString))
-
- //checking the response
- require.Equal(t, 200, response.Code)
- require.Equal(t, string(content), response.Body.String())
- // checking remote file request
- require.Equal(t, "/remote/file", response.Header().Get(testHeader))
- })
- }
-}
+// func TestInject(t *testing.T) {
+// contentLength := 32768 + 1
+// content := strings.Repeat("p", contentLength)
+
+// testCases := []struct {
+// desc string
+// responseWriter http.ResponseWriter
+// contentLength int
+// handlerMustBeCalled bool
+// }{
+// {
+// desc: "the uploading successfully finalized",
+// responseWriter: httptest.NewRecorder(),
+// contentLength: contentLength,
+// handlerMustBeCalled: true,
+// }, {
+// desc: "a user failed to receive the response",
+// responseWriter: &errWriter{},
+// contentLength: contentLength,
+// handlerMustBeCalled: false,
+// }, {
+// desc: "the origin resource server returns partial response",
+// responseWriter: httptest.NewRecorder(),
+// contentLength: contentLength + 1,
+// handlerMustBeCalled: false,
+// },
+// }
+// testhelper.ConfigureSecret()
+
+// for _, tc := range testCases {
+// originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+// w.Header().Set("Content-Length", strconv.Itoa(tc.contentLength))
+// w.Write([]byte(content))
+// }))
+// defer originResourceServer.Close()
+
+// // RequestBody expects http.Handler as its second param, we can create a stub function and verify that
+// // it's only called for successful requests
+// handlerIsCalled := false
+// handlerFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { handlerIsCalled = true })
+
+// bodyUploader := upload.RequestBody(&fakePreAuthHandler{}, handlerFunc, &upload.DefaultPreparer{})
+
+// injector := NewInjector()
+// injector.SetUploadHandler(bodyUploader)
+
+// r := httptest.NewRequest("GET", "/target", nil)
+// sendData := base64.StdEncoding.EncodeToString([]byte(`{"Token": "token", "Url": "` + originResourceServer.URL + `/url"}`))
+
+// injector.Inject(tc.responseWriter, r, sendData)
+
+// require.Equal(t, tc.handlerMustBeCalled, handlerIsCalled, "a partial file must not be saved")
+// }
+// }
+
+// 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", contentLength)
+// w.Header().Set("Content-Type", contentType)
+// w.Header().Set("Docker-Content-Digest", dockerContentDigest)
+// w.Header().Set("Overridden-Header", overriddenHeader)
+// w.Write(content)
+// }))
+// defer originResourceServer.Close()
+
+// uploadHandler := &fakeUploadHandler{
+// handler: func(w http.ResponseWriter, r *http.Request) {
+// w.WriteHeader(200)
+// },
+// }
+
+// injector := NewInjector()
+// injector.SetUploadHandler(uploadHandler)
+
+// response := makeRequest(injector, `{"Token": "token", "Url": "`+originResourceServer.URL+`/url"}`)
+
+// 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 TestValidUploadConfiguration(t *testing.T) {
+// content := []byte("content")
+// contentLength := strconv.Itoa(len(content))
+// contentType := "text/plain"
+// testHeader := "test-received-url"
+// originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+// w.Header().Set(testHeader, r.URL.Path)
+// w.Header().Set("Content-Length", contentLength)
+// w.Header().Set("Content-Type", contentType)
+// w.Write(content)
+// }))
+// defer originResourceServer.Close()
+
+// testCases := []struct {
+// desc string
+// uploadConfig *uploadConfig
+// expectedConfig uploadConfig
+// }{
+// {
+// desc: "with the default values",
+// expectedConfig: uploadConfig{
+// Method: http.MethodPost,
+// Url: "/target/upload",
+// },
+// }, {
+// desc: "with overriden method",
+// uploadConfig: &uploadConfig{
+// Method: http.MethodPut,
+// },
+// expectedConfig: uploadConfig{
+// Method: http.MethodPut,
+// Url: "/target/upload",
+// },
+// }, {
+// desc: "with overriden url",
+// uploadConfig: &uploadConfig{
+// Url: "http://test.org/overriden/upload",
+// },
+// expectedConfig: uploadConfig{
+// Method: http.MethodPost,
+// Url: "http://test.org/overriden/upload",
+// },
+// }, {
+// desc: "with overriden headers",
+// uploadConfig: &uploadConfig{
+// Headers: map[string][]string{"Private-Token": {"123456789"}},
+// },
+// expectedConfig: uploadConfig{
+// Headers: map[string][]string{"Private-Token": {"123456789"}},
+// Method: http.MethodPost,
+// Url: "/target/upload",
+// },
+// },
+// }
+
+// for _, tc := range testCases {
+// t.Run(tc.desc, func(t *testing.T) {
+// uploadHandler := &fakeUploadHandler{
+// handler: func(w http.ResponseWriter, r *http.Request) {
+// require.Equal(t, tc.expectedConfig.Url, r.URL.String())
+// require.Equal(t, tc.expectedConfig.Method, r.Method)
+
+// if tc.expectedConfig.Headers != nil {
+// for k, v := range tc.expectedConfig.Headers {
+// require.Equal(t, v, r.Header[k])
+// }
+// }
+
+// w.WriteHeader(200)
+// },
+// }
+
+// injector := NewInjector()
+// injector.SetUploadHandler(uploadHandler)
+
+// sendData := map[string]interface{}{
+// "Token": "token",
+// "Url": originResourceServer.URL + `/remote/file`,
+// }
+
+// if tc.uploadConfig != nil {
+// sendData["UploadConfig"] = tc.uploadConfig
+// }
+
+// sendDataJsonString, err := json.Marshal(sendData)
+// require.NoError(t, err)
+
+// response := makeRequest(injector, string(sendDataJsonString))
+
+// //checking the response
+// require.Equal(t, 200, response.Code)
+// require.Equal(t, string(content), response.Body.String())
+// // checking remote file request
+// require.Equal(t, "/remote/file", response.Header().Get(testHeader))
+// })
+// }
+// }
func TestInvalidUploadConfiguration(t *testing.T) {
baseSendData := map[string]interface{}{
@@ -348,26 +344,26 @@ func TestIncorrectSendDataUrl(t *testing.T) {
require.Equal(t, "Bad Gateway\n", response.Body.String())
}
-func TestFailedOriginServer(t *testing.T) {
- originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- w.WriteHeader(404)
- w.Write([]byte("Not found"))
- }))
+// func TestFailedOriginServer(t *testing.T) {
+// originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+// w.WriteHeader(404)
+// w.Write([]byte("Not found"))
+// }))
- uploadHandler := &fakeUploadHandler{
- handler: func(w http.ResponseWriter, r *http.Request) {
- require.FailNow(t, "the error response must not be uploaded")
- },
- }
+// uploadHandler := &fakeUploadHandler{
+// handler: func(w http.ResponseWriter, r *http.Request) {
+// require.FailNow(t, "the error response must not be uploaded")
+// },
+// }
- injector := NewInjector()
- injector.SetUploadHandler(uploadHandler)
+// injector := NewInjector()
+// injector.SetUploadHandler(uploadHandler)
- response := makeRequest(injector, `{"Token": "token", "Url": "`+originResourceServer.URL+`/url"}`)
+// response := makeRequest(injector, `{"Token": "token", "Url": "`+originResourceServer.URL+`/url"}`)
- require.Equal(t, 404, response.Code)
- require.Equal(t, "Not found", response.Body.String())
-}
+// require.Equal(t, 404, response.Code)
+// require.Equal(t, "Not found", response.Body.String())
+// }
func makeRequest(injector *Injector, data string) *httptest.ResponseRecorder {
w := httptest.NewRecorder()
diff --git a/workhorse/internal/upload/body_uploader.go b/workhorse/internal/upload/body_uploader.go
index 307c32d2b9fe..bf0635e02f11 100644
--- a/workhorse/internal/upload/body_uploader.go
+++ b/workhorse/internal/upload/body_uploader.go
@@ -58,3 +58,56 @@ func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
h.ServeHTTP(w, r)
}, "/authorize")
}
+
+type WithoutAuthorizeServeHTTPFunc func(http.ResponseWriter, *http.Request, *api.Response)
+
+func (f WithoutAuthorizeServeHTTPFunc) WithoutAuthorizeServeHTTP(w http.ResponseWriter, r *http.Request, a *api.Response) {
+ f(w, r, a)
+}
+
+type WithoutAuthorizeHandler interface {
+ WithoutAuthorizeServeHTTP(http.ResponseWriter, *http.Request, *api.Response)
+}
+
+func WithoutAuthorizeHandlerRequestBody(h http.Handler, p Preparer) WithoutAuthorizeHandler {
+ return WithoutAuthorizeServeHTTPFunc(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
+ opts, err := p.Prepare(a)
+ if err != nil {
+ 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 {
+ fail.Request(w, r, fmt.Errorf("RequestBody: upload failed: %v", err))
+ return
+ }
+
+ data := url.Values{}
+ fields, err := fh.GitLabFinalizeFields("file")
+ if err != nil {
+ fail.Request(w, r, fmt.Errorf("RequestBody: finalize fields failed: %v", err))
+ return
+ }
+
+ for k, v := range fields {
+ data.Set(k, v)
+ }
+
+ // Hijack body
+ body := data.Encode()
+ r.Body = io.NopCloser(strings.NewReader(body))
+ r.ContentLength = int64(len(body))
+ r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+ sft := SavedFileTracker{Request: r}
+ sft.Track("file", fh.LocalPath)
+ if err := sft.Finalize(r.Context()); err != nil {
+ fail.Request(w, r, fmt.Errorf("RequestBody: finalize failed: %v", err))
+ return
+ }
+
+ // And proxy the request
+ h.ServeHTTP(w, r)
+ })
+}
diff --git a/workhorse/internal/upstream/routes.go b/workhorse/internal/upstream/routes.go
index 76ee4f9a2960..c1e2b1d49e4a 100644
--- a/workhorse/internal/upstream/routes.go
+++ b/workhorse/internal/upstream/routes.go
@@ -225,7 +225,8 @@ func configureRoutes(u *upstream) {
ciAPIProxyQueue := queueing.QueueRequests("ci_api_job_requests", tempfileMultipartProxy, u.APILimit, u.APIQueueLimit, u.APIQueueTimeout, prometheus.DefaultRegisterer)
ciAPILongPolling := builds.RegisterHandler(ciAPIProxyQueue, u.watchKeyHandler, u.APICILongPollingDuration)
- dependencyProxyInjector.SetUploadHandler(requestBodyUploader)
+ withoutAuthorizeUploader := upload.WithoutAuthorizeHandlerRequestBody(signingProxy, preparer)
+ dependencyProxyInjector.SetUploadHandler(withoutAuthorizeUploader)
// Serve static files or forward the requests
defaultUpstream := static.ServeExisting(Edited by David Fernandez