Commit c64bd8e3 authored by Stan Hu's avatar Stan Hu

Allow blank S3 regions to be used

If consolidated object storage is used without a region, Workhorse will
fall back to making a direct HTTP PUT call to the S3 endpoint because
GitLab Rails does not send the multipart upload URLs in this mode.

However, this will result a 411 Length Required error because multipart
upload URLs must be used when the Content-Length header is not
available.

An S3 region is not strictly required, particularly with Minio. With
Amazon S3, the region will default to US East (`us-east-1`). If the
bucket does not exist in the region, S3 will return with a more
informative error.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/297227
parent e25137d1
---
title: Allow blank S3 regions to be used
merge_request: 677
author:
type: changed
...@@ -146,7 +146,7 @@ func (c *ObjectStorageConfig) IsGoCloud() bool { ...@@ -146,7 +146,7 @@ func (c *ObjectStorageConfig) IsGoCloud() bool {
func (c *ObjectStorageConfig) IsValid() bool { func (c *ObjectStorageConfig) IsValid() bool {
if c.IsAWS() { if c.IsAWS() {
return c.S3Config.Bucket != "" && c.S3Config.Region != "" && c.s3CredentialsValid() return c.S3Config.Bucket != "" && c.s3CredentialsValid()
} else if c.IsGoCloud() { } else if c.IsGoCloud() {
// We could parse and validate the URL, but GoCloud providers // We could parse and validate the URL, but GoCloud providers
// such as AzureRM don't have a fallback to normal HTTP, so we // such as AzureRM don't have a fallback to normal HTTP, so we
......
...@@ -187,6 +187,9 @@ func TestUseWorkhorseClientEnabled(t *testing.T) { ...@@ -187,6 +187,9 @@ func TestUseWorkhorseClientEnabled(t *testing.T) {
iamConfig := missingCfg iamConfig := missingCfg
iamConfig.S3Config.UseIamProfile = true iamConfig.S3Config.UseIamProfile = true
missingRegion := cfg
missingRegion.S3Config.Region = ""
tests := []struct { tests := []struct {
name string name string
UseWorkhorseClient bool UseWorkhorseClient bool
...@@ -245,6 +248,13 @@ func TestUseWorkhorseClientEnabled(t *testing.T) { ...@@ -245,6 +248,13 @@ func TestUseWorkhorseClientEnabled(t *testing.T) {
}, },
expected: false, expected: false,
}, },
{
name: "missing S3 region",
UseWorkhorseClient: true,
remoteTempObjectID: "test-object",
objectStorageConfig: missingRegion,
expected: true,
},
} }
for _, test := range tests { for _, test := range tests {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment