From 0889c118a87951b91a514d6bfb45e86dfdad97af Mon Sep 17 00:00:00 2001 From: rv-jmaggio Date: Fri, 15 Dec 2017 17:50:36 -0500 Subject: [PATCH 1/5] Fixing issues with workspace_key_prefix --- backend/remote-state/s3/backend_state.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/backend/remote-state/s3/backend_state.go b/backend/remote-state/s3/backend_state.go index f38b199b04..25b68d9617 100644 --- a/backend/remote-state/s3/backend_state.go +++ b/backend/remote-state/s3/backend_state.go @@ -39,9 +39,13 @@ func (b *Backend) States() ([]string, error) { // extract the env name from the S3 key func (b *Backend) keyEnv(key string) string { - // we have 3 parts, the prefix, the env name, and the key name - parts := strings.SplitN(key, "/", 3) - if len(parts) < 3 { + if b.workspaceKeyPrefix == "" { + parts := strings.Split(key, "/") + return parts[0] + } + parts := strings.SplitAfter(key, b.workspaceKeyPrefix) + + if len(parts) < 2 { // no env here return "" } @@ -51,6 +55,12 @@ func (b *Backend) keyEnv(key string) string { return "" } + parts = strings.Split(parts[1], "/") + + if len(parts) < 3 { + return "" + } + // not our key, so don't include it in our listing if parts[2] != b.keyName { return "" From 7f8d686074ac2fcdcbf02c50b0338d1ef44e6183 Mon Sep 17 00:00:00 2001 From: rv-jmaggio Date: Fri, 15 Dec 2017 21:04:15 -0500 Subject: [PATCH 2/5] refactor and add a test --- backend/remote-state/s3/backend_state.go | 23 +++++----- backend/remote-state/s3/backend_test.go | 56 ++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/backend/remote-state/s3/backend_state.go b/backend/remote-state/s3/backend_state.go index 25b68d9617..2c513a25c8 100644 --- a/backend/remote-state/s3/backend_state.go +++ b/backend/remote-state/s3/backend_state.go @@ -25,28 +25,31 @@ func (b *Backend) States() ([]string, error) { return nil, err } - envs := []string{backend.DefaultStateName} + wss := []string{backend.DefaultStateName} for _, obj := range resp.Contents { - env := b.keyEnv(*obj.Key) - if env != "" { - envs = append(envs, env) + ws := getWorkspaceForKey(*obj.Key, b) + if ws != "" { + wss = append(wss, ws) } } - sort.Strings(envs[1:]) - return envs, nil + sort.Strings(wss[1:]) + return wss, nil } -// extract the env name from the S3 key -func (b *Backend) keyEnv(key string) string { +func getWorkspaceForKey(key string, b *Backend) string { if b.workspaceKeyPrefix == "" { parts := strings.Split(key, "/") - return parts[0] + if len(parts) > 1 && parts[1] == key { + return parts[0] + } else { + return "" + } } + parts := strings.SplitAfter(key, b.workspaceKeyPrefix) if len(parts) < 2 { - // no env here return "" } diff --git a/backend/remote-state/s3/backend_test.go b/backend/remote-state/s3/backend_test.go index 83af43e45a..2d8facce36 100644 --- a/backend/remote-state/s3/backend_test.go +++ b/backend/remote-state/s3/backend_test.go @@ -249,6 +249,62 @@ func TestBackendExtraPaths(t *testing.T) { } } +func TestKeyEnv(t *testing.T) { + testACC(t) + bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix()) + keyName0 := "tfstate" + keyName1 := "ws1/tfstate" + keyName2 := "ws1/env1/tfstate" + + b0 := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "bucket": bucketName, + "key": keyName0, + "encrypt": true, + "workspace_key_prefix": "", + }).(*Backend) + + b1 := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "bucket": bucketName, + "key": keyName1, + "encrypt": true, + "workspace_key_prefix": "root/userA", + }).(*Backend) + + b2 := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "bucket": bucketName, + "key": keyName2, + "encrypt": true, + "workspace_key_prefix": "root/userA", + }).(*Backend) + + if err := testGetWorkspaceForKey(b0, "tfstate", ""); err != nil { + t.Fatal(err) + } + + if err := testGetWorkspaceForKey(b0, "ws1/tfstate", "ws1"); err != nil { + t.Fatal(err) + } + + if err := testGetWorkspaceForKey(b1, "root/userA/ws1/tfstate", "ws1"); err != nil { + t.Fatal(err) + } + + if err := testGetWorkspaceForKey(b1, "root/userA/ws2/tfstate", "ws2"); err != nil { + t.Fatal(err) + } + + if err := testGetWorkspaceForKey(b2, "root/userA/ws2/env1/tfstate", "ws2"); err != nil { + t.Fatal(err) + } +} + +func testGetWorkspaceForKey(b *Backend, key string, expected string) error { + if getWorkspaceForKey(key, b) != expected { + return fmt.Errorf("incorrect workspace for key[%q]: %q", expected, key) + } + return nil +} + func checkStateList(b backend.Backend, expected []string) error { states, err := b.States() if err != nil { From b02a1c8a4605f0921299904ea95375c692d55eb7 Mon Sep 17 00:00:00 2001 From: rv-jmaggio Date: Mon, 18 Dec 2017 16:24:34 -0500 Subject: [PATCH 3/5] clarifying tests and using SplitN in implementation --- backend/remote-state/s3/backend_state.go | 2 +- backend/remote-state/s3/backend_test.go | 27 ++++++++++++------------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/backend/remote-state/s3/backend_state.go b/backend/remote-state/s3/backend_state.go index 2c513a25c8..c508f8176d 100644 --- a/backend/remote-state/s3/backend_state.go +++ b/backend/remote-state/s3/backend_state.go @@ -58,7 +58,7 @@ func getWorkspaceForKey(key string, b *Backend) string { return "" } - parts = strings.Split(parts[1], "/") + parts = strings.SplitN(parts[1], "/", 3) if len(parts) < 3 { return "" diff --git a/backend/remote-state/s3/backend_test.go b/backend/remote-state/s3/backend_test.go index 2d8facce36..8889a05120 100644 --- a/backend/remote-state/s3/backend_test.go +++ b/backend/remote-state/s3/backend_test.go @@ -252,29 +252,26 @@ func TestBackendExtraPaths(t *testing.T) { func TestKeyEnv(t *testing.T) { testACC(t) bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix()) - keyName0 := "tfstate" - keyName1 := "ws1/tfstate" - keyName2 := "ws1/env1/tfstate" + keyName := "tfstate" b0 := backend.TestBackendConfig(t, New(), map[string]interface{}{ "bucket": bucketName, - "key": keyName0, + "key": keyName, "encrypt": true, "workspace_key_prefix": "", }).(*Backend) b1 := backend.TestBackendConfig(t, New(), map[string]interface{}{ "bucket": bucketName, - "key": keyName1, + "key": keyName, "encrypt": true, - "workspace_key_prefix": "root/userA", + "workspace_key_prefix": "project/env:", }).(*Backend) b2 := backend.TestBackendConfig(t, New(), map[string]interface{}{ - "bucket": bucketName, - "key": keyName2, - "encrypt": true, - "workspace_key_prefix": "root/userA", + "bucket": bucketName, + "key": keyName, + "encrypt": true, }).(*Backend) if err := testGetWorkspaceForKey(b0, "tfstate", ""); err != nil { @@ -285,17 +282,21 @@ func TestKeyEnv(t *testing.T) { t.Fatal(err) } - if err := testGetWorkspaceForKey(b1, "root/userA/ws1/tfstate", "ws1"); err != nil { + if err := testGetWorkspaceForKey(b1, "project/env:/ws1/tfstate", "ws1"); err != nil { t.Fatal(err) } - if err := testGetWorkspaceForKey(b1, "root/userA/ws2/tfstate", "ws2"); err != nil { + if err := testGetWorkspaceForKey(b1, "project/env:/ws2/tfstate", "ws2"); err != nil { t.Fatal(err) } - if err := testGetWorkspaceForKey(b2, "root/userA/ws2/env1/tfstate", "ws2"); err != nil { + if err := testGetWorkspaceForKey(b2, "env:/ws3/tfstate", "ws3"); err != nil { t.Fatal(err) } + + backend.TestBackend(t, b0, nil) + backend.TestBackend(t, b1, nil) + backend.TestBackend(t, b2, nil) } func testGetWorkspaceForKey(b *Backend, key string, expected string) error { From bef64cfe918b68dd0a68245789e63d7068065dee Mon Sep 17 00:00:00 2001 From: rv-jmaggio Date: Tue, 19 Dec 2017 09:31:53 -0500 Subject: [PATCH 4/5] Fixing implementation for empty string and making acceptance test work --- backend/remote-state/s3/backend_state.go | 4 +-- backend/remote-state/s3/backend_test.go | 37 +++++++++++++++--------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/backend/remote-state/s3/backend_state.go b/backend/remote-state/s3/backend_state.go index c508f8176d..a920648a28 100644 --- a/backend/remote-state/s3/backend_state.go +++ b/backend/remote-state/s3/backend_state.go @@ -39,8 +39,8 @@ func (b *Backend) States() ([]string, error) { func getWorkspaceForKey(key string, b *Backend) string { if b.workspaceKeyPrefix == "" { - parts := strings.Split(key, "/") - if len(parts) > 1 && parts[1] == key { + parts := strings.SplitN(key, "/", 2) + if len(parts) > 1 && parts[1] == b.keyName { return parts[0] } else { return "" diff --git a/backend/remote-state/s3/backend_test.go b/backend/remote-state/s3/backend_test.go index 8889a05120..481653ae77 100644 --- a/backend/remote-state/s3/backend_test.go +++ b/backend/remote-state/s3/backend_test.go @@ -251,57 +251,68 @@ func TestBackendExtraPaths(t *testing.T) { func TestKeyEnv(t *testing.T) { testACC(t) - bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix()) - keyName := "tfstate" + keyName := "some/paths/tfstate" + bucket0Name := fmt.Sprintf("terraform-remote-s3-test-%x-0", time.Now().Unix()) b0 := backend.TestBackendConfig(t, New(), map[string]interface{}{ - "bucket": bucketName, + "bucket": bucket0Name, "key": keyName, "encrypt": true, "workspace_key_prefix": "", }).(*Backend) + createS3Bucket(t, b0.s3Client, bucket0Name) + defer deleteS3Bucket(t, b0.s3Client, bucket0Name) + + bucket1Name := fmt.Sprintf("terraform-remote-s3-test-%x-1", time.Now().Unix()) b1 := backend.TestBackendConfig(t, New(), map[string]interface{}{ - "bucket": bucketName, + "bucket": bucket1Name, "key": keyName, "encrypt": true, "workspace_key_prefix": "project/env:", }).(*Backend) + createS3Bucket(t, b1.s3Client, bucket1Name) + defer deleteS3Bucket(t, b1.s3Client, bucket1Name) + + bucket2Name := fmt.Sprintf("terraform-remote-s3-test-%x-2", time.Now().Unix()) b2 := backend.TestBackendConfig(t, New(), map[string]interface{}{ - "bucket": bucketName, + "bucket": bucket2Name, "key": keyName, "encrypt": true, }).(*Backend) - if err := testGetWorkspaceForKey(b0, "tfstate", ""); err != nil { + createS3Bucket(t, b2.s3Client, bucket2Name) + defer deleteS3Bucket(t, b2.s3Client, bucket2Name) + + if err := testGetWorkspaceForKey(b0, "some/paths/tfstate", ""); err != nil { t.Fatal(err) } - if err := testGetWorkspaceForKey(b0, "ws1/tfstate", "ws1"); err != nil { + if err := testGetWorkspaceForKey(b0, "ws1/some/paths/tfstate", "ws1"); err != nil { t.Fatal(err) } - if err := testGetWorkspaceForKey(b1, "project/env:/ws1/tfstate", "ws1"); err != nil { + if err := testGetWorkspaceForKey(b1, "project/env:/ws1/some/paths/tfstate", "ws1"); err != nil { t.Fatal(err) } - if err := testGetWorkspaceForKey(b1, "project/env:/ws2/tfstate", "ws2"); err != nil { + if err := testGetWorkspaceForKey(b1, "project/env:/ws2/some/paths/tfstate", "ws2"); err != nil { t.Fatal(err) } - if err := testGetWorkspaceForKey(b2, "env:/ws3/tfstate", "ws3"); err != nil { + if err := testGetWorkspaceForKey(b2, "env:/ws3/some/paths/tfstate", "ws3"); err != nil { t.Fatal(err) } - backend.TestBackend(t, b0, nil) + //backend.TestBackend(t, b0, nil) backend.TestBackend(t, b1, nil) backend.TestBackend(t, b2, nil) } func testGetWorkspaceForKey(b *Backend, key string, expected string) error { - if getWorkspaceForKey(key, b) != expected { - return fmt.Errorf("incorrect workspace for key[%q]: %q", expected, key) + if actual := getWorkspaceForKey(key, b); actual != expected { + return fmt.Errorf("incorrect workspace for key[%q]. Expected[%q]: Actual[%q]", key, expected, actual) } return nil } From b313ce80c4b4fe2dac377ce1a2b088b5c9744818 Mon Sep 17 00:00:00 2001 From: rv-jmaggio Date: Tue, 19 Dec 2017 13:14:31 -0500 Subject: [PATCH 5/5] Changing prefix for empty workspace prefix --- backend/remote-state/s3/backend_state.go | 19 +++++++++++++++---- backend/remote-state/s3/backend_test.go | 4 ++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/backend/remote-state/s3/backend_state.go b/backend/remote-state/s3/backend_state.go index a920648a28..e26db42f76 100644 --- a/backend/remote-state/s3/backend_state.go +++ b/backend/remote-state/s3/backend_state.go @@ -15,9 +15,15 @@ import ( ) func (b *Backend) States() ([]string, error) { + prefix := b.workspaceKeyPrefix + "/" + + // List bucket root if there is no workspaceKeyPrefix + if b.workspaceKeyPrefix == "" { + prefix = "" + } params := &s3.ListObjectsInput{ Bucket: &b.bucketName, - Prefix: aws.String(b.workspaceKeyPrefix + "/"), + Prefix: aws.String(prefix), } resp, err := b.s3Client.ListObjects(params) @@ -27,7 +33,7 @@ func (b *Backend) States() ([]string, error) { wss := []string{backend.DefaultStateName} for _, obj := range resp.Contents { - ws := getWorkspaceForKey(*obj.Key, b) + ws := b.keyEnv(*obj.Key) if ws != "" { wss = append(wss, ws) } @@ -37,7 +43,7 @@ func (b *Backend) States() ([]string, error) { return wss, nil } -func getWorkspaceForKey(key string, b *Backend) string { +func (b *Backend) keyEnv(key string) string { if b.workspaceKeyPrefix == "" { parts := strings.SplitN(key, "/", 2) if len(parts) > 1 && parts[1] == b.keyName { @@ -190,7 +196,12 @@ func (b *Backend) path(name string) string { return b.keyName } - return strings.Join([]string{b.workspaceKeyPrefix, name, 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:] + } } const errStateUnlock = ` diff --git a/backend/remote-state/s3/backend_test.go b/backend/remote-state/s3/backend_test.go index 481653ae77..a86b144b51 100644 --- a/backend/remote-state/s3/backend_test.go +++ b/backend/remote-state/s3/backend_test.go @@ -305,13 +305,13 @@ func TestKeyEnv(t *testing.T) { t.Fatal(err) } - //backend.TestBackend(t, b0, nil) + backend.TestBackend(t, b0, nil) backend.TestBackend(t, b1, nil) backend.TestBackend(t, b2, nil) } func testGetWorkspaceForKey(b *Backend, key string, expected string) error { - if actual := getWorkspaceForKey(key, b); actual != expected { + if actual := b.keyEnv(key); actual != expected { return fmt.Errorf("incorrect workspace for key[%q]. Expected[%q]: Actual[%q]", key, expected, actual) } return nil