Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A "unknown_instances" language experiment, allowing unknown values in count and for_each #34561

Merged
merged 5 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions internal/experiments/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ const (
SuppressProviderSensitiveAttrs = Experiment("provider_sensitive_attrs")
ConfigDrivenMove = Experiment("config_driven_move")
PreconditionsPostconditions = Experiment("preconditions_postconditions")
UnknownInstances = Experiment("unknown_instances")
)

func init() {
// Each experiment constant defined above must be registered here as either
// a current or a concluded experiment.
registerCurrentExperiment(UnknownInstances)
registerConcludedExperiment(VariableValidation, "Custom variable validation can now be used by default, without enabling an experiment.")
registerConcludedExperiment(SuppressProviderSensitiveAttrs, "Provider-defined sensitive attributes are now redacted by default, without enabling an experiment.")
registerConcludedExperiment(ConfigDrivenMove, "Declarations of moved resource instances using \"moved\" blocks can now be used by default, without enabling an experiment.")
Expand Down
6 changes: 6 additions & 0 deletions internal/terraform/eval_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/configs/configschema"
"github.com/hashicorp/terraform/internal/experiments"
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/lang"
"github.com/hashicorp/terraform/internal/moduletest/mocking"
Expand Down Expand Up @@ -134,6 +135,11 @@ type EvalContext interface {
// addresses in this context.
EvaluationScope(self addrs.Referenceable, source addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope

// LanguageExperimentActive returns true if the given experiment is
// active in the module associated with this EvalContext, or false
// otherwise.
LanguageExperimentActive(experiment experiments.Experiment) bool

// NamedValues returns the object that tracks the gradual evaluation of
// all input variables, local values, and output values during a graph
// walk.
Expand Down
19 changes: 19 additions & 0 deletions internal/terraform/eval_context_builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/configs/configschema"
"github.com/hashicorp/terraform/internal/experiments"
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/lang"
"github.com/hashicorp/terraform/internal/moduletest/mocking"
Expand Down Expand Up @@ -536,6 +537,24 @@ func (ctx *BuiltinEvalContext) Path() addrs.ModuleInstance {
return ctx.PathValue
}

func (ctx *BuiltinEvalContext) LanguageExperimentActive(experiment experiments.Experiment) bool {
if ctx.Evaluator == nil || ctx.Evaluator.Config == nil {
// Should not get here in normal code, but might get here in test code
// if the context isn't fully populated.
return false
}
if !ctx.pathSet {
// An EvalContext that isn't associated with a module path cannot
// have active experiments.
return false
}
cfg := ctx.Evaluator.Config.DescendentForInstance(ctx.Path())
if cfg == nil {
return false
}
return cfg.Module.ActiveExperiments.Has(experiment)
}

func (ctx *BuiltinEvalContext) NamedValues() *namedvals.State {
return ctx.NamedValuesValue
}
Expand Down
12 changes: 12 additions & 0 deletions internal/terraform/eval_context_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/configs/configschema"
"github.com/hashicorp/terraform/internal/experiments"
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/lang"
"github.com/hashicorp/terraform/internal/moduletest/mocking"
Expand Down Expand Up @@ -119,6 +120,8 @@ type MockEvalContext struct {
PathCalled bool
PathPath addrs.ModuleInstance

LanguageExperimentsActive experiments.Set

NamedValuesCalled bool
NamedValuesState *namedvals.State

Expand Down Expand Up @@ -334,6 +337,15 @@ func (c *MockEvalContext) Path() addrs.ModuleInstance {
return c.PathPath
}

func (c *MockEvalContext) LanguageExperimentActive(experiment experiments.Experiment) bool {
// This particular function uses a live data structure so that tests can
// exercise different experiments being enabled; there is little reason
// to directly test whether this function was called since we use this
// function only temporarily while an experiment is active, and then
// remove the calls once the experiment is concluded.
return c.LanguageExperimentsActive.Has(experiment)
}

func (c *MockEvalContext) NamedValues() *namedvals.State {
c.NamedValuesCalled = true
return c.NamedValuesState
Expand Down
15 changes: 11 additions & 4 deletions internal/terraform/eval_count.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
36 changes: 35 additions & 1 deletion internal/terraform/eval_count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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,
)
}
})
}
}
39 changes: 25 additions & 14 deletions internal/terraform/eval_for_each.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand All @@ -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,
Expand Down
41 changes: 38 additions & 3 deletions internal/terraform/eval_for_each_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))),
Expand All @@ -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))
Expand Down
36 changes: 36 additions & 0 deletions internal/terraform/instance_expanders.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,44 @@

package terraform

import (
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/instances"
)

// graphNodeExpandsInstances is implemented by nodes that causes instances to
// be registered in the instances.Expander.
type graphNodeExpandsInstances interface {
expandsInstances()
}

// forEachModuleInstance is a helper to deal with the common need of doing
// some action for every dynamic module instance associated with a static
// module path.
//
// Many of our plan graph nodes represent configuration constructs that need
// to produce a dynamic subgraph based on the expansion of whatever module
// they are declared inside, and this helper deals with enumerating those
// dynamic addresses so that callers can just focus on building a graph node
// for each one and registering it in the subgraph.
//
// Both of the two callbacks will be called for each instance or set of
// unknown instances. knownCb receives fully-known instance addresses,
// while unknownCb receives partially-expanded addresses. Callers typically
// create a different graph node type in each callback, because
// partially-expanded prefixes conceptually represent an infinite set of
// possible module instance addresses and therefore need quite different
// treatment than a single concrete module instance address.
func forEachModuleInstance(
insts *instances.Expander,
modAddr addrs.Module,
knownCb func(addrs.ModuleInstance),
unknownCb func(addrs.PartialExpandedModule),
) {
for _, instAddr := range insts.ExpandModule(modAddr) {
knownCb(instAddr)
}
for _, instsAddr := range insts.UnknownModuleInstances(modAddr) {
unknownCb(instsAddr)
}
}
Loading
Loading