From 2e5791ab2b76ac019681969686ec904eb015155f Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 9 Aug 2016 16:14:40 -0400 Subject: [PATCH 1/3] Allow the HCL input when prompted We already accept HCL encoded input for -vars, and this expands that to accept HCL when prompted for a value on the command line as well. --- terraform/context.go | 67 ++++++++++++++++------- terraform/context_input_test.go | 41 ++++++++++++++ terraform/context_test.go | 29 +++++++++- terraform/terraform_test.go | 11 ++++ terraform/test-fixtures/input-hcl/main.tf | 12 ++++ 5 files changed, 138 insertions(+), 22 deletions(-) create mode 100644 terraform/test-fixtures/input-hcl/main.tf diff --git a/terraform/context.go b/terraform/context.go index 262b7ce38..8f30fd554 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -317,16 +317,18 @@ func (c *Context) Input(mode InputMode) error { } } + var valueType config.VariableType + v := m[n] - switch v.Type() { + switch valueType = v.Type(); valueType { case config.VariableTypeUnknown: continue case config.VariableTypeMap: - continue + // OK case config.VariableTypeList: - continue + // OK case config.VariableTypeString: - // Good! + // OK default: panic(fmt.Sprintf("Unknown variable type: %#v", v.Type())) } @@ -340,6 +342,12 @@ func (c *Context) Input(mode InputMode) error { } } + // this should only happen during tests + if c.uiInput == nil { + log.Println("[WARN] Content.uiInput is nil") + continue + } + // Ask the user for a value for this variable var value string retry := 0 @@ -355,27 +363,33 @@ func (c *Context) Input(mode InputMode) error { "Error asking for %s: %s", n, err) } - if value == "" && v.Required() { - // Redo if it is required, but abort if we keep getting - // blank entries - if retry > 2 { - return fmt.Errorf("missing required value for %q", n) - } - retry++ - continue - } - if value == "" { - // No value, just exit the loop. With no value, we just - // use whatever is currently set in variables. - break + if v.Required() { + // Redo if it is required, but abort if we keep getting + // blank entries + if retry > 2 { + return fmt.Errorf("missing required value for %q", n) + } + retry++ + continue + } } break } - if value != "" { - c.variables[n] = value + // no value provided, so don't set the variable at all + if value == "" { + continue + } + + decoded, err := parseVariableAsHCL(n, value, valueType) + if err != nil { + return err + } + + if decoded != nil { + c.variables[n] = decoded } } } @@ -656,9 +670,20 @@ func (c *Context) walk( // the name of the variable. In order to get around the restriction of HCL requiring a // top level object, we prepend a sentinel key, decode the user-specified value as its // value and pull the value back out of the resulting map. -func parseVariableAsHCL(name string, input interface{}, targetType config.VariableType) (interface{}, error) { +func parseVariableAsHCL(name string, input string, targetType config.VariableType) (interface{}, error) { + // expecting a string so don't decode anything, just strip quotes if targetType == config.VariableTypeString { - return input, nil + return strings.Trim(input, `"`), nil + } + + // return empty types + if strings.TrimSpace(input) == "" { + switch targetType { + case config.VariableTypeList: + return []interface{}{}, nil + case config.VariableTypeMap: + return make(map[string]interface{}), nil + } } const sentinelValue = "SENTINEL_TERRAFORM_VAR_OVERRIDE_KEY" diff --git a/terraform/context_input_test.go b/terraform/context_input_test.go index 13e372469..f5e0f47a2 100644 --- a/terraform/context_input_test.go +++ b/terraform/context_input_test.go @@ -617,3 +617,44 @@ func TestContext2Input_interpolateVar(t *testing.T) { t.Fatalf("err: %s", err) } } + +func TestContext2Input_hcl(t *testing.T) { + input := new(MockUIInput) + m := testModule(t, "input-hcl") + p := testProvider("hcl") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "hcl": testProviderFuncFixed(p), + }, + Variables: map[string]interface{}{}, + UIInput: input, + }) + + input.InputReturnMap = map[string]string{ + "var.listed": `["a", "b"]`, + "var.mapped": `{x = "y", w = "z"}`, + } + + 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(testTerraformInputHCL) + if actualStr != expectedStr { + t.Logf("expected: \n%s", expectedStr) + t.Fatalf("bad: \n%s", actualStr) + } +} diff --git a/terraform/context_test.go b/terraform/context_test.go index 6d3f1e57f..0d6da97fe 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -6,6 +6,8 @@ import ( "strings" "testing" "time" + + "github.com/hashicorp/terraform/flatmap" ) func TestNewContextState(t *testing.T) { @@ -172,7 +174,16 @@ func testDiffFn( if reflect.DeepEqual(v, []interface{}{}) { attrDiff.New = "" } else { - attrDiff.New = v.(string) + if s, ok := v.(string); ok { + attrDiff.New = s + } else { + // the value is something other than a string + // flatmap it, adding the diff for each value. + for k, attrDiff := range testFlatAttrDiffs(k, v) { + diff.Attributes[k] = attrDiff + } + continue + } } if k == "require_new" { @@ -219,6 +230,22 @@ func testDiffFn( return diff, nil } +// generate ResourceAttrDiffs for nested data structures in tests +func testFlatAttrDiffs(k string, i interface{}) map[string]*ResourceAttrDiff { + flat := flatmap.Flatten(map[string]interface{}{k: i}) + diffs := make(map[string]*ResourceAttrDiff) + + for k, v := range flat { + attrDiff := &ResourceAttrDiff{ + Old: "", + New: v, + } + diffs[k] = attrDiff + } + + return diffs +} + func testProvider(prefix string) *MockResourceProvider { p := new(MockResourceProvider) p.RefreshFn = func(info *InstanceInfo, s *InstanceState) (*InstanceState, error) { diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 7f81bf05a..469d5376d 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -1413,3 +1413,14 @@ module.mod2: STATE: ` + +const testTerraformInputHCL = ` +hcl_instance.hcltest: + ID = foo + bar.w = z + bar.x = y + foo.# = 2 + foo.0 = a + foo.1 = b + type = hcl_instance +` diff --git a/terraform/test-fixtures/input-hcl/main.tf b/terraform/test-fixtures/input-hcl/main.tf new file mode 100644 index 000000000..ca46ee8e9 --- /dev/null +++ b/terraform/test-fixtures/input-hcl/main.tf @@ -0,0 +1,12 @@ +variable "mapped" { + type = "map" +} + +variable "listed" { + type = "list" +} + +resource "hcl_instance" "hcltest" { + foo = "${var.listed}" + bar = "${var.mapped}" +} From d6c8b402011ef2045e8d73e5b2a1bf0ae0e14335 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 10 Aug 2016 11:37:55 -0400 Subject: [PATCH 2/3] unify some of the test code Have all the values in the testDiffFn go through the same code path --- terraform/context_plan_test.go | 1 + terraform/context_test.go | 51 +++++++++++++++++----------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 65ba03566..5a5d50ed1 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1223,6 +1223,7 @@ func TestContext2Plan_countZero(t *testing.T) { actual := strings.TrimSpace(plan.String()) expected := strings.TrimSpace(testTerraformPlanCountZeroStr) if actual != expected { + t.Logf("expected:\n%s", expected) t.Fatalf("bad:\n%s", actual) } } diff --git a/terraform/context_test.go b/terraform/context_test.go index 0d6da97fe..3ec4d9892 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -2,7 +2,6 @@ package terraform import ( "fmt" - "reflect" "strings" "testing" "time" @@ -167,32 +166,15 @@ func testDiffFn( v = c.Config[k] } - attrDiff := &ResourceAttrDiff{ - Old: "", - } - - if reflect.DeepEqual(v, []interface{}{}) { - attrDiff.New = "" - } else { - if s, ok := v.(string); ok { - attrDiff.New = s - } else { - // the value is something other than a string - // flatmap it, adding the diff for each value. - for k, attrDiff := range testFlatAttrDiffs(k, v) { - diff.Attributes[k] = attrDiff - } - continue + for k, attrDiff := range testFlatAttrDiffs(k, v) { + if k == "require_new" { + attrDiff.RequiresNew = true } + if _, ok := c.Raw["__"+k+"_requires_new"]; ok { + attrDiff.RequiresNew = true + } + diff.Attributes[k] = attrDiff } - - if k == "require_new" { - attrDiff.RequiresNew = true - } - if _, ok := c.Raw["__"+k+"_requires_new"]; ok { - attrDiff.RequiresNew = true - } - diff.Attributes[k] = attrDiff } for _, k := range c.ComputedKeys { @@ -232,8 +214,25 @@ func testDiffFn( // generate ResourceAttrDiffs for nested data structures in tests func testFlatAttrDiffs(k string, i interface{}) map[string]*ResourceAttrDiff { - flat := flatmap.Flatten(map[string]interface{}{k: i}) diffs := make(map[string]*ResourceAttrDiff) + // check for strings and empty containers first + switch t := i.(type) { + case string: + diffs[k] = &ResourceAttrDiff{New: t} + return diffs + case map[string]interface{}: + if len(t) == 0 { + diffs[k] = &ResourceAttrDiff{New: ""} + return diffs + } + case []interface{}: + if len(t) == 0 { + diffs[k] = &ResourceAttrDiff{New: ""} + return diffs + } + } + + flat := flatmap.Flatten(map[string]interface{}{k: i}) for k, v := range flat { attrDiff := &ResourceAttrDiff{ From c48a1423a1b68c20166906ce6711f3c174a58ac2 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 10 Aug 2016 16:34:21 -0400 Subject: [PATCH 3/3] Collapse nested if with an && --- terraform/context.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 8f30fd554..f60d5b570 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -363,16 +363,14 @@ func (c *Context) Input(mode InputMode) error { "Error asking for %s: %s", n, err) } - if value == "" { - if v.Required() { - // Redo if it is required, but abort if we keep getting - // blank entries - if retry > 2 { - return fmt.Errorf("missing required value for %q", n) - } - retry++ - continue + if value == "" && v.Required() { + // Redo if it is required, but abort if we keep getting + // blank entries + if retry > 2 { + return fmt.Errorf("missing required value for %q", n) } + retry++ + continue } break