From 8f5ff379b7d79f21f116db6ba54d73b49ef97e7e Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Sun, 7 Feb 2016 11:28:29 -0800 Subject: [PATCH 1/8] Introduced a configurable object path prefix for s3 repositories. Prepends the object path prefix to all s3 paths and allows to have multiple independent restic backup repositories in a single s3 bucket. Removed the hardcoded "restic" prefix from s3 paths. Use "restic" as the default object path prefix for s3 if no other prefix gets specified. This will retain backward compatibility with existing s3 repository configurations. Simplified the parse flow to have a single point where we parse the bucket name and the prefix within the bucket. Added tests for s3 object path prefix and the new default prefix to config_test and location_test. --- backend/s3/config.go | 76 +++++++++++++++++---------------------- backend/s3/config_test.go | 20 +++++++++++ backend/s3/s3.go | 22 ++++++------ location/location_test.go | 26 ++++++++++++++ 4 files changed, 90 insertions(+), 54 deletions(-) diff --git a/backend/s3/config.go b/backend/s3/config.go index b0224925a..4dab8a25c 100644 --- a/backend/s3/config.go +++ b/backend/s3/config.go @@ -13,53 +13,28 @@ type Config struct { UseHTTP bool KeyID, Secret string Bucket string + Prefix string } +const defaultPrefix = "restic" + // ParseConfig parses the string s and extracts the s3 config. The two -// supported configuration formats are s3://host/bucketname and -// s3:host:bucketname. The host can also be a valid s3 region name. +// supported configuration formats are s3://host/bucketname/prefix and +// s3:host:bucketname/prefix. The host can also be a valid s3 region +// name. If no prefix is given the prefix "restic" will be used. func ParseConfig(s string) (interface{}, error) { + var path []string + cfg := Config{} if strings.HasPrefix(s, "s3://") { s = s[5:] - - data := strings.SplitN(s, "/", 2) - if len(data) != 2 { - return nil, errors.New("s3: invalid format, host/region or bucket name not found") - } - - cfg := Config{ - Endpoint: data[0], - Bucket: data[1], - } - - return cfg, nil - } - - data := strings.SplitN(s, ":", 2) - if len(data) != 2 { - return nil, errors.New("s3: invalid format") - } - - if data[0] != "s3" { - return nil, errors.New(`s3: config does not start with "s3"`) - } - - s = data[1] - - cfg := Config{} - rest := strings.Split(s, "/") - if len(rest) < 2 { - return nil, errors.New("s3: region or bucket not found") - } - - if len(rest) == 2 { - // assume that just a region name and a bucket has been specified, in - // the format region/bucket - cfg.Endpoint = rest[0] - cfg.Bucket = rest[1] - } else { - // assume that a URL has been specified, parse it and use the path as - // the bucket name. + path = strings.SplitN(s, "/", 3) + cfg.Endpoint = path[0] + path = path[1:] + } else if strings.HasPrefix(s, "s3:http") { + s = s[3:] + // assume that a URL has been specified, parse it and + // use the host as the endpoint and the path as the + // bucket name and prefix url, err := url.Parse(s) if err != nil { return nil, err @@ -73,8 +48,23 @@ func ParseConfig(s string) (interface{}, error) { if url.Scheme == "http" { cfg.UseHTTP = true } - - cfg.Bucket = url.Path[1:] + path = strings.SplitN(url.Path[1:], "/", 2) + } else if strings.HasPrefix(s, "s3:") { + s = s[3:] + path = strings.SplitN(s, "/", 3) + cfg.Endpoint = path[0] + path = path[1:] + } else { + return nil, errors.New("s3: invalid format") + } + if len(path) < 1 { + return nil, errors.New("s3: invalid format, host/region or bucket name not found") + } + cfg.Bucket = path[0] + if len(path) > 1 { + cfg.Prefix = path[1] + } else { + cfg.Prefix = defaultPrefix } return cfg, nil diff --git a/backend/s3/config_test.go b/backend/s3/config_test.go index 54fc4718a..d1ecbdf60 100644 --- a/backend/s3/config_test.go +++ b/backend/s3/config_test.go @@ -9,18 +9,38 @@ var configTests = []struct { {"s3://eu-central-1/bucketname", Config{ Endpoint: "eu-central-1", Bucket: "bucketname", + Prefix: "restic", + }}, + {"s3://eu-central-1/bucketname/prefix/directory", Config{ + Endpoint: "eu-central-1", + Bucket: "bucketname", + Prefix: "prefix/directory", }}, {"s3:eu-central-1/foobar", Config{ Endpoint: "eu-central-1", Bucket: "foobar", + Prefix: "restic", + }}, + {"s3:eu-central-1/foobar/prefix/directory", Config{ + Endpoint: "eu-central-1", + Bucket: "foobar", + Prefix: "prefix/directory", }}, {"s3:https://hostname:9999/foobar", Config{ Endpoint: "hostname:9999", Bucket: "foobar", + Prefix: "restic", }}, {"s3:http://hostname:9999/foobar", Config{ Endpoint: "hostname:9999", Bucket: "foobar", + Prefix: "restic", + UseHTTP: true, + }}, + {"s3:http://hostname:9999/bucket/prefix/directory", Config{ + Endpoint: "hostname:9999", + Bucket: "bucket", + Prefix: "prefix/directory", UseHTTP: true, }}, } diff --git a/backend/s3/s3.go b/backend/s3/s3.go index ccb6cd42c..0bdf47894 100644 --- a/backend/s3/s3.go +++ b/backend/s3/s3.go @@ -13,13 +13,12 @@ import ( ) const connLimit = 10 -const backendPrefix = "restic" -func s3path(t backend.Type, name string) string { +func s3path(prefix string, t backend.Type, name string) string { if t == backend.Config { - return backendPrefix + "/" + string(t) + return prefix + "/" + string(t) } - return backendPrefix + "/" + string(t) + "/" + name + return prefix + "/" + string(t) + "/" + name } // s3 is a backend which stores the data on an S3 endpoint. @@ -27,6 +26,7 @@ type s3 struct { client minio.CloudStorageClient connChan chan struct{} bucketname string + prefix string } // Open opens the S3 backend at bucket and region. The bucket is created if it @@ -39,7 +39,7 @@ func Open(cfg Config) (backend.Backend, error) { return nil, err } - be := &s3{client: client, bucketname: cfg.Bucket} + be := &s3{client: client, bucketname: cfg.Bucket, prefix: cfg.Prefix} be.createConnections() if err := client.BucketExists(cfg.Bucket); err != nil { @@ -72,7 +72,7 @@ func (be *s3) Location() string { // and saves it in p. Load has the same semantics as io.ReaderAt. func (be s3) Load(h backend.Handle, p []byte, off int64) (int, error) { debug.Log("s3.Load", "%v, offset %v, len %v", h, off, len(p)) - path := s3path(h.Type, h.Name) + path := s3path(be.prefix, h.Type, h.Name) obj, err := be.client.GetObject(be.bucketname, path) if err != nil { debug.Log("s3.GetReader", " err %v", err) @@ -101,7 +101,7 @@ func (be s3) Save(h backend.Handle, p []byte) (err error) { debug.Log("s3.Save", "%v bytes at %d", len(p), h) - path := s3path(h.Type, h.Name) + path := s3path(be.prefix, h.Type, h.Name) // Check key does not already exist _, err = be.client.StatObject(be.bucketname, path) @@ -126,7 +126,7 @@ func (be s3) Save(h backend.Handle, p []byte) (err error) { // Stat returns information about a blob. func (be s3) Stat(h backend.Handle) (backend.BlobInfo, error) { debug.Log("s3.Stat", "%v") - path := s3path(h.Type, h.Name) + path := s3path(be.prefix, h.Type, h.Name) obj, err := be.client.GetObject(be.bucketname, path) if err != nil { debug.Log("s3.Stat", "GetObject() err %v", err) @@ -145,7 +145,7 @@ func (be s3) Stat(h backend.Handle) (backend.BlobInfo, error) { // Test returns true if a blob of the given type and name exists in the backend. func (be *s3) Test(t backend.Type, name string) (bool, error) { found := false - path := s3path(t, name) + path := s3path(be.prefix, t, name) _, err := be.client.StatObject(be.bucketname, path) if err == nil { found = true @@ -157,7 +157,7 @@ func (be *s3) Test(t backend.Type, name string) (bool, error) { // Remove removes the blob with the given name and type. func (be *s3) Remove(t backend.Type, name string) error { - path := s3path(t, name) + path := s3path(be.prefix, t, name) err := be.client.RemoveObject(be.bucketname, path) debug.Log("s3.Remove", "%v %v -> err %v", t, name, err) return err @@ -170,7 +170,7 @@ func (be *s3) List(t backend.Type, done <-chan struct{}) <-chan string { debug.Log("s3.List", "listing %v", t) ch := make(chan string) - prefix := s3path(t, "") + prefix := s3path(be.prefix, t, "") listresp := be.client.ListObjects(be.bucketname, prefix, true, done) diff --git a/location/location_test.go b/location/location_test.go index b0303fad1..ef827dcdd 100644 --- a/location/location_test.go +++ b/location/location_test.go @@ -48,30 +48,56 @@ var parseTests = []struct { Config: s3.Config{ Endpoint: "eu-central-1", Bucket: "bucketname", + Prefix: "restic", }}, }, {"s3://hostname.foo/bucketname", Location{Scheme: "s3", Config: s3.Config{ Endpoint: "hostname.foo", Bucket: "bucketname", + Prefix: "restic", + }}, + }, + {"s3://hostname.foo/bucketname/prefix/directory", Location{Scheme: "s3", + Config: s3.Config{ + Endpoint: "hostname.foo", + Bucket: "bucketname", + Prefix: "prefix/directory", }}, }, {"s3:eu-central-1/repo", Location{Scheme: "s3", Config: s3.Config{ Endpoint: "eu-central-1", Bucket: "repo", + Prefix: "restic", + }}, + }, + {"s3:eu-central-1/repo/prefix/directory", Location{Scheme: "s3", + Config: s3.Config{ + Endpoint: "eu-central-1", + Bucket: "repo", + Prefix: "prefix/directory", }}, }, {"s3:https://hostname.foo/repo", Location{Scheme: "s3", Config: s3.Config{ Endpoint: "hostname.foo", Bucket: "repo", + Prefix: "restic", + }}, + }, + {"s3:https://hostname.foo/repo/prefix/directory", Location{Scheme: "s3", + Config: s3.Config{ + Endpoint: "hostname.foo", + Bucket: "repo", + Prefix: "prefix/directory", }}, }, {"s3:http://hostname.foo/repo", Location{Scheme: "s3", Config: s3.Config{ Endpoint: "hostname.foo", Bucket: "repo", + Prefix: "restic", UseHTTP: true, }}, }, From 535dfaf0978be7315ac6201799d24e980b379e9b Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Sun, 14 Feb 2016 06:40:15 -0800 Subject: [PATCH 2/8] address first round of review comments --- backend/s3/config_test.go | 15 +++++++++++++++ backend/s3/s3.go | 33 ++++++++++++++++++++------------- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/backend/s3/config_test.go b/backend/s3/config_test.go index d1ecbdf60..29f0509b4 100644 --- a/backend/s3/config_test.go +++ b/backend/s3/config_test.go @@ -11,6 +11,11 @@ var configTests = []struct { Bucket: "bucketname", Prefix: "restic", }}, + {"s3://eu-central-1/bucketname/", Config{ + Endpoint: "eu-central-1", + Bucket: "bucketname", + Prefix: "", + }}, {"s3://eu-central-1/bucketname/prefix/directory", Config{ Endpoint: "eu-central-1", Bucket: "bucketname", @@ -21,6 +26,11 @@ var configTests = []struct { Bucket: "foobar", Prefix: "restic", }}, + {"s3:eu-central-1/foobar/", Config{ + Endpoint: "eu-central-1", + Bucket: "foobar", + Prefix: "", + }}, {"s3:eu-central-1/foobar/prefix/directory", Config{ Endpoint: "eu-central-1", Bucket: "foobar", @@ -31,6 +41,11 @@ var configTests = []struct { Bucket: "foobar", Prefix: "restic", }}, + {"s3:https://hostname:9999/foobar/", Config{ + Endpoint: "hostname:9999", + Bucket: "foobar", + Prefix: "", + }}, {"s3:http://hostname:9999/foobar", Config{ Endpoint: "hostname:9999", Bucket: "foobar", diff --git a/backend/s3/s3.go b/backend/s3/s3.go index 0bdf47894..a6657fd2b 100644 --- a/backend/s3/s3.go +++ b/backend/s3/s3.go @@ -14,13 +14,6 @@ import ( const connLimit = 10 -func s3path(prefix string, t backend.Type, name string) string { - if t == backend.Config { - return prefix + "/" + string(t) - } - return prefix + "/" + string(t) + "/" + name -} - // s3 is a backend which stores the data on an S3 endpoint. type s3 struct { client minio.CloudStorageClient @@ -56,6 +49,20 @@ func Open(cfg Config) (backend.Backend, error) { return be, nil } +func (be *s3) s3path(t backend.Type, name string) string { + var path string + + if be.prefix != "" { + path = be.prefix + "/" + } + path += string(t) + + if t == backend.Config { + return path + } + return path + "/" + name +} + func (be *s3) createConnections() { be.connChan = make(chan struct{}, connLimit) for i := 0; i < connLimit; i++ { @@ -72,7 +79,7 @@ func (be *s3) Location() string { // and saves it in p. Load has the same semantics as io.ReaderAt. func (be s3) Load(h backend.Handle, p []byte, off int64) (int, error) { debug.Log("s3.Load", "%v, offset %v, len %v", h, off, len(p)) - path := s3path(be.prefix, h.Type, h.Name) + path := be.s3path(h.Type, h.Name) obj, err := be.client.GetObject(be.bucketname, path) if err != nil { debug.Log("s3.GetReader", " err %v", err) @@ -101,7 +108,7 @@ func (be s3) Save(h backend.Handle, p []byte) (err error) { debug.Log("s3.Save", "%v bytes at %d", len(p), h) - path := s3path(be.prefix, h.Type, h.Name) + path := be.s3path(h.Type, h.Name) // Check key does not already exist _, err = be.client.StatObject(be.bucketname, path) @@ -126,7 +133,7 @@ func (be s3) Save(h backend.Handle, p []byte) (err error) { // Stat returns information about a blob. func (be s3) Stat(h backend.Handle) (backend.BlobInfo, error) { debug.Log("s3.Stat", "%v") - path := s3path(be.prefix, h.Type, h.Name) + path := be.s3path(h.Type, h.Name) obj, err := be.client.GetObject(be.bucketname, path) if err != nil { debug.Log("s3.Stat", "GetObject() err %v", err) @@ -145,7 +152,7 @@ func (be s3) Stat(h backend.Handle) (backend.BlobInfo, error) { // Test returns true if a blob of the given type and name exists in the backend. func (be *s3) Test(t backend.Type, name string) (bool, error) { found := false - path := s3path(be.prefix, t, name) + path := be.s3path(t, name) _, err := be.client.StatObject(be.bucketname, path) if err == nil { found = true @@ -157,7 +164,7 @@ func (be *s3) Test(t backend.Type, name string) (bool, error) { // Remove removes the blob with the given name and type. func (be *s3) Remove(t backend.Type, name string) error { - path := s3path(be.prefix, t, name) + path := be.s3path(t, name) err := be.client.RemoveObject(be.bucketname, path) debug.Log("s3.Remove", "%v %v -> err %v", t, name, err) return err @@ -170,7 +177,7 @@ func (be *s3) List(t backend.Type, done <-chan struct{}) <-chan string { debug.Log("s3.List", "listing %v", t) ch := make(chan string) - prefix := s3path(be.prefix, t, "") + prefix := be.s3path(t, "") listresp := be.client.ListObjects(be.bucketname, prefix, true, done) From 48f85fbb09043a413a06ffb655297e33bc72a517 Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Sun, 14 Feb 2016 07:01:14 -0800 Subject: [PATCH 3/8] replaced if-else chain with switch --- backend/s3/config.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/backend/s3/config.go b/backend/s3/config.go index 4dab8a25c..b65c77488 100644 --- a/backend/s3/config.go +++ b/backend/s3/config.go @@ -25,12 +25,13 @@ const defaultPrefix = "restic" func ParseConfig(s string) (interface{}, error) { var path []string cfg := Config{} - if strings.HasPrefix(s, "s3://") { + switch { + case strings.HasPrefix(s, "s3://"): s = s[5:] path = strings.SplitN(s, "/", 3) cfg.Endpoint = path[0] path = path[1:] - } else if strings.HasPrefix(s, "s3:http") { + case strings.HasPrefix(s, "s3:http"): s = s[3:] // assume that a URL has been specified, parse it and // use the host as the endpoint and the path as the @@ -49,12 +50,12 @@ func ParseConfig(s string) (interface{}, error) { cfg.UseHTTP = true } path = strings.SplitN(url.Path[1:], "/", 2) - } else if strings.HasPrefix(s, "s3:") { + case strings.HasPrefix(s, "s3:"): s = s[3:] path = strings.SplitN(s, "/", 3) cfg.Endpoint = path[0] path = path[1:] - } else { + default: return nil, errors.New("s3: invalid format") } if len(path) < 1 { From 74608531c7b1dcff34449071aa6333e7d5c65093 Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Sun, 14 Feb 2016 07:16:50 -0800 Subject: [PATCH 4/8] strip off the trailing slash for the object prefix --- backend/s3/config.go | 2 +- backend/s3/config_test.go | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/backend/s3/config.go b/backend/s3/config.go index b65c77488..eed4cabe7 100644 --- a/backend/s3/config.go +++ b/backend/s3/config.go @@ -63,7 +63,7 @@ func ParseConfig(s string) (interface{}, error) { } cfg.Bucket = path[0] if len(path) > 1 { - cfg.Prefix = path[1] + cfg.Prefix = strings.TrimRight(path[1], "/") } else { cfg.Prefix = defaultPrefix } diff --git a/backend/s3/config_test.go b/backend/s3/config_test.go index 29f0509b4..63bc2c77e 100644 --- a/backend/s3/config_test.go +++ b/backend/s3/config_test.go @@ -21,6 +21,11 @@ var configTests = []struct { Bucket: "bucketname", Prefix: "prefix/directory", }}, + {"s3://eu-central-1/bucketname/prefix/directory/", Config{ + Endpoint: "eu-central-1", + Bucket: "bucketname", + Prefix: "prefix/directory", + }}, {"s3:eu-central-1/foobar", Config{ Endpoint: "eu-central-1", Bucket: "foobar", @@ -36,6 +41,11 @@ var configTests = []struct { Bucket: "foobar", Prefix: "prefix/directory", }}, + {"s3:eu-central-1/foobar/prefix/directory/", Config{ + Endpoint: "eu-central-1", + Bucket: "foobar", + Prefix: "prefix/directory", + }}, {"s3:https://hostname:9999/foobar", Config{ Endpoint: "hostname:9999", Bucket: "foobar", @@ -52,12 +62,24 @@ var configTests = []struct { Prefix: "restic", UseHTTP: true, }}, + {"s3:http://hostname:9999/foobar/", Config{ + Endpoint: "hostname:9999", + Bucket: "foobar", + Prefix: "", + UseHTTP: true, + }}, {"s3:http://hostname:9999/bucket/prefix/directory", Config{ Endpoint: "hostname:9999", Bucket: "bucket", Prefix: "prefix/directory", UseHTTP: true, }}, + {"s3:http://hostname:9999/bucket/prefix/directory/", Config{ + Endpoint: "hostname:9999", + Bucket: "bucket", + Prefix: "prefix/directory", + UseHTTP: true, + }}, } func TestParseConfig(t *testing.T) { From 32c2cafa89fd5f6c4363e23ae2c379e88ab78af9 Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Sun, 14 Feb 2016 09:10:45 -0800 Subject: [PATCH 5/8] Simplify creation of the Config by moving it to a separate function. Simplify the parsing logic by sharing the handling of s3: and s3:// --- backend/s3/config.go | 56 ++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/backend/s3/config.go b/backend/s3/config.go index eed4cabe7..883939607 100644 --- a/backend/s3/config.go +++ b/backend/s3/config.go @@ -23,20 +23,12 @@ const defaultPrefix = "restic" // s3:host:bucketname/prefix. The host can also be a valid s3 region // name. If no prefix is given the prefix "restic" will be used. func ParseConfig(s string) (interface{}, error) { - var path []string - cfg := Config{} switch { - case strings.HasPrefix(s, "s3://"): - s = s[5:] - path = strings.SplitN(s, "/", 3) - cfg.Endpoint = path[0] - path = path[1:] case strings.HasPrefix(s, "s3:http"): - s = s[3:] // assume that a URL has been specified, parse it and // use the host as the endpoint and the path as the // bucket name and prefix - url, err := url.Parse(s) + url, err := url.Parse(s[3:]) if err != nil { return nil, err } @@ -45,28 +37,36 @@ func ParseConfig(s string) (interface{}, error) { return nil, errors.New("s3: bucket name not found") } - cfg.Endpoint = url.Host - if url.Scheme == "http" { - cfg.UseHTTP = true - } - path = strings.SplitN(url.Path[1:], "/", 2) + path := strings.SplitN(url.Path[1:], "/", 2) + return createConfig(url.Host, path, url.Scheme == "http") case strings.HasPrefix(s, "s3:"): + // use the first entry of the path as the endpoint and the + // remainder as bucket name and prefix s = s[3:] - path = strings.SplitN(s, "/", 3) - cfg.Endpoint = path[0] - path = path[1:] + if strings.HasPrefix(s, "//") { + s = s[2:] + } + path := strings.SplitN(s, "/", 3) + return createConfig(path[0], path[1:], false) default: return nil, errors.New("s3: invalid format") } - if len(path) < 1 { - return nil, errors.New("s3: invalid format, host/region or bucket name not found") - } - cfg.Bucket = path[0] - if len(path) > 1 { - cfg.Prefix = strings.TrimRight(path[1], "/") - } else { - cfg.Prefix = defaultPrefix - } - - return cfg, nil +} + +func createConfig(endpoint string, path []string, useHttp bool) (interface{}, error) { + var prefix string + switch { + case len(path) < 1: + return nil, errors.New("s3: invalid format, host/region or bucket name not found") + case len(path) == 1: + prefix = defaultPrefix + default: + prefix = strings.TrimRight(path[1], "/") + } + return Config{ + Endpoint: endpoint, + UseHTTP: useHttp, + Bucket: path[0], + Prefix: prefix, + }, nil } From 91dc14c9fc1fe5311dcacf1cefaefc60438dfe45 Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Sun, 14 Feb 2016 09:27:00 -0800 Subject: [PATCH 6/8] fix Hound warning --- backend/s3/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/s3/config.go b/backend/s3/config.go index 883939607..985db0aad 100644 --- a/backend/s3/config.go +++ b/backend/s3/config.go @@ -53,7 +53,7 @@ func ParseConfig(s string) (interface{}, error) { } } -func createConfig(endpoint string, path []string, useHttp bool) (interface{}, error) { +func createConfig(endpoint string, path []string, useHTTP bool) (interface{}, error) { var prefix string switch { case len(path) < 1: @@ -65,7 +65,7 @@ func createConfig(endpoint string, path []string, useHttp bool) (interface{}, er } return Config{ Endpoint: endpoint, - UseHTTP: useHttp, + UseHTTP: useHTTP, Bucket: path[0], Prefix: prefix, }, nil From 24b7514fe01942693b376dbc3c2fdbd3ac7fc7cd Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Sun, 14 Feb 2016 09:45:58 -0800 Subject: [PATCH 7/8] cleaner sharing of s3: and s3:// configuration --- backend/s3/config.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/backend/s3/config.go b/backend/s3/config.go index 985db0aad..e2bccd801 100644 --- a/backend/s3/config.go +++ b/backend/s3/config.go @@ -39,18 +39,17 @@ func ParseConfig(s string) (interface{}, error) { path := strings.SplitN(url.Path[1:], "/", 2) return createConfig(url.Host, path, url.Scheme == "http") + case strings.HasPrefix(s, "s3://"): + s = s[5:] case strings.HasPrefix(s, "s3:"): - // use the first entry of the path as the endpoint and the - // remainder as bucket name and prefix s = s[3:] - if strings.HasPrefix(s, "//") { - s = s[2:] - } - path := strings.SplitN(s, "/", 3) - return createConfig(path[0], path[1:], false) default: return nil, errors.New("s3: invalid format") } + // use the first entry of the path as the endpoint and the + // remainder as bucket name and prefix + path := strings.SplitN(s, "/", 3) + return createConfig(path[0], path[1:], false) } func createConfig(endpoint string, path []string, useHTTP bool) (interface{}, error) { From f3f140484993c0cf78c2e516722982caf3a2551a Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Sun, 14 Feb 2016 11:26:46 -0800 Subject: [PATCH 8/8] always use "restic" as the default prefix for parsing. improved test error message to make it easier to find the problematic pattern --- backend/s3/config.go | 11 ++++++----- backend/s3/config_test.go | 14 +++++++------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/backend/s3/config.go b/backend/s3/config.go index e2bccd801..5cd74ce79 100644 --- a/backend/s3/config.go +++ b/backend/s3/config.go @@ -3,6 +3,7 @@ package s3 import ( "errors" "net/url" + "path" "strings" ) @@ -52,20 +53,20 @@ func ParseConfig(s string) (interface{}, error) { return createConfig(path[0], path[1:], false) } -func createConfig(endpoint string, path []string, useHTTP bool) (interface{}, error) { +func createConfig(endpoint string, p []string, useHTTP bool) (interface{}, error) { var prefix string switch { - case len(path) < 1: + case len(p) < 1: return nil, errors.New("s3: invalid format, host/region or bucket name not found") - case len(path) == 1: + case len(p) == 1 || p[1] == "": prefix = defaultPrefix default: - prefix = strings.TrimRight(path[1], "/") + prefix = path.Clean(p[1]) } return Config{ Endpoint: endpoint, UseHTTP: useHTTP, - Bucket: path[0], + Bucket: p[0], Prefix: prefix, }, nil } diff --git a/backend/s3/config_test.go b/backend/s3/config_test.go index 63bc2c77e..3a04d59a2 100644 --- a/backend/s3/config_test.go +++ b/backend/s3/config_test.go @@ -14,7 +14,7 @@ var configTests = []struct { {"s3://eu-central-1/bucketname/", Config{ Endpoint: "eu-central-1", Bucket: "bucketname", - Prefix: "", + Prefix: "restic", }}, {"s3://eu-central-1/bucketname/prefix/directory", Config{ Endpoint: "eu-central-1", @@ -34,7 +34,7 @@ var configTests = []struct { {"s3:eu-central-1/foobar/", Config{ Endpoint: "eu-central-1", Bucket: "foobar", - Prefix: "", + Prefix: "restic", }}, {"s3:eu-central-1/foobar/prefix/directory", Config{ Endpoint: "eu-central-1", @@ -54,7 +54,7 @@ var configTests = []struct { {"s3:https://hostname:9999/foobar/", Config{ Endpoint: "hostname:9999", Bucket: "foobar", - Prefix: "", + Prefix: "restic", }}, {"s3:http://hostname:9999/foobar", Config{ Endpoint: "hostname:9999", @@ -65,7 +65,7 @@ var configTests = []struct { {"s3:http://hostname:9999/foobar/", Config{ Endpoint: "hostname:9999", Bucket: "foobar", - Prefix: "", + Prefix: "restic", UseHTTP: true, }}, {"s3:http://hostname:9999/bucket/prefix/directory", Config{ @@ -86,13 +86,13 @@ func TestParseConfig(t *testing.T) { for i, test := range configTests { cfg, err := ParseConfig(test.s) if err != nil { - t.Errorf("test %d failed: %v", i, err) + t.Errorf("test %d:%s failed: %v", i, test.s, err) continue } if cfg != test.cfg { - t.Errorf("test %d: wrong config, want:\n %v\ngot:\n %v", - i, test.cfg, cfg) + t.Errorf("test %d:\ninput:\n %s\n wrong config, want:\n %v\ngot:\n %v", + i, test.s, test.cfg, cfg) continue } }