diff options
Diffstat (limited to 'workhorse/internal/upload')
6 files changed, 60 insertions, 15 deletions
diff --git a/workhorse/internal/upload/artifacts_upload_test.go b/workhorse/internal/upload/artifacts_upload_test.go index 0a9e4ef3869..96eb3810673 100644 --- a/workhorse/internal/upload/artifacts_upload_test.go +++ b/workhorse/internal/upload/artifacts_upload_test.go @@ -66,7 +66,7 @@ func testArtifactsUploadServer(t *testing.T, authResponse *api.Response, bodyPro if r.Method != "POST" { t.Fatal("Expected POST request") } - if opts.IsLocal() { + if opts.IsLocalTempFile() { if r.FormValue("file.path") == "" { t.Fatal("Expected file to be present") return diff --git a/workhorse/internal/upload/destination/destination.go b/workhorse/internal/upload/destination/destination.go index 7a030e59a64..b18b6e22a99 100644 --- a/workhorse/internal/upload/destination/destination.go +++ b/workhorse/internal/upload/destination/destination.go @@ -128,9 +128,14 @@ func Upload(ctx context.Context, reader io.Reader, size int64, opts *UploadOpts) var uploadDestination consumer var err error switch { - case opts.IsLocal(): - clientMode = "local" + // This case means Workhorse is acting as an upload proxy for Rails and buffers files + // to disk in a temporary location, see: + // https://docs.gitlab.com/ee/development/uploads/background.html#moving-disk-buffering-to-workhorse + case opts.IsLocalTempFile(): + clientMode = "local_tempfile" uploadDestination, err = fh.newLocalFile(ctx, opts) + // All cases below mean we are doing a direct upload to remote i.e. object storage, see: + // https://docs.gitlab.com/ee/development/uploads/background.html#moving-to-object-storage-and-direct-uploads case opts.UseWorkhorseClientEnabled() && opts.ObjectStorageConfig.IsGoCloud(): clientMode = fmt.Sprintf("go_cloud:%s", opts.ObjectStorageConfig.Provider) p := &objectstore.GoCloudObjectParams{ @@ -141,14 +146,14 @@ func Upload(ctx context.Context, reader io.Reader, size int64, opts *UploadOpts) } uploadDestination, err = objectstore.NewGoCloudObject(p) case opts.UseWorkhorseClientEnabled() && opts.ObjectStorageConfig.IsAWS() && opts.ObjectStorageConfig.IsValid(): - clientMode = "s3" + clientMode = "s3_client" uploadDestination, err = objectstore.NewS3Object( opts.RemoteTempObjectID, opts.ObjectStorageConfig.S3Credentials, opts.ObjectStorageConfig.S3Config, ) case opts.IsMultipart(): - clientMode = "multipart" + clientMode = "s3_multipart" uploadDestination, err = objectstore.NewMultipart( opts.PresignedParts, opts.PresignedCompleteMultipart, @@ -158,7 +163,7 @@ func Upload(ctx context.Context, reader io.Reader, size int64, opts *UploadOpts) opts.PartSize, ) default: - clientMode = "http" + clientMode = "presigned_put" uploadDestination, err = objectstore.NewObject( opts.PresignedPut, opts.PresignedDelete, @@ -195,15 +200,15 @@ func Upload(ctx context.Context, reader io.Reader, size int64, opts *UploadOpts) logger := log.WithContextFields(ctx, log.Fields{ "copied_bytes": fh.Size, - "is_local": opts.IsLocal(), + "is_local": opts.IsLocalTempFile(), "is_multipart": opts.IsMultipart(), - "is_remote": !opts.IsLocal(), + "is_remote": !opts.IsLocalTempFile(), "remote_id": opts.RemoteID, "temp_file_prefix": opts.TempFilePrefix, "client_mode": clientMode, }) - if opts.IsLocal() { + if opts.IsLocalTempFile() { logger = logger.WithField("local_temp_path", opts.LocalTempPath) } else { logger = logger.WithField("remote_temp_object", opts.RemoteTempObjectID) diff --git a/workhorse/internal/upload/destination/objectstore/s3_session.go b/workhorse/internal/upload/destination/objectstore/s3_session.go index a0c1f099145..aa38f18ed7a 100644 --- a/workhorse/internal/upload/destination/objectstore/s3_session.go +++ b/workhorse/internal/upload/destination/objectstore/s3_session.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/credentials" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/aws/session" "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" @@ -70,7 +71,23 @@ func setupS3Session(s3Credentials config.S3Credentials, s3Config config.S3Config } if s3Config.Endpoint != "" { - cfg.Endpoint = aws.String(s3Config.Endpoint) + // The administrator has configured an S3 endpoint override, + // e.g. to make use of S3 IPv6 support or S3 FIPS mode. We + // need to configure a custom resolver to make sure that + // the custom endpoint is only used for S3 API calls, and not + // for STS API calls. + s3CustomResolver := func(service, region string, optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) { + if service == endpoints.S3ServiceID { + return endpoints.ResolvedEndpoint{ + URL: s3Config.Endpoint, + SigningRegion: region, + }, nil + } + + return endpoints.DefaultResolver().EndpointFor(service, region, optFns...) + } + + cfg.EndpointResolver = endpoints.ResolverFunc(s3CustomResolver) } sess, err := session.NewSession(cfg) diff --git a/workhorse/internal/upload/destination/objectstore/s3_session_test.go b/workhorse/internal/upload/destination/objectstore/s3_session_test.go index 5d57b4f9af8..4bbe38f90ec 100644 --- a/workhorse/internal/upload/destination/objectstore/s3_session_test.go +++ b/workhorse/internal/upload/destination/objectstore/s3_session_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" @@ -17,7 +18,9 @@ func TestS3SessionSetup(t *testing.T) { sess, err := setupS3Session(credentials, cfg) require.NoError(t, err) - require.Equal(t, aws.StringValue(sess.Config.Region), "us-west-1") + s3Config := sess.ClientConfig(endpoints.S3ServiceID) + require.Equal(t, "https://s3.us-west-1.amazonaws.com", s3Config.Endpoint) + require.Equal(t, "us-west-1", s3Config.SigningRegion) require.True(t, aws.BoolValue(sess.Config.S3ForcePathStyle)) require.Equal(t, len(sessionCache.sessions), 1) @@ -29,6 +32,26 @@ func TestS3SessionSetup(t *testing.T) { ResetS3Session(cfg) } +func TestS3SessionEndpointSetup(t *testing.T) { + credentials := config.S3Credentials{} + const customS3Endpoint = "https://example.com" + const region = "us-west-2" + cfg := config.S3Config{Region: region, PathStyle: true, Endpoint: customS3Endpoint} + + sess, err := setupS3Session(credentials, cfg) + require.NoError(t, err) + + // ClientConfig is what is ultimately used by an S3 client + s3Config := sess.ClientConfig(endpoints.S3ServiceID) + require.Equal(t, customS3Endpoint, s3Config.Endpoint) + require.Equal(t, region, s3Config.SigningRegion) + + stsConfig := sess.ClientConfig(endpoints.StsServiceID) + require.Equal(t, "https://sts.amazonaws.com", stsConfig.Endpoint, "STS should use default endpoint") + + ResetS3Session(cfg) +} + func TestS3SessionExpiry(t *testing.T) { credentials := config.S3Credentials{} cfg := config.S3Config{Region: "us-west-1", PathStyle: true} diff --git a/workhorse/internal/upload/destination/upload_opts.go b/workhorse/internal/upload/destination/upload_opts.go index 750a79d7bc2..77a8927d34f 100644 --- a/workhorse/internal/upload/destination/upload_opts.go +++ b/workhorse/internal/upload/destination/upload_opts.go @@ -70,8 +70,8 @@ func (s *UploadOpts) UseWorkhorseClientEnabled() bool { return s.UseWorkhorseClient && s.ObjectStorageConfig.IsValid() && s.RemoteTempObjectID != "" } -// IsLocal checks if the options require the writing of the file on disk -func (s *UploadOpts) IsLocal() bool { +// IsLocalTempFile checks if the options require the writing of a temporary file on disk +func (s *UploadOpts) IsLocalTempFile() bool { return s.LocalTempPath != "" } diff --git a/workhorse/internal/upload/destination/upload_opts_test.go b/workhorse/internal/upload/destination/upload_opts_test.go index fde726c985d..24a372495c6 100644 --- a/workhorse/internal/upload/destination/upload_opts_test.go +++ b/workhorse/internal/upload/destination/upload_opts_test.go @@ -49,7 +49,7 @@ func TestUploadOptsLocalAndRemote(t *testing.T) { PartSize: test.partSize, } - require.Equal(t, test.isLocal, opts.IsLocal(), "IsLocal() mismatch") + require.Equal(t, test.isLocal, opts.IsLocalTempFile(), "IsLocalTempFile() mismatch") require.Equal(t, test.isMultipart, opts.IsMultipart(), "IsMultipart() mismatch") }) } @@ -336,7 +336,7 @@ func TestGoCloudConfig(t *testing.T) { require.Equal(t, apiResponse.RemoteObject.ObjectStorage.GoCloudConfig, opts.ObjectStorageConfig.GoCloudConfig) require.True(t, opts.UseWorkhorseClientEnabled()) require.Equal(t, test.valid, opts.ObjectStorageConfig.IsValid()) - require.False(t, opts.IsLocal()) + require.False(t, opts.IsLocalTempFile()) }) } } |