diff --git a/builtin/providers/test/resource.go b/builtin/providers/test/resource.go index 7981497c3..64571009b 100644 --- a/builtin/providers/test/resource.go +++ b/builtin/providers/test/resource.go @@ -46,6 +46,11 @@ func testResource() *schema.Resource { Computed: true, ForceNew: true, }, + "computed_from_required": { + Type: schema.TypeString, + Computed: true, + ForceNew: true, + }, "computed_read_only_force_new": { Type: schema.TypeString, Computed: true, @@ -129,6 +134,8 @@ func testResourceCreate(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Missing attribute 'required_map', but it's required!") } + d.Set("computed_from_required", d.Get("required")) + return testResourceRead(d, meta) } diff --git a/builtin/providers/test/splat_flatten_test.go b/builtin/providers/test/splat_flatten_test.go new file mode 100644 index 000000000..29ea74ea2 --- /dev/null +++ b/builtin/providers/test/splat_flatten_test.go @@ -0,0 +1,79 @@ +package test + +import ( + "errors" + "testing" + + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" +) + +// This is actually a test of some core functionality in conjunction with +// helper/schema, rather than of the test provider itself. +// +// Here we're just verifying that unknown splats get flattened when assigned +// to list and set attributes. A variety of other situations are tested in +// an apply context test in the core package, but for this part we lean on +// helper/schema and thus need to exercise it at a higher level. + +func TestSplatFlatten(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: ` +resource "test_resource" "source" { + required = "foo ${count.index}" + required_map = { + key = "value" + } + count = 3 +} + +resource "test_resource" "splatted" { + # This legacy form of splatting into a list is still supported for + # backward-compatibility but no longer suggested. + set = ["${test_resource.source.*.computed_from_required}"] + list = ["${test_resource.source.*.computed_from_required}"] + + required = "yep" + required_map = { + key = "value" + } +} + `, + Check: func(s *terraform.State) error { + gotAttrs := s.RootModule().Resources["test_resource.splatted"].Primary.Attributes + t.Logf("attrs %#v", gotAttrs) + wantAttrs := map[string]string{ + "list.#": "3", + "list.0": "foo 0", + "list.1": "foo 1", + "list.2": "foo 2", + + // This depends on the default set hash implementation. + // If that changes, these keys will need to be updated. + "set.#": "3", + "set.1136855734": "foo 0", + "set.885275168": "foo 1", + "set.2915920794": "foo 2", + } + errored := false + for k, want := range wantAttrs { + got := gotAttrs[k] + if got != want { + t.Errorf("Wrong %s value %q; want %q", k, got, want) + errored = true + } + } + if errored { + return errors.New("incorrect attribute values") + } + return nil + }, + }, + }, + }) + +} diff --git a/config/config.go b/config/config.go index 439c7c567..a15782429 100644 --- a/config/config.go +++ b/config/config.go @@ -705,17 +705,6 @@ func (c *Config) Validate() error { } } - // Check that all variables are in the proper context - for source, rc := range c.rawConfigs() { - walker := &interpolationWalker{ - ContextF: c.validateVarContextFn(source, &errs), - } - if err := reflectwalk.Walk(rc.Raw, walker); err != nil { - errs = append(errs, fmt.Errorf( - "%s: error reading config: %s", source, err)) - } - } - // Validate the self variable for source, rc := range c.rawConfigs() { // Ignore provisioners. This is a pretty brittle way to do this, @@ -787,57 +776,6 @@ func (c *Config) rawConfigs() map[string]*RawConfig { return result } -func (c *Config) validateVarContextFn( - source string, errs *[]error) interpolationWalkerContextFunc { - return func(loc reflectwalk.Location, node ast.Node) { - // If we're in a slice element, then its fine, since you can do - // anything in there. - if loc == reflectwalk.SliceElem { - return - } - - // Otherwise, let's check if there is a splat resource variable - // at the top level in here. We do this by doing a transform that - // replaces everything with a noop node unless its a variable - // access or concat. This should turn the AST into a flat tree - // of Concat(Noop, ...). If there are any variables left that are - // multi-access, then its still broken. - node = node.Accept(func(n ast.Node) ast.Node { - // If it is a concat or variable access, we allow it. - switch n.(type) { - case *ast.Output: - return n - case *ast.VariableAccess: - return n - } - - // Otherwise, noop - return &noopNode{} - }) - - vars, err := DetectVariables(node) - if err != nil { - // Ignore it since this will be caught during parse. This - // actually probably should never happen by the time this - // is called, but its okay. - return - } - - for _, v := range vars { - rv, ok := v.(*ResourceVariable) - if !ok { - return - } - - if rv.Multi && rv.Index == -1 { - *errs = append(*errs, fmt.Errorf( - "%s: use of the splat ('*') operator must be wrapped in a list declaration", - source)) - } - } - } -} - func (c *Config) validateDependsOn( n string, v []string, diff --git a/config/config_test.go b/config/config_test.go index 68a93d9b6..d1ca8d70b 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -593,20 +593,6 @@ func TestConfigValidate_varMultiExactNonSlice(t *testing.T) { } } -func TestConfigValidate_varMultiNonSlice(t *testing.T) { - c := testConfig(t, "validate-var-multi-non-slice") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") - } -} - -func TestConfigValidate_varMultiNonSliceProvisioner(t *testing.T) { - c := testConfig(t, "validate-var-multi-non-slice-provisioner") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") - } -} - func TestConfigValidate_varMultiFunctionCall(t *testing.T) { c := testConfig(t, "validate-var-multi-func") if err := c.Validate(); err != nil { diff --git a/config/test-fixtures/validate-var-multi-non-slice-provisioner/main.tf b/config/test-fixtures/validate-var-multi-non-slice-provisioner/main.tf deleted file mode 100644 index c790b7c84..000000000 --- a/config/test-fixtures/validate-var-multi-non-slice-provisioner/main.tf +++ /dev/null @@ -1,9 +0,0 @@ -resource "aws_instance" "foo" { - count = 3 -} - -resource "aws_instance" "bar" { - provisioner "local-exec" { - foo = "${aws_instance.foo.*.id}" - } -} diff --git a/config/test-fixtures/validate-var-multi-non-slice/main.tf b/config/test-fixtures/validate-var-multi-non-slice/main.tf deleted file mode 100644 index 3f3a996ff..000000000 --- a/config/test-fixtures/validate-var-multi-non-slice/main.tf +++ /dev/null @@ -1,7 +0,0 @@ -resource "aws_instance" "foo" { - count = 3 -} - -resource "aws_instance" "bar" { - foo = "${aws_instance.foo.*.id}" -} diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 7d6a75e97..abe0e524a 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/davecgh/go-spew/spew" "github.com/hashicorp/terraform/config/module" ) @@ -3278,6 +3279,154 @@ func TestContext2Apply_multiVar(t *testing.T) { } } +// This is a holistic test of multi-var (aka "splat variable") handling +// across several different Terraform subsystems. This is here because +// historically there were quirky differences in handling across different +// parts of Terraform and so here we want to assert the expected behavior and +// ensure that it remains consistent in future. +func TestContext2Apply_multiVarComprehensive(t *testing.T) { + m := testModule(t, "apply-multi-var-comprehensive") + p := testProvider("test") + + configs := map[string]*ResourceConfig{} + + p.ApplyFn = testApplyFn + + p.DiffFn = func(info *InstanceInfo, s *InstanceState, c *ResourceConfig) (*InstanceDiff, error) { + configs[info.HumanId()] = c + + // Return a minimal diff to make sure this resource gets included in + // the apply graph and thus the final state, but otherwise we're just + // gathering data for assertions. + return &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "id": &ResourceAttrDiff{ + NewComputed: true, + }, + "name": &ResourceAttrDiff{ + New: info.HumanId(), + }, + }, + }, nil + } + + // First, apply with a count of 3 + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "test": testProviderFuncFixed(p), + }, + Variables: map[string]interface{}{ + "count": "3", + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("error during plan: %s", err) + } + + checkConfig := func(name string, want map[string]interface{}) { + got := configs[name].Config + if !reflect.DeepEqual(got, want) { + t.Errorf( + "wrong config for %s\ngot: %s\nwant: %s", + name, spew.Sdump(got), spew.Sdump(want), + ) + } + } + + checkConfig("test_thing.multi_count_var.0", map[string]interface{}{ + "source_id": unknownValue(), + "source_name": "test_thing.source.0", + }) + checkConfig("test_thing.multi_count_var.2", map[string]interface{}{ + "source_id": unknownValue(), + "source_name": "test_thing.source.2", + }) + checkConfig("test_thing.multi_count_derived.0", map[string]interface{}{ + "source_id": unknownValue(), + "source_name": "test_thing.source.0", + }) + checkConfig("test_thing.multi_count_derived.2", map[string]interface{}{ + "source_id": unknownValue(), + "source_name": "test_thing.source.2", + }) + checkConfig("test_thing.whole_splat", map[string]interface{}{ + "source_ids": unknownValue(), + "source_names": []interface{}{ + "test_thing.source.0", + "test_thing.source.1", + "test_thing.source.2", + }, + "source_ids_from_func": unknownValue(), + "source_names_from_func": []interface{}{ + "test_thing.source.0", + "test_thing.source.1", + "test_thing.source.2", + }, + + // This one ends up being a list with a single unknown value at this + // layer, but is fixed up inside helper/schema. There is a test for + // this inside the "test" provider, since core tests can't exercise + // helper/schema functionality. + "source_ids_wrapped": []interface{}{unknownValue()}, + + "source_names_wrapped": []interface{}{ + "test_thing.source.0", + "test_thing.source.1", + "test_thing.source.2", + }, + "first_source_id": unknownValue(), + "first_source_name": "test_thing.source.0", + }) + checkConfig("module.child.test_thing.whole_splat", map[string]interface{}{ + "source_ids": unknownValue(), + "source_names": []interface{}{ + "test_thing.source.0", + "test_thing.source.1", + "test_thing.source.2", + }, + + // This one ends up being a list with a single unknown value at this + // layer, but is fixed up inside helper/schema. There is a test for + // this inside the "test" provider, since core tests can't exercise + // helper/schema functionality. + "source_ids_wrapped": []interface{}{unknownValue()}, + + "source_names_wrapped": []interface{}{ + "test_thing.source.0", + "test_thing.source.1", + "test_thing.source.2", + }, + }) + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("error during apply: %s", err) + } + + { + want := map[string]interface{}{ + "source_ids": []interface{}{"foo", "foo", "foo"}, + "source_names": []interface{}{ + "test_thing.source.0", + "test_thing.source.1", + "test_thing.source.2", + }, + } + got := map[string]interface{}{} + for k, s := range state.RootModule().Outputs { + got[k] = s.Value + } + if !reflect.DeepEqual(got, want) { + t.Errorf( + "wrong outputs\ngot: %s\nwant: %s", + spew.Sdump(got), spew.Sdump(want), + ) + } + } +} + // Test that multi-var (splat) access is ordered by count, not by // value. func TestContext2Apply_multiVarOrder(t *testing.T) { diff --git a/terraform/test-fixtures/apply-multi-var-comprehensive/child/child.tf b/terraform/test-fixtures/apply-multi-var-comprehensive/child/child.tf new file mode 100644 index 000000000..e208ea6f5 --- /dev/null +++ b/terraform/test-fixtures/apply-multi-var-comprehensive/child/child.tf @@ -0,0 +1,26 @@ + +variable "count" { +} + +variable "source_ids" { + type = "list" +} + +variable "source_names" { + type = "list" +} + +resource "test_thing" "multi_count_var" { + count = "${var.count}" + + # Can pluck a single item out of a multi-var + source_id = "${var.source_ids[count.index]}" +} + +resource "test_thing" "whole_splat" { + # Can "splat" the ids directly into an attribute of type list. + source_ids = "${var.source_ids}" + source_names = "${var.source_names}" + source_ids_wrapped = ["${var.source_ids}"] + source_names_wrapped = ["${var.source_names}"] +} diff --git a/terraform/test-fixtures/apply-multi-var-comprehensive/root.tf b/terraform/test-fixtures/apply-multi-var-comprehensive/root.tf new file mode 100644 index 000000000..b0da72535 --- /dev/null +++ b/terraform/test-fixtures/apply-multi-var-comprehensive/root.tf @@ -0,0 +1,67 @@ + +variable "count" { +} + +resource "test_thing" "source" { + count = "${var.count}" + + # The diffFunc in the test exports "name" here too, which we can use + # to test values that are known during plan. +} + +resource "test_thing" "multi_count_var" { + count = "${var.count}" + + # Can pluck a single item out of a multi-var + source_id = "${test_thing.source.*.id[count.index]}" + source_name = "${test_thing.source.*.name[count.index]}" +} + +resource "test_thing" "multi_count_derived" { + # Can use the source to get the count + count = "${test_thing.source.count}" + + source_id = "${test_thing.source.*.id[count.index]}" + source_name = "${test_thing.source.*.name[count.index]}" +} + +resource "test_thing" "whole_splat" { + # Can "splat" the ids directly into an attribute of type list. + source_ids = "${test_thing.source.*.id}" + source_names = "${test_thing.source.*.name}" + + # Accessing through a function should work. + source_ids_from_func = "${split(" ", join(" ", test_thing.source.*.id))}" + source_names_from_func = "${split(" ", join(" ", test_thing.source.*.name))}" + + # A common pattern of selecting with a default. + first_source_id = "${element(concat(test_thing.source.*.id, list("default")), 0)}" + first_source_name = "${element(concat(test_thing.source.*.name, list("default")), 0)}" + + # Legacy form: Prior to Terraform having comprehensive list support, + # splats were treated as a special case and required to be presented + # in a wrapping list. This is no longer the suggested form, but we + # need it to keep working for compatibility. + # + # This should result in exactly the same result as the above, even + # though it looks like it would result in a list of lists. + source_ids_wrapped = ["${test_thing.source.*.id}"] + source_names_wrapped = ["${test_thing.source.*.name}"] + +} + +module "child" { + source = "./child" + + count = "${var.count}" + source_ids = "${test_thing.source.*.id}" + source_names = "${test_thing.source.*.name}" +} + +output "source_ids" { + value = "${test_thing.source.*.id}" +} + +output "source_names" { + value = "${test_thing.source.*.name}" +} diff --git a/website/source/docs/configuration/interpolation.html.md b/website/source/docs/configuration/interpolation.html.md index 9889ed410..1fada20c9 100644 --- a/website/source/docs/configuration/interpolation.html.md +++ b/website/source/docs/configuration/interpolation.html.md @@ -40,7 +40,7 @@ variable. #### User list variables -The syntax is `["${var.LIST}"]`. For example, `["${var.subnets}"]` +The syntax is `"${var.LIST}"`. For example, `"${var.subnets}"` would get the value of the `subnets` list, as a list. You can also return list elements by index: `${var.subnets[idx]}`. @@ -433,26 +433,26 @@ variable "hostnames" { } data "template_file" "web_init" { - # Expand multiple template files - the same number as we have instances - count = "${var.count}" + # Render the template once for each instance + count = "${length(var.hostnames)}" template = "${file("templates/web_init.tpl")}" vars { - # that gives us access to use count.index to do the lookup - hostname = "${lookup(var.hostnames, count.index)}" + # count.index tells us the index of the instance we are rendering + hostname = "${var.hostnames[count.index]}" } } resource "aws_instance" "web" { - # ... - count = "${var.count}" + # Create one instance for each hostname + count = "${length(var.hostnames)}" - # Link each web instance to the proper template_file - user_data = "${element(data.template_file.web_init.*.rendered, count.index)}" + # Pass each instance its corresponding template_file + user_data = "${data.template_file.web_init.*.rendered[count.index]}" } ``` -With this, we will build a list of `template_file.web_init` data sources which -we can use in combination with our list of `aws_instance.web` resources. +With this, we will build a list of `template_file.web_init` data resources +which we can use in combination with our list of `aws_instance.web` resources. ## Math