From 524da4f278eeb14d95ac86176a842fbcd20df81f Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 8 Dec 2023 18:43:52 -0800 Subject: [PATCH] stackeval: Applyable objects have a set of required components To figure out the required order in which we'll apply the plans for all components we need to know how data flows between them. We'll use what's added here during the plan phase to record in the plan which components depend on which other components, and then the apply phase will use that coarse information to schedule the individual component apply operations into a suitable order. None of that is implemented here though, and will follow in future commits. This commit includes new documentation about the apply scheduling approach which includes some mechanisms that are not actually yet included in this commit, but will follow shortly after. This is just a pragmatic compromise to avoid spending time authoring short-lived partial documentation drafts for the intermediate steps while still keeping these increments small enough for practical review. --- .../stackruntime/internal/stackeval/README.md | 62 ++++++ .../internal/stackeval/applying.go | 13 ++ .../internal/stackeval/component.go | 21 +- .../internal/stackeval/component_instance.go | 8 +- .../internal/stackeval/expression_refs.go | 162 +++++++++++++++ .../internal/stackeval/expressions.go | 44 ---- .../internal/stackeval/input_variable.go | 39 +++- .../stackruntime/internal/stackeval/main.go | 21 +- .../internal/stackeval/output_value.go | 21 +- .../internal/stackeval/planning_test.go | 188 ++++++++++++++++++ .../internal/stackeval/provider.go | 20 +- .../internal/stackeval/provider_instance.go | 11 +- .../stackruntime/internal/stackeval/stack.go | 13 +- .../internal/stackeval/stack_call.go | 20 +- .../internal/stackeval/stack_call_instance.go | 13 +- .../required-components-child.tfstack.hcl | 9 + .../module/required-components.tf | 8 + .../required-components.tfstack.hcl | 50 +++++ 18 files changed, 660 insertions(+), 63 deletions(-) create mode 100644 internal/stacks/stackruntime/internal/stackeval/expression_refs.go create mode 100644 internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/child/required-components-child.tfstack.hcl create mode 100644 internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/module/required-components.tf create mode 100644 internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/required-components.tfstack.hcl diff --git a/internal/stacks/stackruntime/internal/stackeval/README.md b/internal/stacks/stackruntime/internal/stackeval/README.md index 9fb849e72f..60412bf39d 100644 --- a/internal/stacks/stackruntime/internal/stackeval/README.md +++ b/internal/stacks/stackruntime/internal/stackeval/README.md @@ -359,3 +359,65 @@ flow. The runtime achieves this by having any operation that depends on expensive or side-effect-ish work from another object pass the data using the promises and tasks model implemented by [package `promising`](../../../../promising/README.md). + +## Apply-phase Scheduling + +During the validation and planning operations the order of work is driven +entirely by the dynamically-constructed data flow graph that gets assembled +automatically based on control flow between the different functions in this +package. That works under the assumption that those phases should not be +modifying anything outside of Terraform itself and so our only concern is +ensuring that data is available at the appropriate time for other functions +that will make use of it. + +However, the apply phase deals with externally-visible side-effects whose +relative ordering is very important. For example, in some remote APIs an +attempt to destroy one object before destroying another object that depends +on it will either fail with an error or hang until a timeout is reached, and +so it's crucially important that Terraform directly consider the sequence +of operations to make sure that situation cannot possibly arise, even if +the relationship is not implied naturally by data flow. + +We deal with those additional requirements with both an additional scheduling +primitive -- function `ChangeExec` -- and with some explicit dependency data +gathered during the planning phase. + +In practice, it's only _components_ that represent operations with explicit +ordering constraints, because nothing else in the stacks runtime directly +interacts with Terraform's resource instance change lifecycle. Therefore +we can achieve a correct result with only a graph of dependencies between +components, without considering any other objects. Interface `Applyable` +includes the method `RequiredComponents`, which must return a set of all +of the components that a particular applyable object depends on. + +In practice, most of our implementations of `Applyable.RequiredComponents` +wrap a single implementation that works in terms of interface `Referrer`, which +works at a lower level of abstraction that deals only in HCL-level expression +references, regardless of what object types they refer to. The shared +implementation then raises the graph of references into a graph of components +by essentially removing the non-component nodes while preserving the +edges between them. + +Once the plan phase has derived the relationships between components, it +includes that information as part of the plan, so that it's immediately ready +to use in the apply phase without any further graph construction. + +The apply phase then uses the `ChangeExec` function to actually schedule the +changes. That function's own documentation contains more documentation about +its usage, but at a high level it wraps the concepts from +[package `promising`](../../../../promising/README.md) in such a way that +it can oversee the execution of each of the individual component instance apply +phases, and capture the results in a central place for downstream work to +refer to. Each component instance is represented by a single task which blocks +on the completion of the promise of each component it depends on, thus explicitly +ensuring that the component instance changes get applied in the correct +order relative to one another. + +Since the `ChangeExec` usage is concerned only with component instances, the +apply phase still performs a concurrent dynamic walk as described in the +previous section to ensure that all other objects in the configuration will be +visited and have a chance to announce any problems they detect. The significant +difference for the apply phase is that anything which refers to a component +instance will block until the `ChangeExec`-managed apply phase for that +component instance has completed. Otherwise, the usual data-flow-driven +scheduling decides on the evaluation order for all other object types. diff --git a/internal/stacks/stackruntime/internal/stackeval/applying.go b/internal/stacks/stackruntime/internal/stackeval/applying.go index 16a22051bc..642ec3aba3 100644 --- a/internal/stacks/stackruntime/internal/stackeval/applying.go +++ b/internal/stacks/stackruntime/internal/stackeval/applying.go @@ -7,6 +7,7 @@ import ( "context" "github.com/hashicorp/terraform/internal/collections" + "github.com/hashicorp/terraform/internal/stacks/stackaddrs" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/stacks/stackstate/statekeys" "github.com/hashicorp/terraform/internal/tfdiags" @@ -36,6 +37,18 @@ type ApplyOpts struct { // the [ChangeExec] function (see [ApplyPlan]) and then we collect up any // reports to send to the caller separately using this interface. type Applyable interface { + // RequiredComponents returns the set of components that this applyable + // object requires, directly or indirectly, for its functionality. + // + // We use this during the planning phase to understand the dependency + // relationships between components, so that the apply phase can enforce + // the following invariants for all pairs of components A and B: + // - If A depends on B then any create or update actions for A must + // happen before create or update actions for B. + // - If A depends on B then any destroy actions for A must happen only + // after B is destroyed. + RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] + // CheckApply checks the receiver's apply-time result and returns zero // or more applied change descriptions and zero or more diagnostics // describing any problems that occured for this specific object during diff --git a/internal/stacks/stackruntime/internal/stackeval/component.go b/internal/stacks/stackruntime/internal/stackeval/component.go index f5d3c52160..c6537be107 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component.go +++ b/internal/stacks/stackruntime/internal/stackeval/component.go @@ -7,7 +7,10 @@ import ( "context" "fmt" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/promising" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" @@ -16,7 +19,6 @@ import ( "github.com/hashicorp/terraform/internal/stacks/stackruntime/hooks" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) type Component struct { @@ -263,6 +265,23 @@ func (c *Component) PlanChanges(ctx context.Context) ([]stackplan.PlannedChange, return nil, c.checkValid(ctx, PlanPhase) } +// References implements Referrer +func (c *Component) References(ctx context.Context) []stackaddrs.AbsReference { + cfg := c.Declaration(ctx) + var ret []stackaddrs.Reference + ret = append(ret, ReferencesInExpr(ctx, cfg.ForEach)...) + ret = append(ret, ReferencesInExpr(ctx, cfg.Inputs)...) + for _, expr := range cfg.ProviderConfigs { + ret = append(ret, ReferencesInExpr(ctx, expr)...) + } + return makeReferencesAbsolute(ret, c.Addr().Stack) +} + +// RequiredComponents implements Applyable +func (c *Component) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + return c.main.requiredComponentsForReferrer(ctx, c, PlanPhase) +} + // CheckApply implements ApplyChecker. func (c *Component) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { return nil, c.checkValid(ctx, ApplyPhase) diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 65f9104d33..ef4b4de441 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -12,6 +12,7 @@ import ( "github.com/zclconf/go-cty/cty/convert" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/lang/marks" @@ -1130,7 +1131,12 @@ func (c *ComponentInstance) PlanChanges(ctx context.Context) ([]stackplan.Planne return changes, diags } -// CheckApply implements ApplyChecker. +// RequiredComponents implements Applyable +func (c *ComponentInstance) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + return c.call.RequiredComponents(ctx) +} + +// CheckApply implements Applyable. func (c *ComponentInstance) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { var changes []stackstate.AppliedChange var diags tfdiags.Diagnostics diff --git a/internal/stacks/stackruntime/internal/stackeval/expression_refs.go b/internal/stacks/stackruntime/internal/stackeval/expression_refs.go new file mode 100644 index 0000000000..e2deb72338 --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/expression_refs.go @@ -0,0 +1,162 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package stackeval + +import ( + "context" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hcldec" + + "github.com/hashicorp/terraform/internal/collections" + "github.com/hashicorp/terraform/internal/stacks/stackaddrs" +) + +// Referrer is implemented by types that have expressions that can refer to +// [Referenceable] objects. +type Referrer interface { + // References returns descriptions of all of the expression references + // made from the configuration of the receiver. + References(ctx context.Context) []stackaddrs.AbsReference +} + +// ReferencesInExpr returns all of the valid references contained in the given +// HCL expression. +// +// It ignores any invalid references, on the assumption that the expression +// will eventually be evaluated and then those invalid references would be +// reported as errors at that point. +func ReferencesInExpr(ctx context.Context, expr hcl.Expression) []stackaddrs.Reference { + if expr == nil { + return nil + } + return referencesInTraversals(ctx, expr.Variables()) +} + +// ReferencesInBody returns all of the valid references contained in the given +// HCL body. +// +// It ignores any invalid references, on the assumption that the body +// will eventually be evaluated and then those invalid references would be +// reported as errors at that point. +func ReferencesInBody(ctx context.Context, body hcl.Body, spec hcldec.Spec) []stackaddrs.Reference { + if body == nil { + return nil + } + return referencesInTraversals(ctx, hcldec.Variables(body, spec)) +} + +func referencesInTraversals(ctx context.Context, traversals []hcl.Traversal) []stackaddrs.Reference { + if len(traversals) == 0 { + return nil + } + ret := make([]stackaddrs.Reference, 0, len(traversals)) + for _, traversal := range traversals { + ref, _, moreDiags := stackaddrs.ParseReference(traversal) + if moreDiags.HasErrors() { + // We'll ignore any traversals that are not valid references, + // on the assumption that we'd catch them during a subsequent + // evaluation of the same expression/body/etc. + continue + } + ret = append(ret, ref) + } + return ret +} + +func makeReferencesAbsolute(localRefs []stackaddrs.Reference, stackAddr stackaddrs.StackInstance) []stackaddrs.AbsReference { + if len(localRefs) == 0 { + return nil + } + ret := make([]stackaddrs.AbsReference, 0, len(localRefs)) + for _, localRef := range localRefs { + // contextual refs require a more specific scope than an entire + // stack, so they can't be represented as [AbsReference]. + if _, isContextual := localRef.Target.(stackaddrs.ContextualRef); isContextual { + continue + } + ret = append(ret, localRef.Absolute(stackAddr)) + } + return ret +} + +// requiredComponentsForReferrer is the main underlying implementation +// of Applyable.RequiredComponents, allowing the types which directly implement +// that interface to worry only about their own unique way of gathering up +// the relevant references from their configuration, since the work of +// peeling away references until we've found all of the components is the +// same regardless of where the references came from. +// +// This is a best-effort which will produce a complete result only if the +// configuration is completely valid. If not, the result is likely to be +// incomplete, which we accept on the assumption that the invalidity would +// also make the resulting plan non-applyable and thus it doesn't actually +// matter what the required components are. +func (m *Main) requiredComponentsForReferrer(ctx context.Context, obj Referrer, phase EvalPhase) collections.Set[stackaddrs.AbsComponent] { + ret := collections.NewSet[stackaddrs.AbsComponent]() + initialRefs := obj.References(ctx) + + // queued tracks objects we've previously queued -- which may or may not + // still be in the queue -- so that we can avoid re-visiting the same + // object multiple times and thus ensure the following loop will definitely + // eventually terminate, even in the presence of reference cycles, because + // the number of unique reference addresses in the configuration is + // finite. + queued := collections.NewSet[stackaddrs.AbsReferenceable]() + queue := make([]stackaddrs.AbsReferenceable, len(initialRefs)) + for i, ref := range initialRefs { + queue[i] = ref.Target() + queued.Add(queue[i]) + } + + for len(queue) != 0 { + targetAddr, remain := queue[0], queue[1:] + queue = remain + + // If this is a direct reference to a component then we can just + // add it and continue. + if componentAddr, ok := targetAddr.Item.(stackaddrs.Component); ok { + ret.Add(stackaddrs.AbsComponent{ + Stack: targetAddr.Stack, + Item: componentAddr, + }) + continue + } + + // For all other address types, we need to find the corresponding + // object and, if it's also Applyable, ask it for its references. + // + // For all of the fallible situations below, we'll just skip over + // this item on failure, because it's not this function's responsibility + // to report problems with the configuration. + // + // Since we're going to ignore all errors anyway, we can safely use + // a reference with no source location information. + ref := stackaddrs.AbsReference{ + Stack: targetAddr.Stack, + Ref: stackaddrs.Reference{ + Target: targetAddr.Item, + }, + } + target, _ := m.ResolveAbsExpressionReference(ctx, ref, phase) + if target == nil { + continue + } + targetReferrer, ok := target.(Referrer) + if !ok { + // Anything that isn't a referer cannot possibly indirectly + // refer to a component. + continue + } + for _, newRef := range targetReferrer.References(ctx) { + newTargetAddr := newRef.Target() + if !queued.Has(newTargetAddr) { + queue = append(queue, newTargetAddr) + queued.Add(newTargetAddr) + } + } + } + + return ret +} diff --git a/internal/stacks/stackruntime/internal/stackeval/expressions.go b/internal/stacks/stackruntime/internal/stackeval/expressions.go index cbd720fe69..09328266be 100644 --- a/internal/stacks/stackruntime/internal/stackeval/expressions.go +++ b/internal/stacks/stackruntime/internal/stackeval/expressions.go @@ -260,50 +260,6 @@ func EvalBody(ctx context.Context, body hcl.Body, spec hcldec.Spec, phase EvalPh return val, diags } -// ReferencesInExpr returns all of the valid references contained in the given -// HCL expression. -// -// It ignores any invalid references, on the assumption that the expression -// will eventually be evaluated and then those invalid references would be -// reported as errors at that point. -func ReferencesInExpr(ctx context.Context, expr hcl.Expression) []stackaddrs.Reference { - if expr == nil { - return nil - } - return referencesInTraversals(ctx, expr.Variables()) -} - -// ReferencesInBody returns all of the valid references contained in the given -// HCL body. -// -// It ignores any invalid references, on the assumption that the body -// will eventually be evaluated and then those invalid references would be -// reported as errors at that point. -func ReferencesInBody(ctx context.Context, body hcl.Body, spec hcldec.Spec) []stackaddrs.Reference { - if body == nil { - return nil - } - return referencesInTraversals(ctx, hcldec.Variables(body, spec)) -} - -func referencesInTraversals(ctx context.Context, traversals []hcl.Traversal) []stackaddrs.Reference { - if len(traversals) == 0 { - return nil - } - ret := make([]stackaddrs.Reference, 0, len(traversals)) - for _, traversal := range traversals { - ref, _, moreDiags := stackaddrs.ParseReference(traversal) - if moreDiags.HasErrors() { - // We'll ignore any traversals that are not valid references, - // on the assumption that we'd catch them during a subsequent - // evaluation of the same expression/body/etc. - continue - } - ret = append(ret, ref) - } - return ret -} - // ExprResult bundles an arbitrary result value with the expression and // evaluation context it was derived from, allowing the recipient to // potentially emit additional diagnostics if the result is problematic. diff --git a/internal/stacks/stackruntime/internal/stackeval/input_variable.go b/internal/stacks/stackruntime/internal/stackeval/input_variable.go index 0bd7cd2af0..6b0d2867e0 100644 --- a/internal/stacks/stackruntime/internal/stackeval/input_variable.go +++ b/internal/stacks/stackruntime/internal/stackeval/input_variable.go @@ -8,14 +8,16 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" + + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/promising" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" "github.com/hashicorp/terraform/internal/stacks/stackconfig" "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" ) // InputVariable represents an input variable belonging to a [Stack]. @@ -190,7 +192,38 @@ func (v *InputVariable) PlanChanges(ctx context.Context) ([]stackplan.PlannedCha }, diags } -// CheckApply implements ApplyChecker. +// References implements Referrer +func (v *InputVariable) References(ctx context.Context) []stackaddrs.AbsReference { + // The references for an input variable actually come from the + // call that defines it, in the parent stack. + addr := v.Addr() + if addr.Stack.IsRoot() { + // Variables declared in the root module can't refer to anything, + // because they are defined outside of the stack configuration by + // our caller. + return nil + } + stackAddr := addr.Stack + parentStack := v.main.StackUnchecked(ctx, stackAddr.Parent()) + if parentStack == nil { + // Weird, but we'll tolerate it for robustness. + return nil + } + callAddr := stackAddr.Call() + call := parentStack.EmbeddedStackCall(ctx, callAddr.Item) + if call == nil { + // Weird, but we'll tolerate it for robustness. + return nil + } + return call.References(ctx) +} + +// RequiredComponents implements Applyable +func (v *InputVariable) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + return v.main.requiredComponentsForReferrer(ctx, v, PlanPhase) +} + +// CheckApply implements Applyable. func (v *InputVariable) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { return nil, v.checkValid(ctx, ApplyPhase) } diff --git a/internal/stacks/stackruntime/internal/stackeval/main.go b/internal/stacks/stackruntime/internal/stackeval/main.go index ca4364487a..95baa3fb8a 100644 --- a/internal/stacks/stackruntime/internal/stackeval/main.go +++ b/internal/stacks/stackruntime/internal/stackeval/main.go @@ -10,6 +10,9 @@ import ( "time" "github.com/hashicorp/go-slug/sourcebundle" + "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/promising" @@ -17,7 +20,6 @@ import ( "github.com/hashicorp/terraform/internal/stacks/stackconfig" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) // Main is the central node of all data required for performing the major @@ -445,6 +447,23 @@ func (m *Main) RootVariableValue(ctx context.Context, addr stackaddrs.InputVaria } } +// ResolveAbsExpressionReference tries to resolve the given absolute +// expression reference within this evaluation context. +func (m *Main) ResolveAbsExpressionReference(ctx context.Context, ref stackaddrs.AbsReference, phase EvalPhase) (Referenceable, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + stack := m.Stack(ctx, ref.Stack, phase) + if stack == nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to undeclared stack", + Detail: fmt.Sprintf("Cannot resolve reference to object in undeclared stack %s.", ref.Stack), + Subject: ref.SourceRange().ToHCL().Ptr(), + }) + return nil, diags + } + return stack.ResolveExpressionReference(ctx, ref.Ref) +} + // RegisterCleanup registers an arbitrary callback function to run when a // walk driver eventually calls [Main.RunCleanup] on the same receiver. // diff --git a/internal/stacks/stackruntime/internal/stackeval/output_value.go b/internal/stacks/stackruntime/internal/stackeval/output_value.go index 46d3385a5f..fd3978d499 100644 --- a/internal/stacks/stackruntime/internal/stackeval/output_value.go +++ b/internal/stacks/stackruntime/internal/stackeval/output_value.go @@ -8,6 +8,10 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" + + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/promising" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" "github.com/hashicorp/terraform/internal/stacks/stackconfig" @@ -16,8 +20,6 @@ import ( "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" ) // OutputValue represents an input variable belonging to a [Stack]. @@ -167,7 +169,20 @@ func (v *OutputValue) PlanChanges(ctx context.Context) ([]stackplan.PlannedChang return nil, v.checkValid(ctx, PlanPhase) } -// CheckApply implements ApplyChecker. +// References implements Referrer +func (v *OutputValue) References(ctx context.Context) []stackaddrs.AbsReference { + cfg := v.Declaration(ctx) + var ret []stackaddrs.Reference + ret = append(ret, ReferencesInExpr(ctx, cfg.Value)...) + return makeReferencesAbsolute(ret, v.Addr().Stack) +} + +// RequiredComponents implements Applyable +func (v *OutputValue) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + return v.main.requiredComponentsForReferrer(ctx, v, PlanPhase) +} + +// CheckApply implements Applyable. func (v *OutputValue) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { return nil, v.checkValid(ctx, ApplyPhase) } diff --git a/internal/stacks/stackruntime/internal/stackeval/planning_test.go b/internal/stacks/stackruntime/internal/stackeval/planning_test.go index a7c25e1a18..cd1279366b 100644 --- a/internal/stacks/stackruntime/internal/stackeval/planning_test.go +++ b/internal/stacks/stackruntime/internal/stackeval/planning_test.go @@ -4,17 +4,22 @@ package stackeval import ( + "context" "fmt" "testing" + "github.com/google/go-cmp/cmp" "github.com/zclconf/go-cty/cty" "google.golang.org/protobuf/reflect/protoreflect" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/plans" + "github.com/hashicorp/terraform/internal/promising" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" + "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/stacks/stackstate/statekeys" "github.com/hashicorp/terraform/internal/stacks/tfstackdata1" "github.com/hashicorp/terraform/internal/terraform" @@ -321,3 +326,186 @@ func TestPlanning_DestroyMode(t *testing.T) { t.Errorf("no plan for %s", bResourceInstAddr) } } + +func TestPlanning_RequiredComponents(t *testing.T) { + // This test acts both as some unit tests for the component requirement + // analysis of various different object types and as an integration test + // for the overall component dependency analysis during the plan phase, + // ensuring that the dependency graph is reflected correctly in the + // resulting plan. + + cfg := testStackConfig(t, "planning", "required_components") + main := NewForPlanning(cfg, stackstate.NewState(), PlanOpts{ + PlanningMode: plans.NormalMode, + ProviderFactories: ProviderFactories{ + addrs.NewBuiltInProvider("foo"): func() (providers.Interface, error) { + return &terraform.MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + Provider: providers.Schema{ + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "in": { + Type: cty.Map(cty.String), + Optional: true, + }, + }, + }, + }, + }, + ConfigureProviderFn: func(cpr providers.ConfigureProviderRequest) providers.ConfigureProviderResponse { + t.Logf("configuring the provider: %#v", cpr.Config) + return providers.ConfigureProviderResponse{} + }, + }, nil + }, + }, + }) + + cmpA := stackaddrs.AbsComponent{ + Stack: stackaddrs.RootStackInstance, + Item: stackaddrs.Component{Name: "a"}, + } + cmpB := stackaddrs.AbsComponent{ + Stack: stackaddrs.RootStackInstance, + Item: stackaddrs.Component{Name: "b"}, + } + cmpC := stackaddrs.AbsComponent{ + Stack: stackaddrs.RootStackInstance, + Item: stackaddrs.Component{Name: "c"}, + } + + cmpOpts := collections.CmpOptions + + t.Run("integrated", func(t *testing.T) { + _, diags := testPlan(t, main) + assertNoDiagnostics(t, diags) + + // TODO: Once we've got the component dependencies round-tripping + // through the raw plan representation, we should test that the + // plan is capturing the dependencies correctly. + }) + + t.Run("component dependents", func(t *testing.T) { + ctx := context.Background() + promising.MainTask(ctx, func(ctx context.Context) (struct{}, error) { + tests := []struct { + componentAddr stackaddrs.AbsComponent + wantDependencies []stackaddrs.AbsComponent + }{ + { + cmpA, + []stackaddrs.AbsComponent{}, + }, + { + cmpB, + []stackaddrs.AbsComponent{ + cmpA, + }, + }, + { + cmpC, + []stackaddrs.AbsComponent{ + cmpA, + cmpB, + }, + }, + } + + for _, test := range tests { + t.Run(test.componentAddr.String(), func(t *testing.T) { + stack := main.Stack(ctx, test.componentAddr.Stack, PlanPhase) + if stack == nil { + t.Fatalf("no declaration for %s", test.componentAddr.Stack) + } + component := stack.Component(ctx, test.componentAddr.Item) + if component == nil { + t.Fatalf("no declaration for %s", test.componentAddr) + } + + got := component.RequiredComponents(ctx) + want := collections.NewSet[stackaddrs.AbsComponent]() + want.Add(test.wantDependencies...) + + if diff := cmp.Diff(want, got, cmpOpts); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) + } + + return struct{}{}, nil + }) + }) + + subtestInPromisingTask(t, "input variable dependents", func(ctx context.Context, t *testing.T) { + stack := main.Stack(ctx, stackaddrs.RootStackInstance.Child("child", addrs.NoKey), PlanPhase) + if stack == nil { + t.Fatalf("embedded stack isn't declared") + } + ivs := stack.InputVariables(ctx) + iv := ivs[stackaddrs.InputVariable{Name: "in"}] + if iv == nil { + t.Fatalf("input variable isn't declared") + } + + got := iv.RequiredComponents(ctx) + want := collections.NewSet[stackaddrs.AbsComponent]() + want.Add(cmpB) + + if diff := cmp.Diff(want, got, cmpOpts); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) + + subtestInPromisingTask(t, "output value dependents", func(ctx context.Context, t *testing.T) { + stack := main.MainStack(ctx) + ovs := stack.OutputValues(ctx) + ov := ovs[stackaddrs.OutputValue{Name: "out"}] + if ov == nil { + t.Fatalf("output value isn't declared") + } + + got := ov.RequiredComponents(ctx) + want := collections.NewSet[stackaddrs.AbsComponent]() + want.Add(cmpA) + + if diff := cmp.Diff(want, got, cmpOpts); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) + + subtestInPromisingTask(t, "embedded stack dependents", func(ctx context.Context, t *testing.T) { + stack := main.MainStack(ctx) + sc := stack.EmbeddedStackCall(ctx, stackaddrs.StackCall{Name: "child"}) + if sc == nil { + t.Fatalf("embedded stack call isn't declared") + } + + got := sc.RequiredComponents(ctx) + want := collections.NewSet[stackaddrs.AbsComponent]() + want.Add(cmpB) + + if diff := cmp.Diff(want, got, cmpOpts); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) + + subtestInPromisingTask(t, "provider config dependents", func(ctx context.Context, t *testing.T) { + stack := main.MainStack(ctx) + pc := stack.Provider(ctx, stackaddrs.ProviderConfig{ + Provider: addrs.NewBuiltInProvider("foo"), + Name: "bar", + }) + if pc == nil { + t.Fatalf("provider configuration isn't declared") + } + + got := pc.RequiredComponents(ctx) + want := collections.NewSet[stackaddrs.AbsComponent]() + want.Add(cmpA) + want.Add(cmpB) + + if diff := cmp.Diff(want, got, cmpOpts); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) +} diff --git a/internal/stacks/stackruntime/internal/stackeval/provider.go b/internal/stacks/stackruntime/internal/stackeval/provider.go index 69d8c4c0e4..14a223dde7 100644 --- a/internal/stacks/stackruntime/internal/stackeval/provider.go +++ b/internal/stacks/stackruntime/internal/stackeval/provider.go @@ -7,7 +7,10 @@ import ( "context" "fmt" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/promising" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" @@ -15,7 +18,6 @@ import ( "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) // Provider represents a provider configuration in a particular stack config. @@ -246,6 +248,22 @@ func (p *Provider) PlanChanges(ctx context.Context) ([]stackplan.PlannedChange, return nil, p.checkValid(ctx, PlanPhase) } +// References implements Referrer +func (p *Provider) References(ctx context.Context) []stackaddrs.AbsReference { + cfg := p.Declaration(ctx) + var ret []stackaddrs.Reference + ret = append(ret, ReferencesInExpr(ctx, cfg.ForEach)...) + if schema, err := p.ProviderType(ctx).Schema(ctx); err == nil { + ret = append(ret, ReferencesInBody(ctx, cfg.Config, schema.Provider.Block.DecoderSpec())...) + } + return makeReferencesAbsolute(ret, p.Addr().Stack) +} + +// RequiredComponents implements Applyable +func (p *Provider) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + return p.main.requiredComponentsForReferrer(ctx, p, PlanPhase) +} + // CheckApply implements ApplyChecker. func (p *Provider) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { return nil, p.checkValid(ctx, ApplyPhase) diff --git a/internal/stacks/stackruntime/internal/stackeval/provider_instance.go b/internal/stacks/stackruntime/internal/stackeval/provider_instance.go index a4ab2cec1d..1692f8afab 100644 --- a/internal/stacks/stackruntime/internal/stackeval/provider_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/provider_instance.go @@ -9,7 +9,10 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcldec" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/promising" "github.com/hashicorp/terraform/internal/providers" @@ -18,7 +21,6 @@ import ( "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/hashicorp/terraform/version" - "github.com/zclconf/go-cty/cty" ) // ProviderInstance represents one instance of a provider. @@ -285,7 +287,12 @@ func (p *ProviderInstance) PlanChanges(ctx context.Context) ([]stackplan.Planned return nil, p.checkValid(ctx, PlanPhase) } -// CheckApply implements ApplyChecker. +// RequiredComponents implements Applyable +func (p *ProviderInstance) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + return p.provider.RequiredComponents(ctx) +} + +// CheckApply implements Applyable. func (p *ProviderInstance) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { return nil, p.checkValid(ctx, ApplyPhase) } diff --git a/internal/stacks/stackruntime/internal/stackeval/stack.go b/internal/stacks/stackruntime/internal/stackeval/stack.go index 0e2558413c..918608d05d 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack.go @@ -9,7 +9,10 @@ import ( "sync" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" @@ -18,7 +21,6 @@ import ( "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) // Stack represents an instance of a [StackConfig] after it's had its @@ -578,7 +580,14 @@ func (s *Stack) PlanChanges(ctx context.Context) ([]stackplan.PlannedChange, tfd return changes, nil } -// CheckApply implements ApplyChecker. +func (s *Stack) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + // The stack itself doesn't refer to anything and so cannot require + // components. Its _call_ might, but that's handled over in + // [StackCall.RequiredComponents]. + return collections.NewSet[stackaddrs.AbsComponent]() +} + +// CheckApply implements Applyable. func (s *Stack) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { // TODO: We should emit an AppliedChange for each output value, // reporting its final value. diff --git a/internal/stacks/stackruntime/internal/stackeval/stack_call.go b/internal/stacks/stackruntime/internal/stackeval/stack_call.go index 57a6b7e8c7..45d78e7625 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack_call.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack_call.go @@ -7,7 +7,10 @@ import ( "context" "fmt" + "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/promising" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" @@ -15,7 +18,6 @@ import ( "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" ) // StackCall represents a "stack" block in a stack configuration after @@ -253,7 +255,21 @@ func (c *StackCall) PlanChanges(ctx context.Context) ([]stackplan.PlannedChange, return nil, c.checkValid(ctx, PlanPhase) } -// CheckApply implements ApplyChecker. +// References implements Referrer +func (c *StackCall) References(ctx context.Context) []stackaddrs.AbsReference { + cfg := c.Declaration(ctx) + var ret []stackaddrs.Reference + ret = append(ret, ReferencesInExpr(ctx, cfg.ForEach)...) + ret = append(ret, ReferencesInExpr(ctx, cfg.Inputs)...) + return makeReferencesAbsolute(ret, c.Addr().Stack) +} + +// RequiredComponents implements Applyable +func (c *StackCall) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + return c.main.requiredComponentsForReferrer(ctx, c, PlanPhase) +} + +// CheckApply implements Applyable. func (c *StackCall) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { return nil, c.checkValid(ctx, ApplyPhase) } diff --git a/internal/stacks/stackruntime/internal/stackeval/stack_call_instance.go b/internal/stacks/stackruntime/internal/stackeval/stack_call_instance.go index 4f1a73b62d..9f8d000091 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack_call_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack_call_instance.go @@ -8,14 +8,16 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/collections" "github.com/hashicorp/terraform/internal/instances" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" ) // StackCallInstance represents an instance of a [StackCall], acting as @@ -187,7 +189,12 @@ func (c *StackCallInstance) PlanChanges(ctx context.Context) ([]stackplan.Planne return nil, c.checkValid(ctx, PlanPhase) } -// CheckApply implements ApplyChecker by confirming that the input variable +// RequiredComponents implements Applyable +func (c *StackCallInstance) RequiredComponents(ctx context.Context) collections.Set[stackaddrs.AbsComponent] { + return c.call.RequiredComponents(ctx) +} + +// CheckApply implements Applyable by confirming that the input variable // values are still valid after resolving any upstream changes. func (c *StackCallInstance) CheckApply(ctx context.Context) ([]stackstate.AppliedChange, tfdiags.Diagnostics) { return nil, c.checkValid(ctx, ApplyPhase) diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/child/required-components-child.tfstack.hcl b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/child/required-components-child.tfstack.hcl new file mode 100644 index 0000000000..259d917ff0 --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/child/required-components-child.tfstack.hcl @@ -0,0 +1,9 @@ +variable "in" { + type = string + default = "" +} + +output "out" { + type = string + value = var.in +} diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/module/required-components.tf b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/module/required-components.tf new file mode 100644 index 0000000000..806295f926 --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/module/required-components.tf @@ -0,0 +1,8 @@ +variable "in" { + type = string + default = "" +} + +output "out" { + value = var.in +} diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/required-components.tfstack.hcl b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/required-components.tfstack.hcl new file mode 100644 index 0000000000..73ea9b828e --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/required_components/required-components.tfstack.hcl @@ -0,0 +1,50 @@ + +required_providers { + foo = { + source = "terraform.io/builtin/foo" + } +} + +component "a" { + source = "./module" +} + +component "b" { + source = "./module" + + inputs = { + in = component.a.out + } +} + +output "out" { + type = string + value = component.a.out +} + +provider "foo" "bar" { + config { + in = { + a = component.a.out + b = component.b.out + } + } +} + +stack "child" { + source = "./child" + + inputs = { + in = component.b.out + } +} + +component "c" { + source = "./module" + + inputs = { + # stack.child.out depends indirectly on component.b, so therefore + # component.c should transitively depend on component.b. + in = "${component.a.out}-${stack.child.out}" + } +}