From baa0f7805e8b06b9881130c1ed133d1fc3588793 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 23 Jan 2024 16:47:05 -0800 Subject: [PATCH] terraform: Modules can opt in to allowing unknown_instances When the unknown_instances language experiment is active, Terraform will accept unknown values in count and for_each arguments for resource, data, and module blocks. The unknown-ness of the expansion will be registered with the instance expander for later use, but much of the rest of the modules runtime is not yet equipped to deal with that situation, and so as of this commit the experiment will just cause some broken behavior if used. There are therefore no integration tests for this behavior just yet, because there's no reasonable expected behavior for such tests to assert. Behavior when the experiment is not enabled should remain unchanged. Proper handling of the unknown-expansion situation will develop over subsequent commits, along with integration tests once there's enough in place to write them, with this experiment being concluded only after all of that work has completed. --- internal/terraform/node_module_expand.go | 27 +++++++++++++++++--- internal/terraform/node_resource_abstract.go | 27 +++++++++++++++++--- internal/terraform/node_resource_plan.go | 2 +- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/internal/terraform/node_module_expand.go b/internal/terraform/node_module_expand.go index 529dd48dba86..f9cb8f46b37d 100644 --- a/internal/terraform/node_module_expand.go +++ b/internal/terraform/node_module_expand.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/dag" + "github.com/hashicorp/terraform/internal/experiments" "github.com/hashicorp/terraform/internal/lang" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -106,6 +107,15 @@ func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) (diags tfd expander := ctx.InstanceExpander() _, call := n.Addr.Call() + // Allowing unknown values in count and for_each is currently only an + // experimental feature. This will hopefully become the default (and only) + // behavior in future, if the experiment is successful. + // + // If this is false then the codepaths that handle unknown values below + // become unreachable, because the evaluate functions will reject unknown + // values as an error. + allowUnknown := ctx.LanguageExperimentActive(experiments.UnknownInstances) + // nodeExpandModule itself does not have visibility into how its ancestors // were expanded, so we use the expander here to provide all possible paths // to our module, and register module instances with each of them. @@ -113,20 +123,29 @@ 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, false) + count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, ctx, allowUnknown) diags = diags.Append(ctDiags) if diags.HasErrors() { return diags } - expander.SetModuleCount(module, call, count) + if count >= 0 { + expander.SetModuleCount(module, call, count) + } else { + // -1 represents "unknown" + expander.SetModuleCountUnknown(module, call) + } case n.ModuleCall.ForEach != nil: - forEach, _, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx, false) + forEach, known, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx, allowUnknown) diags = diags.Append(feDiags) if diags.HasErrors() { return diags } - expander.SetModuleForEach(module, call, forEach) + if known { + expander.SetModuleForEach(module, call, forEach) + } else { + expander.SetModuleForEachUnknown(module, call) + } default: expander.SetModuleSingle(module, call) diff --git a/internal/terraform/node_resource_abstract.go b/internal/terraform/node_resource_abstract.go index b1708ce66b84..9af4acb5d56e 100644 --- a/internal/terraform/node_resource_abstract.go +++ b/internal/terraform/node_resource_abstract.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/dag" + "github.com/hashicorp/terraform/internal/experiments" "github.com/hashicorp/terraform/internal/lang" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" @@ -402,19 +403,33 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab // to expand the module here to create all resources. expander := ctx.InstanceExpander() + // Allowing unknown values in count and for_each is currently only an + // experimental feature. This will hopefully become the default (and only) + // behavior in future, if the experiment is successful. + // + // If this is false then the codepaths that handle unknown values below + // become unreachable, because the evaluate functions will reject unknown + // values as an error. + allowUnknown := ctx.LanguageExperimentActive(experiments.UnknownInstances) + switch { case n.Config != nil && n.Config.Count != nil: - count, countDiags := evaluateCountExpression(n.Config.Count, ctx, false) + count, countDiags := evaluateCountExpression(n.Config.Count, ctx, allowUnknown) diags = diags.Append(countDiags) if countDiags.HasErrors() { return diags } state.SetResourceProvider(addr, n.ResolvedProvider) - expander.SetResourceCount(addr.Module, n.Addr.Resource, count) + if count >= 0 { + expander.SetResourceCount(addr.Module, n.Addr.Resource, count) + } else { + // -1 represents "unknown" + expander.SetResourceCountUnknown(addr.Module, n.Addr.Resource) + } case n.Config != nil && n.Config.ForEach != nil: - forEach, _, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, false) + forEach, known, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, allowUnknown) diags = diags.Append(forEachDiags) if forEachDiags.HasErrors() { return diags @@ -423,7 +438,11 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab // This method takes care of all of the business logic of updating this // while ensuring that any existing instances are preserved, etc. state.SetResourceProvider(addr, n.ResolvedProvider) - expander.SetResourceForEach(addr.Module, n.Addr.Resource, forEach) + if known { + expander.SetResourceForEach(addr.Module, n.Addr.Resource, forEach) + } else { + expander.SetResourceForEachUnknown(addr.Module, n.Addr.Resource) + } default: state.SetResourceProvider(addr, n.ResolvedProvider) diff --git a/internal/terraform/node_resource_plan.go b/internal/terraform/node_resource_plan.go index 4ab00e9f72f0..8825160c41d0 100644 --- a/internal/terraform/node_resource_plan.go +++ b/internal/terraform/node_resource_plan.go @@ -218,7 +218,7 @@ func (n *nodeExpandPlannableResource) validateExpandedImportTargets() tfdiags.Di // // It has several side-effects: // - Adds a node to Graph g for each leaf resource instance it discovers, whether present or orphaned. -// - Registers the expansion of the resource in the "expander" object embedded inside EvalContext ctx. +// - Registers the expansion of the resource in the "expander" object embedded inside EvalContext globalCtx. // - Adds each present (non-orphaned) resource instance address to checkableAddrs (guaranteed to always be addrs.AbsResourceInstance, despite being declared as addrs.Checkable). // // After calling this for each of the module instances the resource appears