From f6af61f9909c30c7dd35d7ba90a107354014eb9d Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 22 Feb 2019 10:14:53 -0500 Subject: [PATCH 1/2] fix test that never worked The 0.12 update left this test broken. Make sure we're working with the correct objects here. --- backend/remote-state/s3/backend_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/backend/remote-state/s3/backend_test.go b/backend/remote-state/s3/backend_test.go index be711884d..3fc939c80 100644 --- a/backend/remote-state/s3/backend_test.go +++ b/backend/remote-state/s3/backend_test.go @@ -199,6 +199,11 @@ func TestBackendExtraPaths(t *testing.T) { } // remove the state with extra subkey + if err := client.Delete(); err != nil { + t.Fatal(err) + } + + // delete the real workspace if err := b.DeleteWorkspace("s2"); err != nil { t.Fatal(err) } @@ -216,7 +221,7 @@ func TestBackendExtraPaths(t *testing.T) { t.Fatal(err) } - if stateMgr.StateSnapshotMeta().Lineage == s2Lineage { + if s2Mgr.(*remote.State).StateSnapshotMeta().Lineage == s2Lineage { t.Fatal("state s2 was not deleted") } s2 = s2Mgr.State() From 31ca293777bd85c5844aa0c84457579f5ff6c535 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 22 Feb 2019 10:29:10 -0500 Subject: [PATCH 2/2] fix slash handling around workspace_key_prefix The handling of slashes was broken around listing workspaces in workspace_key_prefix. While it worked in most places by splitting an extra time around the spurious slashes, it failed in the case that the prefix ended with a slash of its own. A test was temporarily added to verify that the backend works with the unusual keys, but rather than risking silent breakage around prefixes with trailing slashes, we also add validation to prevent users from entering keys with trailing slashes at all. --- backend/remote-state/s3/backend.go | 13 ++++++--- backend/remote-state/s3/backend_state.go | 34 ++++++++++++------------ 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/backend/remote-state/s3/backend.go b/backend/remote-state/s3/backend.go index ae9bf1fbc..6a9e3acfb 100644 --- a/backend/remote-state/s3/backend.go +++ b/backend/remote-state/s3/backend.go @@ -2,7 +2,7 @@ package s3 import ( "context" - "fmt" + "errors" "strings" "github.com/aws/aws-sdk-go/aws" @@ -31,7 +31,7 @@ func New() backend.Backend { // s3 will strip leading slashes from an object, so while this will // technically be accepted by s3, it will break our workspace hierarchy. if strings.HasPrefix(v.(string), "/") { - return nil, []error{fmt.Errorf("key must not start with '/'")} + return nil, []error{errors.New("key must not start with '/'")} } return nil, nil }, @@ -211,8 +211,15 @@ func New() backend.Backend { "workspace_key_prefix": { Type: schema.TypeString, Optional: true, - Description: "The prefix applied to the non-default state path inside the bucket", + Description: "The prefix applied to the non-default state path inside the bucket.", Default: "env:", + ValidateFunc: func(v interface{}, s string) ([]string, []error) { + prefix := v.(string) + if strings.HasPrefix(prefix, "/") || strings.HasSuffix(prefix, "/") { + return nil, []error{errors.New("workspace_key_prefix must not start or end with '/'")} + } + return nil, nil + }, }, "force_path_style": { diff --git a/backend/remote-state/s3/backend_state.go b/backend/remote-state/s3/backend_state.go index dc134ecf4..b9fe4d0cb 100644 --- a/backend/remote-state/s3/backend_state.go +++ b/backend/remote-state/s3/backend_state.go @@ -3,6 +3,7 @@ package s3 import ( "errors" "fmt" + "path" "sort" "strings" @@ -17,12 +18,12 @@ import ( ) func (b *Backend) Workspaces() ([]string, error) { - prefix := b.workspaceKeyPrefix + "/" + prefix := "" - // List bucket root if there is no workspaceKeyPrefix - if b.workspaceKeyPrefix == "" { - prefix = "" + if b.workspaceKeyPrefix != "" { + prefix = b.workspaceKeyPrefix + "/" } + params := &s3.ListObjectsInput{ Bucket: &b.bucketName, Prefix: aws.String(prefix), @@ -49,7 +50,9 @@ func (b *Backend) Workspaces() ([]string, error) { } func (b *Backend) keyEnv(key string) string { - if b.workspaceKeyPrefix == "" { + prefix := b.workspaceKeyPrefix + + if prefix == "" { parts := strings.SplitN(key, "/", 2) if len(parts) > 1 && parts[1] == b.keyName { return parts[0] @@ -58,29 +61,31 @@ func (b *Backend) keyEnv(key string) string { } } - parts := strings.SplitAfterN(key, b.workspaceKeyPrefix, 2) + // add a slash to treat this as a directory + prefix += "/" + parts := strings.SplitAfterN(key, prefix, 2) if len(parts) < 2 { return "" } // shouldn't happen since we listed by prefix - if parts[0] != b.workspaceKeyPrefix { + if parts[0] != prefix { return "" } - parts = strings.SplitN(parts[1], "/", 3) + parts = strings.SplitN(parts[1], "/", 2) - if len(parts) < 3 { + if len(parts) < 2 { return "" } // not our key, so don't include it in our listing - if parts[2] != b.keyName { + if parts[1] != b.keyName { return "" } - return parts[1] + return parts[0] } func (b *Backend) DeleteWorkspace(name string) error { @@ -201,12 +206,7 @@ func (b *Backend) path(name string) string { return b.keyName } - if b.workspaceKeyPrefix != "" { - return strings.Join([]string{b.workspaceKeyPrefix, name, b.keyName}, "/") - } else { - // Trim the leading / for no workspace prefix - return strings.Join([]string{b.workspaceKeyPrefix, name, b.keyName}, "/")[1:] - } + return path.Join(b.workspaceKeyPrefix, name, b.keyName) } const errStateUnlock = `