From e011dd95f3f2551115bb22f97317ef036a06ff74 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 9 Mar 2018 10:22:18 -0500
Subject: [PATCH] don't let default workspace override environment

The workspace attribute should only override the environment if it's not
the default value.
---
 .../providers/terraform/data_source_state.go  | 15 ++++-----
 .../terraform/data_source_state_test.go       | 31 +++++++++++++++++++
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/builtin/providers/terraform/data_source_state.go b/builtin/providers/terraform/data_source_state.go
index e6797eb4d..1eefe9e8d 100644
--- a/builtin/providers/terraform/data_source_state.go
+++ b/builtin/providers/terraform/data_source_state.go
@@ -64,7 +64,7 @@ func dataSourceRemoteState() *schema.Resource {
 }
 
 func dataSourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error {
-	backend := d.Get("backend").(string)
+	backendType := d.Get("backend").(string)
 
 	// Get the configuration in a type we want.
 	rawConfig, err := config.NewRawConfig(d.Get("config").(map[string]interface{}))
@@ -73,16 +73,16 @@ func dataSourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	// Don't break people using the old _local syntax - but note warning above
-	if backend == "_local" {
+	if backendType == "_local" {
 		log.Println(`[INFO] Switching old (unsupported) backend "_local" to "local"`)
-		backend = "local"
+		backendType = "local"
 	}
 
 	// Create the client to access our remote state
-	log.Printf("[DEBUG] Initializing remote state backend: %s", backend)
-	f := backendinit.Backend(backend)
+	log.Printf("[DEBUG] Initializing remote state backend: %s", backendType)
+	f := backendinit.Backend(backendType)
 	if f == nil {
-		return fmt.Errorf("Unknown backend type: %s", backend)
+		return fmt.Errorf("Unknown backend type: %s", backendType)
 	}
 	b := f()
 
@@ -94,9 +94,10 @@ func dataSourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error {
 	// environment is deprecated in favour of workspace.
 	// If both keys are set workspace should win.
 	name := d.Get("environment").(string)
-	if ws, ok := d.GetOk("workspace"); ok {
+	if ws, ok := d.GetOk("workspace"); ok && ws != backend.DefaultStateName {
 		name = ws.(string)
 	}
+
 	state, err := b.State(name)
 	if err != nil {
 		return fmt.Errorf("error loading the remote state: %s", err)
diff --git a/builtin/providers/terraform/data_source_state_test.go b/builtin/providers/terraform/data_source_state_test.go
index f80b738be..9b2040e82 100644
--- a/builtin/providers/terraform/data_source_state_test.go
+++ b/builtin/providers/terraform/data_source_state_test.go
@@ -129,6 +129,27 @@ func testAccCheckStateValue(id, name, value string) resource.TestCheckFunc {
 	}
 }
 
+// make sure that the deprecated environment field isn't overridden by the
+// default value for workspace.
+func TestState_deprecatedEnvironment(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccState_deprecatedEnvironment,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStateValue(
+						// if the workspace default value overrides the
+						// environment, this will get the foo value from the
+						// default state.
+						"data.terraform_remote_state.foo", "foo", ""),
+				),
+			},
+		},
+	})
+}
+
 const testAccState_basic = `
 data "terraform_remote_state" "foo" {
 	backend = "local"
@@ -190,3 +211,13 @@ data "terraform_remote_state" "foo" {
 		foo = "not bar"
 	}
 }`
+
+const testAccState_deprecatedEnvironment = `
+data "terraform_remote_state" "foo" {
+	backend = "local"
+	environment = "deprecated"
+
+	config {
+		path = "./test-fixtures/basic.tfstate"
+	}
+}`