Add latest changes from gitlab-org/gitlab@15-0-stable-eev15.0.0-rc42

10 files changed, 30 insertions, 191 deletions

diff --git a/workhorse/internal/upload/artifacts_uploader.go b/workhorse/internal/upload/artifacts_uploader.go
index 2a91a05fe3d..c1c49638e21 100644
--- a/workhorse/internal/upload/artifacts_uploader.go
+++ b/workhorse/internal/upload/artifacts_uploader.go
@@ -35,7 +35,6 @@ var zipSubcommandsErrorsCounter = promauto.NewCounterVec(
 	}, []string{"error"})
 
 type artifactsUploadProcessor struct {
-	opts   *destination.UploadOpts
 	format string
 
 	SavedFileTracker
@@ -44,7 +43,7 @@ type artifactsUploadProcessor struct {
 // Artifacts is like a Multipart but specific for artifacts upload.
 func Artifacts(myAPI *api.API, h http.Handler, p Preparer) http.Handler {
 	return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
-		opts, _, err := p.Prepare(a)
+		opts, err := p.Prepare(a)
 		if err != nil {
 			helper.Fail500(w, r, fmt.Errorf("UploadArtifacts: error preparing file storage options"))
 			return
@@ -52,7 +51,7 @@ func Artifacts(myAPI *api.API, h http.Handler, p Preparer) http.Handler {
 
 		format := r.URL.Query().Get(ArtifactFormatKey)
 
-		mg := &artifactsUploadProcessor{opts: opts, format: format, SavedFileTracker: SavedFileTracker{Request: r}}
+		mg := &artifactsUploadProcessor{format: format, SavedFileTracker: SavedFileTracker{Request: r}}
 		interceptMultipartFiles(w, r, h, a, mg, opts)
 	}, "/authorize")
 }
@@ -62,12 +61,9 @@ func (a *artifactsUploadProcessor) generateMetadataFromZip(ctx context.Context,
 	defer metaWriter.Close()
 
 	metaOpts := &destination.UploadOpts{
-		LocalTempPath:  a.opts.LocalTempPath,
+		LocalTempPath:  os.TempDir(),
 		TempFilePrefix: "metadata.gz",
 	}
-	if metaOpts.LocalTempPath == "" {
-		metaOpts.LocalTempPath = os.TempDir()
-	}
 
 	fileName := file.LocalPath
 	if fileName == "" {
diff --git a/workhorse/internal/upload/body_uploader.go b/workhorse/internal/upload/body_uploader.go
index d831f9f43a1..6fb201fe677 100644
--- a/workhorse/internal/upload/body_uploader.go
+++ b/workhorse/internal/upload/body_uploader.go
@@ -17,7 +17,7 @@ import (
 // request to gitlab-rails without the original request body.
 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, verifier, err := p.Prepare(a)
+		opts, err := p.Prepare(a)
 		if err != nil {
 			helper.Fail500(w, r, fmt.Errorf("RequestBody: preparation failed: %v", err))
 			return
@@ -29,13 +29,6 @@ func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
 			return
 		}
 
-		if verifier != nil {
-			if err := verifier.Verify(fh); err != nil {
-				helper.Fail500(w, r, fmt.Errorf("RequestBody: verification failed: %v", err))
-				return
-			}
-		}
-
 		data := url.Values{}
 		fields, err := fh.GitLabFinalizeFields("file")
 		if err != nil {
diff --git a/workhorse/internal/upload/body_uploader_test.go b/workhorse/internal/upload/body_uploader_test.go
index 47490db8780..35772be5bc3 100644
--- a/workhorse/internal/upload/body_uploader_test.go
+++ b/workhorse/internal/upload/body_uploader_test.go
@@ -49,19 +49,6 @@ func TestRequestBodyCustomPreparer(t *testing.T) {
 	require.Equal(t, fileContent, string(uploadEcho))
 }
 
-func TestRequestBodyCustomVerifier(t *testing.T) {
-	body := strings.NewReader(fileContent)
-	verifier := &mockVerifier{}
-
-	resp := testUpload(&rails{}, &alwaysLocalPreparer{verifier: verifier}, echoProxy(t, fileLen), body)
-	require.Equal(t, http.StatusOK, resp.StatusCode)
-
-	uploadEcho, err := ioutil.ReadAll(resp.Body)
-	require.NoError(t, err, "Can't read response body")
-	require.Equal(t, fileContent, string(uploadEcho))
-	require.True(t, verifier.invoked, "Verifier.Verify not invoked")
-}
-
 func TestRequestBodyAuthorizationFailure(t *testing.T) {
 	testNoProxyInvocation(t, http.StatusUnauthorized, &rails{unauthorized: true}, &alwaysLocalPreparer{})
 }
@@ -72,7 +59,6 @@ func TestRequestBodyErrors(t *testing.T) {
 		preparer *alwaysLocalPreparer
 	}{
 		{name: "Prepare failure", preparer: &alwaysLocalPreparer{prepareError: fmt.Errorf("")}},
-		{name: "Verify failure", preparer: &alwaysLocalPreparer{verifier: &alwaysFailsVerifier{}}},
 	}
 
 	for _, test := range tests {
@@ -165,31 +151,14 @@ func (r *rails) PreAuthorizeHandler(next api.HandleFunc, _ string) http.Handler
 }
 
 type alwaysLocalPreparer struct {
-	verifier     Verifier
 	prepareError error
 }
 
-func (a *alwaysLocalPreparer) Prepare(_ *api.Response) (*destination.UploadOpts, Verifier, error) {
+func (a *alwaysLocalPreparer) Prepare(_ *api.Response) (*destination.UploadOpts, error) {
 	opts, err := destination.GetOpts(&api.Response{TempPath: os.TempDir()})
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
-	return opts, a.verifier, a.prepareError
-}
-
-type alwaysFailsVerifier struct{}
-
-func (alwaysFailsVerifier) Verify(handler *destination.FileHandler) error {
-	return fmt.Errorf("Verification failed")
-}
-
-type mockVerifier struct {
-	invoked bool
-}
-
-func (m *mockVerifier) Verify(handler *destination.FileHandler) error {
-	m.invoked = true
-
-	return nil
+	return opts, a.prepareError
 }
diff --git a/workhorse/internal/upload/lfs_preparer.go b/workhorse/internal/upload/lfs_preparer.go
deleted file mode 100644
index e7c5cf16a30..00000000000
--- a/workhorse/internal/upload/lfs_preparer.go
+++ /dev/null
@@ -1,47 +0,0 @@
-package upload
-
-import (
-	"fmt"
-
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/config"
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination"
-)
-
-type object struct {
-	size int64
-	oid  string
-}
-
-func (l *object) Verify(fh *destination.FileHandler) error {
-	if fh.Size != l.size {
-		return fmt.Errorf("LFSObject: expected size %d, wrote %d", l.size, fh.Size)
-	}
-
-	if fh.SHA256() != l.oid {
-		return fmt.Errorf("LFSObject: expected sha256 %s, got %s", l.oid, fh.SHA256())
-	}
-
-	return nil
-}
-
-type uploadPreparer struct {
-	objectPreparer Preparer
-}
-
-// NewLfs returns a new preparer instance which adds capability to a wrapped
-// preparer to set options required for a LFS upload.
-func NewLfsPreparer(c config.Config, objectPreparer Preparer) Preparer {
-	return &uploadPreparer{objectPreparer: objectPreparer}
-}
-
-func (l *uploadPreparer) Prepare(a *api.Response) (*destination.UploadOpts, Verifier, error) {
-	opts, _, err := l.objectPreparer.Prepare(a)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	opts.TempFilePrefix = a.LfsOid
-
-	return opts, &object{oid: a.LfsOid, size: a.LfsSize}, nil
-}
diff --git a/workhorse/internal/upload/lfs_preparer_test.go b/workhorse/internal/upload/lfs_preparer_test.go
deleted file mode 100644
index 6be4a7c2955..00000000000
--- a/workhorse/internal/upload/lfs_preparer_test.go
+++ /dev/null
@@ -1,59 +0,0 @@
-package upload
-
-import (
-	"testing"
-
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/config"
-
-	"github.com/stretchr/testify/require"
-)
-
-func TestLfsPreparerWithConfig(t *testing.T) {
-	lfsOid := "abcd1234"
-	creds := config.S3Credentials{
-		AwsAccessKeyID:     "test-key",
-		AwsSecretAccessKey: "test-secret",
-	}
-
-	c := config.Config{
-		ObjectStorageCredentials: config.ObjectStorageCredentials{
-			Provider:      "AWS",
-			S3Credentials: creds,
-		},
-	}
-
-	r := &api.Response{
-		LfsOid: lfsOid,
-		RemoteObject: api.RemoteObject{
-			ID:                 "the upload ID",
-			UseWorkhorseClient: true,
-			ObjectStorage: &api.ObjectStorageParams{
-				Provider: "AWS",
-			},
-		},
-	}
-
-	uploadPreparer := NewObjectStoragePreparer(c)
-	lfsPreparer := NewLfsPreparer(c, uploadPreparer)
-	opts, verifier, err := lfsPreparer.Prepare(r)
-
-	require.NoError(t, err)
-	require.Equal(t, lfsOid, opts.TempFilePrefix)
-	require.True(t, opts.ObjectStorageConfig.IsAWS())
-	require.True(t, opts.UseWorkhorseClient)
-	require.Equal(t, creds, opts.ObjectStorageConfig.S3Credentials)
-	require.NotNil(t, verifier)
-}
-
-func TestLfsPreparerWithNoConfig(t *testing.T) {
-	c := config.Config{}
-	r := &api.Response{RemoteObject: api.RemoteObject{ID: "the upload ID"}}
-	uploadPreparer := NewObjectStoragePreparer(c)
-	lfsPreparer := NewLfsPreparer(c, uploadPreparer)
-	opts, verifier, err := lfsPreparer.Prepare(r)
-
-	require.NoError(t, err)
-	require.False(t, opts.UseWorkhorseClient)
-	require.NotNil(t, verifier)
-}
diff --git a/workhorse/internal/upload/multipart_uploader.go b/workhorse/internal/upload/multipart_uploader.go
index d0097f9e153..34675d2aa14 100644
--- a/workhorse/internal/upload/multipart_uploader.go
+++ b/workhorse/internal/upload/multipart_uploader.go
@@ -17,7 +17,7 @@ func Multipart(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
 	return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		s := &SavedFileTracker{Request: r}
 
-		opts, _, err := p.Prepare(a)
+		opts, err := p.Prepare(a)
 		if err != nil {
 			helper.Fail500(w, r, fmt.Errorf("Multipart: error preparing file storage options"))
 			return
diff --git a/workhorse/internal/upload/object_storage_preparer.go b/workhorse/internal/upload/object_storage_preparer.go
index f28f598c895..d237a9ca6bc 100644
--- a/workhorse/internal/upload/object_storage_preparer.go
+++ b/workhorse/internal/upload/object_storage_preparer.go
@@ -18,14 +18,14 @@ func NewObjectStoragePreparer(c config.Config) Preparer {
 	return &ObjectStoragePreparer{credentials: c.ObjectStorageCredentials, config: c.ObjectStorageConfig}
 }
 
-func (p *ObjectStoragePreparer) Prepare(a *api.Response) (*destination.UploadOpts, Verifier, error) {
+func (p *ObjectStoragePreparer) Prepare(a *api.Response) (*destination.UploadOpts, error) {
 	opts, err := destination.GetOpts(a)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	opts.ObjectStorageConfig.URLMux = p.config.URLMux
 	opts.ObjectStorageConfig.S3Credentials = p.credentials.S3Credentials
 
-	return opts, nil, nil
+	return opts, nil
 }
diff --git a/workhorse/internal/upload/object_storage_preparer_test.go b/workhorse/internal/upload/object_storage_preparer_test.go
index 5856a1bcc92..56de6bbf7d6 100644
--- a/workhorse/internal/upload/object_storage_preparer_test.go
+++ b/workhorse/internal/upload/object_storage_preparer_test.go
@@ -39,24 +39,22 @@ func TestPrepareWithS3Config(t *testing.T) {
 	}
 
 	p := upload.NewObjectStoragePreparer(c)
-	opts, v, err := p.Prepare(r)
+	opts, err := p.Prepare(r)
 
 	require.NoError(t, err)
 	require.True(t, opts.ObjectStorageConfig.IsAWS())
 	require.True(t, opts.UseWorkhorseClient)
 	require.Equal(t, creds, opts.ObjectStorageConfig.S3Credentials)
 	require.NotNil(t, opts.ObjectStorageConfig.URLMux)
-	require.Equal(t, nil, v)
 }
 
 func TestPrepareWithNoConfig(t *testing.T) {
 	c := config.Config{}
 	r := &api.Response{RemoteObject: api.RemoteObject{ID: "id"}}
 	p := upload.NewObjectStoragePreparer(c)
-	opts, v, err := p.Prepare(r)
+	opts, err := p.Prepare(r)
 
 	require.NoError(t, err)
 	require.False(t, opts.UseWorkhorseClient)
-	require.Nil(t, v)
 	require.Nil(t, opts.ObjectStorageConfig.URLMux)
 }
diff --git a/workhorse/internal/upload/preparer.go b/workhorse/internal/upload/preparer.go
index 46a4cac01b5..4d6d8bd1189 100644
--- a/workhorse/internal/upload/preparer.go
+++ b/workhorse/internal/upload/preparer.go
@@ -5,29 +5,18 @@ import (
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination"
 )
 
-// Verifier is an optional pluggable behavior for upload paths. If
-// Verify() returns an error, Workhorse will return an error response to
-// the client instead of propagating the request to Rails. The motivating
-// use case is Git LFS, where Workhorse checks the size and SHA256
-// checksum of the uploaded file.
-type Verifier interface {
-	// Verify can abort the upload by returning an error
-	Verify(handler *destination.FileHandler) error
-}
-
 // Preparer is a pluggable behavior that interprets a Rails API response
 // and either tells Workhorse how to handle the upload, via the
-// UploadOpts and Verifier, or it rejects the request by returning a
-// non-nil error. Its intended use is to make sure the upload gets stored
-// in the right location: either a local directory, or one of several
-// supported object storage backends.
+// UploadOpts, or it rejects the request by returning a non-nil error.
+// Its intended use is to make sure the upload gets stored in the right
+// location: either a local directory, or one of several supported object
+// storage backends.
 type Preparer interface {
-	Prepare(a *api.Response) (*destination.UploadOpts, Verifier, error)
+	Prepare(a *api.Response) (*destination.UploadOpts, error)
 }
 
 type DefaultPreparer struct{}
 
-func (s *DefaultPreparer) Prepare(a *api.Response) (*destination.UploadOpts, Verifier, error) {
-	opts, err := destination.GetOpts(a)
-	return opts, nil, err
+func (s *DefaultPreparer) Prepare(a *api.Response) (*destination.UploadOpts, error) {
+	return destination.GetOpts(a)
 }
diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go
index 9d787b10d1c..a9c8834d4be 100644
--- a/workhorse/internal/upload/uploads_test.go
+++ b/workhorse/internal/upload/uploads_test.go
@@ -46,7 +46,7 @@ func (a *testFormProcessor) Finalize(ctx context.Context) error {
 func TestUploadTempPathRequirement(t *testing.T) {
 	apiResponse := &api.Response{}
 	preparer := &DefaultPreparer{}
-	_, _, err := preparer.Prepare(apiResponse)
+	_, err := preparer.Prepare(apiResponse)
 	require.Error(t, err)
 }
 
@@ -75,7 +75,7 @@ func TestUploadHandlerForwardingRawData(t *testing.T) {
 	handler := newProxy(ts.URL)
 	apiResponse := &api.Response{TempPath: tempPath}
 	preparer := &DefaultPreparer{}
-	opts, _, err := preparer.Prepare(apiResponse)
+	opts, err := preparer.Prepare(apiResponse)
 	require.NoError(t, err)
 
 	interceptMultipartFiles(response, httpRequest, handler, apiResponse, nil, opts)
@@ -146,7 +146,7 @@ func TestUploadHandlerRewritingMultiPartData(t *testing.T) {
 
 	apiResponse := &api.Response{TempPath: tempPath}
 	preparer := &DefaultPreparer{}
-	opts, _, err := preparer.Prepare(apiResponse)
+	opts, err := preparer.Prepare(apiResponse)
 	require.NoError(t, err)
 
 	interceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)
@@ -215,7 +215,7 @@ func TestUploadHandlerDetectingInjectedMultiPartData(t *testing.T) {
 			handler := newProxy(ts.URL)
 			apiResponse := &api.Response{TempPath: tempPath}
 			preparer := &DefaultPreparer{}
-			opts, _, err := preparer.Prepare(apiResponse)
+			opts, err := preparer.Prepare(apiResponse)
 			require.NoError(t, err)
 
 			interceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)
@@ -245,7 +245,7 @@ func TestUploadProcessingField(t *testing.T) {
 	response := httptest.NewRecorder()
 	apiResponse := &api.Response{TempPath: tempPath}
 	preparer := &DefaultPreparer{}
-	opts, _, err := preparer.Prepare(apiResponse)
+	opts, err := preparer.Prepare(apiResponse)
 	require.NoError(t, err)
 
 	interceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
@@ -276,7 +276,7 @@ func TestUploadingMultipleFiles(t *testing.T) {
 	response := httptest.NewRecorder()
 	apiResponse := &api.Response{TempPath: tempPath}
 	preparer := &DefaultPreparer{}
-	opts, _, err := preparer.Prepare(apiResponse)
+	opts, err := preparer.Prepare(apiResponse)
 	require.NoError(t, err)
 
 	interceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
@@ -332,7 +332,7 @@ func TestUploadProcessingFile(t *testing.T) {
 			response := httptest.NewRecorder()
 			apiResponse := &api.Response{TempPath: tempPath}
 			preparer := &DefaultPreparer{}
-			opts, _, err := preparer.Prepare(apiResponse)
+			opts, err := preparer.Prepare(apiResponse)
 			require.NoError(t, err)
 
 			interceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
@@ -378,7 +378,7 @@ func TestInvalidFileNames(t *testing.T) {
 		response := httptest.NewRecorder()
 		apiResponse := &api.Response{TempPath: tempPath}
 		preparer := &DefaultPreparer{}
-		opts, _, err := preparer.Prepare(apiResponse)
+		opts, err := preparer.Prepare(apiResponse)
 		require.NoError(t, err)
 
 		interceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts)
@@ -444,7 +444,7 @@ func TestContentDispositionRewrite(t *testing.T) {
 			response := httptest.NewRecorder()
 			apiResponse := &api.Response{TempPath: tempPath}
 			preparer := &DefaultPreparer{}
-			opts, _, err := preparer.Prepare(apiResponse)
+			opts, err := preparer.Prepare(apiResponse)
 			require.NoError(t, err)
 
 			interceptMultipartFiles(response, httpRequest, customHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts)
@@ -567,7 +567,7 @@ func runUploadTest(t *testing.T, image []byte, filename string, httpCode int, ts
 	handler := newProxy(ts.URL)
 	apiResponse := &api.Response{TempPath: tempPath}
 	preparer := &DefaultPreparer{}
-	opts, _, err := preparer.Prepare(apiResponse)
+	opts, err := preparer.Prepare(apiResponse)
 	require.NoError(t, err)
 
 	interceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)