fix(s3): add path-style support to fix Ceph RGW S3 compatibility

common/config/datasources.go

@@ -185,26 +185,38 @@ func FactorizeMinioServers(ctx context.Context, existingConfigs map[string]*obje
 			if !update {
 				newSource.ApiSecret = config.ApiSecret
 			} else {
-				// We may have to update signature version
+				// We may have to update signature version and path-style settings
+				if config.GatewayConfiguration == nil {
+					config.GatewayConfiguration = make(map[string]string)
+				}
 				if sv, o := newSource.StorageConfiguration[object.StorageKeySignatureVersion]; o {
-					if config.GatewayConfiguration == nil {
-						config.GatewayConfiguration = make(map[string]string, 1)
-					}
 					config.GatewayConfiguration[object.StorageKeySignatureVersion] = sv
 				}
+				if ps, o := newSource.StorageConfiguration[object.StorageKeyPathStyle]; o {
+					config.GatewayConfiguration[object.StorageKeyPathStyle] = ps
+				}
+				if fps, o := newSource.StorageConfiguration[object.StorageKeyForcePathStyle]; o {
+					config.GatewayConfiguration[object.StorageKeyForcePathStyle] = fps
+				}
 			}
 		} else if update {
 			// Update existing config
 			config = existingConfigs[newSource.ObjectsServiceName]
 			config.ApiKey = newSource.ApiKey
 			config.ApiSecret = newSource.ApiSecret
 			config.EndpointUrl = newSource.StorageConfiguration[object.StorageKeyCustomEndpoint]
+			if config.GatewayConfiguration == nil {
+				config.GatewayConfiguration = make(map[string]string)
+			}
 			if sv, o := newSource.StorageConfiguration[object.StorageKeySignatureVersion]; o {
-				if config.GatewayConfiguration == nil {
-					config.GatewayConfiguration = make(map[string]string, 1)
-				}
 				config.GatewayConfiguration[object.StorageKeySignatureVersion] = sv
 			}
+			if ps, o := newSource.StorageConfiguration[object.StorageKeyPathStyle]; o {
+				config.GatewayConfiguration[object.StorageKeyPathStyle] = ps
+			}
+			if fps, o := newSource.StorageConfiguration[object.StorageKeyForcePathStyle]; o {
+				config.GatewayConfiguration[object.StorageKeyForcePathStyle] = fps
+			}
 		} else {
 			config = &object.MinioConfig{
 				Name:        createConfigName(existingConfigs, object.StorageType_S3),
@@ -214,10 +226,18 @@ func FactorizeMinioServers(ctx context.Context, existingConfigs map[string]*obje
 				RunningPort: createConfigPort(existingConfigs, newSource.ObjectsPort),
 				EndpointUrl: newSource.StorageConfiguration[object.StorageKeyCustomEndpoint],
 			}
+			gatewayConfig := make(map[string]string)
 			if sv, o := newSource.StorageConfiguration[object.StorageKeySignatureVersion]; o {
-				config.GatewayConfiguration = map[string]string{
-					object.StorageKeySignatureVersion: sv,
-				}
+				gatewayConfig[object.StorageKeySignatureVersion] = sv
+			}
+			if ps, o := newSource.StorageConfiguration[object.StorageKeyPathStyle]; o {
+				gatewayConfig[object.StorageKeyPathStyle] = ps
+			}
+			if fps, o := newSource.StorageConfiguration[object.StorageKeyForcePathStyle]; o {
+				gatewayConfig[object.StorageKeyForcePathStyle] = fps
+			}
+			if len(gatewayConfig) > 0 {
+				config.GatewayConfiguration = gatewayConfig
 			}
 		}
 	} else if newSource.StorageType == object.StorageType_AZURE {

common/config/datasources_pathstyle_test.go

@@ -0,0 +1,127 @@
+/*
+ * Copyright (c) 2019-2021. Abstrium SAS <team (at) pydio.com>
+ * This file is part of Pydio Cells.
+ *
+ * Pydio Cells is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Pydio Cells is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Pydio Cells.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * The latest code can be found at <https://pydio.com>.
+ */
+
+package config
+
+import (
+	"context"
+	"testing"
+
+	. "github.com/smartystreets/goconvey/convey"
+
+	"github.com/pydio/cells/v5/common/proto/object"
+)
+
+func TestFactorizeMinioServersPathStyle(t *testing.T) {
+
+	Convey("Testing FactorizeMinioServers path_style propagation", t, func() {
+
+		Convey("New S3 config with path_style should store it in GatewayConfiguration", func() {
+			newSource := &object.DataSource{
+				Name:        "test",
+				StorageType: object.StorageType_S3,
+				ApiKey:      "mykey",
+				ApiSecret:   "mysecret",
+				StorageConfiguration: map[string]string{
+					object.StorageKeyCustomEndpoint: "http://ceph.example.com:7480",
+					object.StorageKeyPathStyle:      "true",
+					object.StorageKeyForcePathStyle: "true",
+				},
+			}
+			cfg, err := FactorizeMinioServers(context.Background(), map[string]*object.MinioConfig{}, newSource, false)
+			So(err, ShouldBeNil)
+			So(cfg, ShouldNotBeNil)
+			So(cfg.GatewayConfiguration, ShouldNotBeNil)
+			So(cfg.GatewayConfiguration[object.StorageKeyPathStyle], ShouldEqual, "true")
+			So(cfg.GatewayConfiguration[object.StorageKeyForcePathStyle], ShouldEqual, "true")
+		})
+
+		Convey("New S3 config without path_style should not set GatewayConfiguration keys", func() {
+			newSource := &object.DataSource{
+				Name:        "test",
+				StorageType: object.StorageType_S3,
+				ApiKey:      "mykey",
+				ApiSecret:   "mysecret",
+				StorageConfiguration: map[string]string{
+					object.StorageKeyCustomEndpoint: "http://s3.amazonaws.com",
+				},
+			}
+			cfg, err := FactorizeMinioServers(context.Background(), map[string]*object.MinioConfig{}, newSource, false)
+			So(err, ShouldBeNil)
+			So(cfg, ShouldNotBeNil)
+			// GatewayConfiguration should be nil or not contain path_style keys
+			if cfg.GatewayConfiguration != nil {
+				_, hasPs := cfg.GatewayConfiguration[object.StorageKeyPathStyle]
+				_, hasFps := cfg.GatewayConfiguration[object.StorageKeyForcePathStyle]
+				So(hasPs, ShouldBeFalse)
+				So(hasFps, ShouldBeFalse)
+			}
+		})
+
+		Convey("Update S3 config with existing gateway should propagate path_style", func() {
+			existingName := "pydio-s3"
+			existing := map[string]*object.MinioConfig{
+				existingName: {
+					Name:        existingName,
+					StorageType: object.StorageType_S3,
+					ApiKey:      "mykey",
+					ApiSecret:   "mysecret",
+				},
+			}
+			newSource := &object.DataSource{
+				Name:                 "test",
+				ObjectsServiceName:   existingName,
+				StorageType:          object.StorageType_S3,
+				ApiKey:               "mykey",
+				ApiSecret:            "newsecret",
+				StorageConfiguration: map[string]string{
+					object.StorageKeyPathStyle:      "true",
+					object.StorageKeyForcePathStyle: "true",
+				},
+			}
+			cfg, err := FactorizeMinioServers(context.Background(), existing, newSource, true)
+			So(err, ShouldBeNil)
+			So(cfg, ShouldNotBeNil)
+			So(cfg.GatewayConfiguration, ShouldNotBeNil)
+			So(cfg.GatewayConfiguration[object.StorageKeyPathStyle], ShouldEqual, "true")
+			So(cfg.GatewayConfiguration[object.StorageKeyForcePathStyle], ShouldEqual, "true")
+		})
+
+		Convey("AZURE storage type should not receive path_style in GatewayConfiguration", func() {
+			newSource := &object.DataSource{
+				Name:        "test-azure",
+				StorageType: object.StorageType_AZURE,
+				ApiKey:      "azurekey",
+				ApiSecret:   "azuresecret",
+				StorageConfiguration: map[string]string{
+					object.StorageKeyPathStyle: "true",
+				},
+			}
+			cfg, err := FactorizeMinioServers(context.Background(), map[string]*object.MinioConfig{}, newSource, false)
+			So(err, ShouldBeNil)
+			So(cfg, ShouldNotBeNil)
+			// AZURE branch does not handle path_style
+			if cfg.GatewayConfiguration != nil {
+				_, hasPs := cfg.GatewayConfiguration[object.StorageKeyPathStyle]
+				So(hasPs, ShouldBeFalse)
+			}
+		})
+	})
+}

common/nodes/objects/mc/client.go

@@ -104,6 +104,9 @@ func New(endpoint, accessKey, secretKey, signatureVersion string, secure bool, c
 	} else if r := s3utils.GetRegionFromURL(*u); r != "" {
 		options.Region = r
 	}
+	if other.Val("path_style").Bool() || other.Val("force_path_style").Bool() {
+		options.BucketLookup = minio.BucketLookupPath
+	}
 
 	c, err := minio.NewCore(endpoint, options)
 	if err != nil {

common/proto/object/datasource.go

@@ -51,6 +51,8 @@ const (
 	StorageKeyJsonCredentials  = "jsonCredentials"
 	StorageKeyStorageClass     = "storageClass"
 	StorageKeySignatureVersion = "signatureVersion"
+	StorageKeyPathStyle        = "path_style"
+	StorageKeyForcePathStyle   = "force_path_style"
 
 	StorageKeyCellsInternal    = "cellsInternal"
 	StorageKeyInitFromBucket   = "initFromBucket"
@@ -85,6 +87,12 @@ func (d *DataSource) ClientConfig(ctx context.Context, p SecretProvider) kv.Valu
 		if sv, o := d.StorageConfiguration[StorageKeySignatureVersion]; o && sv != "" {
 			_ = cfg.Val("signature").Set(sv)
 		}
+		if ps, o := d.StorageConfiguration[StorageKeyPathStyle]; o {
+			_ = cfg.Val(StorageKeyPathStyle).Set(ps == "true")
+		}
+		if fps, o := d.StorageConfiguration[StorageKeyForcePathStyle]; o {
+			_ = cfg.Val(StorageKeyForcePathStyle).Set(fps == "true")
+		}
 	}
 	return cfg
 }
@@ -163,6 +171,12 @@ func (d *MinioConfig) ClientConfig(ctx context.Context, p SecretProvider, agentN
 		if sv, o := d.GatewayConfiguration[StorageKeySignatureVersion]; o && sv != "" {
 			_ = cfg.Val("signature").Set(sv)
 		}
+		if ps, o := d.GatewayConfiguration[StorageKeyPathStyle]; o {
+			_ = cfg.Val(StorageKeyPathStyle).Set(ps == "true")
+		}
+		if fps, o := d.GatewayConfiguration[StorageKeyForcePathStyle]; o {
+			_ = cfg.Val(StorageKeyForcePathStyle).Set(fps == "true")
+		}
 	}
 	return cfg
 }

common/proto/object/datasource_test.go

@@ -0,0 +1,136 @@
+/*
+ * Copyright (c) 2019-2021. Abstrium SAS <team (at) pydio.com>
+ * This file is part of Pydio Cells.
+ *
+ * Pydio Cells is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Pydio Cells is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Pydio Cells.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * The latest code can be found at <https://pydio.com>.
+ */
+
+package object
+
+import (
+	"context"
+	"testing"
+
+	. "github.com/smartystreets/goconvey/convey"
+
+	"github.com/pydio/cells/v5/common/utils/configx"
+	"github.com/pydio/cells/v5/common/utils/kv"
+)
+
+// noopSecretProvider returns an empty config for any secret lookup.
+var noopSecretProvider SecretProvider = func(_ context.Context, _ string) kv.Values {
+	return configx.New()
+}
+
+func TestDataSourceClientConfigPathStyle(t *testing.T) {
+
+	ctx := context.Background()
+
+	Convey("Testing DataSource.ClientConfig path_style propagation", t, func() {
+
+		Convey("path_style=true should set bool true in config", func() {
+			ds := &DataSource{
+				StorageType: StorageType_S3,
+				StorageConfiguration: map[string]string{
+					StorageKeyPathStyle: "true",
+				},
+			}
+			cfg := ds.ClientConfig(ctx, noopSecretProvider)
+			So(cfg.Val(StorageKeyPathStyle).Bool(), ShouldBeTrue)
+		})
+
+		Convey("force_path_style=true should set bool true in config", func() {
+			ds := &DataSource{
+				StorageType: StorageType_S3,
+				StorageConfiguration: map[string]string{
+					StorageKeyForcePathStyle: "true",
+				},
+			}
+			cfg := ds.ClientConfig(ctx, noopSecretProvider)
+			So(cfg.Val(StorageKeyForcePathStyle).Bool(), ShouldBeTrue)
+		})
+
+		Convey("path_style=false should set bool false in config", func() {
+			ds := &DataSource{
+				StorageType: StorageType_S3,
+				StorageConfiguration: map[string]string{
+					StorageKeyPathStyle: "false",
+				},
+			}
+			cfg := ds.ClientConfig(ctx, noopSecretProvider)
+			So(cfg.Val(StorageKeyPathStyle).Bool(), ShouldBeFalse)
+		})
+
+		Convey("missing path_style key should return false", func() {
+			ds := &DataSource{
+				StorageType:          StorageType_S3,
+				StorageConfiguration: map[string]string{},
+			}
+			cfg := ds.ClientConfig(ctx, noopSecretProvider)
+			So(cfg.Val(StorageKeyPathStyle).Bool(), ShouldBeFalse)
+			So(cfg.Val(StorageKeyForcePathStyle).Bool(), ShouldBeFalse)
+		})
+
+		Convey("nil StorageConfiguration should not panic", func() {
+			ds := &DataSource{
+				StorageType:          StorageType_S3,
+				StorageConfiguration: nil,
+			}
+			So(func() { ds.ClientConfig(ctx, noopSecretProvider) }, ShouldNotPanic)
+		})
+	})
+}
+
+func TestMinioConfigClientConfigPathStyle(t *testing.T) {
+
+	ctx := context.Background()
+
+	Convey("Testing MinioConfig.ClientConfig path_style propagation", t, func() {
+
+		Convey("GatewayConfiguration path_style=true should set bool true", func() {
+			mc := &MinioConfig{
+				StorageType: StorageType_S3,
+				GatewayConfiguration: map[string]string{
+					StorageKeyPathStyle: "true",
+				},
+			}
+			cfg := mc.ClientConfig(ctx, noopSecretProvider, "", "")
+			So(cfg.Val(StorageKeyPathStyle).Bool(), ShouldBeTrue)
+		})
+
+		Convey("GatewayConfiguration force_path_style=true should set bool true", func() {
+			mc := &MinioConfig{
+				StorageType: StorageType_S3,
+				GatewayConfiguration: map[string]string{
+					StorageKeyForcePathStyle: "true",
+				},
+			}
+			cfg := mc.ClientConfig(ctx, noopSecretProvider, "", "")
+			So(cfg.Val(StorageKeyForcePathStyle).Bool(), ShouldBeTrue)
+		})
+
+		Convey("nil GatewayConfiguration should not panic and return false", func() {
+			mc := &MinioConfig{
+				StorageType:          StorageType_S3,
+				GatewayConfiguration: nil,
+			}
+			So(func() { mc.ClientConfig(ctx, noopSecretProvider, "", "") }, ShouldNotPanic)
+			cfg := mc.ClientConfig(ctx, noopSecretProvider, "", "")
+			So(cfg.Val(StorageKeyPathStyle).Bool(), ShouldBeFalse)
+			So(cfg.Val(StorageKeyForcePathStyle).Bool(), ShouldBeFalse)
+		})
+	})
+}

common/sync/endpoints/cells/transport/config.go

@@ -76,4 +76,6 @@ type S3Config struct {
 	// IsDebug is a convenience legacy flag to add some logging to this S3 client.
 	// Should be cleaned as soon as we defined the logging strategy for this repo.
 	IsDebug bool `json:"isDebug"`
+	// ForcePathStyle forces the client to use path-style addressing for the buckets.
+	ForcePathStyle bool `json:"forcePathStyle"`
 }