From 2162d6cf3d11ed1edf86bc7511b2db3e953b4608 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Jan 2017 20:32:55 -0800 Subject: [PATCH 1/5] terraform: test a basic static var count passed into a module --- terraform/context_plan_test.go | 34 +++++++++++++++++++ .../plan-count-module-static/child/main.tf | 5 +++ .../plan-count-module-static/main.tf | 6 ++++ 3 files changed, 45 insertions(+) create mode 100644 terraform/test-fixtures/plan-count-module-static/child/main.tf create mode 100644 terraform/test-fixtures/plan-count-module-static/main.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 955f766af..b7636e989 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1477,6 +1477,40 @@ func TestContext2Plan_countComputedModule(t *testing.T) { } } +func TestContext2Plan_countModuleStatic(t *testing.T) { + m := testModule(t, "plan-count-module-static") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(` +DIFF: + +module.child: + CREATE: aws_instance.foo.0 + CREATE: aws_instance.foo.1 + CREATE: aws_instance.foo.2 + +STATE: + + +`) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContext2Plan_countIndex(t *testing.T) { m := testModule(t, "plan-count-index") p := testProvider("aws") diff --git a/terraform/test-fixtures/plan-count-module-static/child/main.tf b/terraform/test-fixtures/plan-count-module-static/child/main.tf new file mode 100644 index 000000000..5b75831fd --- /dev/null +++ b/terraform/test-fixtures/plan-count-module-static/child/main.tf @@ -0,0 +1,5 @@ +variable "value" {} + +resource "aws_instance" "foo" { + count = "${var.value}" +} diff --git a/terraform/test-fixtures/plan-count-module-static/main.tf b/terraform/test-fixtures/plan-count-module-static/main.tf new file mode 100644 index 000000000..547bbde9d --- /dev/null +++ b/terraform/test-fixtures/plan-count-module-static/main.tf @@ -0,0 +1,6 @@ +variable "foo" { default = "3" } + +module "child" { + source = "./child" + value = "${var.foo}" +} From 0ba3fcdc63991f61bb963dc71521952ae3ace93c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Jan 2017 20:38:07 -0800 Subject: [PATCH 2/5] terraform: test static var being passed into grandchild for count --- terraform/context_plan_test.go | 34 +++++++++++++++++++ .../child/child/main.tf | 5 +++ .../child/main.tf | 6 ++++ .../main.tf | 6 ++++ 4 files changed, 51 insertions(+) create mode 100644 terraform/test-fixtures/plan-count-module-static-grandchild/child/child/main.tf create mode 100644 terraform/test-fixtures/plan-count-module-static-grandchild/child/main.tf create mode 100644 terraform/test-fixtures/plan-count-module-static-grandchild/main.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index b7636e989..aaa2c69a5 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1511,6 +1511,40 @@ STATE: } } +func TestContext2Plan_countModuleStaticGrandchild(t *testing.T) { + m := testModule(t, "plan-count-module-static-grandchild") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(` +DIFF: + +module.child.child: + CREATE: aws_instance.foo.0 + CREATE: aws_instance.foo.1 + CREATE: aws_instance.foo.2 + +STATE: + + +`) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContext2Plan_countIndex(t *testing.T) { m := testModule(t, "plan-count-index") p := testProvider("aws") diff --git a/terraform/test-fixtures/plan-count-module-static-grandchild/child/child/main.tf b/terraform/test-fixtures/plan-count-module-static-grandchild/child/child/main.tf new file mode 100644 index 000000000..5b75831fd --- /dev/null +++ b/terraform/test-fixtures/plan-count-module-static-grandchild/child/child/main.tf @@ -0,0 +1,5 @@ +variable "value" {} + +resource "aws_instance" "foo" { + count = "${var.value}" +} diff --git a/terraform/test-fixtures/plan-count-module-static-grandchild/child/main.tf b/terraform/test-fixtures/plan-count-module-static-grandchild/child/main.tf new file mode 100644 index 000000000..4dff927d5 --- /dev/null +++ b/terraform/test-fixtures/plan-count-module-static-grandchild/child/main.tf @@ -0,0 +1,6 @@ +variable "value" {} + +module "child" { + source = "./child" + value = "${var.value}" +} diff --git a/terraform/test-fixtures/plan-count-module-static-grandchild/main.tf b/terraform/test-fixtures/plan-count-module-static-grandchild/main.tf new file mode 100644 index 000000000..547bbde9d --- /dev/null +++ b/terraform/test-fixtures/plan-count-module-static-grandchild/main.tf @@ -0,0 +1,6 @@ +variable "foo" { default = "3" } + +module "child" { + source = "./child" + value = "${var.foo}" +} From cf46e1c3e0921fa857be158d1032d898631f59e5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Jan 2017 21:15:43 -0800 Subject: [PATCH 3/5] terraform: don't validate computed values in validate This disables the computed value check for `count` during the validation pass. This enables partial support for #3888 or #1497: as long as the value is non-computed during the plan, complex values will work in counts. **Notably, this allows data source values to be present in counts!** The "count" value can be disabled during validation safely because we can treat it as if any field that uses `count.index` is computed for validation. We then validate a single instance (as if `count = 1`) just to make sure all required fields are set. --- config/config.go | 16 +++++--------- config/config_test.go | 21 ------------------ terraform/context_validate_test.go | 22 +++++++++++++++++++ terraform/eval_sequence.go | 4 ++++ terraform/eval_validate.go | 1 + terraform/node_resource_abstract_count.go | 11 +++++++++- terraform/node_resource_validate.go | 10 ++++++--- .../validate-count-computed/main.tf | 7 ++++++ 8 files changed, 56 insertions(+), 36 deletions(-) create mode 100644 terraform/test-fixtures/validate-count-computed/main.tf diff --git a/config/config.go b/config/config.go index 80db548a7..14e35d6fa 100644 --- a/config/config.go +++ b/config/config.go @@ -505,23 +505,17 @@ func (c *Config) Validate() error { "%s: resource count can't reference count variable: %s", n, v.FullKey())) - case *ModuleVariable: - errs = append(errs, fmt.Errorf( - "%s: resource count can't reference module variable: %s", - n, - v.FullKey())) - case *ResourceVariable: - errs = append(errs, fmt.Errorf( - "%s: resource count can't reference resource variable: %s", - n, - v.FullKey())) case *SimpleVariable: errs = append(errs, fmt.Errorf( "%s: resource count can't reference variable: %s", n, v.FullKey())) + + // Good + case *ModuleVariable: + case *ResourceVariable: case *UserVariable: - // Good + default: panic(fmt.Sprintf("Unknown type in count var in %s: %T", n, v)) } diff --git a/config/config_test.go b/config/config_test.go index 95acd28b7..f554061ea 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -254,13 +254,6 @@ func TestConfigValidate_countCountVar(t *testing.T) { } } -func TestConfigValidate_countModuleVar(t *testing.T) { - c := testConfig(t, "validate-count-module-var") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") - } -} - func TestConfigValidate_countNotInt(t *testing.T) { c := testConfig(t, "validate-count-not-int") if err := c.Validate(); err == nil { @@ -268,20 +261,6 @@ func TestConfigValidate_countNotInt(t *testing.T) { } } -func TestConfigValidate_countResourceVar(t *testing.T) { - c := testConfig(t, "validate-count-resource-var") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") - } -} - -func TestConfigValidate_countResourceVarMulti(t *testing.T) { - c := testConfig(t, "validate-count-resource-var-multi") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") - } -} - func TestConfigValidate_countUserVar(t *testing.T) { c := testConfig(t, "validate-count-user-var") if err := c.Validate(); err != nil { diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index 9e434a7be..e3e7de419 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -115,6 +115,28 @@ func TestContext2Validate_computedVar(t *testing.T) { } } +// Test that validate allows through computed counts. We do this and allow +// them to fail during "plan" since we can't know if the computed values +// can be realized during a plan. +func TestContext2Validate_countComputed(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "validate-count-computed") + c := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + w, e := c.Validate() + if len(w) > 0 { + t.Fatalf("bad: %#v", w) + } + if len(e) > 0 { + t.Fatalf("bad: %s", e) + } +} + func TestContext2Validate_countNegative(t *testing.T) { p := testProvider("aws") m := testModule(t, "validate-count-negative") diff --git a/terraform/eval_sequence.go b/terraform/eval_sequence.go index 6c3c6a620..82d81782a 100644 --- a/terraform/eval_sequence.go +++ b/terraform/eval_sequence.go @@ -7,6 +7,10 @@ type EvalSequence struct { func (n *EvalSequence) Eval(ctx EvalContext) (interface{}, error) { for _, n := range n.Nodes { + if n == nil { + continue + } + if _, err := EvalRaw(n, ctx); err != nil { return nil, err } diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index 9ae221aa1..a2c122d6a 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -42,6 +42,7 @@ func (n *EvalValidateCount) Eval(ctx EvalContext) (interface{}, error) { c[n.Resource.RawCount.Key] = "1" count = 1 } + err = nil if count < 0 { errs = append(errs, fmt.Errorf( diff --git a/terraform/node_resource_abstract_count.go b/terraform/node_resource_abstract_count.go index 9b4df757f..573570d8e 100644 --- a/terraform/node_resource_abstract_count.go +++ b/terraform/node_resource_abstract_count.go @@ -14,6 +14,14 @@ type NodeAbstractCountResource struct { // GraphNodeEvalable func (n *NodeAbstractCountResource) EvalTree() EvalNode { + // We only check if the count is computed if we're not validating. + // If we're validating we allow computed counts since they just turn + // into more computed values. + var evalCountCheckComputed EvalNode + if !n.Validate { + evalCountCheckComputed = &EvalCountCheckComputed{Resource: n.Config} + } + return &EvalSequence{ Nodes: []EvalNode{ // The EvalTree for a plannable resource primarily involves @@ -24,7 +32,8 @@ func (n *NodeAbstractCountResource) EvalTree() EvalNode { // into the proper number of instances. &EvalInterpolate{Config: n.Config.RawCount}, - &EvalCountCheckComputed{Resource: n.Config}, + // Check if the count is computed + evalCountCheckComputed, // If validation is enabled, perform the validation &EvalIf{ diff --git a/terraform/node_resource_validate.go b/terraform/node_resource_validate.go index 3ea5bfd12..e01518de4 100644 --- a/terraform/node_resource_validate.go +++ b/terraform/node_resource_validate.go @@ -26,9 +26,13 @@ func (n *NodeValidatableResource) DynamicExpand(ctx EvalContext) (*Graph, error) defer lock.RUnlock() // Expand the resource count which must be available by now from EvalTree - count, err := n.Config.Count() - if err != nil { - return nil, err + count := 1 + if n.Config.RawCount.Value() != unknownValue() { + var err error + count, err = n.Config.Count() + if err != nil { + return nil, err + } } // The concrete resource factory we'll use diff --git a/terraform/test-fixtures/validate-count-computed/main.tf b/terraform/test-fixtures/validate-count-computed/main.tf new file mode 100644 index 000000000..e7de125f2 --- /dev/null +++ b/terraform/test-fixtures/validate-count-computed/main.tf @@ -0,0 +1,7 @@ +data "aws_data_source" "foo" { + compute = "value" +} + +resource "aws_instance" "bar" { + count = "${data.aws_data_source.foo.value}" +} From b8c310c61e3d95f25f56fe08f6c6ac4eb81291e6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Jan 2017 21:24:58 -0800 Subject: [PATCH 4/5] command: update test failure to correct message --- command/plan_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/plan_test.go b/command/plan_test.go index 426cac825..3f0e2cf26 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -575,7 +575,7 @@ func TestPlan_validate(t *testing.T) { } actual := ui.ErrorWriter.String() - if !strings.Contains(actual, "can't reference") { + if !strings.Contains(actual, "cannot be computed") { t.Fatalf("bad: %s", actual) } } From dd8ee38ba83dbad2db2742bf0b4d094a934cadef Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 28 Jan 2017 11:09:24 -0800 Subject: [PATCH 5/5] providers/test: additional testing via integration tests --- builtin/providers/test/data_source_test.go | 44 ++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/builtin/providers/test/data_source_test.go b/builtin/providers/test/data_source_test.go index 7a14b58af..3f4e5ada6 100644 --- a/builtin/providers/test/data_source_test.go +++ b/builtin/providers/test/data_source_test.go @@ -2,6 +2,7 @@ package test import ( "errors" + "fmt" "strings" "testing" @@ -55,3 +56,46 @@ resource "test_resource" "foo" { }, }) } + +// Test that the output of a data source can be used as the value for +// a "count" in a real resource. This would fail with "count cannot be computed" +// at some point. +func TestDataSource_valueAsResourceCount(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: func(s *terraform.State) error { + return nil + }, + Steps: []resource.TestStep{ + { + Config: strings.TrimSpace(` +data "test_data_source" "test" { + input = "4" +} + +resource "test_resource" "foo" { + count = "${data.test_data_source.test.output}" + + required = "yep" + required_map = { + key = "value" + } +} + `), + Check: func(s *terraform.State) error { + count := 0 + for k, _ := range s.RootModule().Resources { + if strings.HasPrefix(k, "test_resource.foo.") { + count++ + } + } + + if count != 4 { + return fmt.Errorf("bad count: %d", count) + } + return nil + }, + }, + }, + }) +}