Merge pull request #20432 from hashicorp/jbardin/s3-prefix-key

s3 workspace_key_prefix

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": {

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 = `

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()