From be238b2590d36f256b3564ac7b4d0f46e81fd54f Mon Sep 17 00:00:00 2001
From: btromanova <tr@uber.com>
Date: Thu, 19 Sep 2019 16:16:09 +0200
Subject: [PATCH] Don't pass variables to the modules that don't declare them
 (#40)

Don't pass variable to a module if it doesn't require it
---
 astro/astro_internal_test.go                  | 30 +++++++++++++++++++
 astro/execution.go                            | 11 +++++--
 astro/fixtures/test-pass-variables/astro.yaml | 22 ++++++++++++++
 astro/utils.go                                | 13 --------
 4 files changed, 61 insertions(+), 15 deletions(-)
 create mode 100644 astro/fixtures/test-pass-variables/astro.yaml

diff --git a/astro/astro_internal_test.go b/astro/astro_internal_test.go
index 80340e0..7a25b05 100644
--- a/astro/astro_internal_test.go
+++ b/astro/astro_internal_test.go
@@ -237,3 +237,33 @@ func TestApplyFailModule(t *testing.T) {
 		assert.NoError(t, results[id])
 	}
 }
+
+// Tests that variables are passed to the modules that declare them and not
+// passed to the modules that didn't
+func TestPassVariables(t *testing.T) {
+	t.Parallel()
+
+	c, err := NewProjectFromConfigFile("fixtures/test-pass-variables/astro.yaml")
+	require.NoError(t, err)
+
+	c.config.TerraformDefaults.Path = absolutePath("fixtures/mock-terraform/success")
+
+	_, resultChan, err := c.Plan(PlanExecutionParameters{
+		ExecutionParameters: ExecutionParameters{
+			UserVars: &UserVariables{
+				Values: map[string]string{
+					"region": "east1",
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	results := testReadResults(resultChan)
+
+	// Mocked terraform prints parameters it was called with to stderr.
+	// Check that variables were passed to the module that declared it and not
+	// passed to the one that didn't.
+	assert.Contains(t, results["bar-east1"].TerraformResult().Stderr(), "-var region=east1")
+	assert.NotContains(t, results["foo"].TerraformResult().Stderr(), "-var")
+}
diff --git a/astro/execution.go b/astro/execution.go
index 0c60a17..ee46f70 100644
--- a/astro/execution.go
+++ b/astro/execution.go
@@ -137,10 +137,17 @@ type unboundExecution struct {
 // boundExecution with variable values replaced. An error is returned if
 // not all required user values were provided.
 func (e *unboundExecution) bind(userVars map[string]string) (*boundExecution, error) {
-	boundVars := union(e.Variables(), userVars)
+	// boundVars is the map of execution variables bound to the values provided by user
+	boundVars := make(map[string]string)
 
-	missingVars := []string{}
+	for key, val := range e.Variables() {
+		boundVars[key] = val
+		if userVal, ok := userVars[key]; ok {
+			boundVars[key] = userVal
+		}
+	}
 
+	missingVars := []string{}
 	// Check that the user provided variables replace everything that
 	// needs to be replaced.
 	for _, val := range boundVars {
diff --git a/astro/fixtures/test-pass-variables/astro.yaml b/astro/fixtures/test-pass-variables/astro.yaml
new file mode 100644
index 0000000..6025804
--- /dev/null
+++ b/astro/fixtures/test-pass-variables/astro.yaml
@@ -0,0 +1,22 @@
+---
+
+terraform:
+  path: ../mock-terraform/success
+
+modules:
+
+  - name: foo
+    path: .
+    remote:
+      backend: local
+      backend_config:
+        key: "/tmp/terraform-test/foo.tfstate"
+
+  - name: bar
+    path: .
+    remote:
+      backend: local
+      backend_config:
+        key: "/tmp/terraform-test/bar.tfstate"
+    variables:
+      - name: region
diff --git a/astro/utils.go b/astro/utils.go
index 3eb5080..de93eab 100644
--- a/astro/utils.go
+++ b/astro/utils.go
@@ -52,16 +52,3 @@ func filterMaps(a, b map[string]string) bool {
 	}
 	return true
 }
-
-// union takes a series of maps and returns a new map which is a union of all
-// of them.
-func union(maps ...map[string]string) (result map[string]string) {
-	result = make(map[string]string)
-	for _, m := range maps {
-		for k, v := range m {
-			result[k] = v
-		}
-
-	}
-	return result
-}