From 7d905f6777baf6fdebce91355bcf713274c88c2e Mon Sep 17 00:00:00 2001 From: Pam Selle Date: Wed, 12 Jun 2019 11:07:32 -0400 Subject: [PATCH 1/4] Resource for_each --- addrs/for_each_attr.go | 12 +++ addrs/parse_ref.go | 8 ++ addrs/parse_ref_test.go | 46 +++++++++++ configs/configupgrade/analysis_expr.go | 4 + configs/parser_config_test.go | 5 ++ configs/resource.go | 16 ++-- ...each.tf => resource-count-and-for_each.tf} | 1 + lang/data.go | 1 + lang/data_test.go | 5 ++ lang/eval.go | 10 +++ lang/eval_test.go | 20 +++++ terraform/context_plan_test.go | 37 +++++++++ terraform/eval_apply.go | 6 +- terraform/eval_diff.go | 3 +- terraform/eval_for_each.go | 73 ++++++++++++++++++ terraform/eval_read_data.go | 3 +- terraform/eval_validate.go | 11 +++ terraform/evaluate.go | 51 +++++++++++-- terraform/graph_builder_plan_test.go | 76 +++++++++++++++++++ terraform/node_resource_abstract.go | 2 + terraform/node_resource_apply_instance.go | 15 +--- terraform/node_resource_plan.go | 11 ++- terraform/node_resource_plan_instance.go | 15 +--- terraform/node_resource_refresh.go | 7 ++ terraform/resource_address.go | 2 + terraform/testdata/plan-for-each/main.tf | 32 ++++++++ terraform/transform_orphan_count.go | 31 +++++++- terraform/transform_resource_count.go | 21 ++++- website/docs/configuration/resources.html.md | 58 +++++++++++++- 29 files changed, 530 insertions(+), 52 deletions(-) create mode 100644 addrs/for_each_attr.go rename configs/testdata/invalid-files/{resource-reserved-for_each.tf => resource-count-and-for_each.tf} (78%) create mode 100644 terraform/eval_for_each.go create mode 100644 terraform/testdata/plan-for-each/main.tf diff --git a/addrs/for_each_attr.go b/addrs/for_each_attr.go new file mode 100644 index 000000000..7a6385035 --- /dev/null +++ b/addrs/for_each_attr.go @@ -0,0 +1,12 @@ +package addrs + +// ForEachAttr is the address of an attribute referencing the current "for_each" object in +// the interpolation scope, addressed using the "each" keyword, ex. "each.key" and "each.value" +type ForEachAttr struct { + referenceable + Name string +} + +func (f ForEachAttr) String() string { + return "each." + f.Name +} diff --git a/addrs/parse_ref.go b/addrs/parse_ref.go index 84fe8a0d0..a230d0cd1 100644 --- a/addrs/parse_ref.go +++ b/addrs/parse_ref.go @@ -85,6 +85,14 @@ func parseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) { Remaining: remain, }, diags + case "each": + name, rng, remain, diags := parseSingleAttrRef(traversal) + return &Reference{ + Subject: ForEachAttr{Name: name}, + SourceRange: tfdiags.SourceRangeFromHCL(rng), + Remaining: remain, + }, diags + case "data": if len(traversal) < 3 { diags = diags.Append(&hcl.Diagnostic{ diff --git a/addrs/parse_ref_test.go b/addrs/parse_ref_test.go index 515656b8d..f42799f50 100644 --- a/addrs/parse_ref_test.go +++ b/addrs/parse_ref_test.go @@ -64,6 +64,52 @@ func TestParseRef(t *testing.T) { `The "count" object does not support this operation.`, }, + // each + { + `each.key`, + &Reference{ + Subject: ForEachAttr{ + Name: "key", + }, + SourceRange: tfdiags.SourceRange{ + Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0}, + End: tfdiags.SourcePos{Line: 1, Column: 9, Byte: 8}, + }, + }, + ``, + }, + { + `each.value.blah`, + &Reference{ + Subject: ForEachAttr{ + Name: "value", + }, + SourceRange: tfdiags.SourceRange{ + Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0}, + End: tfdiags.SourcePos{Line: 1, Column: 11, Byte: 10}, + }, + Remaining: hcl.Traversal{ + hcl.TraverseAttr{ + Name: "blah", + SrcRange: hcl.Range{ + Start: hcl.Pos{Line: 1, Column: 11, Byte: 10}, + End: hcl.Pos{Line: 1, Column: 16, Byte: 15}, + }, + }, + }, + }, + ``, + }, + { + `each`, + nil, + `The "each" object cannot be accessed directly. Instead, access one of its attributes.`, + }, + { + `each["hello"]`, + nil, + `The "each" object does not support this operation.`, + }, // data { `data.external.foo`, diff --git a/configs/configupgrade/analysis_expr.go b/configs/configupgrade/analysis_expr.go index 44a65af15..4b69714e2 100644 --- a/configs/configupgrade/analysis_expr.go +++ b/configs/configupgrade/analysis_expr.go @@ -71,6 +71,10 @@ func (d analysisData) GetCountAttr(addr addrs.CountAttr, rng tfdiags.SourceRange return cty.UnknownVal(cty.Number), nil } +func (d analysisData) GetForEachAttr(addr addrs.ForEachAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return cty.DynamicVal, nil +} + func (d analysisData) GetResourceInstance(instAddr addrs.ResourceInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { log.Printf("[TRACE] configupgrade: Determining type for %s", instAddr) addr := instAddr.Resource diff --git a/configs/parser_config_test.go b/configs/parser_config_test.go index bb68ada09..9c8b70ade 100644 --- a/configs/parser_config_test.go +++ b/configs/parser_config_test.go @@ -109,6 +109,11 @@ func TestParserLoadConfigFileFailureMessages(t *testing.T) { hcl.DiagError, "Unsupported block type", }, + { + "invalid-files/resource-count-and-for_each.tf", + hcl.DiagError, + `Invalid combination of "count" and "for_each"`, + }, { "invalid-files/resource-lifecycle-badbool.tf", hcl.DiagError, diff --git a/configs/resource.go b/configs/resource.go index de1a3434a..2528a5583 100644 --- a/configs/resource.go +++ b/configs/resource.go @@ -111,13 +111,15 @@ func decodeResourceBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) { if attr, exists := content.Attributes["for_each"]; exists { r.ForEach = attr.Expr - // We currently parse this, but don't yet do anything with it. - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Reserved argument name in resource block", - Detail: fmt.Sprintf("The name %q is reserved for use in a future version of Terraform.", attr.Name), - Subject: &attr.NameRange, - }) + // Cannot have count and for_each on the same resource block + if r.Count != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid combination of "count" and "for_each"`, + Detail: `The "count" and "for_each" meta-arguments are mutually-exclusive, only one should be used to be explicit about the number of resources to be created.`, + Subject: &attr.NameRange, + }) + } } if attr, exists := content.Attributes["provider"]; exists { diff --git a/configs/testdata/invalid-files/resource-reserved-for_each.tf b/configs/testdata/invalid-files/resource-count-and-for_each.tf similarity index 78% rename from configs/testdata/invalid-files/resource-reserved-for_each.tf rename to configs/testdata/invalid-files/resource-count-and-for_each.tf index 9cfebd89a..530fef74f 100644 --- a/configs/testdata/invalid-files/resource-reserved-for_each.tf +++ b/configs/testdata/invalid-files/resource-count-and-for_each.tf @@ -1,3 +1,4 @@ resource "test" "foo" { + count = 2 for_each = ["a"] } diff --git a/lang/data.go b/lang/data.go index 80313d6c0..eca588ecb 100644 --- a/lang/data.go +++ b/lang/data.go @@ -23,6 +23,7 @@ type Data interface { StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable) tfdiags.Diagnostics GetCountAttr(addrs.CountAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) + GetForEachAttr(addrs.ForEachAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) GetResourceInstance(addrs.ResourceInstance, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) GetLocalValue(addrs.LocalValue, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) GetModuleInstance(addrs.ModuleCallInstance, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) diff --git a/lang/data_test.go b/lang/data_test.go index c8c27b030..dba682958 100644 --- a/lang/data_test.go +++ b/lang/data_test.go @@ -8,6 +8,7 @@ import ( type dataForTests struct { CountAttrs map[string]cty.Value + ForEachAttrs map[string]cty.Value ResourceInstances map[string]cty.Value LocalValues map[string]cty.Value Modules map[string]cty.Value @@ -26,6 +27,10 @@ func (d *dataForTests) GetCountAttr(addr addrs.CountAttr, rng tfdiags.SourceRang return d.CountAttrs[addr.Name], nil } +func (d *dataForTests) GetForEachAttr(addr addrs.ForEachAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + return d.ForEachAttrs[addr.Name], nil +} + func (d *dataForTests) GetResourceInstance(addr addrs.ResourceInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { return d.ResourceInstances[addr.String()], nil } diff --git a/lang/eval.go b/lang/eval.go index a3fb363ec..a8fe8b662 100644 --- a/lang/eval.go +++ b/lang/eval.go @@ -203,6 +203,7 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl pathAttrs := map[string]cty.Value{} terraformAttrs := map[string]cty.Value{} countAttrs := map[string]cty.Value{} + forEachAttrs := map[string]cty.Value{} var self cty.Value for _, ref := range refs { @@ -334,6 +335,14 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl self = val } + case addrs.ForEachAttr: + val, valDiags := normalizeRefValue(s.Data.GetForEachAttr(subj, rng)) + diags = diags.Append(valDiags) + forEachAttrs[subj.Name] = val + if isSelf { + self = val + } + default: // Should never happen panic(fmt.Errorf("Scope.buildEvalContext cannot handle address type %T", rawSubj)) @@ -350,6 +359,7 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl vals["path"] = cty.ObjectVal(pathAttrs) vals["terraform"] = cty.ObjectVal(terraformAttrs) vals["count"] = cty.ObjectVal(countAttrs) + vals["each"] = cty.ObjectVal(forEachAttrs) if self != cty.NilVal { vals["self"] = self } diff --git a/lang/eval_test.go b/lang/eval_test.go index db57eabde..67feb30d0 100644 --- a/lang/eval_test.go +++ b/lang/eval_test.go @@ -20,6 +20,10 @@ func TestScopeEvalContext(t *testing.T) { CountAttrs: map[string]cty.Value{ "index": cty.NumberIntVal(0), }, + ForEachAttrs: map[string]cty.Value{ + "key": cty.StringVal("a"), + "value": cty.NumberIntVal(1), + }, ResourceInstances: map[string]cty.Value{ "null_resource.foo": cty.ObjectVal(map[string]cty.Value{ "attr": cty.StringVal("bar"), @@ -75,6 +79,22 @@ func TestScopeEvalContext(t *testing.T) { }), }, }, + { + `each.key`, + map[string]cty.Value{ + "each": cty.ObjectVal(map[string]cty.Value{ + "key": cty.StringVal("a"), + }), + }, + }, + { + `each.value`, + map[string]cty.Value{ + "each": cty.ObjectVal(map[string]cty.Value{ + "value": cty.NumberIntVal(1), + }), + }, + }, { `local.foo`, map[string]cty.Value{ diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index bf30539f6..0bf293d72 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -3351,6 +3351,43 @@ func TestContext2Plan_countIncreaseWithSplatReference(t *testing.T) { } } +func TestContext2Plan_forEach(t *testing.T) { + m := testModule(t, "plan-for-each") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "aws": testProviderFuncFixed(p), + }, + ), + }) + + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } + + schema := p.GetSchemaReturn.ResourceTypes["aws_instance"] + ty := schema.ImpliedType() + + if len(plan.Changes.Resources) != 8 { + t.Fatal("expected 8 changes, got", len(plan.Changes.Resources)) + } + + for _, res := range plan.Changes.Resources { + if res.Action != plans.Create { + t.Fatalf("expected resource creation, got %s", res.Action) + } + _, err := res.Decode(ty) + if err != nil { + t.Fatal(err) + } + } + +} + func TestContext2Plan_destroy(t *testing.T) { m := testModule(t, "plan-destroy") p := testProvider("aws") diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 09313f7fc..422f372c4 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -61,7 +61,8 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) { configVal := cty.NullVal(cty.DynamicPseudoType) if n.Config != nil { var configDiags tfdiags.Diagnostics - keyData := EvalDataForInstanceKey(n.Addr.Key) + forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx) + keyData := EvalDataForInstanceKey(n.Addr.Key, forEach) configVal, _, configDiags = ctx.EvaluateBlock(n.Config.Config, schema, nil, keyData) diags = diags.Append(configDiags) if configDiags.HasErrors() { @@ -548,7 +549,8 @@ func (n *EvalApplyProvisioners) apply(ctx EvalContext, provs []*configs.Provisio provisioner := ctx.Provisioner(prov.Type) schema := ctx.ProvisionerSchema(prov.Type) - keyData := EvalDataForInstanceKey(instanceAddr.Key) + // TODO the for_each val is not added here, which might causes issues with provisioners + keyData := EvalDataForInstanceKey(instanceAddr.Key, nil) // Evaluate the main provisioner configuration. config, _, configDiags := ctx.EvaluateBlock(prov.Config, schema, instanceAddr, keyData) diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 20af9593c..695b5fe0e 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -133,7 +133,8 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { // Should be caught during validation, so we don't bother with a pretty error here return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type) } - keyData := EvalDataForInstanceKey(n.Addr.Key) + forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx) + keyData := EvalDataForInstanceKey(n.Addr.Key, forEach) configVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData) diags = diags.Append(configDiags) if configDiags.HasErrors() { diff --git a/terraform/eval_for_each.go b/terraform/eval_for_each.go new file mode 100644 index 000000000..f9b2e35cc --- /dev/null +++ b/terraform/eval_for_each.go @@ -0,0 +1,73 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/hcl2/hcl" + "github.com/hashicorp/terraform/tfdiags" + "github.com/zclconf/go-cty/cty" +) + +// evaluateResourceForEachExpression interprets a "for_each" argument on a resource. +// +// Returns a cty.Value map, and diagnostics if necessary. It will return nil if +// the expression is nil, and is used to distinguish between an unset for_each and an +// empty map +func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) { + if expr == nil { + return nil, nil + } + + forEachVal, forEachDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil) + diags = diags.Append(forEachDiags) + if diags.HasErrors() { + return nil, diags + } + + // No-op for dynamic types, so that these pass validation, but are then populated at apply + if forEachVal.Type() == cty.DynamicPseudoType { + return nil, diags + } + + if forEachVal.IsNull() { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid for_each argument", + Detail: `The given "for_each" argument value is unsuitable: the given "for_each" argument value is null. A map, or set of strings is allowed.`, + Subject: expr.Range().Ptr(), + }) + return nil, diags + } + + if !forEachVal.CanIterateElements() || forEachVal.Type().IsListType() { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid for_each argument", + Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: the "for_each" argument must be a map, or set of strings, and you have provided a value of type %s.`, forEachVal.Type().FriendlyName()), + Subject: expr.Range().Ptr(), + }) + return nil, diags + } + + if forEachVal.Type().IsSetType() { + if forEachVal.Type().ElementType() != cty.String { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid for_each set argument", + Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: "for_each" supports maps and sets of strings, but you have provided a set containing type %s.`, forEachVal.Type().ElementType().FriendlyName()), + Subject: expr.Range().Ptr(), + }) + return nil, diags + } + } + + // If the map is empty ({}), return an empty map, because cty will return nil when representing {} AsValueMap + // Also return an empty map if the value is not known -- as this function + // is used to check if the for_each value is valid as well as to apply it, the empty + // map will later be filled in. + if !forEachVal.IsKnown() || forEachVal.LengthInt() == 0 { + return map[string]cty.Value{}, diags + } + + return forEachVal.AsValueMap(), nil +} diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 728719a7a..4999480f5 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -95,7 +95,8 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { objTy := schema.ImpliedType() priorVal := cty.NullVal(objTy) // for data resources, prior is always null because we start fresh every time - keyData := EvalDataForInstanceKey(n.Addr.Key) + forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx) + keyData := EvalDataForInstanceKey(n.Addr.Key, forEach) var configDiags tfdiags.Diagnostics configVal, _, configDiags = ctx.EvaluateBlock(config.Config, schema, nil, keyData) diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index 3edbee408..581a91b0e 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -370,6 +370,17 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) { diags = diags.Append(countDiags) } + if n.Config.ForEach != nil { + keyData = InstanceKeyEvalData{ + EachKey: cty.UnknownVal(cty.String), + EachValue: cty.UnknownVal(cty.DynamicPseudoType), + } + + // Evaluate the for_each expression here so we can expose the diagnostics + _, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx) + diags = diags.Append(forEachDiags) + } + for _, traversal := range n.Config.DependsOn { ref, refDiags := addrs.ParseRef(traversal) diags = diags.Append(refDiags) diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 11a0dac8a..9bb600945 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -120,20 +120,24 @@ type InstanceKeyEvalData struct { // EvalDataForInstanceKey constructs a suitable InstanceKeyEvalData for // evaluating in a context that has the given instance key. -func EvalDataForInstanceKey(key addrs.InstanceKey) InstanceKeyEvalData { - // At the moment we don't actually implement for_each, so we only - // ever populate CountIndex. - // (When we implement for_each later we may need to reorganize this some, - // so that we can resolve the ambiguity that an int key may either be - // a count.index or an each.key where for_each is over a list.) - +func EvalDataForInstanceKey(key addrs.InstanceKey, forEachMap map[string]cty.Value) InstanceKeyEvalData { var countIdx cty.Value + var eachKey cty.Value + var eachVal cty.Value + if intKey, ok := key.(addrs.IntKey); ok { countIdx = cty.NumberIntVal(int64(intKey)) } + if stringKey, ok := key.(addrs.StringKey); ok { + eachKey = cty.StringVal(string(stringKey)) + eachVal = forEachMap[string(stringKey)] + } + return InstanceKeyEvalData{ CountIndex: countIdx, + EachKey: eachKey, + EachValue: eachVal, } } @@ -173,6 +177,37 @@ func (d *evaluationStateData) GetCountAttr(addr addrs.CountAttr, rng tfdiags.Sou } } +func (d *evaluationStateData) GetForEachAttr(addr addrs.ForEachAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + var returnVal cty.Value + switch addr.Name { + + case "key": + returnVal = d.InstanceKeyData.EachKey + case "value": + returnVal = d.InstanceKeyData.EachValue + default: + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid "each" attribute`, + Detail: fmt.Sprintf(`The "each" object does not have an attribute named %q. The supported attributes are each.key and each.value, the current key and value pair of the "for_each" attribute set.`, addr.Name), + Subject: rng.ToHCL().Ptr(), + }) + return cty.DynamicVal, diags + } + + if returnVal == cty.NilVal { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Reference to "each" in context without for_each`, + Detail: fmt.Sprintf(`The "each" object can be used only in "resource" blocks, and only when the "for_each" argument is set.`), + Subject: rng.ToHCL().Ptr(), + }) + return cty.UnknownVal(cty.DynamicPseudoType), diags + } + return returnVal, diags +} + func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics @@ -569,7 +604,7 @@ func (d *evaluationStateData) GetResourceInstance(addr addrs.ResourceInstance, r } case states.EachMap: multi = key == addrs.NoKey - if _, ok := addr.Key.(addrs.IntKey); !multi && !ok { + if _, ok := addr.Key.(addrs.StringKey); !multi && !ok { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid resource index", diff --git a/terraform/graph_builder_plan_test.go b/terraform/graph_builder_plan_test.go index 2993b650c..2eab2c31d 100644 --- a/terraform/graph_builder_plan_test.go +++ b/terraform/graph_builder_plan_test.go @@ -247,6 +247,51 @@ func TestPlanGraphBuilder_targetModule(t *testing.T) { testGraphNotContains(t, g, "module.child1.test_object.foo") } +func TestPlanGraphBuilder_forEach(t *testing.T) { + awsProvider := &MockProvider{ + GetSchemaReturn: &ProviderSchema{ + Provider: simpleTestSchema(), + ResourceTypes: map[string]*configschema.Block{ + "aws_instance": simpleTestSchema(), + }, + }, + } + + components := &basicComponentFactory{ + providers: map[string]providers.Factory{ + "aws": providers.FactoryFixed(awsProvider), + }, + } + + b := &PlanGraphBuilder{ + Config: testModule(t, "plan-for-each"), + Components: components, + Schemas: &Schemas{ + Providers: map[string]*ProviderSchema{ + "aws": awsProvider.GetSchemaReturn, + }, + }, + DisableReduce: true, + } + + g, err := b.Build(addrs.RootModuleInstance) + if err != nil { + t.Fatalf("err: %s", err) + } + + if g.Path.String() != addrs.RootModuleInstance.String() { + t.Fatalf("wrong module path %q", g.Path) + } + + actual := strings.TrimSpace(g.String()) + // We're especially looking for the edge here, where aws_instance.bat + // has a dependency on aws_instance.boo + expected := strings.TrimSpace(testPlanGraphBuilderForEachStr) + if actual != expected { + t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) + } +} + const testPlanGraphBuilderStr = ` aws_instance.web aws_security_group.firewall @@ -290,3 +335,34 @@ root provider.openstack (close) var.foo ` +const testPlanGraphBuilderForEachStr = ` +aws_instance.bar + provider.aws +aws_instance.bat + aws_instance.boo + provider.aws +aws_instance.baz + provider.aws +aws_instance.boo + provider.aws +aws_instance.foo + provider.aws +meta.count-boundary (EachMode fixup) + aws_instance.bar + aws_instance.bat + aws_instance.baz + aws_instance.boo + aws_instance.foo + provider.aws +provider.aws +provider.aws (close) + aws_instance.bar + aws_instance.bat + aws_instance.baz + aws_instance.boo + aws_instance.foo + provider.aws +root + meta.count-boundary (EachMode fixup) + provider.aws (close) +` diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 514845585..d147b42e4 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -187,6 +187,8 @@ func (n *NodeAbstractResource) References() []*addrs.Reference { refs, _ := lang.ReferencesInExpr(c.Count) result = append(result, refs...) + refs, _ = lang.ReferencesInExpr(c.ForEach) + result = append(result, refs...) refs, _ = lang.ReferencesInBlock(c.Config, n.Schema) result = append(result, refs...) if c.Managed != nil { diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index dad7bfc5f..d79532467 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -101,13 +101,6 @@ func (n *NodeApplyableResourceInstance) References() []*addrs.Reference { func (n *NodeApplyableResourceInstance) EvalTree() EvalNode { addr := n.ResourceInstanceAddr() - // State still uses legacy-style internal ids, so we need to shim to get - // a suitable key to use. - stateId := NewLegacyResourceInstanceAddress(addr).stateId() - - // Determine the dependencies for the state. - stateDeps := n.StateReferences() - if n.Config == nil { // This should not be possible, but we've got here in at least one // case as discussed in the following issue: @@ -132,15 +125,15 @@ func (n *NodeApplyableResourceInstance) EvalTree() EvalNode { // Eval info is different depending on what kind of resource this is switch n.Config.Mode { case addrs.ManagedResourceMode: - return n.evalTreeManagedResource(addr, stateId, stateDeps) + return n.evalTreeManagedResource(addr) case addrs.DataResourceMode: - return n.evalTreeDataResource(addr, stateId, stateDeps) + return n.evalTreeDataResource(addr) default: panic(fmt.Errorf("unsupported resource mode %s", n.Config.Mode)) } } -func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResourceInstance, stateId string, stateDeps []addrs.Referenceable) EvalNode { +func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResourceInstance) EvalNode { var provider providers.Interface var providerSchema *ProviderSchema var change *plans.ResourceInstanceChange @@ -206,7 +199,7 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou } } -func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsResourceInstance, stateId string, stateDeps []addrs.Referenceable) EvalNode { +func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsResourceInstance) EvalNode { // Declare a bunch of variables that are used for state during // evaluation. Most of this are written to by-address below. var provider providers.Interface diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 633c1c466..ec4aa9322 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -77,6 +77,11 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { return nil, diags.Err() } + forEachMap, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx) + if forEachDiags.HasErrors() { + return nil, diags.Err() + } + // Next we need to potentially rename an instance address in the state // if we're transitioning whether "count" is set at all. fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1) @@ -119,18 +124,20 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { // Start creating the steps steps := []GraphTransformer{ - // Expand the count. + // Expand the count or for_each (if present) &ResourceCountTransformer{ Concrete: concreteResource, Schema: n.Schema, Count: count, + ForEach: forEachMap, Addr: n.ResourceAddr(), }, - // Add the count orphans + // Add the count/for_each orphans &OrphanResourceCountTransformer{ Concrete: concreteResourceOrphan, Count: count, + ForEach: forEachMap, Addr: n.ResourceAddr(), State: state, }, diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 75e0bcd34..0f74bbe61 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -34,25 +34,18 @@ var ( func (n *NodePlannableResourceInstance) EvalTree() EvalNode { addr := n.ResourceInstanceAddr() - // State still uses legacy-style internal ids, so we need to shim to get - // a suitable key to use. - stateId := NewLegacyResourceInstanceAddress(addr).stateId() - - // Determine the dependencies for the state. - stateDeps := n.StateReferences() - // Eval info is different depending on what kind of resource this is switch addr.Resource.Resource.Mode { case addrs.ManagedResourceMode: - return n.evalTreeManagedResource(addr, stateId, stateDeps) + return n.evalTreeManagedResource(addr) case addrs.DataResourceMode: - return n.evalTreeDataResource(addr, stateId, stateDeps) + return n.evalTreeDataResource(addr) default: panic(fmt.Errorf("unsupported resource mode %s", n.Config.Mode)) } } -func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResourceInstance, stateId string, stateDeps []addrs.Referenceable) EvalNode { +func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResourceInstance) EvalNode { config := n.Config var provider providers.Interface var providerSchema *ProviderSchema @@ -147,7 +140,7 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou } } -func (n *NodePlannableResourceInstance) evalTreeManagedResource(addr addrs.AbsResourceInstance, stateId string, stateDeps []addrs.Referenceable) EvalNode { +func (n *NodePlannableResourceInstance) evalTreeManagedResource(addr addrs.AbsResourceInstance) EvalNode { config := n.Config var provider providers.Interface var providerSchema *ProviderSchema diff --git a/terraform/node_resource_refresh.go b/terraform/node_resource_refresh.go index 950602320..9daeabfa6 100644 --- a/terraform/node_resource_refresh.go +++ b/terraform/node_resource_refresh.go @@ -39,6 +39,11 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, return nil, diags.Err() } + forEachMap, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx) + if forEachDiags.HasErrors() { + return nil, diags.Err() + } + // Next we need to potentially rename an instance address in the state // if we're transitioning whether "count" is set at all. fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1) @@ -66,6 +71,7 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, Concrete: concreteResource, Schema: n.Schema, Count: count, + ForEach: forEachMap, Addr: n.ResourceAddr(), }, @@ -74,6 +80,7 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, &OrphanResourceCountTransformer{ Concrete: concreteResource, Count: count, + ForEach: forEachMap, Addr: n.ResourceAddr(), State: state, }, diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 156ecf5c0..5d8261a67 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -365,6 +365,8 @@ func NewLegacyResourceInstanceAddress(addr addrs.AbsResourceInstance) *ResourceA ret.Index = -1 } else if ik, ok := addr.Resource.Key.(addrs.IntKey); ok { ret.Index = int(ik) + } else if _, ok := addr.Resource.Key.(addrs.StringKey); ok { + ret.Index = -1 } else { panic(fmt.Errorf("cannot shim resource instance with key %#v to legacy ResourceAddress.Index", addr.Resource.Key)) } diff --git a/terraform/testdata/plan-for-each/main.tf b/terraform/testdata/plan-for-each/main.tf new file mode 100644 index 000000000..cbe0c5875 --- /dev/null +++ b/terraform/testdata/plan-for-each/main.tf @@ -0,0 +1,32 @@ +# maps +resource "aws_instance" "foo" { + for_each = { + a = "thing" + b = "another thing" + c = "yet another thing" + } + num = "3" +} + +# sets +resource "aws_instance" "bar" { + for_each = toset(list("z", "y", "x")) +} + +# an empty map should generate no resource +resource "aws_instance" "baz" { + for_each = {} +} + +# references +resource "aws_instance" "boo" { + foo = aws_instance.foo["a"].num +} + +resource "aws_instance" "bat" { + for_each = { + my_key = aws_instance.boo.foo + } + foo = each.value +} + diff --git a/terraform/transform_orphan_count.go b/terraform/transform_orphan_count.go index eec762e55..4f323a7a0 100644 --- a/terraform/transform_orphan_count.go +++ b/terraform/transform_orphan_count.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/states" + "github.com/zclconf/go-cty/cty" ) // OrphanResourceCountTransformer is a GraphTransformer that adds orphans @@ -18,9 +19,10 @@ import ( type OrphanResourceCountTransformer struct { Concrete ConcreteResourceInstanceNodeFunc - Count int // Actual count of the resource, or -1 if count is not set at all - Addr addrs.AbsResource // Addr of the resource to look for orphans - State *states.State // Full global state + Count int // Actual count of the resource, or -1 if count is not set at all + ForEach map[string]cty.Value // The ForEach map on the resource + Addr addrs.AbsResource // Addr of the resource to look for orphans + State *states.State // Full global state } func (t *OrphanResourceCountTransformer) Transform(g *Graph) error { @@ -34,6 +36,10 @@ func (t *OrphanResourceCountTransformer) Transform(g *Graph) error { haveKeys[key] = struct{}{} } + // if for_each is set, use that transformer + if t.ForEach != nil { + return t.transformForEach(haveKeys, g) + } if t.Count < 0 { return t.transformNoCount(haveKeys, g) } @@ -43,6 +49,25 @@ func (t *OrphanResourceCountTransformer) Transform(g *Graph) error { return t.transformCount(haveKeys, g) } +func (t *OrphanResourceCountTransformer) transformForEach(haveKeys map[addrs.InstanceKey]struct{}, g *Graph) error { + for key := range haveKeys { + s, _ := key.(addrs.StringKey) + // If the key is present in our current for_each, carry on + if _, ok := t.ForEach[string(s)]; ok { + continue + } + + abstract := NewNodeAbstractResourceInstance(t.Addr.Instance(key)) + var node dag.Vertex = abstract + if f := t.Concrete; f != nil { + node = f(abstract) + } + log.Printf("[TRACE] OrphanResourceCount(non-zero): adding %s as %T", t.Addr, node) + g.Add(node) + } + return nil +} + func (t *OrphanResourceCountTransformer) transformCount(haveKeys map[addrs.InstanceKey]struct{}, g *Graph) error { // Due to the logic in Transform, we only get in here if our count is // at least one. diff --git a/terraform/transform_resource_count.go b/terraform/transform_resource_count.go index 11237909f..c70a3c144 100644 --- a/terraform/transform_resource_count.go +++ b/terraform/transform_resource_count.go @@ -4,6 +4,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/dag" + "github.com/zclconf/go-cty/cty" ) // ResourceCountTransformer is a GraphTransformer that expands the count @@ -17,12 +18,13 @@ type ResourceCountTransformer struct { // Count is either the number of indexed instances to create, or -1 to // indicate that count is not set at all and thus a no-key instance should // be created. - Count int - Addr addrs.AbsResource + Count int + ForEach map[string]cty.Value + Addr addrs.AbsResource } func (t *ResourceCountTransformer) Transform(g *Graph) error { - if t.Count < 0 { + if t.Count < 0 && t.ForEach == nil { // Negative count indicates that count is not set at all. addr := t.Addr.Instance(addrs.NoKey) @@ -37,6 +39,19 @@ func (t *ResourceCountTransformer) Transform(g *Graph) error { return nil } + // Add nodes related to the for_each expression + for key := range t.ForEach { + addr := t.Addr.Instance(addrs.StringKey(key)) + abstract := NewNodeAbstractResourceInstance(addr) + abstract.Schema = t.Schema + var node dag.Vertex = abstract + if f := t.Concrete; f != nil { + node = f(abstract) + } + + g.Add(node) + } + // For each count, build and add the node for i := 0; i < t.Count; i++ { key := addrs.IntKey(i) diff --git a/website/docs/configuration/resources.html.md b/website/docs/configuration/resources.html.md index 1744a8d62..5c55439e6 100644 --- a/website/docs/configuration/resources.html.md +++ b/website/docs/configuration/resources.html.md @@ -143,7 +143,8 @@ Terraform CLI defines the following meta-arguments, which can be used with any resource type to change the behavior of resources: - [`depends_on`, for specifying hidden dependencies][inpage-depend] -- [`count`, for creating multiple resource instances][inpage-count] +- [`count`, for creating multiple resource instances according to a count][inpage-count] +- [`for_each`, to create multiple instances according to a map, or set of strings][inpage-for_each] - [`provider`, for selecting a non-default provider configuration][inpage-provider] - [`lifecycle`, for lifecycle customizations][inpage-lifecycle] - [`provisioner` and `connection`, for taking extra actions after resource creation][inpage-provisioner] @@ -221,9 +222,9 @@ The `depends_on` argument should be used only as a last resort. When using it, always include a comment explaining why it is being used, to help future maintainers understand the purpose of the additional dependency. -### `count`: Multiple Resource Instances +### `count`: Multiple Resource Instances By Count -[inpage-count]: #count-multiple-resource-instances +[inpage-count]: #count-multiple-resource-instances-by-count By default, a single `resource` block corresponds to only one real infrastructure object. Sometimes it is desirable to instead manage a set @@ -299,6 +300,57 @@ intended. The practice of generating multiple instances from lists should be used sparingly, and with due care given to what will happen if the list is changed later. +### `for_each`: Multiple Resource Instances Defined By a Map, or Set of Strings + +[inpage-for_each]: #for_each-multiple-resource-instances-defined-by-a-map-or-set-of-strings + +When the `for_each` meta-argument is present, Terraform will create instances +based on the keys and values present in a provided map, or set of strings, and expose the values +of the map to the resource for its configuration. + +The keys and values of the map, or strings in the case of a set, are exposed via the `each` attribute, +which can only be used in blocks with a `for_each` argument set. + +```hcl +resource "azurerm_resource_group" "rg" { + for_each = { + a_group = "eastus" + another_group = "westus2" + } + name = each.key + location = each.value +} +``` + +Resources created by `for_each` are identified by the key associated with the instance - +that is, if we have `azurerm_resource_group.rg` as above, the instances will be `azurerm_resource_group.rg["a_group"]` +and `azurerm_resource_group.rg["another_group"]`, as those are the keys in the map provided +to the `for_each` argument. + +The `for_each` argument also supports a set of strings in addition to maps; convert a list +to a set using the `toset` function. As such, we can take the example +in `count` and make it safer to use, as we can change items in our set +and because the string keys are used to identify the instances, +we will only change the items we intend to: + +```hcl +variable "subnet_ids" { + type = list(string) +} + +resource "aws_instance" "server" { + for_each = toset(var.subnet_ids) + + ami = "ami-a1b2c3d4" + instance_type = "t2.micro" + subnet_id = each.key # note, each.key and each.value will be the same on a set + + tags { + Name = "Server ${each.key}" + } +} +``` + ### `provider`: Selecting a Non-default Provider Configuration [inpage-provider]: #provider-selecting-a-non-default-provider-configuration From 7c678d104fe4fbc63260f02c1ffdc0809da630cd Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Thu, 25 Jul 2019 09:51:55 -0600 Subject: [PATCH 2/4] Add support for for_each for data blocks. This also fixes a few things with resource for_each: It makes validation more like validation for count. It makes sure the index is stored in the state properly. --- configs/parser_config_test.go | 5 ++ configs/resource.go | 16 +++--- .../invalid-files/data-count-and-for-each.tf | 4 ++ terraform/eval_for_each.go | 52 +++++++++++++------ terraform/eval_state.go | 14 +++-- terraform/eval_validate.go | 17 +++++- terraform/node_data_refresh.go | 12 +++++ 7 files changed, 91 insertions(+), 29 deletions(-) create mode 100644 configs/testdata/invalid-files/data-count-and-for-each.tf diff --git a/configs/parser_config_test.go b/configs/parser_config_test.go index 9c8b70ade..7b0192353 100644 --- a/configs/parser_config_test.go +++ b/configs/parser_config_test.go @@ -114,6 +114,11 @@ func TestParserLoadConfigFileFailureMessages(t *testing.T) { hcl.DiagError, `Invalid combination of "count" and "for_each"`, }, + { + "invalid-files/data-count-and-for_each.tf", + hcl.DiagError, + `Invalid combination of "count" and "for_each"`, + }, { "invalid-files/resource-lifecycle-badbool.tf", hcl.DiagError, diff --git a/configs/resource.go b/configs/resource.go index 2528a5583..edf822c1b 100644 --- a/configs/resource.go +++ b/configs/resource.go @@ -302,13 +302,15 @@ func decodeDataBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) { if attr, exists := content.Attributes["for_each"]; exists { r.ForEach = attr.Expr - // We currently parse this, but don't yet do anything with it. - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Reserved argument name in module block", - Detail: fmt.Sprintf("The name %q is reserved for use in a future version of Terraform.", attr.Name), - Subject: &attr.NameRange, - }) + // Cannot have count and for_each on the same data block + if r.Count != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid combination of "count" and "for_each"`, + Detail: `The "count" and "for_each" meta-arguments are mutually-exclusive, only one should be used to be explicit about the number of resources to be created.`, + Subject: &attr.NameRange, + }) + } } if attr, exists := content.Attributes["provider"]; exists { diff --git a/configs/testdata/invalid-files/data-count-and-for-each.tf b/configs/testdata/invalid-files/data-count-and-for-each.tf new file mode 100644 index 000000000..7cc476325 --- /dev/null +++ b/configs/testdata/invalid-files/data-count-and-for-each.tf @@ -0,0 +1,4 @@ +data "test" "foo" { + count = 2 + for_each = ["a"] +} diff --git a/terraform/eval_for_each.go b/terraform/eval_for_each.go index f9b2e35cc..a49d0686e 100644 --- a/terraform/eval_for_each.go +++ b/terraform/eval_for_each.go @@ -14,29 +14,50 @@ import ( // the expression is nil, and is used to distinguish between an unset for_each and an // empty map func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) { + forEachMap, known, diags := evaluateResourceForEachExpressionKnown(expr, ctx) + if !known { + // Currently this is a rather bad outcome from a UX standpoint, since we have + // no real mechanism to deal with this situation and all we can do is produce + // an error message. + // FIXME: In future, implement a built-in mechanism for deferring changes that + // can't yet be predicted, and use it to guide the user through several + // plan/apply steps until the desired configuration is eventually reached. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid forEach argument", + Detail: `The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the for_each depends on.`, + }) + panic("uh-oh") + } + return forEachMap, diags +} + +// evaluateResourceForEachExpressionKnown is like evaluateResourceForEachExpression +// except that it handles an unknown result by returning an empty map and +// a known = false, rather than by reporting the unknown value as an error +// diagnostic. +func evaluateResourceForEachExpressionKnown(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, known bool, diags tfdiags.Diagnostics) { if expr == nil { - return nil, nil + return nil, true, nil } forEachVal, forEachDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil) diags = diags.Append(forEachDiags) if diags.HasErrors() { - return nil, diags + return nil, true, diags } - // No-op for dynamic types, so that these pass validation, but are then populated at apply - if forEachVal.Type() == cty.DynamicPseudoType { - return nil, diags - } - - if forEachVal.IsNull() { + switch { + case forEachVal.IsNull(): diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid for_each argument", Detail: `The given "for_each" argument value is unsuitable: the given "for_each" argument value is null. A map, or set of strings is allowed.`, Subject: expr.Range().Ptr(), }) - return nil, diags + return nil, true, diags + case !forEachVal.IsKnown(): + return map[string]cty.Value{}, false, diags } if !forEachVal.CanIterateElements() || forEachVal.Type().IsListType() { @@ -46,7 +67,7 @@ func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (fo Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: the "for_each" argument must be a map, or set of strings, and you have provided a value of type %s.`, forEachVal.Type().FriendlyName()), Subject: expr.Range().Ptr(), }) - return nil, diags + return nil, true, diags } if forEachVal.Type().IsSetType() { @@ -57,17 +78,14 @@ func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (fo Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: "for_each" supports maps and sets of strings, but you have provided a set containing type %s.`, forEachVal.Type().ElementType().FriendlyName()), Subject: expr.Range().Ptr(), }) - return nil, diags + return nil, true, diags } } // If the map is empty ({}), return an empty map, because cty will return nil when representing {} AsValueMap - // Also return an empty map if the value is not known -- as this function - // is used to check if the for_each value is valid as well as to apply it, the empty - // map will later be filled in. - if !forEachVal.IsKnown() || forEachVal.LengthInt() == 0 { - return map[string]cty.Value{}, diags + if forEachVal.LengthInt() == 0 { + return map[string]cty.Value{}, true, diags } - return forEachVal.AsValueMap(), nil + return forEachVal.AsValueMap(), true, nil } diff --git a/terraform/eval_state.go b/terraform/eval_state.go index d506ce3fe..b611113e3 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -424,15 +424,21 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.Err() } - // Currently we ony support NoEach and EachList, because for_each support - // is not fully wired up across Terraform. Once for_each support is added, - // we'll need to handle that here too, setting states.EachMap if the - // assigned expression is a map. eachMode := states.NoEach if count >= 0 { // -1 signals "count not set" eachMode = states.EachList } + forEach, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx) + diags = diags.Append(forEachDiags) + if forEachDiags.HasErrors() { + return nil, diags.Err() + } + + if forEach != nil { + eachMode = states.EachMap + } + // This method takes care of all of the business logic of updating this // while ensuring that any existing instances are preserved, etc. state.SetResourceMeta(absAddr, eachMode, n.ProviderAddr) diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index 581a91b0e..9331143c5 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -377,7 +377,7 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) { } // Evaluate the for_each expression here so we can expose the diagnostics - _, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx) + forEachDiags := n.validateForEach(ctx, n.Config.ForEach) diags = diags.Append(forEachDiags) } @@ -553,3 +553,18 @@ func (n *EvalValidateResource) validateCount(ctx EvalContext, expr hcl.Expressio return diags } + +func (n *EvalValidateResource) validateForEach(ctx EvalContext, expr hcl.Expression) (diags tfdiags.Diagnostics) { + _, known, forEachDiags := evaluateResourceForEachExpressionKnown(expr, ctx) + // If the value isn't known then that's the best we can do for now, but + // we'll check more thoroughly during the plan walk + if !known { + return diags + } + + if forEachDiags.HasErrors() { + diags = diags.Append(forEachDiags) + } + + return diags +} diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go index ab8216341..dd9286648 100644 --- a/terraform/node_data_refresh.go +++ b/terraform/node_data_refresh.go @@ -38,6 +38,16 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er return nil, nil } + forEachMap, forEachKnown, forEachDiags := evaluateResourceForEachExpressionKnown(n.Config.ForEach, ctx) + if forEachDiags.HasErrors() { + return nil, diags.Err() + } + if !forEachKnown { + // If the for_each isn't known yet, we'll skip refreshing and try expansion + // again during the plan walk. + return nil, nil + } + // Next we need to potentially rename an instance address in the state // if we're transitioning whether "count" is set at all. fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1) @@ -77,6 +87,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er Concrete: concreteResource, Schema: n.Schema, Count: count, + ForEach: forEachMap, Addr: n.ResourceAddr(), }, @@ -85,6 +96,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er &OrphanResourceCountTransformer{ Concrete: concreteResourceDestroyable, Count: count, + ForEach: forEachMap, Addr: n.ResourceAddr(), State: state, }, From e7d8ac5ad705734cccbe3b35662df3b0d58fbe1e Mon Sep 17 00:00:00 2001 From: Pam Selle Date: Thu, 25 Jul 2019 17:10:13 -0400 Subject: [PATCH 3/4] Remove panic, update comment --- .tfdev | 5 +++++ ...a-count-and-for-each.tf => data-count-and-for_each.tf} | 0 configs/testdata/invalid-files/data-reserved-for_each.tf | 3 --- terraform/eval_for_each.go | 8 +------- 4 files changed, 6 insertions(+), 10 deletions(-) rename configs/testdata/invalid-files/{data-count-and-for-each.tf => data-count-and-for_each.tf} (100%) delete mode 100644 configs/testdata/invalid-files/data-reserved-for_each.tf diff --git a/.tfdev b/.tfdev index a04d5c102..8779d0181 100644 --- a/.tfdev +++ b/.tfdev @@ -6,3 +6,8 @@ version_info { version_exec = false disable_provider_requirements = true + +platform { + os = "darwin" + arch = "amd64" +} diff --git a/configs/testdata/invalid-files/data-count-and-for-each.tf b/configs/testdata/invalid-files/data-count-and-for_each.tf similarity index 100% rename from configs/testdata/invalid-files/data-count-and-for-each.tf rename to configs/testdata/invalid-files/data-count-and-for_each.tf diff --git a/configs/testdata/invalid-files/data-reserved-for_each.tf b/configs/testdata/invalid-files/data-reserved-for_each.tf deleted file mode 100644 index 1ccc01bfb..000000000 --- a/configs/testdata/invalid-files/data-reserved-for_each.tf +++ /dev/null @@ -1,3 +0,0 @@ -data "test" "foo" { - for_each = ["a"] -} diff --git a/terraform/eval_for_each.go b/terraform/eval_for_each.go index a49d0686e..b86bf3755 100644 --- a/terraform/eval_for_each.go +++ b/terraform/eval_for_each.go @@ -16,18 +16,12 @@ import ( func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) { forEachMap, known, diags := evaluateResourceForEachExpressionKnown(expr, ctx) if !known { - // Currently this is a rather bad outcome from a UX standpoint, since we have - // no real mechanism to deal with this situation and all we can do is produce - // an error message. - // FIXME: In future, implement a built-in mechanism for deferring changes that - // can't yet be predicted, and use it to guide the user through several - // plan/apply steps until the desired configuration is eventually reached. + // Attach a diag as we do with count, with the same downsides diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid forEach argument", Detail: `The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the for_each depends on.`, }) - panic("uh-oh") } return forEachMap, diags } From 1b25cb7d4a289d200a86ec04e7ac2d2c28e1ddbc Mon Sep 17 00:00:00 2001 From: Pam Selle Date: Fri, 26 Jul 2019 10:56:56 -0400 Subject: [PATCH 4/4] Docs updates for data resources, update expressions ref --- website/docs/configuration/data-sources.html.md | 7 ++++--- website/docs/configuration/expressions.html.md | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/website/docs/configuration/data-sources.html.md b/website/docs/configuration/data-sources.html.md index 131192122..1121b2167 100644 --- a/website/docs/configuration/data-sources.html.md +++ b/website/docs/configuration/data-sources.html.md @@ -129,10 +129,11 @@ resources. ## Multiple Resource Instances -Data resources support [the `count` meta-argument](./resources.html#count-multiple-resource-instances) -as defined for managed resources, with the same syntax and behavior. +Data resources support [`count`](./resources.html#count-multiple-resource-instances-by-count) +and [`for_each`](./resources.html#for_each-multiple-resource-instances-defined-by-a-map-or-set-of-strings) +meta-arguments as defined for managed resources, with the same syntax and behavior. -As with managed resources, when `count` is present it is important to +As with managed resources, when `count` or `for_each` is present it is important to distinguish the resource itself from the multiple resource _instances_ it creates. Each instance will separately read from its data source with its own variant of the constraint arguments, producing an indexed result. diff --git a/website/docs/configuration/expressions.html.md b/website/docs/configuration/expressions.html.md index 93491e5b5..67eb7ab1c 100644 --- a/website/docs/configuration/expressions.html.md +++ b/website/docs/configuration/expressions.html.md @@ -275,7 +275,7 @@ for use in references, as follows: `[for k, device in aws_instance.example.device : k => device.size]`. When a particular resource has the special -[`count`](https://www.terraform.io/docs/configuration/resources.html#count-multiple-resource-instances) +[`count`](https://www.terraform.io/docs/configuration/resources.html#count-multiple-resource-instances-by-count) argument set, the resource itself becomes a list of instance objects rather than a single object. In that case, access the attributes of the instances using either [splat expressions](#splat-expressions) or index syntax: