Consolidate upload preparers

In https://gitlab.com/gitlab-org/gitlab/-/issues/351657, the
goal is to document and refactor workhorse code related to
object storage upload. This is to improve readability and
help new contributors.

All preparer types are consolidated in the `upload` package so
they are easier to find and show that when a new type is needed,
this is where it can be added.

Currently, preparers are not mentioned in developer docs. A
separate change will be created for it to guide contributors
whether they need to add a new preparer or use an existing one.

workhorse/internal/upload/body_uploader.go

@@ -16,33 +16,6 @@ type PreAuthorizer interface {
 	PreAuthorizeHandler(next api.HandleFunc, suffix string) http.Handler
 }
 
-// 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 *filestore.FileHandler) error
-}
-
-// Preparer is a pluggable behavior that interprets a Rails API response
-// and either tells Workhorse how to handle the upload, via the
-// SaveFileOpts 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.
-type Preparer interface {
-	Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error)
-}
-
-type DefaultPreparer struct{}
-
-func (s *DefaultPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error) {
-	opts, err := filestore.GetOpts(a)
-	return opts, nil, err
-}
-
 // RequestBody is a request middleware. It will store the request body to
 // a location by determined an api.Response value. It then forwards the
 // request to gitlab-rails without the original request body.

workhorse/internal/lfs/lfs.go → workhorse/internal/upload/lfs_preparer.go

-/*
-In this file we handle git lfs objects downloads and uploads
-*/
-
-package lfs
+package upload
 
 import (
 	"fmt"
@@ -10,7 +6,6 @@ import (
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/config"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/filestore"
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/upload"
 )
 
 type object struct {
@@ -31,14 +26,16 @@ func (l *object) Verify(fh *filestore.FileHandler) error {
 }
 
 type uploadPreparer struct {
-	objectPreparer upload.Preparer
+	objectPreparer Preparer
 }
 
-func NewLfsUploadPreparer(c config.Config, objectPreparer upload.Preparer) upload.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) (*filestore.SaveFileOpts, upload.Verifier, error) {
+func (l *uploadPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error) {
 	opts, _, err := l.objectPreparer.Prepare(a)
 	if err != nil {
 		return nil, nil, err

workhorse/internal/lfs/lfs_test.go → workhorse/internal/upload/lfs_preparer_test.go

-package lfs_test
+package upload
 
 import (
 	"testing"
 
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/config"
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/lfs"
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/upload"
 
 	"github.com/stretchr/testify/require"
 )
 
-func TestLfsUploadPreparerWithConfig(t *testing.T) {
+func TestLfsPreparerWithConfig(t *testing.T) {
 	lfsOid := "abcd1234"
 	creds := config.S3Credentials{
 		AwsAccessKeyID:     "test-key",
@@ -36,8 +34,8 @@ func TestLfsUploadPreparerWithConfig(t *testing.T) {
 		},
 	}
 
-	uploadPreparer := upload.NewObjectStoragePreparer(c)
-	lfsPreparer := lfs.NewLfsUploadPreparer(c, uploadPreparer)
+	uploadPreparer := NewObjectStoragePreparer(c)
+	lfsPreparer := NewLfsPreparer(c, uploadPreparer)
 	opts, verifier, err := lfsPreparer.Prepare(r)
 
 	require.NoError(t, err)
@@ -48,11 +46,11 @@ func TestLfsUploadPreparerWithConfig(t *testing.T) {
 	require.NotNil(t, verifier)
 }
 
-func TestLfsUploadPreparerWithNoConfig(t *testing.T) {
+func TestLfsPreparerWithNoConfig(t *testing.T) {
 	c := config.Config{}
 	r := &api.Response{RemoteObject: api.RemoteObject{ID: "the upload ID"}}
-	uploadPreparer := upload.NewObjectStoragePreparer(c)
-	lfsPreparer := lfs.NewLfsUploadPreparer(c, uploadPreparer)
+	uploadPreparer := NewObjectStoragePreparer(c)
+	lfsPreparer := NewLfsPreparer(c, uploadPreparer)
 	opts, verifier, err := lfsPreparer.Prepare(r)
 
 	require.NoError(t, err)

workhorse/internal/upload/object_storage_preparer.go

@@ -11,6 +11,9 @@ type ObjectStoragePreparer struct {
 	credentials config.ObjectStorageCredentials
 }
 
+// NewObjectStoragePreparer returns a new preparer instance which is responsible for
+// setting the object storage credentials and settings needed by an uploader
+// to upload to object storage.
 func NewObjectStoragePreparer(c config.Config) Preparer {
 	return &ObjectStoragePreparer{credentials: c.ObjectStorageCredentials, config: c.ObjectStorageConfig}
 }

workhorse/internal/upload/preparer.go

+package upload
+
+import (
+	"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
+	"gitlab.com/gitlab-org/gitlab/workhorse/internal/filestore"
+)
+
+// 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 *filestore.FileHandler) error
+}
+
+// Preparer is a pluggable behavior that interprets a Rails API response
+// and either tells Workhorse how to handle the upload, via the
+// SaveFileOpts 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.
+type Preparer interface {
+	Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error)
+}
+
+type DefaultPreparer struct{}
+
+func (s *DefaultPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error) {
+	opts, err := filestore.GetOpts(a)
+	return opts, nil, err
+}

workhorse/internal/upstream/routes.go

@@ -20,7 +20,6 @@ import (
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/git"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/helper"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/imageresizer"
-	"gitlab.com/gitlab-org/gitlab/workhorse/internal/lfs"
 	proxypkg "gitlab.com/gitlab-org/gitlab/workhorse/internal/proxy"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/queueing"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/redis"
@@ -408,7 +407,7 @@ func createUploadPreparers(cfg config.Config) uploadPreparers {
 
 	return uploadPreparers{
 		artifacts: defaultPreparer,
-		lfs:       lfs.NewLfsUploadPreparer(cfg, defaultPreparer),
+		lfs:       upload.NewLfsPreparer(cfg, defaultPreparer),
 		packages:  defaultPreparer,
 		uploads:   defaultPreparer,
 	}