From 5c38456b051725851dbf9a3e37f1555e86a47d20 Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Thu, 2 Jul 2015 10:22:11 -0500
Subject: [PATCH] core: don't prompt for variables with defaults

In `helper/schema` we already makes a distinction between `Default`
which is always applied and `InputDefault` which is displayed to the
user for an empty field.

But for variables we just have `Default` which is treated like
`InputDefault`. This changes it to _not_ prompt the user for a value
when the variable declaration includes a default.

Treating this as a UX bugfix and the "don't prompt for variables w/
defaults set" behavior as the originally expected behavior we were
failing to honor.

Added an already-passing test to verify and cover the `helper/schema`
behavior.

Perhaps down the road we can add a `input_default` attribute to
variables to allow similar behavior to `helper/schema` in variables, but
for now just sticking with the fix.

Fixes #2592
---
 helper/schema/schema_test.go                  | 34 ++++++++++++++
 terraform/context.go                          | 13 ++++--
 terraform/context_test.go                     | 46 +++++++++++++++++++
 .../test-fixtures/input-var-default/main.tf   |  7 +++
 4 files changed, 95 insertions(+), 5 deletions(-)
 create mode 100644 terraform/test-fixtures/input-var-default/main.tf

diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go
index 3f1dce669..4421e4111 100644
--- a/helper/schema/schema_test.go
+++ b/helper/schema/schema_test.go
@@ -2605,6 +2605,40 @@ func TestSchemaMap_Input(t *testing.T) {
 	}
 }
 
+func TestSchemaMap_InputDefault(t *testing.T) {
+	emptyConfig := make(map[string]interface{})
+	c, err := config.NewRawConfig(emptyConfig)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	rc := terraform.NewResourceConfig(c)
+	rc.Config = make(map[string]interface{})
+
+	input := new(terraform.MockUIInput)
+	input.InputFn = func(opts *terraform.InputOpts) (string, error) {
+		t.Fatalf("InputFn should not be called on: %#v", opts)
+		return "", nil
+	}
+
+	schema := map[string]*Schema{
+		"availability_zone": &Schema{
+			Type:     TypeString,
+			Default:  "foo",
+			Optional: true,
+		},
+	}
+	actual, err := schemaMap(schema).Input(input, rc)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	expected := map[string]interface{}{}
+
+	if !reflect.DeepEqual(expected, actual.Config) {
+		t.Fatalf("got: %#v\nexpected: %#v", actual.Config, expected)
+	}
+}
+
 func TestSchemaMap_InternalValidate(t *testing.T) {
 	cases := []struct {
 		In  map[string]*Schema
diff --git a/terraform/context.go b/terraform/context.go
index 531d49310..be01a492a 100644
--- a/terraform/context.go
+++ b/terraform/context.go
@@ -194,7 +194,7 @@ func (c *Context) Input(mode InputMode) error {
 		}
 		sort.Strings(names)
 		for _, n := range names {
-			// If we only care about unset variables, then if the variabel
+			// If we only care about unset variables, then if the variable
 			// is set, continue on.
 			if mode&InputModeVarUnset != 0 {
 				if _, ok := c.variables[n]; ok {
@@ -214,9 +214,13 @@ func (c *Context) Input(mode InputMode) error {
 				panic(fmt.Sprintf("Unknown variable type: %#v", v.Type()))
 			}
 
-			var defaultString string
-			if v.Default != nil {
-				defaultString = v.Default.(string)
+			// If the variable is not already set, and the variable defines a
+			// default, use that for the value.
+			if _, ok := c.variables[n]; !ok {
+				if v.Default != nil {
+					c.variables[n] = v.Default.(string)
+					continue
+				}
 			}
 
 			// Ask the user for a value for this variable
@@ -226,7 +230,6 @@ func (c *Context) Input(mode InputMode) error {
 				value, err = c.uiInput.Input(&InputOpts{
 					Id:          fmt.Sprintf("var.%s", n),
 					Query:       fmt.Sprintf("var.%s", n),
-					Default:     defaultString,
 					Description: v.Description,
 				})
 				if err != nil {
diff --git a/terraform/context_test.go b/terraform/context_test.go
index 022a2f9af..189ffc89a 100644
--- a/terraform/context_test.go
+++ b/terraform/context_test.go
@@ -3572,6 +3572,52 @@ func TestContext2Input_varOnlyUnset(t *testing.T) {
 	}
 }
 
+func TestContext2Input_varWithDefault(t *testing.T) {
+	input := new(MockUIInput)
+	m := testModule(t, "input-var-default")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Variables: map[string]string{},
+		UIInput:   input,
+	})
+
+	input.InputFn = func(opts *InputOpts) (string, error) {
+		t.Fatalf(
+			"Input should never be called because variable has a default: %#v", opts)
+		return "", nil
+	}
+
+	if err := ctx.Input(InputModeVar | InputModeVarUnset); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actualStr := strings.TrimSpace(state.String())
+	expectedStr := strings.TrimSpace(`
+aws_instance.foo:
+  ID = foo
+  foo = 123
+  type = aws_instance
+	`)
+	if actualStr != expectedStr {
+		t.Fatalf("expected: \n%s\ngot: \n%s\n", expectedStr, actualStr)
+	}
+}
+
 func TestContext2Apply(t *testing.T) {
 	m := testModule(t, "apply-good")
 	p := testProvider("aws")
diff --git a/terraform/test-fixtures/input-var-default/main.tf b/terraform/test-fixtures/input-var-default/main.tf
new file mode 100644
index 000000000..59dc34e58
--- /dev/null
+++ b/terraform/test-fixtures/input-var-default/main.tf
@@ -0,0 +1,7 @@
+variable "foo" {
+  default = 123
+}
+
+resource "aws_instance" "foo" {
+  foo = "${var.foo}"
+}