diff --git a/internal/backend/backendrun/operation.go b/internal/backend/backendrun/operation.go index 7a2ce6086f..3ce59a9f4f 100644 --- a/internal/backend/backendrun/operation.go +++ b/internal/backend/backendrun/operation.go @@ -124,6 +124,13 @@ type Operation struct { // the variables set in the plan are used instead, and they must be valid. AllowUnsetVariables bool + // DeferralAllowed enables experimental support for automatically performing + // a partial plan if some objects are not yet plannable. + // + // IMPORTANT: When configuring an Operation, you should only set a value for + // this field if Terraform was built with experimental features enabled. + DeferralAllowed bool + // View implements the logic for all UI interactions. View views.Operation diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 5f1811a840..d34d76f769 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -204,6 +204,7 @@ func (b *Local) localRunDirect(op *backendrun.Operation, run *backendrun.LocalRu SetVariables: variables, SkipRefresh: op.Type != backendrun.OperationTypeRefresh && !op.PlanRefresh, GenerateConfigPath: op.GenerateConfigOut, + DeferralAllowed: op.DeferralAllowed, } run.PlanOpts = planOpts diff --git a/internal/command/apply.go b/internal/command/apply.go index b6a0f3a05d..e332f94b47 100644 --- a/internal/command/apply.go +++ b/internal/command/apply.go @@ -107,6 +107,10 @@ func (c *ApplyCommand) Run(rawArgs []string) int { // Build the operation request opReq, opDiags := c.OperationRequest(be, view, args.ViewType, planFile, args.Operation, args.AutoApprove) diags = diags.Append(opDiags) + if diags.HasErrors() { + view.Diagnostics(diags) + return 1 + } // Collect variable value and add them to the operation request diags = diags.Append(c.GatherVariables(opReq, args.Vars)) @@ -280,6 +284,20 @@ func (c *ApplyCommand) OperationRequest( opReq.Type = backendrun.OperationTypeApply opReq.View = view.Operation() + // EXPERIMENTAL: maybe enable deferred actions + if c.AllowExperimentalFeatures { + opReq.DeferralAllowed = args.DeferralAllowed + } else if args.DeferralAllowed { + // Belated flag parse error, since we don't know about experiments + // support at actual parse time. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + "The -allow-deferral flag is only valid in experimental builds of Terraform.", + )) + return nil, diags + } + var err error opReq.ConfigLoader, err = c.initConfigLoader() if err != nil { diff --git a/internal/command/arguments/extended.go b/internal/command/arguments/extended.go index abc90fd82a..6e9aadea2a 100644 --- a/internal/command/arguments/extended.go +++ b/internal/command/arguments/extended.go @@ -78,6 +78,18 @@ type Operation struct { // learn a use-case for broader matching. ForceReplace []addrs.AbsResourceInstance + // DeferralAllowed enables experimental support for automatically performing + // a partial plan if some objects are not yet plannable (due to unknown + // values in count/for_each, or due to other missing dependencies that can't + // be resolved in a single plan/apply cycle). + // + // IMPORTANT: This feature should only be available when Terraform is built + // with experimental features enabled. Since extendedFlagSet can't currently + // test whether experimental features are enabled, the check needs to happen + // when _reading_ these Operation arguments and transferring values to the + // backendrun.Operation struct. + DeferralAllowed bool + // These private fields are used only temporarily during decoding. Use // method Parse to populate the exported fields from these, validating // the raw values in the process. @@ -223,6 +235,7 @@ func extendedFlagSet(name string, state *State, operation *Operation, vars *Vars if operation != nil { f.IntVar(&operation.Parallelism, "parallelism", DefaultParallelism, "parallelism") + f.BoolVar(&operation.DeferralAllowed, "allow-deferral", false, "allow-deferral") f.BoolVar(&operation.Refresh, "refresh", true, "refresh") f.BoolVar(&operation.destroyRaw, "destroy", false, "destroy") f.BoolVar(&operation.refreshOnlyRaw, "refresh-only", false, "refresh-only") diff --git a/internal/command/plan.go b/internal/command/plan.go index f9423c79bf..056dfd3b97 100644 --- a/internal/command/plan.go +++ b/internal/command/plan.go @@ -165,6 +165,20 @@ func (c *PlanCommand) OperationRequest( opReq.Type = backendrun.OperationTypePlan opReq.View = view.Operation() + // EXPERIMENTAL: maybe enable deferred actions + if c.AllowExperimentalFeatures { + opReq.DeferralAllowed = args.DeferralAllowed + } else if args.DeferralAllowed { + // Belated flag parse error, since we don't know about experiments + // support at actual parse time. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + "The -allow-deferral flag is only valid in experimental builds of Terraform.", + )) + return nil, diags + } + var err error opReq.ConfigLoader, err = c.initConfigLoader() if err != nil { diff --git a/internal/command/refresh.go b/internal/command/refresh.go index 18bab4f25e..15160f98d0 100644 --- a/internal/command/refresh.go +++ b/internal/command/refresh.go @@ -147,6 +147,20 @@ func (c *RefreshCommand) OperationRequest(be backendrun.OperationsBackend, view opReq.Type = backendrun.OperationTypeRefresh opReq.View = view.Operation() + // EXPERIMENTAL: maybe enable deferred actions + if c.AllowExperimentalFeatures { + opReq.DeferralAllowed = args.DeferralAllowed + } else if args.DeferralAllowed { + // Belated flag parse error, since we don't know about experiments + // support at actual parse time. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + "The -allow-deferral flag is only valid in experimental builds of Terraform.", + )) + return nil, diags + } + var err error opReq.ConfigLoader, err = c.initConfigLoader() if err != nil { diff --git a/internal/experiments/experiment.go b/internal/experiments/experiment.go index 22365f51e6..3bfeb9cb41 100644 --- a/internal/experiments/experiment.go +++ b/internal/experiments/experiment.go @@ -27,7 +27,7 @@ const ( func init() { // Each experiment constant defined above must be registered here as either // a current or a concluded experiment. - registerCurrentExperiment(UnknownInstances) + registerConcludedExperiment(UnknownInstances, "Unknown instances are being rolled into a larger feature for deferring unready resources and modules.") 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.") diff --git a/internal/plans/deferring/deferred.go b/internal/plans/deferring/deferred.go index 9ca1612806..35c0bc52f5 100644 --- a/internal/plans/deferring/deferred.go +++ b/internal/plans/deferring/deferred.go @@ -34,6 +34,11 @@ type Deferred struct { // anyway due to its dependencies not yet being fully planned. resourceGraph addrs.DirectedGraph[addrs.ConfigResource] + // deferralAllowed marks whether deferred actions are supported by the + // current runtime. At time of writing, the modules runtime does not support + // deferral, but the stacks runtime does. + deferralAllowed bool + // externalDependencyDeferred marks the special situation where the // subsystem that's calling the modules runtime knows that some external // dependency of the configuration has deferred changes itself, and thus @@ -96,9 +101,10 @@ type Deferred struct { // // Callers must not modify anything reachable through resourceGraph after // calling this function. -func NewDeferred(resourceGraph addrs.DirectedGraph[addrs.ConfigResource]) *Deferred { +func NewDeferred(resourceGraph addrs.DirectedGraph[addrs.ConfigResource], enabled bool) *Deferred { return &Deferred{ resourceGraph: resourceGraph, + deferralAllowed: enabled, resourceInstancesDeferred: addrs.MakeMap[addrs.ConfigResource, addrs.Map[addrs.AbsResourceInstance, deferredResourceInstance]](), partialExpandedResourcesDeferred: addrs.MakeMap[addrs.ConfigResource, addrs.Map[addrs.PartialExpandedResource, deferredPartialExpandedResource]](), partialExpandedModulesDeferred: addrs.MakeSet[addrs.PartialExpandedModule](), @@ -117,6 +123,17 @@ func (d *Deferred) SetExternalDependencyDeferred() { d.externalDependencyDeferred = true } +// DeferralAllowed checks whether deferred actions are supported by the current +// runtime. +func (d *Deferred) DeferralAllowed() bool { + // Gracefully recover from being called on nil, for tests that use + // MockEvalContext without a real Deferred pointer set up. + if d == nil { + return false + } + return d.deferralAllowed +} + // HaveAnyDeferrals returns true if at least one deferral has been registered // with the receiver. // @@ -125,10 +142,11 @@ func (d *Deferred) SetExternalDependencyDeferred() { // as having their own changes deferred without having to duplicate the // modules runtime's rules for what counts as a deferral. func (d *Deferred) HaveAnyDeferrals() bool { - return d.externalDependencyDeferred || - d.resourceInstancesDeferred.Len() != 0 || - d.partialExpandedResourcesDeferred.Len() != 0 || - len(d.partialExpandedModulesDeferred) != 0 + return d.deferralAllowed && + (d.externalDependencyDeferred || + d.resourceInstancesDeferred.Len() != 0 || + d.partialExpandedResourcesDeferred.Len() != 0 || + len(d.partialExpandedModulesDeferred) != 0) } // ShouldDeferResourceChanges returns true if the receiver knows some reason diff --git a/internal/plans/deferring/deferred_test.go b/internal/plans/deferring/deferred_test.go index f1f5b958b3..2f9e8a16be 100644 --- a/internal/plans/deferring/deferred_test.go +++ b/internal/plans/deferring/deferred_test.go @@ -16,7 +16,7 @@ func TestDeferred_externalDependency(t *testing.T) { // defer any resource instance changes regardless. Therefore an empty // graph is just fine. resourceGraph := addrs.NewDirectedGraph[addrs.ConfigResource]() - deferred := NewDeferred(resourceGraph) + deferred := NewDeferred(resourceGraph, true) // This reports that something outside of the modules runtime knows that // everything in this configuration depends on some elsewhere-action @@ -76,7 +76,7 @@ func TestDeferred_absResourceInstanceDeferred(t *testing.T) { resourceGraph := addrs.NewDirectedGraph[addrs.ConfigResource]() resourceGraph.AddDependency(instCAddr.ConfigResource(), instBAddr.ConfigResource()) resourceGraph.AddDependency(instCAddr.ConfigResource(), instAAddr.ConfigResource()) - deferred := NewDeferred(resourceGraph) + deferred := NewDeferred(resourceGraph, true) // Before we report anything, all three addresses should indicate that // they don't need to have their actions deferred. @@ -139,7 +139,7 @@ func TestDeferred_partialExpandedResource(t *testing.T) { resourceGraph := addrs.NewDirectedGraph[addrs.ConfigResource]() resourceGraph.AddDependency(instCAddr.ConfigResource(), instBAddr.ConfigResource()) resourceGraph.AddDependency(instCAddr.ConfigResource(), instAAddr.ConfigResource()) - deferred := NewDeferred(resourceGraph) + deferred := NewDeferred(resourceGraph, true) // Before we report anything, all three addresses should indicate that // they don't need to have their actions deferred. diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 2aa81088f4..6d1d3ef8e5 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -585,6 +585,7 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla Mode: stackPlanOpts.PlanningMode, SetVariables: inputValues, ExternalProviders: providerClients, + DeferralAllowed: stackPlanOpts.DeferralAllowed, ExternalDependencyDeferred: upstreamDeferred, // This is set by some tests but should not be used in main code. diff --git a/internal/stacks/stackruntime/internal/stackeval/planning.go b/internal/stacks/stackruntime/internal/stackeval/planning.go index 3a20b9a814..1fcf80d615 100644 --- a/internal/stacks/stackruntime/internal/stackeval/planning.go +++ b/internal/stacks/stackruntime/internal/stackeval/planning.go @@ -16,6 +16,13 @@ import ( type PlanOpts struct { PlanningMode plans.Mode + // DeferralAllowed is the counterpart of the field of the same name on + // terraform.PlanOpts. + // TODO: We actually want stacks to always allow deferred + // actions, but the feature needs more time in the oven before + // it can be enabled without regressions. + DeferralAllowed bool + InputVariableValues map[stackaddrs.InputVariable]ExternalInputValue ProviderFactories ProviderFactories diff --git a/internal/stacks/stackruntime/internal/stackeval/planning_test.go b/internal/stacks/stackruntime/internal/stackeval/planning_test.go index c61d72c876..7ef5200a21 100644 --- a/internal/stacks/stackruntime/internal/stackeval/planning_test.go +++ b/internal/stacks/stackruntime/internal/stackeval/planning_test.go @@ -593,6 +593,12 @@ func TestPlanning_DeferredChangesPropagation(t *testing.T) { cfg := testStackConfig(t, "planning", "deferred_changes_propagation") main := NewForPlanning(cfg, stackstate.NewState(), PlanOpts{ PlanningMode: plans.NormalMode, + // TEMP: Currently there's no way in normal operation to set this to + // true in the PlanOpts, because it would regress other features. So, + // the test has to set it manually. In the future, deferred actions will + // always be enabled for stacks, and we'll remove this option from the + // stackeval.PlanOpts struct. + DeferralAllowed: true, InputVariableValues: map[stackaddrs.InputVariable]ExternalInputValue{ // This causes the first component to have a module whose // instance count isn't known yet. @@ -617,10 +623,6 @@ func TestPlanning_DeferredChangesPropagation(t *testing.T) { }, }, }) - // TEMP: This test currently relies on the experimental module language - // feature of allowing unknown values in a resource's "count" argument. - // We should remove this if the experiment gets stabilized. - main.AllowLanguageExperiments(true) componentFirstInstAddr := stackaddrs.AbsComponentInstance{ Stack: stackaddrs.RootStackInstance, diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tf b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tf index 1760edf921..91dc06d332 100644 --- a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tf +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tf @@ -5,12 +5,6 @@ terraform { source = "terraform.io/builtin/test" } } - - # TODO: Remove this if this experiment gets stabilized. - # If you're removing this, remember to also update the calling test so - # that it no longer enables the use of experiments, to ensure that we're - # really not depending on any experimental features. - experiments = [unknown_instances] } variable "instance_count" { diff --git a/internal/terraform/context_apply_deferred_test.go b/internal/terraform/context_apply_deferred_test.go index adc8a951b4..f2638c57f6 100644 --- a/internal/terraform/context_apply_deferred_test.go +++ b/internal/terraform/context_apply_deferred_test.go @@ -70,12 +70,6 @@ var ( resourceForEachTest = deferredActionsTest{ configs: map[string]string{ "main.tf": ` -// TEMP: unknown for_each currently requires an experiment opt-in. -// We should remove this block if the experiment gets stabilized. -terraform { - experiments = [unknown_instances] -} - variable "each" { type = set(string) } @@ -379,7 +373,8 @@ func TestContextApply_deferredActions(t *testing.T) { }) plan, diags := ctx.Plan(cfg, state, &PlanOpts{ - Mode: plans.NormalMode, + Mode: plans.NormalMode, + DeferralAllowed: true, SetVariables: func() InputValues { values := InputValues{} for name, value := range stage.inputs { diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index 7facb219c4..cdc1ad9fcd 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -77,6 +77,17 @@ type PlanOpts struct { // fully-functional new object. ForceReplace []addrs.AbsResourceInstance + // DeferralAllowed specifies that the plan is allowed to defer some actions, + // so that a subset of the plan can be applied even if parts of it can't yet + // be planned at all. Plans that contain deferred actions can't converge in + // a single run, and their configuration must be planned again after the + // dependencies of their deferred objects are in a usable state. Various + // events can cause deferrals, including unknown values in count and + // for_each arguments, and deferral notices from providers. If + // DeferralAllowed is false, the plan will error upon encountering an object + // that would be unplannable until after the apply. + DeferralAllowed bool + // ExternalReferences allows the external caller to pass in references to // nodes that should not be pruned even if they are not referenced within // the actual graph. @@ -674,6 +685,7 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, o Config: config, InputState: prevRunState, ExternalProviderConfigs: externalProviderConfigs, + DeferralAllowed: opts.DeferralAllowed, ExternalDependencyDeferred: opts.ExternalDependencyDeferred, Changes: changes, MoveResults: moveResults, diff --git a/internal/terraform/context_plan2_test.go b/internal/terraform/context_plan2_test.go index 6cbcc58fa2..2433b36a3e 100644 --- a/internal/terraform/context_plan2_test.go +++ b/internal/terraform/context_plan2_test.go @@ -4818,6 +4818,7 @@ func TestContext2Apply_externalDependencyDeferred(t *testing.T) { plan, diags := ctx.Plan(cfg, states.NewState(), &PlanOpts{ Mode: plans.NormalMode, + DeferralAllowed: true, ExternalDependencyDeferred: true, }) assertNoDiagnostics(t, diags) diff --git a/internal/terraform/context_walk.go b/internal/terraform/context_walk.go index ccdaf7bf70..831e422f70 100644 --- a/internal/terraform/context_walk.go +++ b/internal/terraform/context_walk.go @@ -44,6 +44,9 @@ type graphWalkOpts struct { // always take into account what walk type it's dealing with. ExternalProviderConfigs map[addrs.RootProviderConfig]providers.Interface + // DeferralAlowed indicates that the current runtime supports deferred actions. + DeferralAllowed bool + // ExternalDependencyDeferred indicates that something that this entire // configuration depends on (outside the view of this modules runtime) // has deferred changes, and therefore we must treat _all_ actions @@ -168,7 +171,7 @@ func (c *Context) graphWalker(graph *Graph, operation walkOperation, opts *graph // We'll produce a derived graph that only includes the static resource // blocks, since we need that for deferral tracking. resourceGraph := graph.ResourceGraph() - deferred := deferring.NewDeferred(resourceGraph) + deferred := deferring.NewDeferred(resourceGraph, opts.DeferralAllowed) if opts.ExternalDependencyDeferred { deferred.SetExternalDependencyDeferred() } diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 9be98c34b6..235a7f4974 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -574,20 +574,21 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc return cty.DynamicVal, diags } - // Much of this function was written before we had factored out the - // handling of instance keys into the separate instance expander model, - // and so it does a bunch of instance-related work itself below. While - // the possibility of unknown instance keys remains experimental - // (behind the unknown_instances language experiment) we'll use this - // function only for detecting that experimental situation, but leave - // the rest of this function unchanged for now to minimize the chances - // of the experiment code affecting someone who isn't participating. + // Much of this function was written before we had factored out the handling + // of instance keys into the separate instance expander model, and so it + // does a bunch of instance-related work itself below. // - // TODO: If we decide to stabilize the unknown_instances experiment - // then it would be nice to finally rework this function to rely - // on the ResourceInstanceKeys result for _all_ of its work, rather - // than continuing to duplicate a bunch of the logic we've tried to - // encapsulate over ther already. + // Currently, unknown instance keys are only possible when planning with + // DeferralAllowed set to true in the PlanOpts, which should only be the + // case in the stacks runtime (not the "normal terraform" modules runtime). + // Thus, we have some amount of duplicated code remaining, to be more + // certain that stacks-specific behaviors won't leak out into the standard + // runtime. + // + // TODO: When deferred actions are more stable and robust in stacks, it + // would be nice to rework this function to rely on the ResourceInstanceKeys + // result for _all_ of its work, rather than continuing to duplicate a bunch + // of the logic we've tried to encapsulate over ther already. if d.Operation == walkPlan { if _, _, hasUnknownKeys := d.Evaluator.Instances.ResourceInstanceKeys(addr.Absolute(moduleAddr)); hasUnknownKeys { // There really isn't anything interesting we can do in this situation, diff --git a/internal/terraform/node_module_expand.go b/internal/terraform/node_module_expand.go index 7d1e1822f9..379e7ec930 100644 --- a/internal/terraform/node_module_expand.go +++ b/internal/terraform/node_module_expand.go @@ -9,7 +9,6 @@ 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/langrefs" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -107,21 +106,19 @@ func (n *nodeExpandModule) Execute(globalCtx EvalContext, op walkOperation) (dia expander := globalCtx.InstanceExpander() _, call := n.Addr.Call() + // Allowing unknown values in count and for_each is a top-level plan option. + // + // 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 := globalCtx.Deferrals().DeferralAllowed() + // 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. for _, module := range expander.ExpandModule(n.Addr.Parent()) { moduleCtx := evalContextForModuleInstance(globalCtx, module) - // 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 := moduleCtx.LanguageExperimentActive(experiments.UnknownInstances) - switch { case n.ModuleCall.Count != nil: count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, moduleCtx, allowUnknown) diff --git a/internal/terraform/node_resource_abstract.go b/internal/terraform/node_resource_abstract.go index c07c5152c6..c29e74c2aa 100644 --- a/internal/terraform/node_resource_abstract.go +++ b/internal/terraform/node_resource_abstract.go @@ -11,7 +11,6 @@ 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/langrefs" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" @@ -83,13 +82,6 @@ type NodeAbstractResource struct { // generateConfigPath tells this node which file to write generated config // into. If empty, then config should not be generated. generateConfigPath string - - // TEMP: [ConfigTransformer] sets this to true when at least one module - // in the configuration has opted in to the unknown_instances experiment. - // See the field of the same name in [ConfigTransformer] for more details. - // (And if that field has been removed already, then this one should've - // been too!) - unknownInstancesExperimentEnabled bool } var ( @@ -410,14 +402,12 @@ 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. + // Allowing unknown values in count and for_each is a top-level plan option. // // 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) + allowUnknown := ctx.Deferrals().DeferralAllowed() switch { case n.Config != nil && n.Config.Count != nil: diff --git a/internal/terraform/node_resource_plan.go b/internal/terraform/node_resource_plan.go index 2ed9ac4152..09c9ff88f7 100644 --- a/internal/terraform/node_resource_plan.go +++ b/internal/terraform/node_resource_plan.go @@ -108,28 +108,15 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, tf expander := ctx.InstanceExpander() moduleInstances := expander.ExpandModule(n.Addr.Module) - // The possibility of partial-expanded modules and resources is - // currently guarded by a language experiment, and so to minimize the - // risk of that experiment impacting mainline behavior we currently - // branch off into an entirely-separate codepath in those situations, - // at the expense of duplicating some of the logic for behavior this - // method would normally handle. - // - // Normally language experiments are confined to only a single module, - // but this one has potential cross-module impact once enabled for at - // least one, and so this flag is true if _any_ module in the configuration - // has opted in to the experiment. Our intent is for this different - // codepath to produce the same results when there aren't any - // partial-expanded modules, but bugs might make that not true and so - // this is conservative to minimize the risk of breaking things for - // those who aren't participating in the experiment. - // - // TODO: If this experiment is stablized then we should aim to combine - // these two codepaths back together, so that the behavior is less likely - // to diverge under future maintenence. - if n.unknownInstancesExperimentEnabled { + // The possibility of partial-expanded modules and resources is guarded by a + // top-level option for the whole plan, so that we can preserve mainline + // behavior for the modules runtime. So, we currently branch off into an + // entirely-separate codepath in those situations, at the expense of + // duplicating some of the logic for behavior this method would normally + // handle. + if ctx.Deferrals().DeferralAllowed() { pem := expander.UnknownModuleInstances(n.Addr.Module) - return n.dynamicExpandWithUnknownInstancesExperiment(ctx, moduleInstances, pem) + return n.dynamicExpandWithDeferralAllowed(ctx, moduleInstances, pem) } // Lock the state while we inspect it @@ -218,22 +205,14 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, tf return &g, diags } -// dynamicExpandWithUnknownInstancesExperiment is a temporary experimental -// variant of DynamicExpand that we use when at least one module is -// participating in the "unknown_instances" language experiment. -// -// This is not exactly in the typical spirit of language experiments in that -// the effect is not scoped only to the module where the opt-in is declared: -// if there are bugs in this method then they could potentially also affect -// resources in modules not directly participating. We're accepting that -// as a pragmatic compromise here since unknown expansion of a module call -// is inherently a cross-module concern. +// dynamicExpandWithDeferralAllowed is a variant of DynamicExpand that we use +// when deferred actions are enabled for the current plan. // -// If we move forward with unknown instances as a stable feature then we -// should find a way to meld this logic with the main DynamicExpand logic, -// but it's separate for now to minimize the risk of the experiment impacting -// configurations that are not opted into it. -func (n *nodeExpandPlannableResource) dynamicExpandWithUnknownInstancesExperiment(globalCtx EvalContext, knownInsts []addrs.ModuleInstance, partialInsts addrs.Set[addrs.PartialExpandedModule]) (*Graph, tfdiags.Diagnostics) { +// Once deferred actions are more stable and robust in the stacks runtime, it +// would be nice to integrate this logic a little better with the main +// DynamicExpand logic, but it's separate for now to minimize the risk of +// stacks-specific behavior impacting configurations that are not opted into it. +func (n *nodeExpandPlannableResource) dynamicExpandWithDeferralAllowed(globalCtx EvalContext, knownInsts []addrs.ModuleInstance, partialInsts addrs.Set[addrs.PartialExpandedModule]) (*Graph, tfdiags.Diagnostics) { var g Graph var diags tfdiags.Diagnostics diff --git a/internal/terraform/transform_config.go b/internal/terraform/transform_config.go index 878fbc85ce..ead1adb53d 100644 --- a/internal/terraform/transform_config.go +++ b/internal/terraform/transform_config.go @@ -11,7 +11,6 @@ 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/tfdiags" ) @@ -51,14 +50,6 @@ type ConfigTransformer struct { // try to delete the imported resource unless the config is updated // manually. generateConfigPathForImportTargets string - - // TEMP: [ConfigTransformer.Transform] sets this to true if at least one - // module in the configuration has the "unknown_instances" language - // experiment enabled, because this particular experiment has cross-module - // implications (a module call with unknown instances affects everything - // beneath it in the tree) but we want to avoid activating the experimental - // code in the common case where no module is using it at all. - unknownInstancesExperimentEnabled bool } func (t *ConfigTransformer) Transform(g *Graph) error { @@ -71,16 +62,6 @@ func (t *ConfigTransformer) Transform(g *Graph) error { return nil } - // TEMP: Before we go further, we'll decide whether we're going to activate - // the experimental new behavior for the "unknown_instances" experiment. - // See the docstring for [ConfigTransformer.unknownInstancesExperimentEnabled] - // for more details. - t.Config.DeepEach(func(c *configs.Config) { - if c.Module != nil && c.Module.ActiveExperiments.Has(experiments.UnknownInstances) { - t.unknownInstancesExperimentEnabled = true - } - }) - // Start the transformation process return t.transform(g, t.Config) } @@ -180,10 +161,6 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config) er Module: path, }, importTargets: imports, - - // TEMP: See the docs for this field in [ConfigTransformer] for - // more information. - unknownInstancesExperimentEnabled: t.unknownInstancesExperimentEnabled, } var node dag.Vertex = abstract