From 39e3d8ea9bb9e8062e985103a7edc995498435e2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 10 Dec 2016 14:10:25 -0500 Subject: [PATCH 1/5] helper/variables: helpers for working with vars --- helper/variables/flag.go | 151 +++++++++++++++++++++++++ helper/variables/flag_file.go | 65 +++++++++++ helper/variables/flag_file_test.go | 107 ++++++++++++++++++ helper/variables/flag_test.go | 174 +++++++++++++++++++++++++++++ helper/variables/merge.go | 66 +++++++++++ helper/variables/merge_test.go | 93 +++++++++++++++ helper/variables/variables.go | 3 + helper/variables/variables_test.go | 20 ++++ 8 files changed, 679 insertions(+) create mode 100644 helper/variables/flag.go create mode 100644 helper/variables/flag_file.go create mode 100644 helper/variables/flag_file_test.go create mode 100644 helper/variables/flag_test.go create mode 100644 helper/variables/merge.go create mode 100644 helper/variables/merge_test.go create mode 100644 helper/variables/variables.go create mode 100644 helper/variables/variables_test.go diff --git a/helper/variables/flag.go b/helper/variables/flag.go new file mode 100644 index 000000000..40087a514 --- /dev/null +++ b/helper/variables/flag.go @@ -0,0 +1,151 @@ +package variables + +import ( + "fmt" + "regexp" + "strconv" + "strings" + + "github.com/hashicorp/hcl" +) + +// Flag a flag.Value implementation for parsing user variables +// from the command-line in the format of '-var key=value', where value is +// a type intended for use as a Terraform variable. +type Flag map[string]interface{} + +func (v *Flag) String() string { + return "" +} + +func (v *Flag) Set(raw string) error { + key, value, err := parseVarFlag(raw) + if err != nil { + return err + } + + *v = Merge(*v, map[string]interface{}{key: value}) + return nil +} + +var ( + // This regular expression is how we check if a value for a variable + // matches what we'd expect a rich HCL value to be. For example: { + // definitely signals a map. If a value DOESN'T match this, we return + // it as a raw string. + varFlagHCLRe = regexp.MustCompile(`^["\[\{]`) +) + +// parseVarFlag parses the value of a single variable specified +// on the command line via -var or in an environment variable named TF_VAR_x, +// where x is 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 parseVarFlag(input string) (string, interface{}, error) { + idx := strings.Index(input, "=") + if idx == -1 { + return "", nil, fmt.Errorf("No '=' value in variable: %s", input) + } + + probablyName := input[0:idx] + value := input[idx+1:] + trimmed := strings.TrimSpace(value) + + // If the value is a simple number, don't parse it as hcl because the + // variable type may actually be a string, and HCL will convert it to the + // numberic value. We could check this in the validation later, but the + // conversion may alter the string value. + if _, err := strconv.ParseInt(trimmed, 10, 64); err == nil { + return probablyName, value, nil + } + if _, err := strconv.ParseFloat(trimmed, 64); err == nil { + return probablyName, value, nil + } + + // HCL will also parse hex as a number + if strings.HasPrefix(trimmed, "0x") { + if _, err := strconv.ParseInt(trimmed[2:], 16, 64); err == nil { + return probablyName, value, nil + } + } + + // If the value is a boolean value, also convert it to a simple string + // since Terraform core doesn't accept primitives as anything other + // than string for now. + if _, err := strconv.ParseBool(trimmed); err == nil { + return probablyName, value, nil + } + + parsed, err := hcl.Parse(input) + if err != nil { + // If it didn't parse as HCL, we check if it doesn't match our + // whitelist of TF-accepted HCL types for inputs. If not, then + // we let it through as a raw string. + if !varFlagHCLRe.MatchString(trimmed) { + return probablyName, value, nil + } + + // This covers flags of the form `foo=bar` which is not valid HCL + // At this point, probablyName is actually the name, and the remainder + // of the expression after the equals sign is the value. + if regexp.MustCompile(`Unknown token: \d+:\d+ IDENT`).Match([]byte(err.Error())) { + return probablyName, value, nil + } + + return "", nil, fmt.Errorf( + "Cannot parse value for variable %s (%q) as valid HCL: %s", + probablyName, input, err) + } + + var decoded map[string]interface{} + if hcl.DecodeObject(&decoded, parsed); err != nil { + return "", nil, fmt.Errorf( + "Cannot parse value for variable %s (%q) as valid HCL: %s", + probablyName, input, err) + } + + // Cover cases such as key= + if len(decoded) == 0 { + return probablyName, "", nil + } + + if len(decoded) > 1 { + return "", nil, fmt.Errorf( + "Cannot parse value for variable %s (%q) as valid HCL. "+ + "Only one value may be specified.", + probablyName, input) + } + + err = flattenMultiMaps(decoded) + if err != nil { + return probablyName, "", err + } + + var k string + var v interface{} + for k, v = range decoded { + break + } + + return k, v, nil +} + +// Variables don't support any type that can be configured via multiple +// declarations of the same HCL map, so any instances of +// []map[string]interface{} are either a single map that can be flattened, or +// are invalid config. +func flattenMultiMaps(m map[string]interface{}) error { + for k, v := range m { + switch v := v.(type) { + case []map[string]interface{}: + switch { + case len(v) > 1: + return fmt.Errorf("multiple map declarations not supported for variables") + case len(v) == 1: + m[k] = v[0] + } + } + } + return nil +} diff --git a/helper/variables/flag_file.go b/helper/variables/flag_file.go new file mode 100644 index 000000000..372a6a839 --- /dev/null +++ b/helper/variables/flag_file.go @@ -0,0 +1,65 @@ +package variables + +import ( + "fmt" + "io/ioutil" + + "github.com/hashicorp/hcl" + "github.com/mitchellh/go-homedir" +) + +// FlagFile is a flag.Value implementation for parsing user variables +// from the command line in the form of files. i.e. '-var-file=foo' +type FlagFile map[string]interface{} + +func (v *FlagFile) String() string { + return "" +} + +func (v *FlagFile) Set(raw string) error { + vs, err := loadKVFile(raw) + if err != nil { + return err + } + + *v = Merge(*v, vs) + return nil +} + +func loadKVFile(rawPath string) (map[string]interface{}, error) { + path, err := homedir.Expand(rawPath) + if err != nil { + return nil, fmt.Errorf( + "Error expanding path: %s", err) + } + + // Read the HCL file and prepare for parsing + d, err := ioutil.ReadFile(path) + if err != nil { + return nil, fmt.Errorf( + "Error reading %s: %s", path, err) + } + + // Parse it + obj, err := hcl.Parse(string(d)) + if err != nil { + return nil, fmt.Errorf( + "Error parsing %s: %s", path, err) + } + + var result map[string]interface{} + if err := hcl.DecodeObject(&result, obj); err != nil { + return nil, fmt.Errorf( + "Error decoding Terraform vars file: %s\n\n"+ + "The vars file should be in the format of `key = \"value\"`.\n"+ + "Decoding errors are usually caused by an invalid format.", + err) + } + + err = flattenMultiMaps(result) + if err != nil { + return nil, err + } + + return result, nil +} diff --git a/helper/variables/flag_file_test.go b/helper/variables/flag_file_test.go new file mode 100644 index 000000000..50f8fe662 --- /dev/null +++ b/helper/variables/flag_file_test.go @@ -0,0 +1,107 @@ +package variables + +import ( + "flag" + "fmt" + "io/ioutil" + "reflect" + "testing" +) + +func TestFlagFile_impl(t *testing.T) { + var _ flag.Value = new(FlagFile) +} + +func TestFlagFile(t *testing.T) { + inputLibucl := ` +foo = "bar" +` + inputMap := ` +foo = { + k = "v" +}` + + inputJson := `{ + "foo": "bar"}` + + cases := []struct { + Input interface{} + Output map[string]interface{} + Error bool + }{ + { + inputLibucl, + map[string]interface{}{"foo": "bar"}, + false, + }, + + { + inputJson, + map[string]interface{}{"foo": "bar"}, + false, + }, + + { + `map.key = "foo"`, + map[string]interface{}{"map.key": "foo"}, + false, + }, + + { + inputMap, + map[string]interface{}{ + "foo": map[string]interface{}{ + "k": "v", + }, + }, + false, + }, + + { + []string{ + `foo = { "k" = "v"}`, + `foo = { "j" = "v" }`, + }, + map[string]interface{}{ + "foo": map[string]interface{}{ + "k": "v", + "j": "v", + }, + }, + false, + }, + } + + path := testTempFile(t) + + for i, tc := range cases { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + var input []string + switch i := tc.Input.(type) { + case string: + input = []string{i} + case []string: + input = i + default: + t.Fatalf("bad input type: %T", i) + } + + f := new(FlagFile) + for _, input := range input { + if err := ioutil.WriteFile(path, []byte(input), 0644); err != nil { + t.Fatalf("err: %s", err) + } + + err := f.Set(path) + if err != nil != tc.Error { + t.Fatalf("bad error. Input: %#v, err: %s", input, err) + } + } + + actual := map[string]interface{}(*f) + if !reflect.DeepEqual(actual, tc.Output) { + t.Fatalf("bad: %#v", actual) + } + }) + } +} diff --git a/helper/variables/flag_test.go b/helper/variables/flag_test.go new file mode 100644 index 000000000..f88b42cd1 --- /dev/null +++ b/helper/variables/flag_test.go @@ -0,0 +1,174 @@ +package variables + +import ( + "flag" + "fmt" + "reflect" + "testing" + + "github.com/davecgh/go-spew/spew" +) + +func TestFlag_impl(t *testing.T) { + var _ flag.Value = new(Flag) +} + +func TestFlag(t *testing.T) { + cases := []struct { + Input interface{} + Output map[string]interface{} + Error bool + }{ + { + "key=value", + map[string]interface{}{"key": "value"}, + false, + }, + + { + "key=", + map[string]interface{}{"key": ""}, + false, + }, + + { + "key=foo=bar", + map[string]interface{}{"key": "foo=bar"}, + false, + }, + + { + "key=false", + map[string]interface{}{"key": "false"}, + false, + }, + + { + "map.key=foo", + map[string]interface{}{"map.key": "foo"}, + false, + }, + + { + "key", + nil, + true, + }, + + { + `key=["hello", "world"]`, + map[string]interface{}{"key": []interface{}{"hello", "world"}}, + false, + }, + + { + `key={"hello" = "world", "foo" = "bar"}`, + map[string]interface{}{ + "key": map[string]interface{}{ + "hello": "world", + "foo": "bar", + }, + }, + false, + }, + + { + `key={"hello" = "world", "foo" = "bar"}\nkey2="invalid"`, + nil, + true, + }, + + { + "key=/path", + map[string]interface{}{"key": "/path"}, + false, + }, + + { + "key=1234.dkr.ecr.us-east-1.amazonaws.com/proj:abcdef", + map[string]interface{}{"key": "1234.dkr.ecr.us-east-1.amazonaws.com/proj:abcdef"}, + false, + }, + + // simple values that can parse as numbers should remain strings + { + "key=1", + map[string]interface{}{ + "key": "1", + }, + false, + }, + { + "key=1.0", + map[string]interface{}{ + "key": "1.0", + }, + false, + }, + { + "key=0x10", + map[string]interface{}{ + "key": "0x10", + }, + false, + }, + + // Test setting multiple times + { + []string{ + "foo=bar", + "bar=baz", + }, + map[string]interface{}{ + "foo": "bar", + "bar": "baz", + }, + false, + }, + + // Test map merging + { + []string{ + `foo={ foo = "bar" }`, + `foo={ bar = "baz" }`, + }, + map[string]interface{}{ + "foo": map[string]interface{}{ + "foo": "bar", + "bar": "baz", + }, + }, + false, + }, + } + + for i, tc := range cases { + t.Run(fmt.Sprintf("%d-%s", i, tc.Input), func(t *testing.T) { + var input []string + switch v := tc.Input.(type) { + case string: + input = []string{v} + case []string: + input = v + default: + t.Fatalf("bad input type: %T", tc.Input) + } + + f := new(Flag) + for i, single := range input { + err := f.Set(single) + + // Only check for expected errors on the final input + expected := tc.Error && i == len(input)-1 + if err != nil != expected { + t.Fatalf("bad error. Input: %#v\n\nError: %s", single, err) + } + } + + actual := map[string]interface{}(*f) + if !reflect.DeepEqual(actual, tc.Output) { + t.Fatalf("bad:\nexpected: %s\n\n got: %s\n", spew.Sdump(tc.Output), spew.Sdump(actual)) + } + }) + } +} diff --git a/helper/variables/merge.go b/helper/variables/merge.go new file mode 100644 index 000000000..6fe18188e --- /dev/null +++ b/helper/variables/merge.go @@ -0,0 +1,66 @@ +package variables + +// Merge merges raw variable values b into a. +// +// The parameters given here should be the full map of set variables, such +// as those created by Flag and FlagFile. +// +// The merge behavior is to override the top-level key except for map +// types. Map types are merged together by key. Any other types are overwritten: +// primitives and lists. +// +// This returns the resulting map. This merges into a but if a is nil a new +// map will be allocated. A non-nil "a" value is returned regardless. +func Merge(a, b map[string]interface{}) map[string]interface{} { + if a == nil { + a = map[string]interface{}{} + } + + for k, raw := range b { + switch v := raw.(type) { + case map[string]interface{}: + // For maps, we do a deep merge. If the value in the original + // map (a) is not a map, we just overwrite. For invalid types + // they're caught later in the validation step in Terraform. + + // If there is no value set, just set it + rawA, ok := a[k] + if !ok { + a[k] = v + continue + } + + // If the value is not a map, just set it + mapA, ok := rawA.(map[string]interface{}) + if !ok { + a[k] = v + continue + } + + // Go over the values in the map. If we're setting a raw value, + // then override. If we're setting a nested map, then recurse. + for k, v := range v { + // If the value isn't a map, then there is nothing to merge + // further so we just set it. + mv, ok := v.(map[string]interface{}) + if !ok { + mapA[k] = v + continue + } + + switch av := mapA[k].(type) { + case map[string]interface{}: + mapA[k] = Merge(av, mv) + default: + // Unset or non-map, just set it + mapA[k] = mv + } + } + default: + // Any other type we just set directly + a[k] = v + } + } + + return a +} diff --git a/helper/variables/merge_test.go b/helper/variables/merge_test.go new file mode 100644 index 000000000..25f5ae570 --- /dev/null +++ b/helper/variables/merge_test.go @@ -0,0 +1,93 @@ +package variables + +import ( + "fmt" + "reflect" + "testing" +) + +func TestMerge(t *testing.T) { + cases := []struct { + Name string + A, B map[string]interface{} + Expected map[string]interface{} + }{ + { + "basic key/value", + map[string]interface{}{ + "foo": "bar", + }, + map[string]interface{}{ + "bar": "baz", + }, + map[string]interface{}{ + "foo": "bar", + "bar": "baz", + }, + }, + + { + "map unset", + map[string]interface{}{ + "foo": "bar", + }, + map[string]interface{}{ + "bar": map[string]interface{}{ + "foo": "bar", + }, + }, + map[string]interface{}{ + "foo": "bar", + "bar": map[string]interface{}{ + "foo": "bar", + }, + }, + }, + + { + "map merge", + map[string]interface{}{ + "foo": "bar", + "bar": map[string]interface{}{ + "bar": "baz", + }, + }, + map[string]interface{}{ + "bar": map[string]interface{}{ + "foo": "bar", + }, + }, + map[string]interface{}{ + "foo": "bar", + "bar": map[string]interface{}{ + "foo": "bar", + "bar": "baz", + }, + }, + }, + + { + "basic k/v with lists", + map[string]interface{}{ + "foo": "bar", + "bar": []interface{}{"foo"}, + }, + map[string]interface{}{ + "bar": []interface{}{"bar"}, + }, + map[string]interface{}{ + "foo": "bar", + "bar": []interface{}{"bar"}, + }, + }, + } + + for i, tc := range cases { + t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) { + actual := Merge(tc.A, tc.B) + if !reflect.DeepEqual(tc.Expected, actual) { + t.Fatalf("bad: %#v", actual) + } + }) + } +} diff --git a/helper/variables/variables.go b/helper/variables/variables.go new file mode 100644 index 000000000..5a1b5e7a3 --- /dev/null +++ b/helper/variables/variables.go @@ -0,0 +1,3 @@ +// Package variables provides functions and types for working with +// Terraform variables provided as input. +package variables diff --git a/helper/variables/variables_test.go b/helper/variables/variables_test.go new file mode 100644 index 000000000..52c0637db --- /dev/null +++ b/helper/variables/variables_test.go @@ -0,0 +1,20 @@ +package variables + +import ( + "io/ioutil" + "path/filepath" + "testing" +) + +func testTempFile(t *testing.T) string { + return filepath.Join(testTempDir(t), "temp.dat") +} + +func testTempDir(t *testing.T) string { + d, err := ioutil.TempDir("", "tf") + if err != nil { + t.Fatalf("err: %s", err) + } + + return d +} From 77efacf30e63ef7a4786627ea7fda9e6c2f3452b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 10 Dec 2016 14:30:40 -0500 Subject: [PATCH 2/5] command: use helper/variables for flags and parsing --- command/flag_kv.go | 204 ---------------------------------------- command/flag_kv_test.go | 192 ------------------------------------- command/meta.go | 7 +- 3 files changed, 4 insertions(+), 399 deletions(-) diff --git a/command/flag_kv.go b/command/flag_kv.go index 8d73e43b0..b084c5135 100644 --- a/command/flag_kv.go +++ b/command/flag_kv.go @@ -2,38 +2,9 @@ package command import ( "fmt" - "io/ioutil" - "regexp" - "strconv" "strings" - - "github.com/hashicorp/hcl" - "github.com/mitchellh/go-homedir" ) -// FlagTypedKVis a flag.Value implementation for parsing user variables -// from the command-line in the format of '-var key=value', where value is -// a type intended for use as a Terraform variable -type FlagTypedKV map[string]interface{} - -func (v *FlagTypedKV) String() string { - return "" -} - -func (v *FlagTypedKV) Set(raw string) error { - key, value, err := parseVarFlagAsHCL(raw) - if err != nil { - return err - } - - if *v == nil { - *v = make(map[string]interface{}) - } - - (*v)[key] = value - return nil -} - // FlagStringKV is a flag.Value implementation for parsing user variables // from the command-line in the format of '-var key=value', where value is // only ever a primitive. @@ -58,71 +29,8 @@ func (v *FlagStringKV) Set(raw string) error { return nil } -// FlagKVFile is a flag.Value implementation for parsing user variables -// from the command line in the form of files. i.e. '-var-file=foo' -type FlagKVFile map[string]interface{} - -func (v *FlagKVFile) String() string { - return "" -} - -func (v *FlagKVFile) Set(raw string) error { - vs, err := loadKVFile(raw) - if err != nil { - return err - } - - if *v == nil { - *v = make(map[string]interface{}) - } - - for key, value := range vs { - (*v)[key] = value - } - - return nil -} - -func loadKVFile(rawPath string) (map[string]interface{}, error) { - path, err := homedir.Expand(rawPath) - if err != nil { - return nil, fmt.Errorf( - "Error expanding path: %s", err) - } - - // Read the HCL file and prepare for parsing - d, err := ioutil.ReadFile(path) - if err != nil { - return nil, fmt.Errorf( - "Error reading %s: %s", path, err) - } - - // Parse it - obj, err := hcl.Parse(string(d)) - if err != nil { - return nil, fmt.Errorf( - "Error parsing %s: %s", path, err) - } - - var result map[string]interface{} - if err := hcl.DecodeObject(&result, obj); err != nil { - return nil, fmt.Errorf( - "Error decoding Terraform vars file: %s\n\n"+ - "The vars file should be in the format of `key = \"value\"`.\n"+ - "Decoding errors are usually caused by an invalid format.", - err) - } - err = flattenMultiMaps(result) - if err != nil { - return nil, err - } - - return result, nil -} - // FlagStringSlice is a flag.Value implementation for parsing targets from the // command line, e.g. -target=aws_instance.foo -target=aws_vpc.bar - type FlagStringSlice []string func (v *FlagStringSlice) String() string { @@ -133,115 +41,3 @@ func (v *FlagStringSlice) Set(raw string) error { return nil } - -var ( - // This regular expression is how we check if a value for a variable - // matches what we'd expect a rich HCL value to be. For example: { - // definitely signals a map. If a value DOESN'T match this, we return - // it as a raw string. - varFlagHCLRe = regexp.MustCompile(`^["\[\{]`) -) - -// parseVarFlagAsHCL parses the value of a single variable as would have been specified -// on the command line via -var or in an environment variable named TF_VAR_x, where x is -// 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 parseVarFlagAsHCL(input string) (string, interface{}, error) { - idx := strings.Index(input, "=") - if idx == -1 { - return "", nil, fmt.Errorf("No '=' value in variable: %s", input) - } - probablyName := input[0:idx] - value := input[idx+1:] - trimmed := strings.TrimSpace(value) - - // If the value is a simple number, don't parse it as hcl because the - // variable type may actually be a string, and HCL will convert it to the - // numberic value. We could check this in the validation later, but the - // conversion may alter the string value. - if _, err := strconv.ParseInt(trimmed, 10, 64); err == nil { - return probablyName, value, nil - } - if _, err := strconv.ParseFloat(trimmed, 64); err == nil { - return probablyName, value, nil - } - - // HCL will also parse hex as a number - if strings.HasPrefix(trimmed, "0x") { - if _, err := strconv.ParseInt(trimmed[2:], 16, 64); err == nil { - return probablyName, value, nil - } - } - - // If the value is a boolean value, also convert it to a simple string - // since Terraform core doesn't accept primitives as anything other - // than string for now. - if _, err := strconv.ParseBool(trimmed); err == nil { - return probablyName, value, nil - } - - parsed, err := hcl.Parse(input) - if err != nil { - // If it didn't parse as HCL, we check if it doesn't match our - // whitelist of TF-accepted HCL types for inputs. If not, then - // we let it through as a raw string. - if !varFlagHCLRe.MatchString(trimmed) { - return probablyName, value, nil - } - - // This covers flags of the form `foo=bar` which is not valid HCL - // At this point, probablyName is actually the name, and the remainder - // of the expression after the equals sign is the value. - if regexp.MustCompile(`Unknown token: \d+:\d+ IDENT`).Match([]byte(err.Error())) { - return probablyName, value, nil - } - - return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL: %s", probablyName, input, err) - } - - var decoded map[string]interface{} - if hcl.DecodeObject(&decoded, parsed); err != nil { - return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL: %s", probablyName, input, err) - } - - // Cover cases such as key= - if len(decoded) == 0 { - return probablyName, "", nil - } - - if len(decoded) > 1 { - return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL. Only one value may be specified.", probablyName, input) - } - - err = flattenMultiMaps(decoded) - if err != nil { - return probablyName, "", err - } - - var k string - var v interface{} - for k, v = range decoded { - break - } - return k, v, nil -} - -// Variables don't support any type that can be configured via multiple -// declarations of the same HCL map, so any instances of -// []map[string]interface{} are either a single map that can be flattened, or -// are invalid config. -func flattenMultiMaps(m map[string]interface{}) error { - for k, v := range m { - switch v := v.(type) { - case []map[string]interface{}: - switch { - case len(v) > 1: - return fmt.Errorf("multiple map declarations not supported for variables") - case len(v) == 1: - m[k] = v[0] - } - } - } - return nil -} diff --git a/command/flag_kv_test.go b/command/flag_kv_test.go index d42c88d36..0f39211fb 100644 --- a/command/flag_kv_test.go +++ b/command/flag_kv_test.go @@ -2,11 +2,8 @@ package command import ( "flag" - "io/ioutil" "reflect" "testing" - - "github.com/davecgh/go-spew/spew" ) func TestFlagStringKV_impl(t *testing.T) { @@ -69,192 +66,3 @@ func TestFlagStringKV(t *testing.T) { } } } - -func TestFlagTypedKV_impl(t *testing.T) { - var _ flag.Value = new(FlagTypedKV) -} - -func TestFlagTypedKV(t *testing.T) { - cases := []struct { - Input string - Output map[string]interface{} - Error bool - }{ - { - "key=value", - map[string]interface{}{"key": "value"}, - false, - }, - - { - "key=", - map[string]interface{}{"key": ""}, - false, - }, - - { - "key=foo=bar", - map[string]interface{}{"key": "foo=bar"}, - false, - }, - - { - "key=false", - map[string]interface{}{"key": "false"}, - false, - }, - - { - "map.key=foo", - map[string]interface{}{"map.key": "foo"}, - false, - }, - - { - "key", - nil, - true, - }, - - { - `key=["hello", "world"]`, - map[string]interface{}{"key": []interface{}{"hello", "world"}}, - false, - }, - - { - `key={"hello" = "world", "foo" = "bar"}`, - map[string]interface{}{ - "key": map[string]interface{}{ - "hello": "world", - "foo": "bar", - }, - }, - false, - }, - - { - `key={"hello" = "world", "foo" = "bar"}\nkey2="invalid"`, - nil, - true, - }, - - { - "key=/path", - map[string]interface{}{"key": "/path"}, - false, - }, - - { - "key=1234.dkr.ecr.us-east-1.amazonaws.com/proj:abcdef", - map[string]interface{}{"key": "1234.dkr.ecr.us-east-1.amazonaws.com/proj:abcdef"}, - false, - }, - - // simple values that can parse as numbers should remain strings - { - "key=1", - map[string]interface{}{ - "key": "1", - }, - false, - }, - { - "key=1.0", - map[string]interface{}{ - "key": "1.0", - }, - false, - }, - { - "key=0x10", - map[string]interface{}{ - "key": "0x10", - }, - false, - }, - } - - for _, tc := range cases { - f := new(FlagTypedKV) - err := f.Set(tc.Input) - if err != nil != tc.Error { - t.Fatalf("bad error. Input: %#v\n\nError: %s", tc.Input, err) - } - - actual := map[string]interface{}(*f) - if !reflect.DeepEqual(actual, tc.Output) { - t.Fatalf("bad:\nexpected: %s\n\n got: %s\n", spew.Sdump(tc.Output), spew.Sdump(actual)) - } - } -} - -func TestFlagKVFile_impl(t *testing.T) { - var _ flag.Value = new(FlagKVFile) -} - -func TestFlagKVFile(t *testing.T) { - inputLibucl := ` -foo = "bar" -` - inputMap := ` -foo = { - k = "v" -}` - - inputJson := `{ - "foo": "bar"}` - - cases := []struct { - Input string - Output map[string]interface{} - Error bool - }{ - { - inputLibucl, - map[string]interface{}{"foo": "bar"}, - false, - }, - - { - inputJson, - map[string]interface{}{"foo": "bar"}, - false, - }, - - { - `map.key = "foo"`, - map[string]interface{}{"map.key": "foo"}, - false, - }, - - { - inputMap, - map[string]interface{}{ - "foo": map[string]interface{}{ - "k": "v", - }, - }, - false, - }, - } - - path := testTempFile(t) - - for _, tc := range cases { - if err := ioutil.WriteFile(path, []byte(tc.Input), 0644); err != nil { - t.Fatalf("err: %s", err) - } - - f := new(FlagKVFile) - err := f.Set(path) - if err != nil != tc.Error { - t.Fatalf("bad error. Input: %#v, err: %s", tc.Input, err) - } - - actual := map[string]interface{}(*f) - if !reflect.DeepEqual(actual, tc.Output) { - t.Fatalf("bad: %#v", actual) - } - } -} diff --git a/command/meta.go b/command/meta.go index 07ac45627..5566c2669 100644 --- a/command/meta.go +++ b/command/meta.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/helper/experiment" + "github.com/hashicorp/terraform/helper/variables" "github.com/hashicorp/terraform/helper/wrappedstreams" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" @@ -358,12 +359,12 @@ func (m *Meta) contextOpts() *terraform.ContextOpts { func (m *Meta) flagSet(n string) *flag.FlagSet { f := flag.NewFlagSet(n, flag.ContinueOnError) f.BoolVar(&m.input, "input", true, "input") - f.Var((*FlagTypedKV)(&m.variables), "var", "variables") - f.Var((*FlagKVFile)(&m.variables), "var-file", "variable file") + f.Var((*variables.Flag)(&m.variables), "var", "variables") + f.Var((*variables.FlagFile)(&m.variables), "var-file", "variable file") f.Var((*FlagStringSlice)(&m.targets), "target", "resource to target") if m.autoKey != "" { - f.Var((*FlagKVFile)(&m.autoVariables), m.autoKey, "variable file") + f.Var((*variables.FlagFile)(&m.autoVariables), m.autoKey, "variable file") } // Advanced (don't need documentation, or unlikely to be set) From 83e72b0361fcbead7454bfe214e4db4726a23c79 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 10 Dec 2016 14:44:17 -0500 Subject: [PATCH 3/5] helper/variables: ParseInput for consistent parsing --- helper/variables/flag.go | 134 ++------------------------------- helper/variables/parse.go | 118 +++++++++++++++++++++++++++++ helper/variables/parse_test.go | 81 ++++++++++++++++++++ 3 files changed, 206 insertions(+), 127 deletions(-) create mode 100644 helper/variables/parse.go create mode 100644 helper/variables/parse_test.go diff --git a/helper/variables/flag.go b/helper/variables/flag.go index 40087a514..393586234 100644 --- a/helper/variables/flag.go +++ b/helper/variables/flag.go @@ -2,11 +2,7 @@ package variables import ( "fmt" - "regexp" - "strconv" "strings" - - "github.com/hashicorp/hcl" ) // Flag a flag.Value implementation for parsing user variables @@ -19,7 +15,13 @@ func (v *Flag) String() string { } func (v *Flag) Set(raw string) error { - key, value, err := parseVarFlag(raw) + idx := strings.Index(raw, "=") + if idx == -1 { + return fmt.Errorf("No '=' value in arg: %s", raw) + } + + key, input := raw[0:idx], raw[idx+1:] + value, err := ParseInput(input) if err != nil { return err } @@ -27,125 +29,3 @@ func (v *Flag) Set(raw string) error { *v = Merge(*v, map[string]interface{}{key: value}) return nil } - -var ( - // This regular expression is how we check if a value for a variable - // matches what we'd expect a rich HCL value to be. For example: { - // definitely signals a map. If a value DOESN'T match this, we return - // it as a raw string. - varFlagHCLRe = regexp.MustCompile(`^["\[\{]`) -) - -// parseVarFlag parses the value of a single variable specified -// on the command line via -var or in an environment variable named TF_VAR_x, -// where x is 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 parseVarFlag(input string) (string, interface{}, error) { - idx := strings.Index(input, "=") - if idx == -1 { - return "", nil, fmt.Errorf("No '=' value in variable: %s", input) - } - - probablyName := input[0:idx] - value := input[idx+1:] - trimmed := strings.TrimSpace(value) - - // If the value is a simple number, don't parse it as hcl because the - // variable type may actually be a string, and HCL will convert it to the - // numberic value. We could check this in the validation later, but the - // conversion may alter the string value. - if _, err := strconv.ParseInt(trimmed, 10, 64); err == nil { - return probablyName, value, nil - } - if _, err := strconv.ParseFloat(trimmed, 64); err == nil { - return probablyName, value, nil - } - - // HCL will also parse hex as a number - if strings.HasPrefix(trimmed, "0x") { - if _, err := strconv.ParseInt(trimmed[2:], 16, 64); err == nil { - return probablyName, value, nil - } - } - - // If the value is a boolean value, also convert it to a simple string - // since Terraform core doesn't accept primitives as anything other - // than string for now. - if _, err := strconv.ParseBool(trimmed); err == nil { - return probablyName, value, nil - } - - parsed, err := hcl.Parse(input) - if err != nil { - // If it didn't parse as HCL, we check if it doesn't match our - // whitelist of TF-accepted HCL types for inputs. If not, then - // we let it through as a raw string. - if !varFlagHCLRe.MatchString(trimmed) { - return probablyName, value, nil - } - - // This covers flags of the form `foo=bar` which is not valid HCL - // At this point, probablyName is actually the name, and the remainder - // of the expression after the equals sign is the value. - if regexp.MustCompile(`Unknown token: \d+:\d+ IDENT`).Match([]byte(err.Error())) { - return probablyName, value, nil - } - - return "", nil, fmt.Errorf( - "Cannot parse value for variable %s (%q) as valid HCL: %s", - probablyName, input, err) - } - - var decoded map[string]interface{} - if hcl.DecodeObject(&decoded, parsed); err != nil { - return "", nil, fmt.Errorf( - "Cannot parse value for variable %s (%q) as valid HCL: %s", - probablyName, input, err) - } - - // Cover cases such as key= - if len(decoded) == 0 { - return probablyName, "", nil - } - - if len(decoded) > 1 { - return "", nil, fmt.Errorf( - "Cannot parse value for variable %s (%q) as valid HCL. "+ - "Only one value may be specified.", - probablyName, input) - } - - err = flattenMultiMaps(decoded) - if err != nil { - return probablyName, "", err - } - - var k string - var v interface{} - for k, v = range decoded { - break - } - - return k, v, nil -} - -// Variables don't support any type that can be configured via multiple -// declarations of the same HCL map, so any instances of -// []map[string]interface{} are either a single map that can be flattened, or -// are invalid config. -func flattenMultiMaps(m map[string]interface{}) error { - for k, v := range m { - switch v := v.(type) { - case []map[string]interface{}: - switch { - case len(v) > 1: - return fmt.Errorf("multiple map declarations not supported for variables") - case len(v) == 1: - m[k] = v[0] - } - } - } - return nil -} diff --git a/helper/variables/parse.go b/helper/variables/parse.go new file mode 100644 index 000000000..b7474df81 --- /dev/null +++ b/helper/variables/parse.go @@ -0,0 +1,118 @@ +package variables + +import ( + "fmt" + "regexp" + "strconv" + "strings" + + "github.com/hashicorp/hcl" +) + +// ParseInput parses a manually inputed variable to a richer value. +// +// This will turn raw input into rich types such as `[]` to a real list or +// `{}` to a real map. This function should be used to parse any manual untyped +// input for variables in order to provide a consistent experience. +func ParseInput(value string) (interface{}, error) { + trimmed := strings.TrimSpace(value) + + // If the value is a simple number, don't parse it as hcl because the + // variable type may actually be a string, and HCL will convert it to the + // numberic value. We could check this in the validation later, but the + // conversion may alter the string value. + if _, err := strconv.ParseInt(trimmed, 10, 64); err == nil { + return value, nil + } + if _, err := strconv.ParseFloat(trimmed, 64); err == nil { + return value, nil + } + + // HCL will also parse hex as a number + if strings.HasPrefix(trimmed, "0x") { + if _, err := strconv.ParseInt(trimmed[2:], 16, 64); err == nil { + return value, nil + } + } + + // If the value is a boolean value, also convert it to a simple string + // since Terraform core doesn't accept primitives as anything other + // than string for now. + if _, err := strconv.ParseBool(trimmed); err == nil { + return value, nil + } + + parsed, err := hcl.Parse(fmt.Sprintf("foo=%s", trimmed)) + if err != nil { + // If it didn't parse as HCL, we check if it doesn't match our + // whitelist of TF-accepted HCL types for inputs. If not, then + // we let it through as a raw string. + if !varFlagHCLRe.MatchString(trimmed) { + return value, nil + } + + // This covers flags of the form `foo=bar` which is not valid HCL + // At this point, probablyName is actually the name, and the remainder + // of the expression after the equals sign is the value. + if regexp.MustCompile(`Unknown token: \d+:\d+ IDENT`).Match([]byte(err.Error())) { + return value, nil + } + + return nil, fmt.Errorf( + "Cannot parse value for variable %s (%q) as valid HCL: %s", + value, err) + } + + var decoded map[string]interface{} + if hcl.DecodeObject(&decoded, parsed); err != nil { + return nil, fmt.Errorf( + "Cannot parse value for variable %s (%q) as valid HCL: %s", + value, err) + } + + // Cover cases such as key= + if len(decoded) == 0 { + return "", nil + } + + if len(decoded) > 1 { + return nil, fmt.Errorf( + "Cannot parse value for variable %s (%q) as valid HCL. "+ + "Only one value may be specified.", + value) + } + + err = flattenMultiMaps(decoded) + if err != nil { + return "", err + } + + return decoded["foo"], nil +} + +var ( + // This regular expression is how we check if a value for a variable + // matches what we'd expect a rich HCL value to be. For example: { + // definitely signals a map. If a value DOESN'T match this, we return + // it as a raw string. + varFlagHCLRe = regexp.MustCompile(`^["\[\{]`) +) + +// Variables don't support any type that can be configured via multiple +// declarations of the same HCL map, so any instances of +// []map[string]interface{} are either a single map that can be flattened, or +// are invalid config. +func flattenMultiMaps(m map[string]interface{}) error { + for k, v := range m { + switch v := v.(type) { + case []map[string]interface{}: + switch { + case len(v) > 1: + return fmt.Errorf("multiple map declarations not supported for variables") + case len(v) == 1: + m[k] = v[0] + } + } + } + return nil +} diff --git a/helper/variables/parse_test.go b/helper/variables/parse_test.go new file mode 100644 index 000000000..7c18df38c --- /dev/null +++ b/helper/variables/parse_test.go @@ -0,0 +1,81 @@ +package variables + +import ( + "fmt" + "reflect" + "testing" +) + +func TestParseInput(t *testing.T) { + cases := []struct { + Name string + Input string + Result interface{} + Error bool + }{ + { + "unquoted string", + "foo", + "foo", + false, + }, + + { + "number", + "1", + "1", + false, + }, + + { + "float", + "1.2", + "1.2", + false, + }, + + { + "hex number", + "0x12", + "0x12", + false, + }, + + { + "bool", + "true", + "true", + false, + }, + + { + "list", + `["foo"]`, + []interface{}{"foo"}, + false, + }, + + { + "map", + `{ foo = "bar" }`, + map[string]interface{}{"foo": "bar"}, + false, + }, + } + + for i, tc := range cases { + t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) { + actual, err := ParseInput(tc.Input) + if (err != nil) != tc.Error { + t.Fatalf("err: %s", err) + } + if err != nil { + return + } + + if !reflect.DeepEqual(actual, tc.Result) { + t.Fatalf("bad: %#v", actual) + } + }) + } +} From da418b25821c0645de90d3d2a72675df885108ab Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 10 Dec 2016 15:05:58 -0500 Subject: [PATCH 4/5] website: note about map/var merging --- .../docs/configuration/variables.html.md | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/website/source/docs/configuration/variables.html.md b/website/source/docs/configuration/variables.html.md index 31b3b6918..223dcb1c0 100644 --- a/website/source/docs/configuration/variables.html.md +++ b/website/source/docs/configuration/variables.html.md @@ -294,7 +294,39 @@ terraform apply -var-file=foo.tfvars -var-file=bar.tfvars on the command line. If a variable is defined in more than one variable file, the last value specified is effective. -### Precedence example +### Variable Merging + +When variables are conflicting, map values are merged and all are values are +overridden. Map values are always merged. + +For example, if you set a variable twice on the command line: + +``` +terraform apply -var foo=bar -var foo=baz +``` + +Then the value of `foo` will be `baz` since it was the last value seen. + +However, for maps, the values are merged: + +``` +terraform apply -var 'foo={foo="bar"}' -var 'foo={bar="baz"}' +``` + +The resulting value of `foo` will be: + +``` +{ + foo = "bar" + bar = "baz" +} +``` + +There is no way currently to unset map values in Terraform. Whenever a map +is modified either via variable input or being passed into a module, the +values are always merged. + +### Variable Precedence Both these files have the variable `baz` defined: From 42edd22566efe522aaca5f21b2e6facd0c6cecc8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 10 Dec 2016 20:32:50 -0500 Subject: [PATCH 5/5] helper/variables: address go vet --- helper/variables/parse.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helper/variables/parse.go b/helper/variables/parse.go index b7474df81..f4370f1ae 100644 --- a/helper/variables/parse.go +++ b/helper/variables/parse.go @@ -59,14 +59,14 @@ func ParseInput(value string) (interface{}, error) { } return nil, fmt.Errorf( - "Cannot parse value for variable %s (%q) as valid HCL: %s", + "Cannot parse value for variable (%q) as valid HCL: %s", value, err) } var decoded map[string]interface{} if hcl.DecodeObject(&decoded, parsed); err != nil { return nil, fmt.Errorf( - "Cannot parse value for variable %s (%q) as valid HCL: %s", + "Cannot parse value for variable (%q) as valid HCL: %s", value, err) } @@ -77,7 +77,7 @@ func ParseInput(value string) (interface{}, error) { if len(decoded) > 1 { return nil, fmt.Errorf( - "Cannot parse value for variable %s (%q) as valid HCL. "+ + "Cannot parse value for variable (%q) as valid HCL. "+ "Only one value may be specified.", value) }