From b0ce89b80525f8c58b5102db5e4be6638d73d227 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 07:32:36 -0700 Subject: [PATCH 01/25] config: change Default to an interface{} --- config/config.go | 8 +++----- config/loader_libucl.go | 11 +---------- config/loader_test.go | 14 +++++++------- config/merge_test.go | 4 ++-- 4 files changed, 13 insertions(+), 24 deletions(-) diff --git a/config/config.go b/config/config.go index bb7ab76bf..2083c7fc7 100644 --- a/config/config.go +++ b/config/config.go @@ -53,9 +53,8 @@ type Provisioner struct { // Variable is a variable defined within the configuration. type Variable struct { Name string - Default string + Default interface{} Description string - defaultSet bool } // Output is an output defined within the configuration. An output is @@ -310,9 +309,8 @@ func (v *Variable) Merge(v2 *Variable) *Variable { // The names should be the same, but the second name always wins. result.Name = v2.Name - if v2.defaultSet { + if v2.Default != nil { result.Default = v2.Default - result.defaultSet = true } if v2.Description != "" { result.Description = v2.Description @@ -331,7 +329,7 @@ func (v *Variable) mergerMerge(m merger) merger { // Required tests whether a variable is required or not. func (v *Variable) Required() bool { - return !v.defaultSet + return v.Default == nil } func NewResourceVariable(key string) (*ResourceVariable, error) { diff --git a/config/loader_libucl.go b/config/loader_libucl.go index c6050d922..398c2f5fd 100644 --- a/config/loader_libucl.go +++ b/config/loader_libucl.go @@ -30,7 +30,7 @@ func (t *libuclConfigurable) Config() (*Config, error) { } type LibuclVariable struct { - Default string + Default interface{} Description string Fields []string `libucl:",decodedFields"` } @@ -51,19 +51,10 @@ func (t *libuclConfigurable) Config() (*Config, error) { if len(rawConfig.Variable) > 0 { config.Variables = make([]*Variable, 0, len(rawConfig.Variable)) for k, v := range rawConfig.Variable { - defaultSet := false - for _, f := range v.Fields { - if f == "Default" { - defaultSet = true - break - } - } - config.Variables = append(config.Variables, &Variable{ Name: k, Default: v.Default, Description: v.Description, - defaultSet: defaultSet, }) } } diff --git a/config/loader_test.go b/config/loader_test.go index 658e1c248..0cb8183cf 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -437,20 +437,20 @@ func variablesStr(vs []*Variable) string { for _, k := range ks { v := m[k] - if v.Default == "" { + required := "" + if v.Required() { + required = " (required)" + } + + if v.Default == nil || v.Default == "" { v.Default = "<>" } if v.Description == "" { v.Description = "<>" } - required := "" - if v.Required() { - required = " (required)" - } - result += fmt.Sprintf( - "%s%s\n %s\n %s\n", + "%s%s\n %v\n %s\n", k, required, v.Default, diff --git a/config/merge_test.go b/config/merge_test.go index fb52677b5..10ef5f37a 100644 --- a/config/merge_test.go +++ b/config/merge_test.go @@ -103,7 +103,7 @@ func TestMerge(t *testing.T) { &Resource{Name: "bar"}, }, Variables: []*Variable{ - &Variable{Name: "foo", Default: "bar", defaultSet: true}, + &Variable{Name: "foo", Default: "bar"}, &Variable{Name: "bar"}, }, @@ -124,7 +124,7 @@ func TestMerge(t *testing.T) { &Resource{Name: "bar"}, }, Variables: []*Variable{ - &Variable{Name: "foo", Default: "bar", defaultSet: true}, + &Variable{Name: "foo", Default: "bar"}, &Variable{Name: "foo"}, &Variable{Name: "bar"}, }, From b8a0a0221748abd63efa05612913bfa12e3e098b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 08:34:44 -0700 Subject: [PATCH 02/25] config: TODO tests for validation --- config/config.go | 3 +++ config/config_test.go | 18 ++++++++++++++++++ config/loader_libucl.go | 6 ++++-- .../validate-var-default-bad-type/main.tf | 3 +++ .../test-fixtures/validate-var-default/main.tf | 9 +++++++++ 5 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 config/test-fixtures/validate-var-default-bad-type/main.tf create mode 100644 config/test-fixtures/validate-var-default/main.tf diff --git a/config/config.go b/config/config.go index 2083c7fc7..656f33a62 100644 --- a/config/config.go +++ b/config/config.go @@ -131,6 +131,9 @@ func (c *Config) Validate() error { varMap[v.Name] = v } + // TODO(mitchellh): Validate that variable defaults are only a string + // or mapping of strings. + // Check for references to user variables that do not actually // exist and record those errors. for source, vs := range vars { diff --git a/config/config_test.go b/config/config_test.go index 4c66466b1..f4a2ce0d7 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -64,6 +64,24 @@ func TestConfigValidate_unknownVar(t *testing.T) { } } +func TestConfigValidate_varDefault(t *testing.T) { + c := testConfig(t, "validate-var-default") + if err := c.Validate(); err != nil { + t.Fatalf("should be valid: %s", err) + } +} + +func TestConfigValidate_varDefaultBadType(t *testing.T) { + t.Skip() + + // TODO(mitchellh): FIX + + c := testConfig(t, "validate-var-default-bad-type") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + func TestNewResourceVariable(t *testing.T) { v, err := NewResourceVariable("foo.bar.baz") if err != nil { diff --git a/config/loader_libucl.go b/config/loader_libucl.go index 398c2f5fd..176c588bd 100644 --- a/config/loader_libucl.go +++ b/config/loader_libucl.go @@ -51,11 +51,13 @@ func (t *libuclConfigurable) Config() (*Config, error) { if len(rawConfig.Variable) > 0 { config.Variables = make([]*Variable, 0, len(rawConfig.Variable)) for k, v := range rawConfig.Variable { - config.Variables = append(config.Variables, &Variable{ + newVar := &Variable{ Name: k, Default: v.Default, Description: v.Description, - }) + } + + config.Variables = append(config.Variables, newVar) } } diff --git a/config/test-fixtures/validate-var-default-bad-type/main.tf b/config/test-fixtures/validate-var-default-bad-type/main.tf new file mode 100644 index 000000000..c7975db38 --- /dev/null +++ b/config/test-fixtures/validate-var-default-bad-type/main.tf @@ -0,0 +1,3 @@ +variable "foo" { + default = ["foo", "bar"] +} diff --git a/config/test-fixtures/validate-var-default/main.tf b/config/test-fixtures/validate-var-default/main.tf new file mode 100644 index 000000000..75a6bc9e0 --- /dev/null +++ b/config/test-fixtures/validate-var-default/main.tf @@ -0,0 +1,9 @@ +variable "foo" { + default = "bar" +} + +variable "foo" { + default = { + "foo" = "bar" + } +} From b772f8078dd7ea3599782d2591f0879026a8ad11 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 08:55:45 -0700 Subject: [PATCH 03/25] config: detect UserMapVariable --- config/config.go | 87 -------------------------- config/config_test.go | 86 -------------------------- config/variable.go | 132 +++++++++++++++++++++++++++++++++++++++- config/variable_test.go | 122 +++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 176 deletions(-) diff --git a/config/config.go b/config/config.go index 656f33a62..be4bf00f5 100644 --- a/config/config.go +++ b/config/config.go @@ -4,7 +4,6 @@ package config import ( "fmt" - "strconv" "strings" "github.com/hashicorp/terraform/helper/multierror" @@ -64,38 +63,6 @@ type Output struct { RawConfig *RawConfig } -// An InterpolatedVariable is a variable that is embedded within a string -// in the configuration, such as "hello ${world}" (world in this case is -// an interpolated variable). -// -// These variables can come from a variety of sources, represented by -// implementations of this interface. -type InterpolatedVariable interface { - FullKey() string -} - -// A ResourceVariable is a variable that is referencing the field -// of a resource, such as "${aws_instance.foo.ami}" -type ResourceVariable struct { - Type string // Resource type, i.e. "aws_instance" - Name string // Resource name - Field string // Resource field - - Multi bool // True if multi-variable: aws_instance.foo.*.id - Index int // Index for multi-variable: aws_instance.foo.1.id == 1 - - key string -} - -// A UserVariable is a variable that is referencing a user variable -// that is inputted from outside the configuration. This looks like -// "${var.foo}" -type UserVariable struct { - Name string - - key string -} - // ProviderConfigName returns the name of the provider configuration in // the given mapping that maps to the proper provider configuration // for this resource. @@ -334,57 +301,3 @@ func (v *Variable) mergerMerge(m merger) merger { func (v *Variable) Required() bool { return v.Default == nil } - -func NewResourceVariable(key string) (*ResourceVariable, error) { - parts := strings.SplitN(key, ".", 3) - field := parts[2] - multi := false - var index int - - if idx := strings.Index(field, "."); idx != -1 { - indexStr := field[:idx] - multi = indexStr == "*" - index = -1 - - if !multi { - indexInt, err := strconv.ParseInt(indexStr, 0, 0) - if err == nil { - multi = true - index = int(indexInt) - } - } - - if multi { - field = field[idx+1:] - } - } - - return &ResourceVariable{ - Type: parts[0], - Name: parts[1], - Field: field, - Multi: multi, - Index: index, - key: key, - }, nil -} - -func (v *ResourceVariable) ResourceId() string { - return fmt.Sprintf("%s.%s", v.Type, v.Name) -} - -func (v *ResourceVariable) FullKey() string { - return v.key -} - -func NewUserVariable(key string) (*UserVariable, error) { - name := key[len("var."):] - return &UserVariable{ - key: key, - Name: name, - }, nil -} - -func (v *UserVariable) FullKey() string { - return v.key -} diff --git a/config/config_test.go b/config/config_test.go index f4a2ce0d7..000719ee5 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -82,92 +82,6 @@ func TestConfigValidate_varDefaultBadType(t *testing.T) { } } -func TestNewResourceVariable(t *testing.T) { - v, err := NewResourceVariable("foo.bar.baz") - if err != nil { - t.Fatalf("err: %s", err) - } - - if v.Type != "foo" { - t.Fatalf("bad: %#v", v) - } - if v.Name != "bar" { - t.Fatalf("bad: %#v", v) - } - if v.Field != "baz" { - t.Fatalf("bad: %#v", v) - } - if v.Multi { - t.Fatal("should not be multi") - } - - if v.FullKey() != "foo.bar.baz" { - t.Fatalf("bad: %#v", v) - } -} - -func TestResourceVariable_Multi(t *testing.T) { - v, err := NewResourceVariable("foo.bar.*.baz") - if err != nil { - t.Fatalf("err: %s", err) - } - - if v.Type != "foo" { - t.Fatalf("bad: %#v", v) - } - if v.Name != "bar" { - t.Fatalf("bad: %#v", v) - } - if v.Field != "baz" { - t.Fatalf("bad: %#v", v) - } - if !v.Multi { - t.Fatal("should be multi") - } -} - -func TestResourceVariable_MultiIndex(t *testing.T) { - cases := []struct { - Input string - Index int - Field string - }{ - {"foo.bar.*.baz", -1, "baz"}, - {"foo.bar.0.baz", 0, "baz"}, - {"foo.bar.5.baz", 5, "baz"}, - } - - for _, tc := range cases { - v, err := NewResourceVariable(tc.Input) - if err != nil { - t.Fatalf("err: %s", err) - } - if !v.Multi { - t.Fatalf("should be multi: %s", tc.Input) - } - if v.Index != tc.Index { - t.Fatalf("bad: %d\n\n%s", v.Index, tc.Input) - } - if v.Field != tc.Field { - t.Fatalf("bad: %s\n\n%s", v.Field, tc.Input) - } - } -} - -func TestNewUserVariable(t *testing.T) { - v, err := NewUserVariable("var.bar") - if err != nil { - t.Fatalf("err: %s", err) - } - - if v.Name != "bar" { - t.Fatalf("bad: %#v", v.Name) - } - if v.FullKey() != "var.bar" { - t.Fatalf("bad: %#v", v) - } -} - func TestProviderConfigName(t *testing.T) { pcs := []*ProviderConfig{ &ProviderConfig{Name: "aw"}, diff --git a/config/variable.go b/config/variable.go index 40a3b28df..96b49f9e6 100644 --- a/config/variable.go +++ b/config/variable.go @@ -4,6 +4,7 @@ import ( "fmt" "reflect" "regexp" + "strconv" "strings" "github.com/mitchellh/reflectwalk" @@ -16,6 +17,126 @@ func init() { varRegexp = regexp.MustCompile(`(?i)(\$+)\{([*-.a-z0-9_]+)\}`) } +// An InterpolatedVariable is a variable that is embedded within a string +// in the configuration, such as "hello ${world}" (world in this case is +// an interpolated variable). +// +// These variables can come from a variety of sources, represented by +// implementations of this interface. +type InterpolatedVariable interface { + FullKey() string +} + +// A ResourceVariable is a variable that is referencing the field +// of a resource, such as "${aws_instance.foo.ami}" +type ResourceVariable struct { + Type string // Resource type, i.e. "aws_instance" + Name string // Resource name + Field string // Resource field + + Multi bool // True if multi-variable: aws_instance.foo.*.id + Index int // Index for multi-variable: aws_instance.foo.1.id == 1 + + key string +} + +func (v *ResourceVariable) ResourceId() string { + return fmt.Sprintf("%s.%s", v.Type, v.Name) +} + +func (v *ResourceVariable) FullKey() string { + return v.key +} + +// A UserVariable is a variable that is referencing a user variable +// that is inputted from outside the configuration. This looks like +// "${var.foo}" +type UserVariable struct { + Name string + + key string +} + +func (v *UserVariable) FullKey() string { + return v.key +} + +// A UserMapVariable is a variable that is referencing a user +// variable that is a map. This looks like "${var.amis.us-east-1}" +type UserMapVariable struct { + Name string + Elem string + + key string +} + +func NewUserMapVariable(key string) (*UserMapVariable, error) { + name := key[len("var."):] + idx := strings.Index(name, ".") + if idx == -1 { + return nil, fmt.Errorf("not a user map variable: %s", key) + } + + elem := name[idx+1:] + name = name[:idx] + return &UserMapVariable{ + Name: name, + Elem: elem, + + key: key, + }, nil +} + +func (v *UserMapVariable) FullKey() string { + return v.key +} + +func (v *UserMapVariable) GoString() string { + return fmt.Sprintf("%#v", *v) +} + +func NewResourceVariable(key string) (*ResourceVariable, error) { + parts := strings.SplitN(key, ".", 3) + field := parts[2] + multi := false + var index int + + if idx := strings.Index(field, "."); idx != -1 { + indexStr := field[:idx] + multi = indexStr == "*" + index = -1 + + if !multi { + indexInt, err := strconv.ParseInt(indexStr, 0, 0) + if err == nil { + multi = true + index = int(indexInt) + } + } + + if multi { + field = field[idx+1:] + } + } + + return &ResourceVariable{ + Type: parts[0], + Name: parts[1], + Field: field, + Multi: multi, + Index: index, + key: key, + }, nil +} + +func NewUserVariable(key string) (*UserVariable, error) { + name := key[len("var."):] + return &UserVariable{ + key: key, + Name: name, + }, nil +} + // ReplaceVariables takes a configuration and a mapping of variables // and performs the structure walking necessary to properly replace // all the variables. @@ -80,10 +201,15 @@ func (w *variableDetectWalker) Primitive(v reflect.Value) error { key, key) } - if strings.HasPrefix(key, "var.") { - iv, err = NewUserVariable(key) - } else { + if !strings.HasPrefix(key, "var.") { iv, err = NewResourceVariable(key) + } else { + varKey := key[len("var."):] + if strings.Index(varKey, ".") == -1 { + iv, err = NewUserVariable(key) + } else { + iv, err = NewUserMapVariable(key) + } } if err != nil { diff --git a/config/variable_test.go b/config/variable_test.go index 7e3b5fc11..1dacc4f41 100644 --- a/config/variable_test.go +++ b/config/variable_test.go @@ -36,6 +36,44 @@ func BenchmarkVariableReplaceWalker(b *testing.B) { } } +func TestNewResourceVariable(t *testing.T) { + v, err := NewResourceVariable("foo.bar.baz") + if err != nil { + t.Fatalf("err: %s", err) + } + + if v.Type != "foo" { + t.Fatalf("bad: %#v", v) + } + if v.Name != "bar" { + t.Fatalf("bad: %#v", v) + } + if v.Field != "baz" { + t.Fatalf("bad: %#v", v) + } + if v.Multi { + t.Fatal("should not be multi") + } + + if v.FullKey() != "foo.bar.baz" { + t.Fatalf("bad: %#v", v) + } +} + +func TestNewUserVariable(t *testing.T) { + v, err := NewUserVariable("var.bar") + if err != nil { + t.Fatalf("err: %s", err) + } + + if v.Name != "bar" { + t.Fatalf("bad: %#v", v.Name) + } + if v.FullKey() != "var.bar" { + t.Fatalf("bad: %#v", v) + } +} + func TestReplaceVariables(t *testing.T) { input := "foo-${var.bar}" expected := "foo-bar" @@ -55,6 +93,66 @@ func TestReplaceVariables(t *testing.T) { } } +func TestResourceVariable_impl(t *testing.T) { + var _ InterpolatedVariable = new(ResourceVariable) +} + +func TestResourceVariable_Multi(t *testing.T) { + v, err := NewResourceVariable("foo.bar.*.baz") + if err != nil { + t.Fatalf("err: %s", err) + } + + if v.Type != "foo" { + t.Fatalf("bad: %#v", v) + } + if v.Name != "bar" { + t.Fatalf("bad: %#v", v) + } + if v.Field != "baz" { + t.Fatalf("bad: %#v", v) + } + if !v.Multi { + t.Fatal("should be multi") + } +} + +func TestResourceVariable_MultiIndex(t *testing.T) { + cases := []struct { + Input string + Index int + Field string + }{ + {"foo.bar.*.baz", -1, "baz"}, + {"foo.bar.0.baz", 0, "baz"}, + {"foo.bar.5.baz", 5, "baz"}, + } + + for _, tc := range cases { + v, err := NewResourceVariable(tc.Input) + if err != nil { + t.Fatalf("err: %s", err) + } + if !v.Multi { + t.Fatalf("should be multi: %s", tc.Input) + } + if v.Index != tc.Index { + t.Fatalf("bad: %d\n\n%s", v.Index, tc.Input) + } + if v.Field != tc.Field { + t.Fatalf("bad: %s\n\n%s", v.Field, tc.Input) + } + } +} + +func TestUserVariable_impl(t *testing.T) { + var _ InterpolatedVariable = new(UserVariable) +} + +func TestUserMapVariable_impl(t *testing.T) { + var _ InterpolatedVariable = new(UserMapVariable) +} + func TestVariableDetectWalker(t *testing.T) { w := new(variableDetectWalker) @@ -138,6 +236,30 @@ func TestVariableDetectWalker_empty(t *testing.T) { } } +func TestVariableDetectWalker_userMap(t *testing.T) { + w := new(variableDetectWalker) + + str := `foo ${var.foo.bar}` + if err := w.Primitive(reflect.ValueOf(str)); err != nil { + t.Fatalf("err: %s", err) + } + + if len(w.Variables) != 1 { + t.Fatalf("bad: %#v", w.Variables) + } + + v := w.Variables["var.foo.bar"].(*UserMapVariable) + if v.FullKey() != "var.foo.bar" { + t.Fatalf("bad: %#v", w.Variables) + } + if v.Name != "foo" { + t.Fatalf("bad: %#v", w.Variables) + } + if v.Elem != "bar" { + t.Fatalf("bad: %#v", w.Variables) + } +} + func TestVariableReplaceWalker(t *testing.T) { w := &variableReplaceWalker{ Values: map[string]string{ From 582b0cf43e5d49dbc3551590a453281125f1788e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 10:39:55 -0700 Subject: [PATCH 04/25] config: introduce Interpolation, not hooked up completely yet --- config/interpolate.go | 157 +++++++++++++++++++++++++++++++++++++ config/interpolate_test.go | 136 ++++++++++++++++++++++++++++++++ config/variable.go | 122 +--------------------------- config/variable_test.go | 98 ----------------------- 4 files changed, 294 insertions(+), 219 deletions(-) create mode 100644 config/interpolate.go create mode 100644 config/interpolate_test.go diff --git a/config/interpolate.go b/config/interpolate.go new file mode 100644 index 000000000..7c0a9812c --- /dev/null +++ b/config/interpolate.go @@ -0,0 +1,157 @@ +package config + +import ( + "fmt" + "strconv" + "strings" +) + +// Interpolation is something that can be contained in a "${}" in a +// configuration value. +// +// Interpolations might be simple variable references, or it might be +// function calls, or even nested function calls. +type Interpolation interface { + FullString() string + Interpolate(map[string]string) (string, error) + Variables() map[string]InterpolatedVariable +} + +// An InterpolatedVariable is a variable reference within an interpolation. +// +// Implementations of this interface represents various sources where +// variables can come from: user variables, resources, etc. +type InterpolatedVariable interface { + FullKey() string +} + +// VariableInterpolation implements Interpolation for simple variable +// interpolation. Ex: "${var.foo}" or "${aws_instance.foo.bar}" +type VariableInterpolation struct { + Variable InterpolatedVariable + + key string +} + +func (i *VariableInterpolation) FullString() string { + return i.key +} + +func (i *VariableInterpolation) Interpolate( + vs map[string]string) (string, error) { + return vs[i.key], nil +} + +func (i *VariableInterpolation) Variables() map[string]InterpolatedVariable { + return map[string]InterpolatedVariable{i.key: i.Variable} +} + +// A ResourceVariable is a variable that is referencing the field +// of a resource, such as "${aws_instance.foo.ami}" +type ResourceVariable struct { + Type string // Resource type, i.e. "aws_instance" + Name string // Resource name + Field string // Resource field + + Multi bool // True if multi-variable: aws_instance.foo.*.id + Index int // Index for multi-variable: aws_instance.foo.1.id == 1 + + key string +} + +func NewResourceVariable(key string) (*ResourceVariable, error) { + parts := strings.SplitN(key, ".", 3) + field := parts[2] + multi := false + var index int + + if idx := strings.Index(field, "."); idx != -1 { + indexStr := field[:idx] + multi = indexStr == "*" + index = -1 + + if !multi { + indexInt, err := strconv.ParseInt(indexStr, 0, 0) + if err == nil { + multi = true + index = int(indexInt) + } + } + + if multi { + field = field[idx+1:] + } + } + + return &ResourceVariable{ + Type: parts[0], + Name: parts[1], + Field: field, + Multi: multi, + Index: index, + key: key, + }, nil +} + +func (v *ResourceVariable) ResourceId() string { + return fmt.Sprintf("%s.%s", v.Type, v.Name) +} + +func (v *ResourceVariable) FullKey() string { + return v.key +} + +// A UserVariable is a variable that is referencing a user variable +// that is inputted from outside the configuration. This looks like +// "${var.foo}" +type UserVariable struct { + Name string + + key string +} + +func NewUserVariable(key string) (*UserVariable, error) { + name := key[len("var."):] + return &UserVariable{ + key: key, + Name: name, + }, nil +} + +func (v *UserVariable) FullKey() string { + return v.key +} + +// A UserMapVariable is a variable that is referencing a user +// variable that is a map. This looks like "${var.amis.us-east-1}" +type UserMapVariable struct { + Name string + Elem string + + key string +} + +func NewUserMapVariable(key string) (*UserMapVariable, error) { + name := key[len("var."):] + idx := strings.Index(name, ".") + if idx == -1 { + return nil, fmt.Errorf("not a user map variable: %s", key) + } + + elem := name[idx+1:] + name = name[:idx] + return &UserMapVariable{ + Name: name, + Elem: elem, + + key: key, + }, nil +} + +func (v *UserMapVariable) FullKey() string { + return v.key +} + +func (v *UserMapVariable) GoString() string { + return fmt.Sprintf("%#v", *v) +} diff --git a/config/interpolate_test.go b/config/interpolate_test.go new file mode 100644 index 000000000..5f77f64b7 --- /dev/null +++ b/config/interpolate_test.go @@ -0,0 +1,136 @@ +package config + +import ( + "reflect" + "testing" +) + +func TestNewResourceVariable(t *testing.T) { + v, err := NewResourceVariable("foo.bar.baz") + if err != nil { + t.Fatalf("err: %s", err) + } + + if v.Type != "foo" { + t.Fatalf("bad: %#v", v) + } + if v.Name != "bar" { + t.Fatalf("bad: %#v", v) + } + if v.Field != "baz" { + t.Fatalf("bad: %#v", v) + } + if v.Multi { + t.Fatal("should not be multi") + } + + if v.FullKey() != "foo.bar.baz" { + t.Fatalf("bad: %#v", v) + } +} + +func TestNewUserVariable(t *testing.T) { + v, err := NewUserVariable("var.bar") + if err != nil { + t.Fatalf("err: %s", err) + } + + if v.Name != "bar" { + t.Fatalf("bad: %#v", v.Name) + } + if v.FullKey() != "var.bar" { + t.Fatalf("bad: %#v", v) + } +} + +func TestResourceVariable_impl(t *testing.T) { + var _ InterpolatedVariable = new(ResourceVariable) +} + +func TestResourceVariable_Multi(t *testing.T) { + v, err := NewResourceVariable("foo.bar.*.baz") + if err != nil { + t.Fatalf("err: %s", err) + } + + if v.Type != "foo" { + t.Fatalf("bad: %#v", v) + } + if v.Name != "bar" { + t.Fatalf("bad: %#v", v) + } + if v.Field != "baz" { + t.Fatalf("bad: %#v", v) + } + if !v.Multi { + t.Fatal("should be multi") + } +} + +func TestResourceVariable_MultiIndex(t *testing.T) { + cases := []struct { + Input string + Index int + Field string + }{ + {"foo.bar.*.baz", -1, "baz"}, + {"foo.bar.0.baz", 0, "baz"}, + {"foo.bar.5.baz", 5, "baz"}, + } + + for _, tc := range cases { + v, err := NewResourceVariable(tc.Input) + if err != nil { + t.Fatalf("err: %s", err) + } + if !v.Multi { + t.Fatalf("should be multi: %s", tc.Input) + } + if v.Index != tc.Index { + t.Fatalf("bad: %d\n\n%s", v.Index, tc.Input) + } + if v.Field != tc.Field { + t.Fatalf("bad: %s\n\n%s", v.Field, tc.Input) + } + } +} + +func TestUserVariable_impl(t *testing.T) { + var _ InterpolatedVariable = new(UserVariable) +} + +func TestUserMapVariable_impl(t *testing.T) { + var _ InterpolatedVariable = new(UserMapVariable) +} + +func TestVariableInterpolation_impl(t *testing.T) { + var _ Interpolation = new(VariableInterpolation) +} + +func TestVariableInterpolation(t *testing.T) { + uv, err := NewUserVariable("var.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + i := &VariableInterpolation{Variable: uv, key: "var.foo"} + if i.FullString() != "var.foo" { + t.Fatalf("err: %#v", i) + } + + expected := map[string]InterpolatedVariable{"var.foo": uv} + if !reflect.DeepEqual(i.Variables(), expected) { + t.Fatalf("bad: %#v", i.Variables()) + } + + actual, err := i.Interpolate(map[string]string{ + "var.foo": "bar", + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + if actual != "bar" { + t.Fatalf("bad: %#v", actual) + } +} diff --git a/config/variable.go b/config/variable.go index 96b49f9e6..d57463ae8 100644 --- a/config/variable.go +++ b/config/variable.go @@ -4,7 +4,6 @@ import ( "fmt" "reflect" "regexp" - "strconv" "strings" "github.com/mitchellh/reflectwalk" @@ -17,126 +16,6 @@ func init() { varRegexp = regexp.MustCompile(`(?i)(\$+)\{([*-.a-z0-9_]+)\}`) } -// An InterpolatedVariable is a variable that is embedded within a string -// in the configuration, such as "hello ${world}" (world in this case is -// an interpolated variable). -// -// These variables can come from a variety of sources, represented by -// implementations of this interface. -type InterpolatedVariable interface { - FullKey() string -} - -// A ResourceVariable is a variable that is referencing the field -// of a resource, such as "${aws_instance.foo.ami}" -type ResourceVariable struct { - Type string // Resource type, i.e. "aws_instance" - Name string // Resource name - Field string // Resource field - - Multi bool // True if multi-variable: aws_instance.foo.*.id - Index int // Index for multi-variable: aws_instance.foo.1.id == 1 - - key string -} - -func (v *ResourceVariable) ResourceId() string { - return fmt.Sprintf("%s.%s", v.Type, v.Name) -} - -func (v *ResourceVariable) FullKey() string { - return v.key -} - -// A UserVariable is a variable that is referencing a user variable -// that is inputted from outside the configuration. This looks like -// "${var.foo}" -type UserVariable struct { - Name string - - key string -} - -func (v *UserVariable) FullKey() string { - return v.key -} - -// A UserMapVariable is a variable that is referencing a user -// variable that is a map. This looks like "${var.amis.us-east-1}" -type UserMapVariable struct { - Name string - Elem string - - key string -} - -func NewUserMapVariable(key string) (*UserMapVariable, error) { - name := key[len("var."):] - idx := strings.Index(name, ".") - if idx == -1 { - return nil, fmt.Errorf("not a user map variable: %s", key) - } - - elem := name[idx+1:] - name = name[:idx] - return &UserMapVariable{ - Name: name, - Elem: elem, - - key: key, - }, nil -} - -func (v *UserMapVariable) FullKey() string { - return v.key -} - -func (v *UserMapVariable) GoString() string { - return fmt.Sprintf("%#v", *v) -} - -func NewResourceVariable(key string) (*ResourceVariable, error) { - parts := strings.SplitN(key, ".", 3) - field := parts[2] - multi := false - var index int - - if idx := strings.Index(field, "."); idx != -1 { - indexStr := field[:idx] - multi = indexStr == "*" - index = -1 - - if !multi { - indexInt, err := strconv.ParseInt(indexStr, 0, 0) - if err == nil { - multi = true - index = int(indexInt) - } - } - - if multi { - field = field[idx+1:] - } - } - - return &ResourceVariable{ - Type: parts[0], - Name: parts[1], - Field: field, - Multi: multi, - Index: index, - key: key, - }, nil -} - -func NewUserVariable(key string) (*UserVariable, error) { - name := key[len("var."):] - return &UserVariable{ - key: key, - Name: name, - }, nil -} - // ReplaceVariables takes a configuration and a mapping of variables // and performs the structure walking necessary to properly replace // all the variables. @@ -229,6 +108,7 @@ func (w *variableDetectWalker) Primitive(v reflect.Value) error { // will _panic_. The variableDetectWalker will tell you all variables // you need. type variableReplaceWalker struct { + Variables map[string]InterpolatedVariable Values map[string]string UnknownKeys []string diff --git a/config/variable_test.go b/config/variable_test.go index 1dacc4f41..803857880 100644 --- a/config/variable_test.go +++ b/config/variable_test.go @@ -36,44 +36,6 @@ func BenchmarkVariableReplaceWalker(b *testing.B) { } } -func TestNewResourceVariable(t *testing.T) { - v, err := NewResourceVariable("foo.bar.baz") - if err != nil { - t.Fatalf("err: %s", err) - } - - if v.Type != "foo" { - t.Fatalf("bad: %#v", v) - } - if v.Name != "bar" { - t.Fatalf("bad: %#v", v) - } - if v.Field != "baz" { - t.Fatalf("bad: %#v", v) - } - if v.Multi { - t.Fatal("should not be multi") - } - - if v.FullKey() != "foo.bar.baz" { - t.Fatalf("bad: %#v", v) - } -} - -func TestNewUserVariable(t *testing.T) { - v, err := NewUserVariable("var.bar") - if err != nil { - t.Fatalf("err: %s", err) - } - - if v.Name != "bar" { - t.Fatalf("bad: %#v", v.Name) - } - if v.FullKey() != "var.bar" { - t.Fatalf("bad: %#v", v) - } -} - func TestReplaceVariables(t *testing.T) { input := "foo-${var.bar}" expected := "foo-bar" @@ -93,66 +55,6 @@ func TestReplaceVariables(t *testing.T) { } } -func TestResourceVariable_impl(t *testing.T) { - var _ InterpolatedVariable = new(ResourceVariable) -} - -func TestResourceVariable_Multi(t *testing.T) { - v, err := NewResourceVariable("foo.bar.*.baz") - if err != nil { - t.Fatalf("err: %s", err) - } - - if v.Type != "foo" { - t.Fatalf("bad: %#v", v) - } - if v.Name != "bar" { - t.Fatalf("bad: %#v", v) - } - if v.Field != "baz" { - t.Fatalf("bad: %#v", v) - } - if !v.Multi { - t.Fatal("should be multi") - } -} - -func TestResourceVariable_MultiIndex(t *testing.T) { - cases := []struct { - Input string - Index int - Field string - }{ - {"foo.bar.*.baz", -1, "baz"}, - {"foo.bar.0.baz", 0, "baz"}, - {"foo.bar.5.baz", 5, "baz"}, - } - - for _, tc := range cases { - v, err := NewResourceVariable(tc.Input) - if err != nil { - t.Fatalf("err: %s", err) - } - if !v.Multi { - t.Fatalf("should be multi: %s", tc.Input) - } - if v.Index != tc.Index { - t.Fatalf("bad: %d\n\n%s", v.Index, tc.Input) - } - if v.Field != tc.Field { - t.Fatalf("bad: %s\n\n%s", v.Field, tc.Input) - } - } -} - -func TestUserVariable_impl(t *testing.T) { - var _ InterpolatedVariable = new(UserVariable) -} - -func TestUserMapVariable_impl(t *testing.T) { - var _ InterpolatedVariable = new(UserMapVariable) -} - func TestVariableDetectWalker(t *testing.T) { w := new(variableDetectWalker) From 4c9e0f395c05056ec222f09f5d324285dab7b115 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 11:24:44 -0700 Subject: [PATCH 05/25] config: basic interpolationWalker --- config/interpolate.go | 97 ++++++++++++++++++++--------- config/interpolate_walk.go | 107 ++++++++++++++++++++++++++++++++ config/interpolate_walk_test.go | 55 ++++++++++++++++ 3 files changed, 228 insertions(+), 31 deletions(-) create mode 100644 config/interpolate_walk.go create mode 100644 config/interpolate_walk_test.go diff --git a/config/interpolate.go b/config/interpolate.go index 7c0a9812c..dc4f39fd1 100644 --- a/config/interpolate.go +++ b/config/interpolate.go @@ -33,19 +33,6 @@ type VariableInterpolation struct { key string } -func (i *VariableInterpolation) FullString() string { - return i.key -} - -func (i *VariableInterpolation) Interpolate( - vs map[string]string) (string, error) { - return vs[i.key], nil -} - -func (i *VariableInterpolation) Variables() map[string]InterpolatedVariable { - return map[string]InterpolatedVariable{i.key: i.Variable} -} - // A ResourceVariable is a variable that is referencing the field // of a resource, such as "${aws_instance.foo.ami}" type ResourceVariable struct { @@ -59,6 +46,72 @@ type ResourceVariable struct { key string } +// A UserVariable is a variable that is referencing a user variable +// that is inputted from outside the configuration. This looks like +// "${var.foo}" +type UserVariable struct { + Name string + + key string +} + +// A UserMapVariable is a variable that is referencing a user +// variable that is a map. This looks like "${var.amis.us-east-1}" +type UserMapVariable struct { + Name string + Elem string + + key string +} + +// NewInterpolation takes some string and returns the valid +// Interpolation associated with it, or error if a valid +// interpolation could not be found or the interpolation itself +// is invalid. +func NewInterpolation(v string) (Interpolation, error) { + if idx := strings.Index(v, "."); idx >= 0 { + v, err := NewInterpolatedVariable(v) + if err != nil { + return nil, err + } + + return &VariableInterpolation{ + Variable: v, + key: v.FullKey(), + }, nil + } + + return nil, fmt.Errorf( + "Interpolation '%s' is not a valid interpolation. " + + "Please check your syntax and try again.") +} + +func NewInterpolatedVariable(v string) (InterpolatedVariable, error) { + if !strings.HasPrefix(v, "var.") { + return NewResourceVariable(v) + } + + varKey := v[len("var."):] + if strings.Index(varKey, ".") == -1 { + return NewUserVariable(v) + } else { + return NewUserMapVariable(v) + } +} + +func (i *VariableInterpolation) FullString() string { + return i.key +} + +func (i *VariableInterpolation) Interpolate( + vs map[string]string) (string, error) { + return vs[i.key], nil +} + +func (i *VariableInterpolation) Variables() map[string]InterpolatedVariable { + return map[string]InterpolatedVariable{i.key: i.Variable} +} + func NewResourceVariable(key string) (*ResourceVariable, error) { parts := strings.SplitN(key, ".", 3) field := parts[2] @@ -101,15 +154,6 @@ func (v *ResourceVariable) FullKey() string { return v.key } -// A UserVariable is a variable that is referencing a user variable -// that is inputted from outside the configuration. This looks like -// "${var.foo}" -type UserVariable struct { - Name string - - key string -} - func NewUserVariable(key string) (*UserVariable, error) { name := key[len("var."):] return &UserVariable{ @@ -122,15 +166,6 @@ func (v *UserVariable) FullKey() string { return v.key } -// A UserMapVariable is a variable that is referencing a user -// variable that is a map. This looks like "${var.amis.us-east-1}" -type UserMapVariable struct { - Name string - Elem string - - key string -} - func NewUserMapVariable(key string) (*UserMapVariable, error) { name := key[len("var."):] idx := strings.Index(name, ".") diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go new file mode 100644 index 000000000..15bc84586 --- /dev/null +++ b/config/interpolate_walk.go @@ -0,0 +1,107 @@ +package config + +import ( + "reflect" + "regexp" + + "github.com/mitchellh/reflectwalk" +) + +// interpRegexp is a regexp that matches interpolations such as ${foo.bar} +var interpRegexp *regexp.Regexp = regexp.MustCompile( + `(?i)(\$+)\{([*-.a-z0-9_]+)\}`) + +// interpolationWalker implements interfaces for the reflectwalk package +// (github.com/mitchellh/reflectwalk) that can be used to automatically +// execute a callback for an interpolation. +type interpolationWalker struct { + // F must be one of interpolationWalkerFunc or + // interpolationReplaceWalkerFunc. + F interpolationWalkerFunc + Replace bool + + key []string + loc reflectwalk.Location + cs []reflect.Value + csData interface{} +} + +type interpolationWalkerFunc func(Interpolation) (string, error) + +func (w *interpolationWalker) Enter(loc reflectwalk.Location) error { + w.loc = loc + return nil +} + +func (w *interpolationWalker) Exit(loc reflectwalk.Location) error { + w.loc = reflectwalk.None + + switch loc { + case reflectwalk.Map: + w.cs = w.cs[:len(w.cs)-1] + case reflectwalk.MapValue: + w.key = w.key[:len(w.key)-1] + } + + return nil +} + +func (w *interpolationWalker) Map(m reflect.Value) error { + w.cs = append(w.cs, m) + return nil +} + +func (w *interpolationWalker) MapElem(m, k, v reflect.Value) error { + w.csData = k + w.key = append(w.key, k.String()) + return nil +} + +func (w *interpolationWalker) Primitive(v reflect.Value) error { + // We only care about strings + if v.Kind() == reflect.Interface { + v = v.Elem() + } + if v.Kind() != reflect.String { + return nil + } + + // XXX: This can be a lot more efficient if we used a real + // parser. A regexp is a hammer though that will get this working. + + matches := interpRegexp.FindAllStringSubmatch(v.String(), -1) + if len(matches) == 0 { + return nil + } + + for _, match := range matches { + dollars := len(match[1]) + + // If there are even amounts of dollar signs, then it is escaped + if dollars%2 == 0 { + continue + } + + // Interpolation found, instantiate it + key := match[2] + + i, err := NewInterpolation(key) + if err != nil { + return err + } + + replaceVal, err := w.F(i) + if err != nil { + return err + } + + if w.Replace { + // TODO(mitchellh): replace + println(replaceVal) + } + + return nil + } + + return nil +} diff --git a/config/interpolate_walk_test.go b/config/interpolate_walk_test.go new file mode 100644 index 000000000..15fa29d50 --- /dev/null +++ b/config/interpolate_walk_test.go @@ -0,0 +1,55 @@ +package config + +import ( + "reflect" + "testing" + + "github.com/mitchellh/reflectwalk" +) + +func TestInterpolationWalker_detect(t *testing.T) { + cases := []struct { + Input interface{} + Result []Interpolation + }{ + { + Input: map[string]interface{}{ + "foo": "$${var.foo}", + }, + Result: nil, + }, + + { + Input: map[string]interface{}{ + "foo": "${var.foo}", + }, + Result: []Interpolation{ + &VariableInterpolation{ + Variable: &UserVariable{ + Name: "foo", + key: "var.foo", + }, + key: "var.foo", + }, + }, + }, + } + + for i, tc := range cases { + var actual []Interpolation + + detectFn := func(i Interpolation) (string, error) { + actual = append(actual, i) + return "", nil + } + + w := &interpolationWalker{F: detectFn} + if err := reflectwalk.Walk(tc.Input, w); err != nil { + t.Fatalf("err: %s", err) + } + + if !reflect.DeepEqual(actual, tc.Result) { + t.Fatalf("%d: bad:\n\n%#v", i, actual) + } + } +} From e8fe26488a07f21b9551337b0c631205415594df Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 11:30:43 -0700 Subject: [PATCH 06/25] config: interpolationWalk seems to work --- config/interpolate_walk.go | 30 ++++++++++++++++++++----- config/interpolate_walk_test.go | 40 +++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go index 15bc84586..b4ee9ab98 100644 --- a/config/interpolate_walk.go +++ b/config/interpolate_walk.go @@ -3,6 +3,7 @@ package config import ( "reflect" "regexp" + "strings" "github.com/mitchellh/reflectwalk" ) @@ -15,8 +16,6 @@ var interpRegexp *regexp.Regexp = regexp.MustCompile( // (github.com/mitchellh/reflectwalk) that can be used to automatically // execute a callback for an interpolation. type interpolationWalker struct { - // F must be one of interpolationWalkerFunc or - // interpolationReplaceWalkerFunc. F interpolationWalkerFunc Replace bool @@ -26,6 +25,12 @@ type interpolationWalker struct { csData interface{} } +// interpolationWalkerFunc is the callback called by interpolationWalk. +// It is called with any interpolation found. It should return a value +// to replace the interpolation with, along with any errors. +// +// If Replace is set to false in interpolationWalker, then the replace +// value can be anything as it will have no effect. type interpolationWalkerFunc func(Interpolation) (string, error) func (w *interpolationWalker) Enter(loc reflectwalk.Location) error { @@ -58,8 +63,11 @@ func (w *interpolationWalker) MapElem(m, k, v reflect.Value) error { } func (w *interpolationWalker) Primitive(v reflect.Value) error { + setV := v + // We only care about strings if v.Kind() == reflect.Interface { + setV = v v = v.Elem() } if v.Kind() != reflect.String { @@ -74,6 +82,7 @@ func (w *interpolationWalker) Primitive(v reflect.Value) error { return nil } + result := v.String() for _, match := range matches { dollars := len(match[1]) @@ -96,11 +105,22 @@ func (w *interpolationWalker) Primitive(v reflect.Value) error { } if w.Replace { - // TODO(mitchellh): replace - println(replaceVal) + result = strings.Replace(result, match[0], replaceVal, -1) } + } - return nil + if w.Replace { + resultVal := reflect.ValueOf(result) + if w.loc == reflectwalk.MapValue { + // If we're in a map, then the only way to set a map value is + // to set it directly. + m := w.cs[len(w.cs)-1] + mk := w.csData.(reflect.Value) + m.SetMapIndex(mk, resultVal) + } else { + // Otherwise, we should be addressable + setV.Set(resultVal) + } } return nil diff --git a/config/interpolate_walk_test.go b/config/interpolate_walk_test.go index 15fa29d50..77249d71d 100644 --- a/config/interpolate_walk_test.go +++ b/config/interpolate_walk_test.go @@ -53,3 +53,43 @@ func TestInterpolationWalker_detect(t *testing.T) { } } } + +func TestInterpolationWalker_replace(t *testing.T) { + cases := []struct { + Input interface{} + Output interface{} + }{ + { + Input: map[string]interface{}{ + "foo": "$${var.foo}", + }, + Output: map[string]interface{}{ + "foo": "$${var.foo}", + }, + }, + + { + Input: map[string]interface{}{ + "foo": "${var.foo}", + }, + Output: map[string]interface{}{ + "foo": "bar", + }, + }, + } + + for i, tc := range cases { + fn := func(i Interpolation) (string, error) { + return "bar", nil + } + + w := &interpolationWalker{F: fn, Replace: true} + if err := reflectwalk.Walk(tc.Input, w); err != nil { + t.Fatalf("err: %s", err) + } + + if !reflect.DeepEqual(tc.Input, tc.Output) { + t.Fatalf("%d: bad:\n\n%#v", i, tc.Input) + } + } +} From 4099c6483300856ff17356ce255e57c63e8be2e5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 11:36:21 -0700 Subject: [PATCH 07/25] config: tests, so many tests --- config/interpolate_test.go | 63 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/config/interpolate_test.go b/config/interpolate_test.go index 5f77f64b7..3e399bf65 100644 --- a/config/interpolate_test.go +++ b/config/interpolate_test.go @@ -5,6 +5,69 @@ import ( "testing" ) +func TestNewInterpolation(t *testing.T) { + cases := []struct { + Input string + Result Interpolation + Error bool + }{ + { + "foo", + nil, + true, + }, + + { + "var.foo", + &VariableInterpolation{ + Variable: &UserVariable{ + Name: "foo", + key: "var.foo", + }, + key: "var.foo", + }, + false, + }, + } + + for i, tc := range cases { + actual, err := NewInterpolation(tc.Input) + if (err != nil) != tc.Error { + t.Fatalf("%d. Error: %s", i, err) + } + if !reflect.DeepEqual(actual, tc.Result) { + t.Fatalf("%d bad: %#v", i, actual) + } + } +} + +func TestNewInterpolatedVariable(t *testing.T) { + cases := []struct { + Input string + Result InterpolatedVariable + Error bool + }{ + { + "var.foo", + &UserVariable{ + Name: "foo", + key: "var.foo", + }, + false, + }, + } + + for i, tc := range cases { + actual, err := NewInterpolatedVariable(tc.Input) + if (err != nil) != tc.Error { + t.Fatalf("%d. Error: %s", i, err) + } + if !reflect.DeepEqual(actual, tc.Result) { + t.Fatalf("%d bad: %#v", i, actual) + } + } +} + func TestNewResourceVariable(t *testing.T) { v, err := NewResourceVariable("foo.bar.baz") if err != nil { From cabc007ec45f1b49218a4cd4b0d2f807293589fe Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 11:45:56 -0700 Subject: [PATCH 08/25] config: get rid of the variable*Walkers, replace with more generic --- config/interpolate_walk.go | 32 +++- config/interpolate_walk_test.go | 4 +- config/raw_config.go | 29 ++- config/variable.go | 222 ----------------------- config/variable_test.go | 306 -------------------------------- 5 files changed, 53 insertions(+), 540 deletions(-) delete mode 100644 config/variable.go delete mode 100644 config/variable_test.go diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go index b4ee9ab98..90b98e1ec 100644 --- a/config/interpolate_walk.go +++ b/config/interpolate_walk.go @@ -19,10 +19,11 @@ type interpolationWalker struct { F interpolationWalkerFunc Replace bool - key []string - loc reflectwalk.Location - cs []reflect.Value - csData interface{} + key []string + loc reflectwalk.Location + cs []reflect.Value + csData interface{} + unknownKeys []string } // interpolationWalkerFunc is the callback called by interpolationWalk. @@ -105,6 +106,13 @@ func (w *interpolationWalker) Primitive(v reflect.Value) error { } if w.Replace { + // If this is an unknown variable, then we remove it from + // the configuration. + if replaceVal == UnknownVariableValue { + w.removeCurrent() + return nil + } + result = strings.Replace(result, match[0], replaceVal, -1) } } @@ -125,3 +133,19 @@ func (w *interpolationWalker) Primitive(v reflect.Value) error { return nil } + +func (w *interpolationWalker) removeCurrent() { + c := w.cs[len(w.cs)-1] + switch c.Kind() { + case reflect.Map: + // Zero value so that we delete the map key + var val reflect.Value + + // Get the key and delete it + k := w.csData.(reflect.Value) + c.SetMapIndex(k, val) + } + + // Append the key to the unknown keys + w.unknownKeys = append(w.unknownKeys, strings.Join(w.key, ".")) +} diff --git a/config/interpolate_walk_test.go b/config/interpolate_walk_test.go index 77249d71d..255e0de5b 100644 --- a/config/interpolate_walk_test.go +++ b/config/interpolate_walk_test.go @@ -70,10 +70,10 @@ func TestInterpolationWalker_replace(t *testing.T) { { Input: map[string]interface{}{ - "foo": "${var.foo}", + "foo": "hello, ${var.foo}", }, Output: map[string]interface{}{ - "foo": "bar", + "foo": "hello, bar", }, }, } diff --git a/config/raw_config.go b/config/raw_config.go index f16921fd1..c97b29c52 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -69,26 +69,43 @@ func (r *RawConfig) Interpolate(vs map[string]string) error { if err != nil { return err } - - w := &variableReplaceWalker{Values: vs} r.config = config.(map[string]interface{}) + + fn := func(i Interpolation) (string, error) { + return i.Interpolate(vs) + } + + w := &interpolationWalker{F: fn, Replace: true} err = reflectwalk.Walk(r.config, w) if err != nil { return err } - r.unknownKeys = w.UnknownKeys + r.unknownKeys = w.unknownKeys return nil } func (r *RawConfig) init() error { - walker := new(variableDetectWalker) + r.config = r.Raw + r.Variables = nil + + fn := func(i Interpolation) (string, error) { + for k, v := range i.Variables() { + if r.Variables == nil { + r.Variables = make(map[string]InterpolatedVariable) + } + + r.Variables[k] = v + } + + return "", nil + } + + walker := &interpolationWalker{F: fn} if err := reflectwalk.Walk(r.Raw, walker); err != nil { return err } - r.Variables = walker.Variables - r.config = r.Raw return nil } diff --git a/config/variable.go b/config/variable.go deleted file mode 100644 index d57463ae8..000000000 --- a/config/variable.go +++ /dev/null @@ -1,222 +0,0 @@ -package config - -import ( - "fmt" - "reflect" - "regexp" - "strings" - - "github.com/mitchellh/reflectwalk" -) - -// varRegexp is a regexp that matches variables such as ${foo.bar} -var varRegexp *regexp.Regexp - -func init() { - varRegexp = regexp.MustCompile(`(?i)(\$+)\{([*-.a-z0-9_]+)\}`) -} - -// ReplaceVariables takes a configuration and a mapping of variables -// and performs the structure walking necessary to properly replace -// all the variables. -func ReplaceVariables( - c interface{}, - vs map[string]string) ([]string, error) { - w := &variableReplaceWalker{Values: vs} - if err := reflectwalk.Walk(c, w); err != nil { - return nil, err - } - - return w.UnknownKeys, nil -} - -// variableDetectWalker implements interfaces for the reflectwalk package -// (github.com/mitchellh/reflectwalk) that can be used to automatically -// pull out the variables that need replacing. -type variableDetectWalker struct { - Variables map[string]InterpolatedVariable -} - -func (w *variableDetectWalker) Primitive(v reflect.Value) error { - // We only care about strings - if v.Kind() == reflect.Interface { - v = v.Elem() - } - if v.Kind() != reflect.String { - return nil - } - - // XXX: This can be a lot more efficient if we used a real - // parser. A regexp is a hammer though that will get this working. - - matches := varRegexp.FindAllStringSubmatch(v.String(), -1) - if len(matches) == 0 { - return nil - } - - for _, match := range matches { - dollars := len(match[1]) - - // If there are even amounts of dollar signs, then it is escaped - if dollars%2 == 0 { - continue - } - - // Otherwise, record it - key := match[2] - if w.Variables == nil { - w.Variables = make(map[string]InterpolatedVariable) - } - if _, ok := w.Variables[key]; ok { - continue - } - - var err error - var iv InterpolatedVariable - if strings.Index(key, ".") == -1 { - return fmt.Errorf( - "Interpolated variable '%s' has bad format. "+ - "Did you mean 'var.%s'?", - key, key) - } - - if !strings.HasPrefix(key, "var.") { - iv, err = NewResourceVariable(key) - } else { - varKey := key[len("var."):] - if strings.Index(varKey, ".") == -1 { - iv, err = NewUserVariable(key) - } else { - iv, err = NewUserMapVariable(key) - } - } - - if err != nil { - return err - } - - w.Variables[key] = iv - } - - return nil -} - -// variableReplaceWalker implements interfaces for reflectwalk that -// is used to replace variables with their values. -// -// If Values does not have every available value, then the program -// will _panic_. The variableDetectWalker will tell you all variables -// you need. -type variableReplaceWalker struct { - Variables map[string]InterpolatedVariable - Values map[string]string - UnknownKeys []string - - key []string - loc reflectwalk.Location - cs []reflect.Value - csData interface{} -} - -func (w *variableReplaceWalker) Enter(loc reflectwalk.Location) error { - w.loc = loc - return nil -} - -func (w *variableReplaceWalker) Exit(loc reflectwalk.Location) error { - w.loc = reflectwalk.None - - switch loc { - case reflectwalk.Map: - w.cs = w.cs[:len(w.cs)-1] - case reflectwalk.MapValue: - w.key = w.key[:len(w.key)-1] - } - - return nil -} - -func (w *variableReplaceWalker) Map(m reflect.Value) error { - w.cs = append(w.cs, m) - return nil -} - -func (w *variableReplaceWalker) MapElem(m, k, v reflect.Value) error { - w.csData = k - w.key = append(w.key, k.String()) - return nil -} - -func (w *variableReplaceWalker) Primitive(v reflect.Value) error { - // We only care about strings - setV := v - if v.Kind() == reflect.Interface { - setV = v - v = v.Elem() - } - if v.Kind() != reflect.String { - return nil - } - - matches := varRegexp.FindAllStringSubmatch(v.String(), -1) - if len(matches) == 0 { - return nil - } - - result := v.String() - for _, match := range matches { - dollars := len(match[1]) - - // If there are even amounts of dollar signs, then it is escaped - if dollars%2 == 0 { - continue - } - - // Get the key - key := match[2] - value, ok := w.Values[key] - if !ok { - panic("no value for variable key: " + key) - } - - // If this is an unknown variable, then we remove it from - // the configuration. - if value == UnknownVariableValue { - w.removeCurrent() - return nil - } - - // Replace - result = strings.Replace(result, match[0], value, -1) - } - - resultVal := reflect.ValueOf(result) - if w.loc == reflectwalk.MapValue { - // If we're in a map, then the only way to set a map value is - // to set it directly. - m := w.cs[len(w.cs)-1] - mk := w.csData.(reflect.Value) - m.SetMapIndex(mk, resultVal) - } else { - // Otherwise, we should be addressable - setV.Set(resultVal) - } - - return nil -} - -func (w *variableReplaceWalker) removeCurrent() { - c := w.cs[len(w.cs)-1] - switch c.Kind() { - case reflect.Map: - // Zero value so that we delete the map key - var val reflect.Value - - // Get the key and delete it - k := w.csData.(reflect.Value) - c.SetMapIndex(k, val) - } - - // Append the key to the unknown keys - w.UnknownKeys = append(w.UnknownKeys, strings.Join(w.key, ".")) -} diff --git a/config/variable_test.go b/config/variable_test.go deleted file mode 100644 index 803857880..000000000 --- a/config/variable_test.go +++ /dev/null @@ -1,306 +0,0 @@ -package config - -import ( - "reflect" - "testing" - - "github.com/mitchellh/reflectwalk" -) - -func BenchmarkVariableDetectWalker(b *testing.B) { - w := new(variableDetectWalker) - str := reflect.ValueOf(`foo ${var.bar} bar ${bar.baz.bing} $${escaped}`) - - b.ResetTimer() - for i := 0; i < b.N; i++ { - w.Variables = nil - w.Primitive(str) - } -} - -func BenchmarkVariableReplaceWalker(b *testing.B) { - w := &variableReplaceWalker{ - Values: map[string]string{ - "var.bar": "bar", - "bar.baz.bing": "baz", - }, - } - - str := `foo ${var.bar} bar ${bar.baz.bing} $${escaped}` - - b.ResetTimer() - for i := 0; i < b.N; i++ { - if err := reflectwalk.Walk(&str, w); err != nil { - panic(err) - } - } -} - -func TestReplaceVariables(t *testing.T) { - input := "foo-${var.bar}" - expected := "foo-bar" - - unk, err := ReplaceVariables(&input, map[string]string{ - "var.bar": "bar", - }) - if err != nil { - t.Fatalf("err: %s", err) - } - if len(unk) > 0 { - t.Fatal("bad: %#v", unk) - } - - if input != expected { - t.Fatalf("bad: %#v", input) - } -} - -func TestVariableDetectWalker(t *testing.T) { - w := new(variableDetectWalker) - - str := `foo ${var.bar}` - if err := w.Primitive(reflect.ValueOf(str)); err != nil { - t.Fatalf("err: %s", err) - } - - if len(w.Variables) != 1 { - t.Fatalf("bad: %#v", w.Variables) - } - if w.Variables["var.bar"].(*UserVariable).FullKey() != "var.bar" { - t.Fatalf("bad: %#v", w.Variables) - } -} - -func TestVariableDetectWalker_resource(t *testing.T) { - w := new(variableDetectWalker) - - str := `foo ${ec2.foo.bar}` - if err := w.Primitive(reflect.ValueOf(str)); err != nil { - t.Fatalf("err: %s", err) - } - - if len(w.Variables) != 1 { - t.Fatalf("bad: %#v", w.Variables) - } - if w.Variables["ec2.foo.bar"].(*ResourceVariable).FullKey() != "ec2.foo.bar" { - t.Fatalf("bad: %#v", w.Variables) - } -} - -func TestVariableDetectWalker_resourceMulti(t *testing.T) { - w := new(variableDetectWalker) - - str := `foo ${ec2.foo.*.bar}` - if err := w.Primitive(reflect.ValueOf(str)); err != nil { - t.Fatalf("err: %s", err) - } - - if len(w.Variables) != 1 { - t.Fatalf("bad: %#v", w.Variables) - } - if w.Variables["ec2.foo.*.bar"].(*ResourceVariable).FullKey() != "ec2.foo.*.bar" { - t.Fatalf("bad: %#v", w.Variables) - } -} - -func TestVariableDetectWalker_bad(t *testing.T) { - w := new(variableDetectWalker) - - str := `foo ${bar}` - if err := w.Primitive(reflect.ValueOf(str)); err == nil { - t.Fatal("should error") - } -} - -func TestVariableDetectWalker_escaped(t *testing.T) { - w := new(variableDetectWalker) - - str := `foo $${var.bar}` - if err := w.Primitive(reflect.ValueOf(str)); err != nil { - t.Fatalf("err: %s", err) - } - - if len(w.Variables) > 0 { - t.Fatalf("bad: %#v", w.Variables) - } -} - -func TestVariableDetectWalker_empty(t *testing.T) { - w := new(variableDetectWalker) - - str := `foo` - if err := w.Primitive(reflect.ValueOf(str)); err != nil { - t.Fatalf("err: %s", err) - } - - if len(w.Variables) > 0 { - t.Fatalf("bad: %#v", w.Variables) - } -} - -func TestVariableDetectWalker_userMap(t *testing.T) { - w := new(variableDetectWalker) - - str := `foo ${var.foo.bar}` - if err := w.Primitive(reflect.ValueOf(str)); err != nil { - t.Fatalf("err: %s", err) - } - - if len(w.Variables) != 1 { - t.Fatalf("bad: %#v", w.Variables) - } - - v := w.Variables["var.foo.bar"].(*UserMapVariable) - if v.FullKey() != "var.foo.bar" { - t.Fatalf("bad: %#v", w.Variables) - } - if v.Name != "foo" { - t.Fatalf("bad: %#v", w.Variables) - } - if v.Elem != "bar" { - t.Fatalf("bad: %#v", w.Variables) - } -} - -func TestVariableReplaceWalker(t *testing.T) { - w := &variableReplaceWalker{ - Values: map[string]string{ - "var.bar": "bar", - "var.unknown": UnknownVariableValue, - }, - } - - cases := []struct { - Input interface{} - Output interface{} - }{ - { - `foo ${var.bar}`, - "foo bar", - }, - { - []string{"foo", "${var.bar}"}, - []string{"foo", "bar"}, - }, - { - map[string]interface{}{ - "ami": "${var.bar}", - "security_groups": []interface{}{ - "foo", - "${var.bar}", - }, - }, - map[string]interface{}{ - "ami": "bar", - "security_groups": []interface{}{ - "foo", - "bar", - }, - }, - }, - { - map[string]interface{}{ - "foo": map[string]interface{}{ - "foo": []string{"${var.bar}"}, - }, - }, - map[string]interface{}{ - "foo": map[string]interface{}{ - "foo": []string{"bar"}, - }, - }, - }, - { - map[string]interface{}{ - "foo": "bar", - "bar": "hello${var.unknown}world", - }, - map[string]interface{}{ - "foo": "bar", - }, - }, - { - map[string]interface{}{ - "foo": []string{"foo", "${var.unknown}", "bar"}, - }, - map[string]interface{}{}, - }, - } - - for i, tc := range cases { - var input interface{} = tc.Input - if reflect.ValueOf(tc.Input).Kind() == reflect.String { - input = &tc.Input - } - - if err := reflectwalk.Walk(input, w); err != nil { - t.Fatalf("err: %s", err) - } - - if !reflect.DeepEqual(tc.Input, tc.Output) { - t.Fatalf("bad %d: %#v", i, tc.Input) - } - } -} - -func TestVariableReplaceWalker_unknown(t *testing.T) { - cases := []struct { - Input interface{} - Output interface{} - Keys []string - }{ - { - map[string]interface{}{ - "foo": "bar", - "bar": "hello${var.unknown}world", - }, - map[string]interface{}{ - "foo": "bar", - }, - []string{"bar"}, - }, - { - map[string]interface{}{ - "foo": []string{"foo", "${var.unknown}", "bar"}, - }, - map[string]interface{}{}, - []string{"foo"}, - }, - { - map[string]interface{}{ - "foo": map[string]interface{}{ - "bar": "${var.unknown}", - }, - }, - map[string]interface{}{ - "foo": map[string]interface{}{}, - }, - []string{"foo.bar"}, - }, - } - - for i, tc := range cases { - var input interface{} = tc.Input - w := &variableReplaceWalker{ - Values: map[string]string{ - "var.unknown": UnknownVariableValue, - }, - } - - if reflect.ValueOf(tc.Input).Kind() == reflect.String { - input = &tc.Input - } - - if err := reflectwalk.Walk(input, w); err != nil { - t.Fatalf("err: %s", err) - } - - if !reflect.DeepEqual(tc.Input, tc.Output) { - t.Fatalf("bad %d: %#v", i, tc.Input) - } - - if !reflect.DeepEqual(tc.Keys, w.UnknownKeys) { - t.Fatalf("bad: %#v", w.UnknownKeys) - } - } -} From 6a191d73958af791fe259bca4c95013faaf80487 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 12:56:03 -0700 Subject: [PATCH 09/25] config: function calls work --- config/interpolate.go | 86 +++++++++++++++++++++++++++++++++++++ config/interpolate_funcs.go | 17 ++++++++ config/interpolate_test.go | 76 ++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+) create mode 100644 config/interpolate_funcs.go diff --git a/config/interpolate.go b/config/interpolate.go index dc4f39fd1..ab2cd26a4 100644 --- a/config/interpolate.go +++ b/config/interpolate.go @@ -2,10 +2,15 @@ package config import ( "fmt" + "regexp" "strconv" "strings" ) +// We really need to replace this with a real parser. +var funcRegexp *regexp.Regexp = regexp.MustCompile( + `(?i)([a-z0-9_]+)\(\s*(?:([.a-z0-9_]+)\s*,\s*)*([.a-z0-9_]+)\s*\)`) + // Interpolation is something that can be contained in a "${}" in a // configuration value. // @@ -17,6 +22,10 @@ type Interpolation interface { Variables() map[string]InterpolatedVariable } +// InterpolationFunc is the function signature for implementing +// callable functions in Terraform configurations. +type InterpolationFunc func(map[string]string, ...string) (string, error) + // An InterpolatedVariable is a variable reference within an interpolation. // // Implementations of this interface represents various sources where @@ -25,6 +34,15 @@ type InterpolatedVariable interface { FullKey() string } +// FunctionInterpolation is an Interpolation that executes a function +// with some variable number of arguments to generate a value. +type FunctionInterpolation struct { + Func InterpolationFunc + Args []InterpolatedVariable + + key string +} + // VariableInterpolation implements Interpolation for simple variable // interpolation. Ex: "${var.foo}" or "${aws_instance.foo.bar}" type VariableInterpolation struct { @@ -69,6 +87,33 @@ type UserMapVariable struct { // interpolation could not be found or the interpolation itself // is invalid. func NewInterpolation(v string) (Interpolation, error) { + match := funcRegexp.FindStringSubmatch(v) + if match != nil { + fn, ok := Funcs[match[1]] + if !ok { + return nil, fmt.Errorf( + "%s: Unknown function '%s'", + v, match[1]) + } + + args := make([]InterpolatedVariable, 0, len(match)-2) + for i := 2; i < len(match); i++ { + v, err := NewInterpolatedVariable(match[i]) + if err != nil { + return nil, err + } + + args = append(args, v) + } + + return &FunctionInterpolation{ + Func: fn, + Args: args, + + key: v, + }, nil + } + if idx := strings.Index(v, "."); idx >= 0 { v, err := NewInterpolatedVariable(v) if err != nil { @@ -99,6 +144,43 @@ func NewInterpolatedVariable(v string) (InterpolatedVariable, error) { } } +func (i *FunctionInterpolation) FullString() string { + return i.key +} + +func (i *FunctionInterpolation) Interpolate( + vs map[string]string) (string, error) { + args := make([]string, len(i.Args)) + for idx, a := range i.Args { + k := a.FullKey() + v, ok := vs[k] + if !ok { + return "", fmt.Errorf( + "%s: variable argument value unknown: %s", + i.FullString(), + k) + } + + args[idx] = v + } + + return i.Func(vs, args...) +} + +func (i *FunctionInterpolation) Variables() map[string]InterpolatedVariable { + result := make(map[string]InterpolatedVariable) + for _, a := range i.Args { + k := a.FullKey() + if _, ok := result[k]; ok { + continue + } + + result[k] = a + } + + return result +} + func (i *VariableInterpolation) FullString() string { return i.key } @@ -166,6 +248,10 @@ func (v *UserVariable) FullKey() string { return v.key } +func (v *UserVariable) GoString() string { + return fmt.Sprintf("*%#v", *v) +} + func NewUserMapVariable(key string) (*UserMapVariable, error) { name := key[len("var."):] idx := strings.Index(name, ".") diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go new file mode 100644 index 000000000..6e5ec98df --- /dev/null +++ b/config/interpolate_funcs.go @@ -0,0 +1,17 @@ +package config + +// Funcs is the mapping of built-in functions for configuration. +var Funcs map[string]InterpolationFunc + +func init() { + Funcs = map[string]InterpolationFunc{ + "lookup": interpolationFuncLookup, + } +} + +// interpolationFuncLookup implements the "lookup" function that allows +// dynamic lookups of map types within a Terraform configuration. +func interpolationFuncLookup( + vs map[string]string, args ...string) (string, error) { + return "", nil +} diff --git a/config/interpolate_test.go b/config/interpolate_test.go index 3e399bf65..87c2be0ca 100644 --- a/config/interpolate_test.go +++ b/config/interpolate_test.go @@ -2,6 +2,7 @@ package config import ( "reflect" + "strings" "testing" ) @@ -28,6 +29,25 @@ func TestNewInterpolation(t *testing.T) { }, false, }, + + { + "lookup(var.foo, var.bar)", + &FunctionInterpolation{ + Func: nil, // Funcs["lookup"] + Args: []InterpolatedVariable{ + &UserVariable{ + Name: "foo", + key: "var.foo", + }, + &UserVariable{ + Name: "bar", + key: "var.bar", + }, + }, + key: "lookup(var.foo, var.bar)", + }, + false, + }, } for i, tc := range cases { @@ -35,6 +55,13 @@ func TestNewInterpolation(t *testing.T) { if (err != nil) != tc.Error { t.Fatalf("%d. Error: %s", i, err) } + + // This is jank, but reflect.DeepEqual never has functions + // being the same. + if f, ok := actual.(*FunctionInterpolation); ok { + f.Func = nil + } + if !reflect.DeepEqual(actual, tc.Result) { t.Fatalf("%d bad: %#v", i, actual) } @@ -106,6 +133,55 @@ func TestNewUserVariable(t *testing.T) { } } +func TestFunctionInterpolation_impl(t *testing.T) { + var _ Interpolation = new(FunctionInterpolation) +} + +func TestFunctionInterpolation(t *testing.T) { + v1, err := NewInterpolatedVariable("var.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + v2, err := NewInterpolatedVariable("var.bar") + if err != nil { + t.Fatalf("err: %s", err) + } + + fn := func(vs map[string]string, args ...string) (string, error) { + return strings.Join(args, " "), nil + } + + i := &FunctionInterpolation{ + Func: fn, + Args: []InterpolatedVariable{v1, v2}, + key: "foo", + } + if i.FullString() != "foo" { + t.Fatalf("err: %#v", i) + } + + expected := map[string]InterpolatedVariable{ + "var.foo": v1, + "var.bar": v2, + } + if !reflect.DeepEqual(i.Variables(), expected) { + t.Fatalf("bad: %#v", i.Variables()) + } + + actual, err := i.Interpolate(map[string]string{ + "var.foo": "bar", + "var.bar": "baz", + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + if actual != "bar baz" { + t.Fatalf("bad: %#v", actual) + } +} + func TestResourceVariable_impl(t *testing.T) { var _ InterpolatedVariable = new(ResourceVariable) } From 8b5cc5d534f87ea3558ec959c8d761d659ad2a01 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 21 Jul 2014 13:12:43 -0700 Subject: [PATCH 10/25] config: lookup function works + tests --- config/interpolate_funcs.go | 20 +++++++++++- config/interpolate_funcs_test.go | 54 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 config/interpolate_funcs_test.go diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 6e5ec98df..9c076db2d 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -1,5 +1,10 @@ package config +import ( + "fmt" + "strings" +) + // Funcs is the mapping of built-in functions for configuration. var Funcs map[string]InterpolationFunc @@ -13,5 +18,18 @@ func init() { // dynamic lookups of map types within a Terraform configuration. func interpolationFuncLookup( vs map[string]string, args ...string) (string, error) { - return "", nil + if len(args) != 2 { + return "", fmt.Errorf( + "lookup expects 2 arguments, got %d", len(args)) + } + + k := fmt.Sprintf("var.%s", strings.Join(args, ".")) + v, ok := vs[k] + if !ok { + return "", fmt.Errorf( + "lookup in '%s' failed to find '%s'", + args[0], args[1]) + } + + return v, nil } diff --git a/config/interpolate_funcs_test.go b/config/interpolate_funcs_test.go new file mode 100644 index 000000000..20f964c43 --- /dev/null +++ b/config/interpolate_funcs_test.go @@ -0,0 +1,54 @@ +package config + +import ( + "testing" +) + +func TestInterpolateFuncLookup(t *testing.T) { + cases := []struct { + M map[string]string + Args []string + Result string + Error bool + }{ + { + map[string]string{ + "var.foo.bar": "baz", + }, + []string{"foo", "bar"}, + "baz", + false, + }, + + // Invalid key + { + map[string]string{ + "var.foo.bar": "baz", + }, + []string{"foo", "baz"}, + "", + true, + }, + + // Too many args + { + map[string]string{ + "var.foo.bar": "baz", + }, + []string{"foo", "bar", "baz"}, + "", + true, + }, + } + + for i, tc := range cases { + actual, err := interpolationFuncLookup(tc.M, tc.Args...) + if (err != nil) != tc.Error { + t.Fatalf("%d: err: %s", i, err) + } + + if actual != tc.Result { + t.Fatalf("%d: bad: %#v", i, actual) + } + } +} From 7578fb8bdc3c1744861b78c002800a0fa2e8cb7a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 06:43:04 -0700 Subject: [PATCH 11/25] config: interpolationWalker detects functions --- config/interpolate.go | 12 ++++++++++++ config/interpolate_walk.go | 2 +- config/interpolate_walk_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/config/interpolate.go b/config/interpolate.go index ab2cd26a4..1c6aeefc9 100644 --- a/config/interpolate.go +++ b/config/interpolate.go @@ -98,6 +98,12 @@ func NewInterpolation(v string) (Interpolation, error) { args := make([]InterpolatedVariable, 0, len(match)-2) for i := 2; i < len(match); i++ { + // This can be empty if we have a single argument + // due to the format of the regexp. + if match[i] == "" { + continue + } + v, err := NewInterpolatedVariable(match[i]) if err != nil { return nil, err @@ -196,6 +202,12 @@ func (i *VariableInterpolation) Variables() map[string]InterpolatedVariable { func NewResourceVariable(key string) (*ResourceVariable, error) { parts := strings.SplitN(key, ".", 3) + if len(parts) < 3 { + return nil, fmt.Errorf( + "%s: resource variables must be three parts: type.name.attr", + key) + } + field := parts[2] multi := false var index int diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go index 90b98e1ec..fd8a70833 100644 --- a/config/interpolate_walk.go +++ b/config/interpolate_walk.go @@ -10,7 +10,7 @@ import ( // interpRegexp is a regexp that matches interpolations such as ${foo.bar} var interpRegexp *regexp.Regexp = regexp.MustCompile( - `(?i)(\$+)\{([*-.a-z0-9_]+)\}`) + `(?i)(\$+)\{([\s*-.,\(\)a-z0-9_]+)\}`) // interpolationWalker implements interfaces for the reflectwalk package // (github.com/mitchellh/reflectwalk) that can be used to automatically diff --git a/config/interpolate_walk_test.go b/config/interpolate_walk_test.go index 255e0de5b..a731f8615 100644 --- a/config/interpolate_walk_test.go +++ b/config/interpolate_walk_test.go @@ -33,6 +33,24 @@ func TestInterpolationWalker_detect(t *testing.T) { }, }, }, + + { + Input: map[string]interface{}{ + "foo": "${lookup(var.foo)}", + }, + Result: []Interpolation{ + &FunctionInterpolation{ + Func: nil, + Args: []InterpolatedVariable{ + &UserVariable{ + Name: "foo", + key: "var.foo", + }, + }, + key: "lookup(var.foo)", + }, + }, + }, } for i, tc := range cases { @@ -48,6 +66,14 @@ func TestInterpolationWalker_detect(t *testing.T) { t.Fatalf("err: %s", err) } + for _, a := range actual { + // This is jank, but reflect.DeepEqual never has functions + // being the same. + if f, ok := a.(*FunctionInterpolation); ok { + f.Func = nil + } + } + if !reflect.DeepEqual(actual, tc.Result) { t.Fatalf("%d: bad:\n\n%#v", i, actual) } From aeb085c5f0e98ca4c5648335d2ad2ead6219432d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 06:51:02 -0700 Subject: [PATCH 12/25] config: error if variable interpolation can't find variable --- config/interpolate.go | 8 +++++++- config/interpolate_test.go | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/config/interpolate.go b/config/interpolate.go index 1c6aeefc9..77bc15117 100644 --- a/config/interpolate.go +++ b/config/interpolate.go @@ -193,7 +193,13 @@ func (i *VariableInterpolation) FullString() string { func (i *VariableInterpolation) Interpolate( vs map[string]string) (string, error) { - return vs[i.key], nil + v, ok := vs[i.key] + if !ok { + return "", fmt.Errorf( + "%s: value for variable '%s' not found", v) + } + + return v, nil } func (i *VariableInterpolation) Variables() map[string]InterpolatedVariable { diff --git a/config/interpolate_test.go b/config/interpolate_test.go index 87c2be0ca..f9ccb619f 100644 --- a/config/interpolate_test.go +++ b/config/interpolate_test.go @@ -273,3 +273,18 @@ func TestVariableInterpolation(t *testing.T) { t.Fatalf("bad: %#v", actual) } } + +func TestVariableInterpolation_missing(t *testing.T) { + uv, err := NewUserVariable("var.foo") + if err != nil { + t.Fatalf("err: %s", err) + } + + i := &VariableInterpolation{Variable: uv, key: "var.foo"} + _, err = i.Interpolate(map[string]string{ + "var.bar": "bar", + }) + if err == nil { + t.Fatal("should error") + } +} From c988be9ce75001d27a3113252a932bf651a6eb78 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 07:41:55 -0700 Subject: [PATCH 13/25] config: DefaultsMap --- config/config.go | 45 +++++++++++++++++++++++++++++++++++++++++++ config/config_test.go | 32 ++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/config/config.go b/config/config.go index be4bf00f5..adacf5ec9 100644 --- a/config/config.go +++ b/config/config.go @@ -6,7 +6,9 @@ import ( "fmt" "strings" + "github.com/hashicorp/terraform/flatmap" "github.com/hashicorp/terraform/helper/multierror" + "github.com/mitchellh/mapstructure" ) // Config is the configuration that comes from loading a collection @@ -63,6 +65,14 @@ type Output struct { RawConfig *RawConfig } +type VariableType byte + +const ( + VariableTypeUnknown VariableType = iota + VariableTypeString + VariableTypeMap +) + // ProviderConfigName returns the name of the provider configuration in // the given mapping that maps to the proper provider configuration // for this resource. @@ -271,6 +281,20 @@ func (r *Resource) mergerMerge(m merger) merger { return &result } +// DefaultsMap returns a map of default values for this variable. +func (v *Variable) DefaultsMap() map[string]string { + switch v.Type() { + case VariableTypeString: + return map[string]string{v.Name: v.Default.(string)} + case VariableTypeMap: + return flatmap.Flatten(map[string]interface{}{ + v.Name: v.Default.(map[string]string), + }) + default: + return nil + } +} + // Merge merges two variables to create a new third variable. func (v *Variable) Merge(v2 *Variable) *Variable { // Shallow copy the variable @@ -289,6 +313,27 @@ func (v *Variable) Merge(v2 *Variable) *Variable { return &result } +// Type returns the type of varialbe this is. +func (v *Variable) Type() VariableType { + if v.Default == nil { + return VariableTypeString + } + + var strVal string + if err := mapstructure.WeakDecode(v.Default, &strVal); err == nil { + v.Default = strVal + return VariableTypeString + } + + var m map[string]string + if err := mapstructure.WeakDecode(v.Default, &m); err == nil { + v.Default = m + return VariableTypeMap + } + + return VariableTypeUnknown +} + func (v *Variable) mergerName() string { return v.Name } diff --git a/config/config_test.go b/config/config_test.go index 000719ee5..40f54b5c7 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -2,6 +2,7 @@ package config import ( "path/filepath" + "reflect" "testing" ) @@ -96,6 +97,37 @@ func TestProviderConfigName(t *testing.T) { } } +func TestVariableDefaultsMap(t *testing.T) { + cases := []struct { + Default interface{} + Output map[string]string + }{ + { + "foo", + map[string]string{"foo": "foo"}, + }, + + { + map[interface{}]interface{}{ + "foo": "bar", + "bar": "baz", + }, + map[string]string{ + "foo.foo": "bar", + "foo.bar": "baz", + }, + }, + } + + for i, tc := range cases { + v := &Variable{Name: "foo", Default: tc.Default} + actual := v.DefaultsMap() + if !reflect.DeepEqual(actual, tc.Output) { + t.Fatalf("%d: bad: %#v", i, actual) + } + } +} + func testConfig(t *testing.T, name string) *Config { c, err := Load(filepath.Join(fixtureDir, name, "main.tf")) if err != nil { From c9a20c3c5849b58c2ab1145b3810cfee1e0f42a7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:06:09 -0700 Subject: [PATCH 14/25] terraform: test that mapping lookups work --- config/config.go | 11 ++++++++--- config/config_test.go | 7 ++++--- config/loader_libucl.go | 14 ++++++++++++++ terraform/context.go | 22 ++++++++++++++++++++-- terraform/context_test.go | 2 +- terraform/terraform_test.go | 3 ++- terraform/test-fixtures/apply-vars/main.tf | 8 ++++++++ 7 files changed, 57 insertions(+), 10 deletions(-) diff --git a/config/config.go b/config/config.go index adacf5ec9..99f2a66cb 100644 --- a/config/config.go +++ b/config/config.go @@ -283,13 +283,18 @@ func (r *Resource) mergerMerge(m merger) merger { // DefaultsMap returns a map of default values for this variable. func (v *Variable) DefaultsMap() map[string]string { + n := fmt.Sprintf("var.%s", v.Name) + switch v.Type() { case VariableTypeString: - return map[string]string{v.Name: v.Default.(string)} + return map[string]string{n: v.Default.(string)} case VariableTypeMap: - return flatmap.Flatten(map[string]interface{}{ - v.Name: v.Default.(map[string]string), + result := flatmap.Flatten(map[string]interface{}{ + n: v.Default.(map[string]string), }) + result[n] = v.Name + + return result default: return nil } diff --git a/config/config_test.go b/config/config_test.go index 40f54b5c7..3fe651743 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -104,7 +104,7 @@ func TestVariableDefaultsMap(t *testing.T) { }{ { "foo", - map[string]string{"foo": "foo"}, + map[string]string{"var.foo": "foo"}, }, { @@ -113,8 +113,9 @@ func TestVariableDefaultsMap(t *testing.T) { "bar": "baz", }, map[string]string{ - "foo.foo": "bar", - "foo.bar": "baz", + "var.foo": "foo", + "var.foo.foo": "bar", + "var.foo.bar": "baz", }, }, } diff --git a/config/loader_libucl.go b/config/loader_libucl.go index 176c588bd..cd14774fd 100644 --- a/config/loader_libucl.go +++ b/config/loader_libucl.go @@ -51,6 +51,20 @@ func (t *libuclConfigurable) Config() (*Config, error) { if len(rawConfig.Variable) > 0 { config.Variables = make([]*Variable, 0, len(rawConfig.Variable)) for k, v := range rawConfig.Variable { + // Defaults turn into a slice of map[string]interface{} and + // we need to make sure to convert that down into the + // proper type for Config. + if ms, ok := v.Default.([]map[string]interface{}); ok { + def := make(map[string]interface{}) + for _, m := range ms { + for k, v := range m { + def[k] = v + } + } + + v.Default = def + } + newVar := &Variable{ Name: k, Default: v.Default, diff --git a/terraform/context.go b/terraform/context.go index 1fa95ae0d..5555f2e98 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -30,6 +30,7 @@ type Context struct { providers map[string]ResourceProviderFactory provisioners map[string]ResourceProvisionerFactory variables map[string]string + defaultVars map[string]string l sync.Mutex // Lock acquired during any task parCh chan struct{} // Semaphore used to limit parallelism @@ -72,6 +73,14 @@ func NewContext(opts *ContextOpts) *Context { } parCh := make(chan struct{}, par) + // Calculate all the default variables + defaultVars := make(map[string]string) + for _, v := range opts.Config.Variables { + for k, val := range v.DefaultsMap() { + defaultVars[k] = val + } + } + return &Context{ config: opts.Config, diff: opts.Diff, @@ -80,6 +89,7 @@ func NewContext(opts *ContextOpts) *Context { providers: opts.Providers, provisioners: opts.Provisioners, variables: opts.Variables, + defaultVars: defaultVars, parCh: parCh, sh: sh, @@ -296,8 +306,13 @@ func (c *Context) computeVars(raw *config.RawConfig) error { return nil } - // Go through each variable and find it + // Start building up the variables. First, defaults vs := make(map[string]string) + for k, v := range c.defaultVars { + vs[k] = v + } + + // Next, the actual computed variables for n, rawV := range raw.Variables { switch v := rawV.(type) { case *config.ResourceVariable: @@ -314,7 +329,10 @@ func (c *Context) computeVars(raw *config.RawConfig) error { vs[n] = attr case *config.UserVariable: - vs[n] = c.variables[v.Name] + val, ok := c.variables[v.Name] + if ok { + vs[n] = val + } } } diff --git a/terraform/context_test.go b/terraform/context_test.go index 40143f36d..504941b64 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -1045,7 +1045,7 @@ func TestContextApply_vars(t *testing.T) { "aws": testProviderFuncFixed(p), }, Variables: map[string]string{ - "foo": "bar", + "foo": "us-west-2", }, }) diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 635148d93..ac452148f 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -221,7 +221,8 @@ aws_instance.foo: const testTerraformApplyVarsStr = ` aws_instance.bar: ID = foo - foo = bar + bar = foo + foo = us-west-2 type = aws_instance aws_instance.foo: ID = foo diff --git a/terraform/test-fixtures/apply-vars/main.tf b/terraform/test-fixtures/apply-vars/main.tf index f5c9c684f..986954562 100644 --- a/terraform/test-fixtures/apply-vars/main.tf +++ b/terraform/test-fixtures/apply-vars/main.tf @@ -1,7 +1,15 @@ +variable "amis" { + default = { + "us-east-1": "foo", + "us-west-2": "foo", + } +} + resource "aws_instance" "foo" { num = "2" } resource "aws_instance" "bar" { foo = "${var.foo}" + bar = "${lookup(var.amis, var.foo)}" } From fe2285898a7e332e5e0d9f762b66bb7e40bf59cc Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:10:06 -0700 Subject: [PATCH 15/25] config: DefaultsMap should return nil if nil --- config/config.go | 5 ++++- config/config_test.go | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 99f2a66cb..46a2ce431 100644 --- a/config/config.go +++ b/config/config.go @@ -283,8 +283,11 @@ func (r *Resource) mergerMerge(m merger) merger { // DefaultsMap returns a map of default values for this variable. func (v *Variable) DefaultsMap() map[string]string { - n := fmt.Sprintf("var.%s", v.Name) + if v.Default == nil { + return nil + } + n := fmt.Sprintf("var.%s", v.Name) switch v.Type() { case VariableTypeString: return map[string]string{n: v.Default.(string)} diff --git a/config/config_test.go b/config/config_test.go index 3fe651743..a060798ee 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -102,6 +102,11 @@ func TestVariableDefaultsMap(t *testing.T) { Default interface{} Output map[string]string }{ + { + nil, + nil, + }, + { "foo", map[string]string{"var.foo": "foo"}, From 5795db9232da89e2460f4169137e07c32db151aa Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:14:20 -0700 Subject: [PATCH 16/25] terraform: test regular variable default --- terraform/terraform_test.go | 1 + terraform/test-fixtures/apply-vars/main.tf | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index ac452148f..747ed3ead 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -226,6 +226,7 @@ aws_instance.bar: type = aws_instance aws_instance.foo: ID = foo + bar = baz num = 2 type = aws_instance ` diff --git a/terraform/test-fixtures/apply-vars/main.tf b/terraform/test-fixtures/apply-vars/main.tf index 986954562..997f1ffe7 100644 --- a/terraform/test-fixtures/apply-vars/main.tf +++ b/terraform/test-fixtures/apply-vars/main.tf @@ -5,8 +5,13 @@ variable "amis" { } } +variable "bar" { + default = "baz" +} + resource "aws_instance" "foo" { num = "2" + bar = "${var.bar}" } resource "aws_instance" "bar" { From 83ba038423710414e631892bd4a1f415d3900fa1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:18:53 -0700 Subject: [PATCH 17/25] terraform: mapping overrides work --- terraform/context.go | 7 +++++++ terraform/context_test.go | 3 ++- terraform/terraform_test.go | 1 + terraform/test-fixtures/apply-vars/main.tf | 1 + 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index 5555f2e98..af508f8c0 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -332,6 +332,13 @@ func (c *Context) computeVars(raw *config.RawConfig) error { val, ok := c.variables[v.Name] if ok { vs[n] = val + continue + } + + for k, val := range c.variables { + if strings.HasPrefix(k, v.Name+".") { + vs["var."+k] = val + } } } } diff --git a/terraform/context_test.go b/terraform/context_test.go index 504941b64..ec49e5a90 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -1045,7 +1045,8 @@ func TestContextApply_vars(t *testing.T) { "aws": testProviderFuncFixed(p), }, Variables: map[string]string{ - "foo": "us-west-2", + "foo": "us-west-2", + "amis.us-east-1": "override", }, }) diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 747ed3ead..24e780f9e 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -222,6 +222,7 @@ const testTerraformApplyVarsStr = ` aws_instance.bar: ID = foo bar = foo + baz = override foo = us-west-2 type = aws_instance aws_instance.foo: diff --git a/terraform/test-fixtures/apply-vars/main.tf b/terraform/test-fixtures/apply-vars/main.tf index 997f1ffe7..e250d0f98 100644 --- a/terraform/test-fixtures/apply-vars/main.tf +++ b/terraform/test-fixtures/apply-vars/main.tf @@ -17,4 +17,5 @@ resource "aws_instance" "foo" { resource "aws_instance" "bar" { foo = "${var.foo}" bar = "${lookup(var.amis, var.foo)}" + baz = "${var.amis.us-east-1}" } From fed0a89c364b00671708c578cb4c6190286687cb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:21:11 -0700 Subject: [PATCH 18/25] terraform: comments --- terraform/context.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/terraform/context.go b/terraform/context.go index af508f8c0..7674714ac 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -335,6 +335,8 @@ func (c *Context) computeVars(raw *config.RawConfig) error { continue } + // Look up if we have any variables with this prefix because + // those are map overrides. Include those. for k, val := range c.variables { if strings.HasPrefix(k, v.Name+".") { vs["var."+k] = val From c6474b3e5cce843e384bc2f30d8200ac54721100 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:27:16 -0700 Subject: [PATCH 19/25] terraform: more test cases --- helper/multierror/error.go | 4 ++++ terraform/context_test.go | 8 ++++++++ terraform/semantics_test.go | 9 +++++++++ terraform/test-fixtures/apply-vars/main.tf | 2 ++ terraform/test-fixtures/smc-uservars/main.tf | 7 +++++++ 5 files changed, 30 insertions(+) diff --git a/helper/multierror/error.go b/helper/multierror/error.go index cddd2fc84..5bfe2b14d 100644 --- a/helper/multierror/error.go +++ b/helper/multierror/error.go @@ -23,6 +23,10 @@ func (e *Error) Error() string { len(e.Errors), strings.Join(points, "\n")) } +func (e *Error) GoString() string { + return fmt.Sprintf("*%#v", *e) +} + // ErrorAppend is a helper function that will append more errors // onto a Error in order to create a larger multi-error. If the // original error is not a Error, it will be turned into one. diff --git a/terraform/context_test.go b/terraform/context_test.go index ec49e5a90..1cf797c7e 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -1050,6 +1050,14 @@ func TestContextApply_vars(t *testing.T) { }, }) + w, e := ctx.Validate() + if len(w) > 0 { + t.Fatalf("bad: %#v", w) + } + if len(e) > 0 { + t.Fatalf("bad: %s", e) + } + if _, err := ctx.Plan(nil); err != nil { t.Fatalf("err: %s", err) } diff --git a/terraform/semantics_test.go b/terraform/semantics_test.go index 9d7d30ebc..018f448b1 100644 --- a/terraform/semantics_test.go +++ b/terraform/semantics_test.go @@ -18,4 +18,13 @@ func TestSMCUserVariables(t *testing.T) { if len(errs) != 0 { t.Fatalf("err: %#v", errs) } + + // Mapping element override + errs = smcUserVariables(c, map[string]string{ + "foo": "bar", + "map.foo": "baz", + }) + if len(errs) != 0 { + t.Fatalf("err: %#v", errs) + } } diff --git a/terraform/test-fixtures/apply-vars/main.tf b/terraform/test-fixtures/apply-vars/main.tf index e250d0f98..6645e532d 100644 --- a/terraform/test-fixtures/apply-vars/main.tf +++ b/terraform/test-fixtures/apply-vars/main.tf @@ -9,6 +9,8 @@ variable "bar" { default = "baz" } +variable "foo" {} + resource "aws_instance" "foo" { num = "2" bar = "${var.bar}" diff --git a/terraform/test-fixtures/smc-uservars/main.tf b/terraform/test-fixtures/smc-uservars/main.tf index 7e2d10c11..cfa329378 100644 --- a/terraform/test-fixtures/smc-uservars/main.tf +++ b/terraform/test-fixtures/smc-uservars/main.tf @@ -6,3 +6,10 @@ variable "foo" { variable "bar" { default = "baz" } + +# Mapping +variable "map" { + default = { + "foo" = "bar"; + } +} From e59ff6e92cc54168aadc062447b5feabc62d494b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:29:49 -0700 Subject: [PATCH 20/25] terraform: fix tests --- config/interpolate.go | 3 ++- terraform/context_test.go | 4 ---- terraform/test-fixtures/apply-compute/main.tf | 4 ++++ 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/config/interpolate.go b/config/interpolate.go index 77bc15117..0b0bfc460 100644 --- a/config/interpolate.go +++ b/config/interpolate.go @@ -196,7 +196,8 @@ func (i *VariableInterpolation) Interpolate( v, ok := vs[i.key] if !ok { return "", fmt.Errorf( - "%s: value for variable '%s' not found", v) + "%s: value for variable not found", + i.key) } return v, nil diff --git a/terraform/context_test.go b/terraform/context_test.go index 1cf797c7e..9d161ea6b 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -1168,10 +1168,6 @@ func TestContextPlan_computed(t *testing.T) { t.Fatalf("err: %s", err) } - if len(plan.Diff.Resources) < 2 { - t.Fatalf("bad: %#v", plan.Diff.Resources) - } - actual := strings.TrimSpace(plan.String()) expected := strings.TrimSpace(testTerraformPlanComputedStr) if actual != expected { diff --git a/terraform/test-fixtures/apply-compute/main.tf b/terraform/test-fixtures/apply-compute/main.tf index 3999b5912..8fe55ed17 100644 --- a/terraform/test-fixtures/apply-compute/main.tf +++ b/terraform/test-fixtures/apply-compute/main.tf @@ -1,3 +1,7 @@ +variable "value" { + default = "" +} + resource "aws_instance" "foo" { num = "2" compute = "dynamical" From 61938c070f2824ebb9eb65a3ab35da0a42d53f2f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:34:24 -0700 Subject: [PATCH 21/25] config: validate type of default to string or mapping for var --- config/config.go | 9 +++++++-- config/config_test.go | 4 ---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/config/config.go b/config/config.go index 46a2ce431..4ed008ee2 100644 --- a/config/config.go +++ b/config/config.go @@ -108,8 +108,13 @@ func (c *Config) Validate() error { varMap[v.Name] = v } - // TODO(mitchellh): Validate that variable defaults are only a string - // or mapping of strings. + for _, v := range c.Variables { + if v.Type() == VariableTypeUnknown { + errs = append(errs, fmt.Errorf( + "Variable '%s': must be string or mapping", + v.Name)) + } + } // Check for references to user variables that do not actually // exist and record those errors. diff --git a/config/config_test.go b/config/config_test.go index a060798ee..9b141fa8f 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -73,10 +73,6 @@ func TestConfigValidate_varDefault(t *testing.T) { } func TestConfigValidate_varDefaultBadType(t *testing.T) { - t.Skip() - - // TODO(mitchellh): FIX - c := testConfig(t, "validate-var-default-bad-type") if err := c.Validate(); err == nil { t.Fatal("should not be valid") From f9e6754763d008b3d047557ee3722fdd5154fd81 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:43:16 -0700 Subject: [PATCH 22/25] config: comments --- config/config.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/config.go b/config/config.go index 4ed008ee2..7c68f6cec 100644 --- a/config/config.go +++ b/config/config.go @@ -65,6 +65,8 @@ type Output struct { RawConfig *RawConfig } +// VariableType is the type of value a variable is holding, and returned +// by the Type() function on variables. type VariableType byte const ( From b10b67832673c2e60b9ae7a43e2872335b8114d3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:47:10 -0700 Subject: [PATCH 23/25] config: fix error message in validation --- config/config.go | 4 ++-- config/test-fixtures/validate-good/main.tf | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index 7c68f6cec..cab5f728a 100644 --- a/config/config.go +++ b/config/config.go @@ -213,8 +213,8 @@ func (c *Config) Validate() error { // are valid in the Validate step. func (c *Config) allVariables() map[string][]InterpolatedVariable { result := make(map[string][]InterpolatedVariable) - for n, pc := range c.ProviderConfigs { - source := fmt.Sprintf("provider config '%s'", n) + for _, pc := range c.ProviderConfigs { + source := fmt.Sprintf("provider config '%s'", pc.Name) for _, v := range pc.RawConfig.Variables { result[source] = append(result[source], v) } diff --git a/config/test-fixtures/validate-good/main.tf b/config/test-fixtures/validate-good/main.tf index 7cd229611..965877a10 100644 --- a/config/test-fixtures/validate-good/main.tf +++ b/config/test-fixtures/validate-good/main.tf @@ -3,6 +3,12 @@ variable "foo" { description = "bar"; } +variable "amis" { + default = { + "east": "foo", + } +} + provider "aws" { access_key = "foo"; secret_key = "bar"; @@ -16,7 +22,7 @@ resource "aws_security_group" "firewall" { } resource aws_instance "web" { - ami = "${var.foo}" + ami = "${var.amis.east}" security_groups = [ "foo", "${aws_security_group.firewall.foo}" From 7b3a462ad135daed9094e51985c346944fc7cf7a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:51:50 -0700 Subject: [PATCH 24/25] config: remove UserMapVariable --- config/interpolate.go | 50 ++++++++------------------------------ config/interpolate_test.go | 4 --- 2 files changed, 10 insertions(+), 44 deletions(-) diff --git a/config/interpolate.go b/config/interpolate.go index 0b0bfc460..59f14a83b 100644 --- a/config/interpolate.go +++ b/config/interpolate.go @@ -69,14 +69,6 @@ type ResourceVariable struct { // "${var.foo}" type UserVariable struct { Name string - - key string -} - -// A UserMapVariable is a variable that is referencing a user -// variable that is a map. This looks like "${var.amis.us-east-1}" -type UserMapVariable struct { - Name string Elem string key string @@ -142,12 +134,7 @@ func NewInterpolatedVariable(v string) (InterpolatedVariable, error) { return NewResourceVariable(v) } - varKey := v[len("var."):] - if strings.Index(varKey, ".") == -1 { - return NewUserVariable(v) - } else { - return NewUserMapVariable(v) - } + return NewUserVariable(v) } func (i *FunctionInterpolation) FullString() string { @@ -257,9 +244,17 @@ func (v *ResourceVariable) FullKey() string { func NewUserVariable(key string) (*UserVariable, error) { name := key[len("var."):] + elem := "" + if idx := strings.Index(name, "."); idx > -1 { + elem = name[idx+1:] + name = name[:idx] + } + return &UserVariable{ - key: key, + key: key, + Name: name, + Elem: elem, }, nil } @@ -270,28 +265,3 @@ func (v *UserVariable) FullKey() string { func (v *UserVariable) GoString() string { return fmt.Sprintf("*%#v", *v) } - -func NewUserMapVariable(key string) (*UserMapVariable, error) { - name := key[len("var."):] - idx := strings.Index(name, ".") - if idx == -1 { - return nil, fmt.Errorf("not a user map variable: %s", key) - } - - elem := name[idx+1:] - name = name[:idx] - return &UserMapVariable{ - Name: name, - Elem: elem, - - key: key, - }, nil -} - -func (v *UserMapVariable) FullKey() string { - return v.key -} - -func (v *UserMapVariable) GoString() string { - return fmt.Sprintf("%#v", *v) -} diff --git a/config/interpolate_test.go b/config/interpolate_test.go index f9ccb619f..32d1f9869 100644 --- a/config/interpolate_test.go +++ b/config/interpolate_test.go @@ -238,10 +238,6 @@ func TestUserVariable_impl(t *testing.T) { var _ InterpolatedVariable = new(UserVariable) } -func TestUserMapVariable_impl(t *testing.T) { - var _ InterpolatedVariable = new(UserMapVariable) -} - func TestVariableInterpolation_impl(t *testing.T) { var _ Interpolation = new(VariableInterpolation) } From 935fa1d6fb6295361563ea8a9640497469114ac7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 22 Jul 2014 08:53:03 -0700 Subject: [PATCH 25/25] config: tests --- config/interpolate_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/config/interpolate_test.go b/config/interpolate_test.go index 32d1f9869..3b01c6cd7 100644 --- a/config/interpolate_test.go +++ b/config/interpolate_test.go @@ -133,6 +133,23 @@ func TestNewUserVariable(t *testing.T) { } } +func TestNewUserVariable_map(t *testing.T) { + v, err := NewUserVariable("var.bar.baz") + if err != nil { + t.Fatalf("err: %s", err) + } + + if v.Name != "bar" { + t.Fatalf("bad: %#v", v.Name) + } + if v.Elem != "baz" { + t.Fatalf("bad: %#v", v.Elem) + } + if v.FullKey() != "var.bar.baz" { + t.Fatalf("bad: %#v", v) + } +} + func TestFunctionInterpolation_impl(t *testing.T) { var _ Interpolation = new(FunctionInterpolation) }