Merge pull request #1563 from jtackaberry/s3-path-style

Allow configurable S3 path style for auto backups

auto/backup/uploader.go

@@ -173,6 +173,7 @@ func (u *Uploader) upload(ctx context.Context) error {
 		stats.Get(lastUploadBytes).(*expvar.Int).Set(cr.Count())
 		u.lastUploadTime = time.Now()
 		u.lastUploadDuration = time.Since(startTime)
+		u.logger.Printf("completed auto upload to %s in %s", u.storageClient, u.lastUploadDuration)
 	}
 	return err
 }

auto/restore/downloader.go

@@ -39,7 +39,7 @@ func DownloadFile(ctx context.Context, cfgPath string) (path string, errOK bool,
 		return "", false, fmt.Errorf("failed to parse auto-restore file: %s", err.Error())
 	}
 	sc := aws.NewS3Client(s3cfg.Endpoint, s3cfg.Region, s3cfg.AccessKeyID, s3cfg.SecretAccessKey,
-		s3cfg.Bucket, s3cfg.Path)
+		s3cfg.Bucket, s3cfg.Path, s3cfg.ForcePathStyle)
 	d := NewDownloader(sc)
 
 	// Create a temporary file to download to.

aws/s3.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/credentials"
@@ -20,16 +21,18 @@ type S3Config struct {
 	SecretAccessKey string `json:"secret_access_key"`
 	Bucket          string `json:"bucket"`
 	Path            string `json:"path"`
+	ForcePathStyle  bool   `json:"force_path_style"`
 }
 
 // S3Client is a client for uploading data to S3.
 type S3Client struct {
-	endpoint  string
-	region    string
-	accessKey string
-	secretKey string
-	bucket    string
-	key       string
+	endpoint       string
+	region         string
+	accessKey      string
+	secretKey      string
+	bucket         string
+	key            string
+	forcePathStyle bool
 
 	// These fields are used for testing via dependency injection.
 	uploader   uploader
@@ -37,20 +40,30 @@ type S3Client struct {
 }
 
 // NewS3Client returns an instance of an S3Client.
-func NewS3Client(endpoint, region, accessKey, secretKey, bucket, key string) *S3Client {
+func NewS3Client(endpoint, region, accessKey, secretKey, bucket, key string, forcePathStyle bool) *S3Client {
 	return &S3Client{
-		endpoint:  endpoint,
-		region:    region,
-		accessKey: accessKey,
-		secretKey: secretKey,
-		bucket:    bucket,
-		key:       key,
+		endpoint:       endpoint,
+		region:         region,
+		accessKey:      accessKey,
+		secretKey:      secretKey,
+		bucket:         bucket,
+		key:            key,
+		forcePathStyle: forcePathStyle,
 	}
 }
 
 // String returns a string representation of the S3Client.
 func (s *S3Client) String() string {
-	return fmt.Sprintf("s3://%s/%s", s.bucket, s.key)
+	if s.endpoint == "" || strings.HasSuffix(s.endpoint, "amazonaws.com") {
+		// Native Amazon S3, use AWS's S3 URL format
+		return fmt.Sprintf("s3://%s/%s", s.bucket, s.key)
+	} else if !s.forcePathStyle {
+		// Endpoint specified but not using path style (e.g. Wasabi)
+		return fmt.Sprintf("s3://%s.%s/%s", s.bucket, s.endpoint, s.key)
+	} else {
+		// Endpoint specified and using path style (e.g. MinIO)
+		return fmt.Sprintf("s3://%s/%s/%s", s.endpoint, s.bucket, s.key)
+	}
 }
 
 // Upload uploads data to S3.
@@ -108,9 +121,10 @@ func (s *S3Client) Download(ctx context.Context, writer io.WriterAt) error {
 
 func (s *S3Client) createSession() (*session.Session, error) {
 	sess, err := session.NewSession(&aws.Config{
-		Endpoint:    aws.String(s.endpoint),
-		Region:      aws.String(s.region),
-		Credentials: credentials.NewStaticCredentials(s.accessKey, s.secretKey, ""),
+		Endpoint:         aws.String(s.endpoint),
+		Region:           aws.String(s.region),
+		Credentials:      credentials.NewStaticCredentials(s.accessKey, s.secretKey, ""),
+		S3ForcePathStyle: aws.Bool(s.forcePathStyle),
 	})
 	if err != nil {
 		return nil, fmt.Errorf("failed to create S3 session: %w", err)

aws/s3_test.go

@@ -14,7 +14,7 @@ import (
 )
 
 func Test_NewS3Client(t *testing.T) {
-	c := NewS3Client("endpoint1", "region1", "access", "secret", "bucket2", "key3")
+	c := NewS3Client("endpoint1", "region1", "access", "secret", "bucket2", "key3", true)
 	if c.region != "region1" {
 		t.Fatalf("expected region to be %q, got %q", "region1", c.region)
 	}
@@ -30,13 +30,32 @@ func Test_NewS3Client(t *testing.T) {
 	if c.key != "key3" {
 		t.Fatalf("expected key to be %q, got %q", "key3", c.key)
 	}
+	if c.forcePathStyle != true {
+		t.Fatalf("expected forcePathStyle to be %v, got %v", true, c.forcePathStyle)
+	}
 }
 
 func Test_S3Client_String(t *testing.T) {
-	c := NewS3Client("endpoint1", "region1", "access", "secret", "bucket2", "key3")
+	// Test native S3 with implicit endpoint
+	c := NewS3Client("", "region1", "access", "secret", "bucket2", "key3", false)
+	if c.String() != "s3://bucket2/key3" {
+		t.Fatalf("expected String() to be %q, got %q", "s3://bucket2/key3", c.String())
+	}
+	// Test native S3 with explicit endpoint
+	c = NewS3Client("s3.amazonaws.com", "region1", "access", "secret", "bucket2", "key3", false)
 	if c.String() != "s3://bucket2/key3" {
 		t.Fatalf("expected String() to be %q, got %q", "s3://bucket2/key3", c.String())
 	}
+	// Test non-native S3 (explicit endpoint) with non-path style (e.g. Wasabi)
+	c = NewS3Client("s3.ca-central-1.wasabisys.com", "region1", "access", "secret", "bucket2", "key3", false)
+	if c.String() != "s3://bucket2.s3.ca-central-1.wasabisys.com/key3" {
+		t.Fatalf("expected String() to be %q, got %q", "s3://bucket2.s3.ca-central-1.wasabisys.com/key3", c.String())
+	}
+	// Test non-native S3 (explicit endpoint) with forced path style (e.g. MinIO)
+	c = NewS3Client("s3.minio.example.com", "region1", "access", "secret", "bucket2", "key3", true)
+	if c.String() != "s3://s3.minio.example.com/bucket2/key3" {
+		t.Fatalf("expected String() to be %q, got %q", "s3://s3.minio.example.com/bucket2/key3", c.String())
+	}
 }
 
 func TestS3ClientUploadOK(t *testing.T) {

cmd/rqlited/main.go

@@ -254,7 +254,7 @@ func startAutoBackups(ctx context.Context, cfg *Config, str *store.Store) (*back
 	}
 	provider := store.NewProvider(str, false)
 	sc := aws.NewS3Client(s3cfg.Endpoint, s3cfg.Region, s3cfg.AccessKeyID, s3cfg.SecretAccessKey,
-		s3cfg.Bucket, s3cfg.Path)
+		s3cfg.Bucket, s3cfg.Path, s3cfg.ForcePathStyle)
 	u := backup.NewUploader(sc, provider, time.Duration(uCfg.Interval), !uCfg.NoCompress)
 	u.Start(ctx, nil)
 	return u, nil