diff --git a/internal/terraform/eval_count.go b/internal/terraform/eval_count.go index 9e2144d9e0f6..3d1f091bf4e1 100644 --- a/internal/terraform/eval_count.go +++ b/internal/terraform/eval_count.go @@ -6,10 +6,11 @@ package terraform import ( "fmt" - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/tfdiags" ) // evaluateCountExpression is our standard mechanism for interpreting an @@ -20,9 +21,15 @@ import ( // evaluateCountExpression differs from evaluateCountExpressionValue by // returning an error if the count value is not known, and converting the // cty.Value to an integer. -func evaluateCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags.Diagnostics) { +// +// If allowUnknown is false then this function will return error diagnostics +// whenever the expression returns an unknown value. Setting allowUnknown to +// true instead permits unknown values, indicating them by returning the +// placeholder value -1. Callers can assume that a return value of -1 without +// any error diagnostics represents a valid unknown value. +func evaluateCountExpression(expr hcl.Expression, ctx EvalContext, allowUnknown bool) (int, tfdiags.Diagnostics) { countVal, diags := evaluateCountExpressionValue(expr, ctx) - if !countVal.IsKnown() { + if !allowUnknown && !countVal.IsKnown() { // 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. diff --git a/internal/terraform/eval_count_test.go b/internal/terraform/eval_count_test.go index 74bcfbdf721d..d4d785c37dc6 100644 --- a/internal/terraform/eval_count_test.go +++ b/internal/terraform/eval_count_test.go @@ -32,7 +32,7 @@ func TestEvaluateCountExpression(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - countVal, diags := evaluateCountExpression(test.Expr, ctx) + countVal, diags := evaluateCountExpression(test.Expr, ctx, false) if len(diags) != 0 { t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) @@ -47,3 +47,37 @@ func TestEvaluateCountExpression(t *testing.T) { }) } } + +func TestEvaluateCountExpression_allowUnknown(t *testing.T) { + tests := map[string]struct { + Expr hcl.Expression + Count int + }{ + "unknown number": { + hcltest.MockExprLiteral(cty.UnknownVal(cty.Number)), + -1, + }, + "dynamicval": { + hcltest.MockExprLiteral(cty.DynamicVal), + -1, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + ctx := &MockEvalContext{} + ctx.installSimpleEval() + countVal, diags := evaluateCountExpression(test.Expr, ctx, true) + + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + + if !reflect.DeepEqual(countVal, test.Count) { + t.Errorf( + "wrong result\ngot: %#v\nwant: %#v", + countVal, test.Count, + ) + } + }) + } +} diff --git a/internal/terraform/eval_for_each.go b/internal/terraform/eval_for_each.go index 3782234e9b0e..c567a543970f 100644 --- a/internal/terraform/eval_for_each.go +++ b/internal/terraform/eval_for_each.go @@ -19,20 +19,21 @@ import ( // evaluateForEachExpression differs from evaluateForEachExpressionValue by // returning an error if the count value is not known, and converting the // cty.Value to a map[string]cty.Value for compatibility with other calls. -func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) { - return newForEachEvaluator(expr, ctx).ResourceValue() +func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext, allowUnknown bool) (forEach map[string]cty.Value, known bool, diags tfdiags.Diagnostics) { + return newForEachEvaluator(expr, ctx, allowUnknown).ResourceValue() } // forEachEvaluator is the standard mechanism for interpreting an expression // given for a "for_each" argument on a resource, module, or import. -func newForEachEvaluator(expr hcl.Expression, ctx EvalContext) *forEachEvaluator { +func newForEachEvaluator(expr hcl.Expression, ctx EvalContext, allowUnknown bool) *forEachEvaluator { if ctx == nil { panic("nil EvalContext") } return &forEachEvaluator{ - ctx: ctx, - expr: expr, + ctx: ctx, + expr: expr, + allowUnknown: allowUnknown, } } @@ -48,44 +49,54 @@ type forEachEvaluator struct { ctx EvalContext expr hcl.Expression + // TEMP: If allowUnknown is set then we skip the usual restriction that + // unknown values are not allowed in for_each. A caller that sets this + // must therefore be ready to deal with the result being unknown. + // This will eventually become the default behavior, once we've updated + // the rest of this package to handle that situation in a reasonable way. + allowUnknown bool + // internal hclCtx *hcl.EvalContext } // ResourceForEachValue returns a known for_each map[string]cty.Value // appropriate for use within resource expansion. -func (ev *forEachEvaluator) ResourceValue() (map[string]cty.Value, tfdiags.Diagnostics) { +func (ev *forEachEvaluator) ResourceValue() (map[string]cty.Value, bool, tfdiags.Diagnostics) { res := map[string]cty.Value{} // no expression always results in an empty map if ev.expr == nil { - return res, nil + return res, true, nil } forEachVal, diags := ev.Value() if diags.HasErrors() { - return res, diags + return res, false, diags } // ensure our value is known for use in resource expansion - diags = diags.Append(ev.ensureKnownForResource(forEachVal)) - if diags.HasErrors() { - return res, diags + unknownDiags := ev.ensureKnownForResource(forEachVal) + if unknownDiags.HasErrors() { + if !ev.allowUnknown { + diags = diags.Append(unknownDiags) + } + return res, false, diags } // validate the for_each value for use in resource expansion diags = diags.Append(ev.validateResource(forEachVal)) if diags.HasErrors() { - return res, diags + return res, false, diags } if forEachVal.IsNull() || !forEachVal.IsKnown() || markSafeLengthInt(forEachVal) == 0 { // we check length, because an empty set returns a nil map which will panic below - return res, diags + return res, true, diags } res = forEachVal.AsValueMap() - return res, diags + return res, true, diags } // ImportValue returns the for_each map for use within an import block, diff --git a/internal/terraform/eval_for_each_test.go b/internal/terraform/eval_for_each_test.go index 3f7717d09015..6f4885b1d489 100644 --- a/internal/terraform/eval_for_each_test.go +++ b/internal/terraform/eval_for_each_test.go @@ -72,7 +72,7 @@ func TestEvaluateForEachExpression_valid(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - forEachMap, diags := evaluateForEachExpression(test.Expr, ctx) + forEachMap, _, diags := evaluateForEachExpression(test.Expr, ctx, false) if len(diags) != 0 { t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) @@ -176,7 +176,7 @@ func TestEvaluateForEachExpression_errors(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - _, diags := evaluateForEachExpression(test.Expr, ctx) + _, _, diags := evaluateForEachExpression(test.Expr, ctx, false) if len(diags) != 1 { t.Fatalf("got %d diagnostics; want 1", len(diags)) @@ -211,6 +211,41 @@ func TestEvaluateForEachExpression_errors(t *testing.T) { } } +func TestEvaluateForEachExpression_allowUnknown(t *testing.T) { + tests := map[string]struct { + Expr hcl.Expression + }{ + "unknown string set": { + hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))), + }, + "unknown map": { + hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))), + }, + "set containing unknown value": { + hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)})), + }, + "set containing dynamic unknown value": { + hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.DynamicPseudoType)})), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + ctx := &MockEvalContext{} + ctx.installSimpleEval() + _, known, diags := evaluateForEachExpression(test.Expr, ctx, true) + + // With allowUnknown set, all of these expressions should be treated + // as valid for_each values. + assertNoDiagnostics(t, diags) + + if known { + t.Errorf("result is known; want unknown") + } + }) + } +} + func TestEvaluateForEachExpressionKnown(t *testing.T) { tests := map[string]hcl.Expression{ "unknown string set": hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))), @@ -221,7 +256,7 @@ func TestEvaluateForEachExpressionKnown(t *testing.T) { t.Run(name, func(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - diags := newForEachEvaluator(expr, ctx).ValidateResourceValue() + diags := newForEachEvaluator(expr, ctx, false).ValidateResourceValue() if len(diags) != 0 { t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) diff --git a/internal/terraform/node_module_expand.go b/internal/terraform/node_module_expand.go index cb6f3a88bca1..529dd48dba86 100644 --- a/internal/terraform/node_module_expand.go +++ b/internal/terraform/node_module_expand.go @@ -113,7 +113,7 @@ func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) (diags tfd ctx = ctx.WithPath(module) switch { case n.ModuleCall.Count != nil: - count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, ctx) + count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, ctx, false) diags = diags.Append(ctDiags) if diags.HasErrors() { return diags @@ -121,7 +121,7 @@ func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) (diags tfd expander.SetModuleCount(module, call, count) case n.ModuleCall.ForEach != nil: - forEach, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx) + forEach, _, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx, false) diags = diags.Append(feDiags) if diags.HasErrors() { return diags @@ -256,7 +256,7 @@ func (n *nodeValidateModule) Execute(ctx EvalContext, op walkOperation) (diags t diags = diags.Append(countDiags) case n.ModuleCall.ForEach != nil: - forEachDiags := newForEachEvaluator(n.ModuleCall.ForEach, ctx).ValidateResourceValue() + forEachDiags := newForEachEvaluator(n.ModuleCall.ForEach, ctx, false).ValidateResourceValue() diags = diags.Append(forEachDiags) } diff --git a/internal/terraform/node_resource_abstract.go b/internal/terraform/node_resource_abstract.go index cbf4f1d16e40..b1708ce66b84 100644 --- a/internal/terraform/node_resource_abstract.go +++ b/internal/terraform/node_resource_abstract.go @@ -404,7 +404,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab switch { case n.Config != nil && n.Config.Count != nil: - count, countDiags := evaluateCountExpression(n.Config.Count, ctx) + count, countDiags := evaluateCountExpression(n.Config.Count, ctx, false) diags = diags.Append(countDiags) if countDiags.HasErrors() { return diags @@ -414,7 +414,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab expander.SetResourceCount(addr.Module, n.Addr.Resource, count) case n.Config != nil && n.Config.ForEach != nil: - forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx) + forEach, _, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, false) diags = diags.Append(forEachDiags) if forEachDiags.HasErrors() { return diags diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 696645a91b8f..b7d44b2c298c 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -777,7 +777,7 @@ func (n *NodeAbstractResourceInstance) plan( } // Evaluate the configuration - forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx) + forEach, _, _ := evaluateForEachExpression(n.Config.ForEach, ctx, false) keyData = EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach) @@ -1709,7 +1709,7 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule objTy := schema.ImpliedType() priorVal := cty.NullVal(objTy) - forEach, _ := evaluateForEachExpression(config.ForEach, ctx) + forEach, _, _ := evaluateForEachExpression(config.ForEach, ctx, false) keyData = EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach) checkDiags := evalCheckRules( @@ -1989,7 +1989,7 @@ func (n *NodeAbstractResourceInstance) applyDataSource(ctx EvalContext, planned return nil, keyData, diags } - forEach, _ := evaluateForEachExpression(config.ForEach, ctx) + forEach, _, _ := evaluateForEachExpression(config.ForEach, ctx, false) keyData = EvalDataForInstanceKey(n.Addr.Resource.Key, forEach) checkDiags := evalCheckRules( @@ -2293,7 +2293,7 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state func (n *NodeAbstractResourceInstance) evalProvisionerConfig(ctx EvalContext, body hcl.Body, self cty.Value, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx) + forEach, _, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, false) diags = diags.Append(forEachDiags) keyData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach) diff --git a/internal/terraform/node_resource_plan.go b/internal/terraform/node_resource_plan.go index 041d414293bd..4ab00e9f72f0 100644 --- a/internal/terraform/node_resource_plan.go +++ b/internal/terraform/node_resource_plan.go @@ -378,7 +378,7 @@ func (n nodeExpandPlannableResource) expandResourceImports(ctx EvalContext, addr continue } - forEachData, forEachDiags := newForEachEvaluator(imp.Config.ForEach, ctx).ImportValues() + forEachData, forEachDiags := newForEachEvaluator(imp.Config.ForEach, ctx, false).ImportValues() diags = diags.Append(forEachDiags) if forEachDiags.HasErrors() { return imports, diags diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 6322097d5ae4..d00b9f9bb998 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -355,7 +355,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) // values, which could result in a post-condition check relying on that // value being inaccurate. Unless we decide to store the value of the // for-each expression in state, this is unavoidable. - forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx) + forEach, _, _ := evaluateForEachExpression(n.Config.ForEach, ctx, false) repeatData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach) checkDiags := evalCheckRules( @@ -463,7 +463,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs. return nil, diags } - forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx) + forEach, _, _ := evaluateForEachExpression(n.Config.ForEach, ctx, false) keyData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach) configVal, _, configDiags := ctx.EvaluateBlock(n.Config.Config, schema, nil, keyData) if configDiags.HasErrors() { diff --git a/internal/terraform/node_resource_validate.go b/internal/terraform/node_resource_validate.go index 253bcb0097b6..9d6df7094578 100644 --- a/internal/terraform/node_resource_validate.go +++ b/internal/terraform/node_resource_validate.go @@ -306,7 +306,7 @@ func (n *NodeValidatableResource) validateResource(ctx EvalContext) tfdiags.Diag } // Evaluate the for_each expression here so we can expose the diagnostics - forEachDiags := newForEachEvaluator(n.Config.ForEach, ctx).ValidateResourceValue() + forEachDiags := newForEachEvaluator(n.Config.ForEach, ctx, false).ValidateResourceValue() diags = diags.Append(forEachDiags) }