From dcff9e4d2cf92c5f5bbdab50aa03d79b6d67e213 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 24 Apr 2025 09:22:50 +0200 Subject: [PATCH] stacks: improve test coverage for removed blocks (#36914) * stacks: improve test coverage for removed blocks * remove leftover comment --- internal/stacks/stackconfig/config.go | 69 ++- internal/stacks/stackruntime/apply_test.go | 4 + .../internal/stackeval/component.go | 41 +- .../internal/stackeval/input_variable.go | 8 +- .../internal/stackeval/removed.go | 81 +++ .../internal/stackeval/removed_component.go | 35 +- .../internal/stackeval/removed_stack_call.go | 56 +- .../stackruntime/internal/stackeval/stack.go | 121 +++- .../internal/stackeval/stack_call.go | 41 +- .../internal/stackeval/stack_config.go | 2 +- .../internal/stackeval/walk_dynamic.go | 55 +- internal/stacks/stackruntime/plan_test.go | 516 +++++++++++++++++- .../multiple-components.tfstack.hcl | 34 ++ .../removed/removed.tfstack.hcl | 11 + .../orphaned-component.tfstack.hcl | 19 + ...oved-stack-from-embedded-stack.tfstack.hcl | 38 ++ ...removed-stack-instance-dynamic.tfstack.hcl | 26 + 17 files changed, 1027 insertions(+), 130 deletions(-) create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/multiple-components/multiple-components.tfstack.hcl create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/multiple-components/removed/removed.tfstack.hcl create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/orphaned-component/orphaned-component.tfstack.hcl create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/removed-stack-from-embedded-stack/removed-stack-from-embedded-stack.tfstack.hcl diff --git a/internal/stacks/stackconfig/config.go b/internal/stacks/stackconfig/config.go index 43bfd49364..2c90597645 100644 --- a/internal/stacks/stackconfig/config.go +++ b/internal/stacks/stackconfig/config.go @@ -5,6 +5,7 @@ package stackconfig import ( "fmt" + "sort" "strings" "github.com/apparentlymart/go-versions/versions" @@ -190,34 +191,70 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun ret.Children[call.Name] = childNode } } - for _, blocks := range stack.RemovedEmbeddedStacks.All() { - for _, rmvd := range blocks { - effectiveSourceAddr, err := resolveFinalSourceAddr(sourceAddr, rmvd.SourceAddr, rmvd.VersionConstraints, sources) + + var removedTargets []stackaddrs.ConfigStackCall + for target := range stack.RemovedEmbeddedStacks.All() { + // removed embedded stacks can point to deeply embedded stacks, + // which we actually want to load into the embedded stacks if they + // naturally exist. But, the parents of those deeply embedded stacks + // will only exist if we have already added their parents to the + // tree of objects. So, we're going to store all our removed blocks + // in a flattened list and sort them so we add children before + // grandchildren and onwards and properly build the list to place + // everything in the correct place. + removedTargets = append(removedTargets, target) + } + + sort.Slice(removedTargets, func(i, j int) bool { + return len(removedTargets[i].Stack) < len(removedTargets[j].Stack) + }) + + for _, target := range removedTargets { + for _, block := range stack.RemovedEmbeddedStacks.Get(target) { + effectiveSourceAddr, err := resolveFinalSourceAddr(sourceAddr, block.SourceAddr, block.VersionConstraints, sources) if err != nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid source address", Detail: fmt.Sprintf( "Cannot use %q as a source address here: %s.", - rmvd.SourceAddr, err, + block.SourceAddr, err, ), - Subject: rmvd.SourceAddrRange.ToHCL().Ptr(), + Subject: block.SourceAddrRange.ToHCL().Ptr(), }) continue } - rmvd.FinalSourceAddr = effectiveSourceAddr - - next := rmvd.From.TargetStack()[0].Name - if _, ok := ret.Children[next]; ok { - // Then we've already loaded the configuration for this - // stack in the direct stack call. - continue + block.FinalSourceAddr = effectiveSourceAddr + + current := ret + for _, step := range target.Stack { + current = current.Children[step.Name] + if current == nil { + // this is invalid, we can't have orphaned removed blocks + // so we'll just return an error and skip this block. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid removed block", + Detail: "The linked removed block was not executed because the `from` attribute of the removed block targets a component or embedded stack within an orphaned embedded stack.\n\nIn order to remove an entire stack, update your removed block to target the entire removed stack itself instead of the specific elements within it.", + Subject: block.SourceAddrRange.ToHCL().Ptr(), + }) + break + } } - childNode, moreDiags := loadConfigDir(effectiveSourceAddr, sources, append(callers, sourceAddr)) - diags = diags.Append(moreDiags) - if childNode != nil { - ret.Children[next] = childNode + if current != nil { + next := target.Item.Name + if _, ok := current.Children[next]; ok { + // Then we've already loaded the configuration for this + // stack in the direct stack call. + continue + } + + childNode, moreDiags := loadConfigDir(effectiveSourceAddr, sources, append(callers, sourceAddr)) + diags = diags.Append(moreDiags) + if childNode != nil { + current.Children[next] = childNode + } } } } diff --git a/internal/stacks/stackruntime/apply_test.go b/internal/stacks/stackruntime/apply_test.go index 05ebd98315..78542a5d40 100644 --- a/internal/stacks/stackruntime/apply_test.go +++ b/internal/stacks/stackruntime/apply_test.go @@ -1119,6 +1119,10 @@ func TestApply(t *testing.T) { "removed": cty.StringVal("removed"), }), }, + &stackstate.AppliedChangeInputVariable{ + Addr: mustStackInputVariable("removed-direct"), + Value: cty.SetValEmpty(cty.String), + }, }, }, }, diff --git a/internal/stacks/stackruntime/internal/stackeval/component.go b/internal/stacks/stackruntime/internal/stackeval/component.go index 46e297e55d..fff1123105 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component.go +++ b/internal/stacks/stackruntime/internal/stackeval/component.go @@ -6,6 +6,7 @@ package stackeval import ( "context" "fmt" + "sync" "github.com/zclconf/go-cty/cty" @@ -27,9 +28,11 @@ type Component struct { stack *Stack config *ComponentConfig - forEachValue perEvalPhase[promising.Once[withDiagnostics[cty.Value]]] - instances perEvalPhase[promising.Once[withDiagnostics[instancesResult[*ComponentInstance]]]] - unknownInstance perEvalPhase[promising.Once[*ComponentInstance]] + forEachValue perEvalPhase[promising.Once[withDiagnostics[cty.Value]]] + instances perEvalPhase[promising.Once[withDiagnostics[instancesResult[*ComponentInstance]]]] + + unknownInstancesMutex sync.Mutex + unknownInstances map[addrs.InstanceKey]*ComponentInstance } var _ Plannable = (*Component)(nil) @@ -38,10 +41,11 @@ var _ Referenceable = (*Component)(nil) func newComponent(main *Main, addr stackaddrs.AbsComponent, stack *Stack, config *ComponentConfig) *Component { return &Component{ - addr: addr, - main: main, - stack: stack, - config: config, + addr: addr, + main: main, + stack: stack, + config: config, + unknownInstances: make(map[addrs.InstanceKey]*ComponentInstance), } } @@ -164,15 +168,22 @@ func (c *Component) CheckInstances(ctx context.Context, phase EvalPhase) (map[ad return result.insts, result.unknown, diags } -func (c *Component) UnknownInstance(ctx context.Context, phase EvalPhase) *ComponentInstance { - inst, err := c.unknownInstance.For(phase).Do(ctx, c.tracingName()+" unknown instance", func(ctx context.Context) (*ComponentInstance, error) { - return newComponentInstance(c, stackaddrs.AbsComponentToInstance(c.addr, addrs.WildcardKey), instances.UnknownForEachRepetitionData(c.ForEachValue(ctx, phase).Type()), c.stack.mode, true), nil - }) - if err != nil { - // Since we never return an error from the function we pass to Do, - // this should never happen. - panic(err) +func (c *Component) UnknownInstance(ctx context.Context, key addrs.InstanceKey, phase EvalPhase) *ComponentInstance { + c.unknownInstancesMutex.Lock() + defer c.unknownInstancesMutex.Unlock() + + if inst, ok := c.unknownInstances[key]; ok { + return inst } + + forEachType := c.ForEachValue(ctx, phase).Type() + repetitionData := instances.UnknownForEachRepetitionData(forEachType) + if key != addrs.WildcardKey { + repetitionData.EachKey = key.Value() + } + + inst := newComponentInstance(c, stackaddrs.AbsComponentToInstance(c.addr, key), repetitionData, c.stack.mode, true) + c.unknownInstances[key] = inst return inst } diff --git a/internal/stacks/stackruntime/internal/stackeval/input_variable.go b/internal/stacks/stackruntime/internal/stackeval/input_variable.go index 42b06cc40b..da9004626f 100644 --- a/internal/stacks/stackruntime/internal/stackeval/input_variable.go +++ b/internal/stacks/stackruntime/internal/stackeval/input_variable.go @@ -67,18 +67,20 @@ func (v *InputVariable) DefinedByStackCallInstance(ctx context.Context, phase Ev // actually exist, which is odd but we'll tolerate it. return nil } + + lastStep := declarerAddr[len(declarerAddr)-1] + instKey := lastStep.Key + callInsts, unknown := call.Instances(ctx, phase) if unknown { // Return our static unknown instance for this variable. - return call.UnknownInstance(ctx, phase) + return call.UnknownInstance(ctx, instKey, phase) } if callInsts == nil { // Could get here if the call's for_each is invalid. return nil } - lastStep := declarerAddr[len(declarerAddr)-1] - instKey := lastStep.Key return callInsts[instKey] } diff --git a/internal/stacks/stackruntime/internal/stackeval/removed.go b/internal/stacks/stackruntime/internal/stackeval/removed.go index 74ddae6576..4fc72ca748 100644 --- a/internal/stacks/stackruntime/internal/stackeval/removed.go +++ b/internal/stacks/stackruntime/internal/stackeval/removed.go @@ -4,13 +4,19 @@ package stackeval import ( + "context" "sync" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" + "github.com/hashicorp/terraform/internal/stacks/stackconfig" ) // Removed encapsulates the somewhat complicated logic for tracking and // managing the removed block instances in a given stack. +// +// The Removed block does actually capture the entire tree of removed blocks +// in a single instance via the children field. Each Stack has a reference to +// its Removed instance, from which it can access all of its children. type Removed struct { sync.Mutex @@ -71,3 +77,78 @@ func (removed *Removed) AddStackCall(addr stackaddrs.ConfigStackCall, stackCalls Item: addr.Item, }, stackCalls) } + +// validateMissingInstanceAgainstRemovedBlocks matches the function of the same +// name defined on Stack. +// +// This function should only ever be called from inside that function and it +// performs the same purpose except it exclusively looks for orphaned blocks +// with the children. +// +// This function assumes all the checks made in the equivalent function in Stack +// have been completed, so again (!!!) it should only be called from within +// the other function. +func (removed *Removed) validateMissingInstanceAgainstRemovedBlocks(ctx context.Context, addr stackaddrs.StackInstance, target stackaddrs.AbsComponentInstance, phase EvalPhase) (*stackconfig.Removed, *stackconfig.Component) { + + // we're just jumping directly into checking the children, the removed + // stack calls should have already been checked by the function on + // Stack. + + if len(target.Stack) == 0 { + + if components, ok := removed.components[target.Item.Component]; ok { + for _, component := range components { + insts, _ := component.InstancesFor(ctx, addr, phase) + for _, inst := range insts { + if inst.from.Item.Key == target.Item.Key { + // then we have actually found it! this is a removed + // block that targets the target address, but isn't + // in any stacks. + return inst.call.config.config, nil + } + } + } + } + + return nil, nil // we found no potential blocks + } + + // otherwise, we'll keep looking! + + // first, we'll check to see if we have a removed block targeting + // the entire stack. + + next := target.Stack[0] + rest := stackaddrs.AbsComponentInstance{ + Stack: target.Stack[1:], + Item: target.Item, + } + + if calls, ok := removed.stackCalls[stackaddrs.StackCall{Name: next.Name}]; ok { + for _, call := range calls { + insts, _ := call.InstancesFor(ctx, append(addr, next), phase) + for _, inst := range insts { + stack := inst.Stack(ctx, phase) + + // now, hand the search back over to the stack to check if + // the target instance is actually claimed by this removed + // stack. + removed, component := stack.validateMissingInstanceAgainstRemovedBlocks(ctx, rest, phase) + if removed != nil || component != nil { + // if we found any match, then return this removed block + // as the original source + return call.config.config, nil + } + } + } + + } + + // finally, we'll keep going through the children of the next one. + + if child, ok := removed.children[next.Name]; ok { + return child.validateMissingInstanceAgainstRemovedBlocks(ctx, append(addr, next), rest, phase) + } + + return nil, nil +} diff --git a/internal/stacks/stackruntime/internal/stackeval/removed_component.go b/internal/stacks/stackruntime/internal/stackeval/removed_component.go index 91dcf8db86..51e22379b2 100644 --- a/internal/stacks/stackruntime/internal/stackeval/removed_component.go +++ b/internal/stacks/stackruntime/internal/stackeval/removed_component.go @@ -6,12 +6,14 @@ package stackeval import ( "context" "fmt" + "sync" "time" "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/lang" "github.com/hashicorp/terraform/internal/promising" @@ -34,17 +36,20 @@ type RemovedComponent struct { stack *Stack main *Main - forEachValue perEvalPhase[promising.Once[withDiagnostics[cty.Value]]] - instances perEvalPhase[promising.Once[withDiagnostics[instancesResult[*RemovedComponentInstance]]]] - unknownInstance perEvalPhase[promising.Once[*RemovedComponentInstance]] + forEachValue perEvalPhase[promising.Once[withDiagnostics[cty.Value]]] + instances perEvalPhase[promising.Once[withDiagnostics[instancesResult[*RemovedComponentInstance]]]] + + unknownInstancesMutex sync.Mutex + unknownInstances collections.Map[stackaddrs.AbsComponentInstance, *RemovedComponentInstance] } func newRemovedComponent(main *Main, target stackaddrs.ConfigComponent, stack *Stack, config *RemovedComponentConfig) *RemovedComponent { return &RemovedComponent{ - target: target, - main: main, - config: config, - stack: stack, + target: target, + main: main, + config: config, + stack: stack, + unknownInstances: collections.NewMap[stackaddrs.AbsComponentInstance, *RemovedComponentInstance](), } } @@ -189,6 +194,22 @@ func (r *RemovedComponent) Instances(ctx context.Context, phase EvalPhase) (map[ return result.insts, result.unknown, diags } +func (r *RemovedComponent) UnknownInstance(ctx context.Context, from stackaddrs.AbsComponentInstance, phase EvalPhase) *RemovedComponentInstance { + r.unknownInstancesMutex.Lock() + defer r.unknownInstancesMutex.Unlock() + + if inst, ok := r.unknownInstances.GetOk(from); ok { + return inst + } + + forEachType, _ := r.ForEachValue(ctx, phase) + repetitionData := instances.UnknownForEachRepetitionData(forEachType.Type()) + + inst := newRemovedComponentInstance(r, from, repetitionData, true) + r.unknownInstances.Put(from, inst) + return inst +} + func (r *RemovedComponent) PlanIsComplete(ctx context.Context, stack stackaddrs.StackInstance) bool { if !r.main.Planning() { panic("PlanIsComplete used when not in the planning phase") diff --git a/internal/stacks/stackruntime/internal/stackeval/removed_stack_call.go b/internal/stacks/stackruntime/internal/stackeval/removed_stack_call.go index 6cbf674256..68fea75671 100644 --- a/internal/stacks/stackruntime/internal/stackeval/removed_stack_call.go +++ b/internal/stacks/stackruntime/internal/stackeval/removed_stack_call.go @@ -6,12 +6,14 @@ package stackeval import ( "context" "fmt" + "sync" "time" "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/lang" "github.com/hashicorp/terraform/internal/promising" @@ -34,14 +36,18 @@ type RemovedStackCall struct { forEachValue perEvalPhase[promising.Once[withDiagnostics[cty.Value]]] instances perEvalPhase[promising.Once[withDiagnostics[instancesResult[*RemovedStackCallInstance]]]] + + unknownInstancesMutex sync.Mutex + unknownInstances collections.Map[stackaddrs.StackInstance, *RemovedStackCallInstance] } func newRemovedStackCall(main *Main, target stackaddrs.ConfigStackCall, stack *Stack, config *RemovedStackCallConfig) *RemovedStackCall { return &RemovedStackCall{ - stack: stack, - target: target, - config: config, - main: main, + stack: stack, + target: target, + config: config, + main: main, + unknownInstances: collections.NewMap[stackaddrs.StackInstance, *RemovedStackCallInstance](), } } @@ -123,18 +129,20 @@ func (r *RemovedStackCall) Instances(ctx context.Context, phase EvalPhase) (map[ Name: rsc.from[len(rsc.from)-1].Name, }) - insts, _ := embeddedCall.Instances(ctx, phase) - if _, exists := insts[key]; exists { - // error, we have an embedded stack call and a removed block - // pointing at the same instance - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Cannot remove stack instance", - Detail: fmt.Sprintf("The stack instance %s is targeted by an embedded stack block and cannot be removed. The relevant embedded stack is defined at %s.", rsc.from, embeddedCall.config.config.DeclRange.ToHCL()), - Subject: rsc.call.config.config.DeclRange.ToHCL().Ptr(), - }) - - continue // don't add this to the known instances + if embeddedCall != nil { + insts, _ := embeddedCall.Instances(ctx, phase) + if _, exists := insts[key]; exists { + // error, we have an embedded stack call and a removed block + // pointing at the same instance + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Cannot remove stack instance", + Detail: fmt.Sprintf("The stack instance %s is targeted by an embedded stack block and cannot be removed. The relevant embedded stack is defined at %s.", rsc.from, embeddedCall.config.config.DeclRange.ToHCL()), + Subject: rsc.call.config.config.DeclRange.ToHCL().Ptr(), + }) + + continue // don't add this to the known instances + } } } @@ -162,6 +170,22 @@ func (r *RemovedStackCall) Instances(ctx context.Context, phase EvalPhase) (map[ return result.insts, result.unknown, diags } +func (r *RemovedStackCall) UnknownInstance(ctx context.Context, from stackaddrs.StackInstance, phase EvalPhase) *RemovedStackCallInstance { + r.unknownInstancesMutex.Lock() + defer r.unknownInstancesMutex.Unlock() + + if inst, ok := r.unknownInstances.GetOk(from); ok { + return inst + } + + forEachType, _ := r.ForEachValue(ctx, phase) + repetitionData := instances.UnknownForEachRepetitionData(forEachType.Type()) + + inst := newRemovedStackCallInstance(r, from, repetitionData, true) + r.unknownInstances.Put(from, inst) + return inst +} + func (r *RemovedStackCall) PlanChanges(ctx context.Context) ([]stackplan.PlannedChange, tfdiags.Diagnostics) { _, _, diags := r.Instances(ctx, PlanPhase) return nil, diags diff --git a/internal/stacks/stackruntime/internal/stackeval/stack.go b/internal/stacks/stackruntime/internal/stackeval/stack.go index ac4b48200f..186c8f0470 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" + "github.com/hashicorp/terraform/internal/stacks/stackconfig" "github.com/hashicorp/terraform/internal/stacks/stackconfig/typeexpr" "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/stacks/stackstate" @@ -83,7 +84,7 @@ func (s *Stack) ChildStack(ctx context.Context, addr stackaddrs.StackInstanceSte if call := s.EmbeddedStackCalls()[callAddr]; call != nil { instances, unknown := call.Instances(ctx, phase) if unknown { - return call.UnknownInstance(ctx, phase).Stack(ctx, phase) + return call.UnknownInstance(ctx, addr.Key, phase).Stack(ctx, phase) } if instance, exists := instances[addr.Key]; exists { @@ -93,8 +94,13 @@ func (s *Stack) ChildStack(ctx context.Context, addr stackaddrs.StackInstanceSte calls := s.Removed().stackCalls[callAddr] for _, call := range calls { - instances, _ := call.InstancesFor(ctx, s.addr, phase) - if instance, exists := instances[addr.Key]; exists { + absolute := append(s.addr, addr) + + instances, unknown := call.InstancesFor(ctx, absolute, phase) + if unknown { + return call.UnknownInstance(ctx, absolute, phase).Stack(ctx, phase) + } + for _, instance := range instances { return instance.Stack(ctx, phase) } } @@ -702,6 +708,37 @@ Instance: continue } + // Normally, this is a simple error. The user has deleted an entire + // stack without adding an equivalent removed block for the stack + // so now the instances in that stack are all unclaimed. + // + // However, the user may have tried to write removed blocks that + // target specific components within a removed stack instead of + // just targeting the entire stack. This is invalid, for one it is + // easier for the user if they could just remove the whole stack, + // and for two it is very difficult for us to reconcile orphaned + // removed components and removed embedded stacks that could be + // floating anywhere in the configuration - instead, we'll just + // not allow this. + // + // In this case, we want to change the error message to be more + // user-friendly than the generic one, so we need to discover if + // this has happened here, and if so, modify the error message. + + removed, _ := s.validateMissingInstanceAgainstRemovedBlocks(ctx, inst, PlanPhase) + if removed != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid removed block", + Detail: fmt.Sprintf("The component instance %s could not be removed. The linked removed block was not executed because the `from` attribute of the removed block targets a component or embedded stack within an orphaned embedded stack.\n\nIn order to remove an entire stack, update your removed block to target the entire removed stack itself instead of the specific elements within it.", inst.String()), + Subject: removed.DeclRange.ToHCL().Ptr(), + }) + continue + } + + // If we fall out here, then we found no relevant removed blocks + // so we can return the generic error message! + diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Unclaimed component instance", @@ -856,3 +893,81 @@ func (s *Stack) tracingName() string { } return addr.String() } + +// validateMissingInstanceAgainstRemovedBlocks returns the removed config most +// applicable to the target address if it exists. +// +// We have an edge case where a user has written a removed block that targets +// a stacks or components within stacks that are not defined anywhere in the +// stack (either in a removed blocks or an embedded stack). We consider this to +// be an error - if you remove an entire stack from the configuration then you +// should write a removed block that targets that stack not several removed +// blocks that target things inside the removed block. +// +// The above edge case is exposed when we check that all component instances +// in state are included in the plan. This function is called with the absolute +// address of the problematic component (the target). The error we would +// normally return would say that the component isn't targeted by any component +// or removed blocks. This is misleading for the discussed edge case, as the +// user may have written a removed block that targets the component specifically +// but it is just not getting executed as it is in a stack that is also not +// in the configuration. +// +// The function aims to discover if a removed block does exist that might target +// this component. Note, that since we can have removed blocks that target +// entire stacks we do check both removed blocks and direct components on the +// assumption that a removed stack might expand to include the target component +// and we want to capture that removed stack specifically. +func (s *Stack) validateMissingInstanceAgainstRemovedBlocks(ctx context.Context, target stackaddrs.AbsComponentInstance, phase EvalPhase) (*stackconfig.Removed, *stackconfig.Component) { + if len(target.Stack) == 0 { + + // First, we'll handle the simple case. This means we are actually + // targeting a component that should be in the current stack, so we'll + // just look to see if there is a removed block that targets this + // component directly. + + components, ok := s.Removed().components[target.Item.Component] + if ok { + for _, component := range components { + // we have the component, let's check the + insts, _ := component.InstancesFor(ctx, s.addr, phase) + if inst, ok := insts[target.Item.Key]; ok { + return inst.call.config.config, nil + } + } + } + + if component := s.Component(target.Item.Component); component != nil { + insts, _ := component.Instances(ctx, phase) + if inst, ok := insts[target.Item.Key]; ok { + return nil, inst.call.config.config + } + } + + return nil, nil + } + + // more complicated now, we need to look into a child stack + + next := target.Stack[0] + rest := stackaddrs.AbsComponentInstance{ + Stack: target.Stack[1:], + Item: target.Item, + } + + if child := s.ChildStack(ctx, next, phase); child != nil { + return child.validateMissingInstanceAgainstRemovedBlocks(ctx, rest, phase) + } + + // if we get here, then we had no child stack to check against. But, things + // are not over yet! we also have might have orphaned removed blocks. + // these are tracked in the Removed() struct directly, so we'll also look + // into there. this is the actual troublesome case we're checking for so + // we do expect to actually get here for these checks. + + if child, ok := s.Removed().children[next.Name]; ok { + return child.validateMissingInstanceAgainstRemovedBlocks(ctx, append(s.addr, next), rest, phase) + } + + return nil, nil +} diff --git a/internal/stacks/stackruntime/internal/stackeval/stack_call.go b/internal/stacks/stackruntime/internal/stackeval/stack_call.go index 2ad3c1fc44..eecfcb382b 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack_call.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack_call.go @@ -6,6 +6,7 @@ package stackeval import ( "context" "fmt" + "sync" "github.com/zclconf/go-cty/cty" @@ -27,9 +28,11 @@ type StackCall struct { main *Main - forEachValue perEvalPhase[promising.Once[withDiagnostics[cty.Value]]] - instances perEvalPhase[promising.Once[withDiagnostics[instancesResult[*StackCallInstance]]]] - unknownInstance perEvalPhase[promising.Once[*StackCallInstance]] + forEachValue perEvalPhase[promising.Once[withDiagnostics[cty.Value]]] + instances perEvalPhase[promising.Once[withDiagnostics[instancesResult[*StackCallInstance]]]] + + unknownInstancesMutex sync.Mutex + unknownInstances map[addrs.InstanceKey]*StackCallInstance } var _ Plannable = (*StackCall)(nil) @@ -37,10 +40,11 @@ var _ Referenceable = (*StackCall)(nil) func newStackCall(main *Main, addr stackaddrs.AbsStackCall, stack *Stack, config *StackCallConfig) *StackCall { return &StackCall{ - addr: addr, - main: main, - stack: stack, - config: config, + addr: addr, + main: main, + stack: stack, + config: config, + unknownInstances: make(map[addrs.InstanceKey]*StackCallInstance), } } @@ -156,15 +160,22 @@ func (c *StackCall) CheckInstances(ctx context.Context, phase EvalPhase) (map[ad return result.insts, result.unknown, diags } -func (c *StackCall) UnknownInstance(ctx context.Context, phase EvalPhase) *StackCallInstance { - inst, err := c.unknownInstance.For(phase).Do(ctx, c.tracingName()+" unknown instace", func(ctx context.Context) (*StackCallInstance, error) { - return newStackCallInstance(c, addrs.WildcardKey, instances.UnknownForEachRepetitionData(c.ForEachValue(ctx, phase).Type()), c.stack.mode, true), nil - }) - if err != nil { - // Since we never return an error from the function we pass to Do, - // this should never happen. - panic(err) +func (c *StackCall) UnknownInstance(ctx context.Context, key addrs.InstanceKey, phase EvalPhase) *StackCallInstance { + c.unknownInstancesMutex.Lock() + defer c.unknownInstancesMutex.Unlock() + + if inst, ok := c.unknownInstances[key]; ok { + return inst } + + forEachType := c.ForEachValue(ctx, phase).Type() + repetitionData := instances.UnknownForEachRepetitionData(forEachType) + if key != addrs.WildcardKey { + repetitionData.EachKey = key.Value() + } + + inst := newStackCallInstance(c, key, repetitionData, c.stack.mode, true) + c.unknownInstances[key] = inst return inst } diff --git a/internal/stacks/stackruntime/internal/stackeval/stack_config.go b/internal/stacks/stackruntime/internal/stackeval/stack_config.go index e3d7b95b71..7d07cdb468 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack_config.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack_config.go @@ -354,7 +354,7 @@ func (s *StackConfig) StackCalls() map[stackaddrs.StackCall]*StackCallConfig { return nil } ret := make(map[stackaddrs.StackCall]*StackCallConfig, len(s.config.Children)) - for n := range s.config.Children { + for n := range s.config.Stack.EmbeddedStacks { stepAddr := stackaddrs.StackCall{Name: n} ret[stepAddr] = s.StackCall(stepAddr) } diff --git a/internal/stacks/stackruntime/internal/stackeval/walk_dynamic.go b/internal/stacks/stackruntime/internal/stackeval/walk_dynamic.go index c45b117569..b63233eecb 100644 --- a/internal/stacks/stackruntime/internal/stackeval/walk_dynamic.go +++ b/internal/stacks/stackruntime/internal/stackeval/walk_dynamic.go @@ -7,11 +7,8 @@ import ( "context" "sync" - "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/stacks/stackaddrs" ) @@ -247,25 +244,8 @@ func walkComponent[Output any]( // instance dynamically and off we go unknownComponentBlockClaimedSomething = true - - if inst.Key == addrs.WildcardKey { - // use the preset unknown instance if the key is actually - // the wildcard - inst := unknownComponentBlock.UnknownInstance(ctx, phase) - visit(ctx, walk, inst) - - continue - } - - inst := newComponentInstance(unknownComponentBlock, stackaddrs.AbsComponentInstance{ - Stack: stack.addr, - Item: inst, - }, instances.RepetitionData{ - EachKey: inst.Key.Value(), - EachValue: cty.UnknownVal(cty.DynamicPseudoType), - }, stack.mode, true) + inst := unknownComponentBlock.UnknownInstance(ctx, inst.Key, phase) visit(ctx, walk, inst) - continue } @@ -273,13 +253,11 @@ func walkComponent[Output any]( // then we didn't have an unknown component block, but we do // have an unknown removed component block to claim it - inst := newRemovedComponentInstance(unknownRemovedComponentBlock, stackaddrs.AbsComponentInstance{ + from := stackaddrs.AbsComponentInstance{ Stack: stack.addr, Item: inst, - }, instances.RepetitionData{ - EachKey: inst.Key.Value(), - EachValue: cty.UnknownVal(cty.DynamicPseudoType), - }, true) + } + inst := unknownRemovedComponentBlock.UnknownInstance(ctx, from, phase) visit(ctx, walk, inst) continue @@ -292,7 +270,7 @@ func walkComponent[Output any]( if !unknownComponentBlockClaimedSomething { // then we want to include the partial unknown component instance - inst := unknownComponentBlock.UnknownInstance(ctx, phase) + inst := unknownComponentBlock.UnknownInstance(ctx, addrs.WildcardKey, phase) visit(ctx, walk, inst) } }) @@ -393,20 +371,7 @@ func walkEmbeddedStack[Output any]( if unknownStackCall != nil { unknownStackCallClaimedSomething = true - - if inst[len(inst)-1].Key == addrs.WildcardKey { - inst := unknownStackCall.UnknownInstance(ctx, phase) - visit(ctx, walk, inst) - childStack := inst.Stack(ctx, phase) - walkDynamicObjectsInStack(ctx, walk, childStack, phase, visit) - continue - } - - inst := newStackCallInstance(unknownStackCall, inst[len(inst)-1].Key, instances.RepetitionData{ - EachKey: inst[len(inst)-1].Key.Value(), - EachValue: cty.UnknownVal(cty.DynamicPseudoType), - }, stack.mode, true) - + inst := unknownStackCall.UnknownInstance(ctx, inst[len(inst)-1].Key, phase) visit(ctx, walk, inst) childStack := inst.Stack(ctx, phase) walkDynamicObjectsInStack(ctx, walk, childStack, phase, visit) @@ -415,11 +380,7 @@ func walkEmbeddedStack[Output any]( } if unknownRemovedStackCall != nil { - inst := newRemovedStackCallInstance(unknownRemovedStackCall, inst, instances.RepetitionData{ - EachKey: inst[len(inst)-1].Key.Value(), - EachValue: cty.UnknownVal(cty.DynamicPseudoType), - }, true) - + inst := unknownRemovedStackCall.UnknownInstance(ctx, inst, phase) visit(ctx, walk, inst) childStack := inst.Stack(ctx, phase) walkDynamicObjectsInStack(ctx, walk, childStack, phase, visit) @@ -430,7 +391,7 @@ func walkEmbeddedStack[Output any]( if !unknownStackCallClaimedSomething { // then we want to include the partial unknown component instance - inst := unknownStackCall.UnknownInstance(ctx, phase) + inst := unknownStackCall.UnknownInstance(ctx, addrs.WildcardKey, phase) visit(ctx, walk, inst) childStack := inst.Stack(ctx, phase) walkDynamicObjectsInStack(ctx, walk, childStack, phase, visit) diff --git a/internal/stacks/stackruntime/plan_test.go b/internal/stacks/stackruntime/plan_test.go index 7757c01bc1..b7fa858a5f 100644 --- a/internal/stacks/stackruntime/plan_test.go +++ b/internal/stacks/stackruntime/plan_test.go @@ -596,7 +596,7 @@ func TestPlan(t *testing.T) { PlanComplete: false, Action: plans.Update, PlannedInputValues: map[string]plans.DynamicValue{ - "id": mustPlanDynamicValueDynamicType(cty.UnknownVal(cty.String)), + "id": mustPlanDynamicValueDynamicType(cty.StringVal("deferred")), "input": mustPlanDynamicValueDynamicType(cty.UnknownVal(cty.String)), }, PlannedOutputValues: map[string]cty.Value{}, @@ -644,18 +644,14 @@ func TestPlan(t *testing.T) { Module: addrs.RootModule, Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), }, - ActionReason: plans.ResourceInstanceReplaceBecauseCannotUpdate, - RequiredReplace: cty.NewPathSet( - cty.GetAttrPath("id"), - ), ChangeSrc: plans.ChangeSrc{ - Action: plans.DeleteThenCreate, + Action: plans.Update, Before: mustPlanDynamicValueSchema(cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("deferred"), "value": cty.StringVal("deferred"), }), stacks_testing_provider.TestingResourceSchema.Body), After: mustPlanDynamicValueSchema(cty.ObjectVal(map[string]cty.Value{ - "id": cty.UnknownVal(cty.String), + "id": cty.StringVal("deferred"), "value": cty.UnknownVal(cty.String), }), stacks_testing_provider.TestingResourceSchema.Body), AfterSensitivePaths: nil, @@ -912,6 +908,512 @@ func TestPlan(t *testing.T) { "component": cty.StringVal("component"), }), }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "removed-direct"}, + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.SetValEmpty(cty.String), + }, + }, + }, + }, + "embedded stack in state but not in configuration": { + path: filepath.Join("with-single-input", "valid"), + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("stack.child.component.self"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("stack.child.component.self.testing_resource.data")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "leftover", + "value": "leftover", + }), + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("leftover", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("leftover"), + "value": cty.StringVal("leftover"), + })). + Build(), + cycle: TestCycle{ + planInputs: map[string]cty.Value{ + "input": cty.StringVal("input"), + }, + wantPlannedDiags: initDiags(func(diags tfdiags.Diagnostics) tfdiags.Diagnostics { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unclaimed component instance", + Detail: "The component instance stack.child.component.self is not claimed by any component or removed block in the configuration. Make sure it is instantiated by a component block, or targeted for removal by a removed block.", + }) + }), + }, + }, + "removed and stack block target the same stack": { + path: filepath.Join("with-single-input", "removed-stack-instance-dynamic"), + cycle: TestCycle{ + planInputs: map[string]cty.Value{ + "input": cty.MapVal(map[string]cty.Value{ + "component": cty.StringVal("component"), + }), + "removed": cty.MapVal(map[string]cty.Value{ + "component": cty.StringVal("component"), + }), + }, + wantPlannedDiags: initDiags(func(diags tfdiags.Diagnostics) tfdiags.Diagnostics { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Cannot remove stack instance", + Detail: "The stack instance stack.simple[\"component\"] is targeted by an embedded stack block and cannot be removed. The relevant embedded stack is defined at git::https://example.com/test.git//with-single-input/removed-stack-instance-dynamic/removed-stack-instance-dynamic.tfstack.hcl:25,1-15.", + Subject: &hcl.Range{ + Filename: "git::https://example.com/test.git//with-single-input/removed-stack-instance-dynamic/removed-stack-instance-dynamic.tfstack.hcl", + Start: hcl.Pos{Line: 36, Column: 1, Byte: 441}, + End: hcl.Pos{Line: 36, Column: 8, Byte: 448}, + }, + }) + }), + }, + }, + "removed targets stack block in embedded stack that exists": { + path: filepath.Join("with-single-input", "removed-stack-from-embedded-stack"), + cycle: TestCycle{ + planInputs: map[string]cty.Value{ + "input": cty.MapVal(map[string]cty.Value{ + "component": cty.MapVal(map[string]cty.Value{ + "component": cty.StringVal("component"), + }), + }), + "removed": cty.MapVal(map[string]cty.Value{ + "component": cty.MapVal(map[string]cty.Value{ + "id": cty.StringVal("component"), + "input": cty.StringVal("component"), + }), + }), + }, + wantPlannedDiags: initDiags(func(diags tfdiags.Diagnostics) tfdiags.Diagnostics { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Cannot remove stack instance", + Detail: "The stack instance stack.embedded[\"component\"].stack.simple[\"component\"] is targeted by an embedded stack block and cannot be removed. The relevant embedded stack is defined at git::https://example.com/test.git//with-single-input/removed-stack-instance-dynamic/removed-stack-instance-dynamic.tfstack.hcl:25,1-15.", + Subject: &hcl.Range{ + Filename: "git::https://example.com/test.git//with-single-input/removed-stack-from-embedded-stack/removed-stack-from-embedded-stack.tfstack.hcl", + Start: hcl.Pos{Line: 28, Column: 1, Byte: 360}, + End: hcl.Pos{Line: 28, Column: 8, Byte: 367}, + }, + }) + }), + }, + }, + "removed block targets component inside removed stack": { + path: filepath.Join("with-single-input", "removed-stack-instance-dynamic"), + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("stack.simple[\"component\"].component.self")). + AddInputVariable("id", cty.StringVal("component")). + AddInputVariable("input", cty.StringVal("component"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("stack.simple[\"component\"].component.self.testing_resource.data")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "component", + "value": "component", + }), + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("component", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("component"), + "value": cty.StringVal("component"), + })). + Build(), + cycle: TestCycle{ + planInputs: map[string]cty.Value{ + "removed": cty.MapVal(map[string]cty.Value{ + "component": cty.StringVal("component"), + }), + "removed-direct": cty.SetVal([]cty.Value{ + cty.StringVal("component"), + }), + }, + wantPlannedDiags: initDiags(func(diags tfdiags.Diagnostics) tfdiags.Diagnostics { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Cannot remove component instance", + Detail: "The component instance stack.simple[\"component\"].component.self is targeted by a component block and cannot be removed. The relevant component is defined at git::https://example.com/test.git//with-single-input/valid/valid.tfstack.hcl:19,1-17.", + Subject: &hcl.Range{ + Filename: "git::https://example.com/test.git//with-single-input/removed-stack-instance-dynamic/removed-stack-instance-dynamic.tfstack.hcl", + Start: hcl.Pos{Line: 51, Column: 1, Byte: 708}, + End: hcl.Pos{Line: 51, Column: 8, Byte: 715}, + }, + }) + }), + }, + }, + "removed block targets orphaned component": { + path: filepath.Join("with-single-input", "removed-component-from-stack-dynamic"), + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("stack.simple[\"component\"].component.self")). + AddInputVariable("id", cty.StringVal("component")). + AddInputVariable("input", cty.StringVal("component"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("stack.simple[\"component\"].component.self.testing_resource.data")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "component", + "value": "component", + }), + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("component", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("component"), + "value": cty.StringVal("component"), + })). + Build(), + cycle: TestCycle{ + planInputs: map[string]cty.Value{ + "simple_input": cty.MapValEmpty(cty.String), + "simple_removed": cty.SetVal([]cty.Value{ + cty.StringVal("component"), + }), + }, + wantPlannedDiags: initDiags(func(diags tfdiags.Diagnostics) tfdiags.Diagnostics { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid removed block", + Detail: "The component instance stack.simple[\"component\"].component.self could not be removed. The linked removed block was not executed because the `from` attribute of the removed block targets a component or embedded stack within an orphaned embedded stack.\n\nIn order to remove an entire stack, update your removed block to target the entire removed stack itself instead of the specific elements within it.", + Subject: &hcl.Range{ + Filename: "git::https://example.com/test.git//with-single-input/removed-component-from-stack-dynamic/removed-component-from-stack-dynamic.tfstack.hcl", + Start: hcl.Pos{Line: 60, Column: 1, Byte: 811}, + End: hcl.Pos{Line: 60, Column: 8, Byte: 818}, + }, + }) + }), + wantPlannedChanges: []stackplan.PlannedChange{ + &stackplan.PlannedChangeApplyable{ + Applyable: false, + }, + &stackplan.PlannedChangeHeader{ + TerraformVersion: version.SemVer, + }, + &stackplan.PlannedChangePlannedTimestamp{ + PlannedTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "for_each_input"}, + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.MapValEmpty(cty.String), + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "for_each_removed"}, + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.SetValEmpty(cty.String), + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "simple_input"}, + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.MapValEmpty(cty.String), + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "simple_removed"}, + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.SetVal([]cty.Value{ + cty.StringVal("component"), + }), + }, + }, + }, + }, + "removed block targets orphaned stack": { + path: filepath.Join("with-single-input", "removed-stack-from-embedded-stack"), + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("stack.embedded[\"component\"].stack.simple[\"component\"].component.self")). + AddInputVariable("id", cty.StringVal("component")). + AddInputVariable("input", cty.StringVal("component"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("stack.embedded[\"component\"].stack.simple[\"component\"].component.self.testing_resource.data")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "component", + "value": "component", + }), + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("component", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("component"), + "value": cty.StringVal("component"), + })). + Build(), + cycle: TestCycle{ + planInputs: map[string]cty.Value{ + "input": cty.MapValEmpty(cty.Map(cty.String)), + "removed": cty.MapVal(map[string]cty.Value{ + "component": cty.MapVal(map[string]cty.Value{ + "id": cty.StringVal("component"), + "input": cty.StringVal("component"), + }), + }), + }, + wantPlannedDiags: initDiags(func(diags tfdiags.Diagnostics) tfdiags.Diagnostics { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid removed block", + Detail: "The component instance stack.embedded[\"component\"].stack.simple[\"component\"].component.self could not be removed. The linked removed block was not executed because the `from` attribute of the removed block targets a component or embedded stack within an orphaned embedded stack.\n\nIn order to remove an entire stack, update your removed block to target the entire removed stack itself instead of the specific elements within it.", + Subject: &hcl.Range{ + Filename: "git::https://example.com/test.git//with-single-input/removed-stack-from-embedded-stack/removed-stack-from-embedded-stack.tfstack.hcl", + Start: hcl.Pos{Line: 28, Column: 1, Byte: 360}, + End: hcl.Pos{Line: 28, Column: 8, Byte: 367}, + }, + }) + }), + }, + }, + "removed block targets orphaned component without config definition": { + path: filepath.Join("with-single-input", "orphaned-component"), + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("stack.embedded.component.self")). + AddInputVariable("id", cty.StringVal("component")). + AddInputVariable("input", cty.StringVal("component"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("stack.embedded.component.self.testing_resource.data")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "component", + "value": "component", + }), + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("component", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("component"), + "value": cty.StringVal("component"), + })). + Build(), + cycle: TestCycle{ + wantPlannedDiags: initDiags(func(diags tfdiags.Diagnostics) tfdiags.Diagnostics { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid removed block", + Detail: "The component instance stack.embedded.component.self could not be removed. The linked removed block was not executed because the `from` attribute of the removed block targets a component or embedded stack within an orphaned embedded stack.\n\nIn order to remove an entire stack, update your removed block to target the entire removed stack itself instead of the specific elements within it.", + Subject: &hcl.Range{ + Filename: "git::https://example.com/test.git//with-single-input/orphaned-component/orphaned-component.tfstack.hcl", + Start: hcl.Pos{Line: 10, Column: 1, Byte: 131}, + End: hcl.Pos{Line: 10, Column: 8, Byte: 138}, + }, + }) + }), + }, + }, + "unknown embedded stack with internal component targeted by concrete removed block": { + path: filepath.Join("with-single-input", "removed-stack-instance-dynamic"), + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("stack.simple[\"component\"].component.self")). + AddInputVariable("id", cty.StringVal("component")). + AddInputVariable("input", cty.StringVal("component"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("stack.simple[\"component\"].component.self.testing_resource.data")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "component", + "value": "component", + }), + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("component", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("component"), + "value": cty.StringVal("component"), + })). + Build(), + cycle: TestCycle{ + planInputs: map[string]cty.Value{ + "removed": cty.UnknownVal(cty.Map(cty.String)), + }, + wantPlannedChanges: []stackplan.PlannedChange{ + &stackplan.PlannedChangeApplyable{ + Applyable: true, + }, + &stackplan.PlannedChangeHeader{ + TerraformVersion: version.SemVer, + }, + &stackplan.PlannedChangePlannedTimestamp{ + PlannedTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeComponentInstance{ + Addr: mustAbsComponentInstance("stack.simple[\"component\"].component.self"), + Action: plans.Delete, + Mode: plans.DestroyMode, + PlannedInputValues: map[string]plans.DynamicValue{ + "id": mustPlanDynamicValueDynamicType(cty.UnknownVal(cty.String)), + "input": mustPlanDynamicValueDynamicType(cty.UnknownVal(cty.String)), + }, + PlannedInputValueMarks: map[string][]cty.PathValueMarks{ + "id": nil, + "input": nil, + }, + PlannedOutputValues: make(map[string]cty.Value), + PlannedCheckResults: &states.CheckResults{}, + PlanTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeDeferredResourceInstancePlanned{ + ResourceInstancePlanned: stackplan.PlannedChangeResourceInstancePlanned{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("stack.simple[\"component\"].component.self.testing_resource.data"), + ChangeSrc: &plans.ResourceInstanceChangeSrc{ + Addr: mustAbsResourceInstance("testing_resource.data"), + PrevRunAddr: mustAbsResourceInstance("testing_resource.data"), + ProviderAddr: mustDefaultRootProvider("testing"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Delete, + Before: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("component"), + "value": cty.StringVal("component"), + })), + After: mustPlanDynamicValue(cty.NullVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + "value": cty.String, + }))), + }, + }, + PriorStateSrc: &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "component", + "value": "component", + }), + Dependencies: make([]addrs.ConfigResource, 0), + }, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.TestingResourceSchema, + }, + DeferredReason: providers.DeferredReasonDeferredPrereq, + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "input"}, + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.MapValEmpty(cty.String), + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "removed"}, + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.UnknownVal(cty.Map(cty.String)), + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "removed-direct"}, + Action: plans.Create, + Before: cty.NullVal(cty.DynamicPseudoType), + After: cty.SetValEmpty(cty.String), + }, + }, + }, + }, + "remove partial stack": { + path: filepath.Join("with-single-input", "multiple-components", "removed"), + state: stackstate.NewStateBuilder(). + AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("stack.multiple.component.one")). + AddInputVariable("id", cty.StringVal("one")). + AddInputVariable("input", cty.StringVal("one"))). + AddResourceInstance(stackstate.NewResourceInstanceBuilder(). + SetAddr(mustAbsResourceInstanceObject("stack.multiple.component.one.testing_resource.data")). + SetProviderAddr(mustDefaultRootProvider("testing")). + SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "one", + "value": "one", + }), + })). + Build(), + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("one", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("one"), + "value": cty.StringVal("one"), + })). + Build(), + cycle: TestCycle{ + wantPlannedChanges: []stackplan.PlannedChange{ + &stackplan.PlannedChangeApplyable{ + Applyable: true, + }, + &stackplan.PlannedChangeHeader{ + TerraformVersion: version.SemVer, + }, + &stackplan.PlannedChangePlannedTimestamp{ + PlannedTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeComponentInstance{ + Addr: mustAbsComponentInstance("stack.multiple.component.one"), + PlanApplyable: true, + PlanComplete: true, + Action: plans.Delete, + Mode: plans.DestroyMode, + PlannedInputValues: map[string]plans.DynamicValue{ + "id": mustPlanDynamicValueDynamicType(cty.StringVal("one")), + "input": mustPlanDynamicValueDynamicType(cty.StringVal("one")), + }, + PlannedInputValueMarks: map[string][]cty.PathValueMarks{ + "id": nil, + "input": nil, + }, + PlannedOutputValues: make(map[string]cty.Value), + PlannedCheckResults: &states.CheckResults{}, + PlanTimestamp: fakePlanTimestamp, + }, + &stackplan.PlannedChangeResourceInstancePlanned{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("stack.multiple.component.one.testing_resource.data"), + ChangeSrc: &plans.ResourceInstanceChangeSrc{ + Addr: mustAbsResourceInstance("testing_resource.data"), + PrevRunAddr: mustAbsResourceInstance("testing_resource.data"), + ProviderAddr: mustDefaultRootProvider("testing"), + ChangeSrc: plans.ChangeSrc{ + Action: plans.Delete, + Before: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("one"), + "value": cty.StringVal("one"), + })), + After: mustPlanDynamicValue(cty.NullVal(cty.Object(map[string]cty.Type{ + "id": cty.String, + "value": cty.String, + }))), + }, + }, + PriorStateSrc: &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: mustMarshalJSONAttrs(map[string]any{ + "id": "one", + "value": "one", + }), + Dependencies: make([]addrs.ConfigResource, 0), + }, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.TestingResourceSchema, + }, + &stackplan.PlannedChangeComponentInstance{ + Addr: mustAbsComponentInstance("stack.multiple.component.two"), + PlanApplyable: true, + PlanComplete: true, + Action: plans.Delete, + Mode: plans.DestroyMode, + PlannedOutputValues: make(map[string]cty.Value), + PlanTimestamp: fakePlanTimestamp, + }, }, }, }, diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/multiple-components/multiple-components.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/multiple-components/multiple-components.tfstack.hcl new file mode 100644 index 0000000000..cf4076afa5 --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/multiple-components/multiple-components.tfstack.hcl @@ -0,0 +1,34 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +provider "testing" "default" {} + +component "one" { + source = "../" + + providers = { + testing = provider.testing.default + } + + inputs = { + id = "one" + input = "one" + } +} + +component "two" { + source = "../" + + providers = { + testing = provider.testing.default + } + + inputs = { + id = "two" + input = "two" + } +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/multiple-components/removed/removed.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/multiple-components/removed/removed.tfstack.hcl new file mode 100644 index 0000000000..6823a34f0a --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/multiple-components/removed/removed.tfstack.hcl @@ -0,0 +1,11 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +removed { + from = stack.multiple + source = "../" +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/orphaned-component/orphaned-component.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/orphaned-component/orphaned-component.tfstack.hcl new file mode 100644 index 0000000000..b0b65a73da --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/orphaned-component/orphaned-component.tfstack.hcl @@ -0,0 +1,19 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +provider "testing" "default" {} + +removed { + // this is invalid, without a definition of the stack itself we can't remove + // components from it directly, instead we should removed the whole stack + from = stack.embedded.component.self + source = "../" + + providers = { + testing = provider.testing.default + } +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/removed-stack-from-embedded-stack/removed-stack-from-embedded-stack.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/removed-stack-from-embedded-stack/removed-stack-from-embedded-stack.tfstack.hcl new file mode 100644 index 0000000000..5a805d9d8c --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/removed-stack-from-embedded-stack/removed-stack-from-embedded-stack.tfstack.hcl @@ -0,0 +1,38 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +variable "input" { + type = map(map(string)) + default = {} +} + +variable "removed" { + type = map(map(string)) + default = {} +} + +stack "embedded" { + source = "../removed-stack-instance-dynamic" + + for_each = var.input + + inputs = { + input = each.value + } +} + +removed { + for_each = var.removed + + from = stack.embedded[each.key].stack.simple[each.value["id"]] + source = "../valid" + + inputs = { + id = each.value["id"] + input = each.value["input"] + } +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/removed-stack-instance-dynamic/removed-stack-instance-dynamic.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/removed-stack-instance-dynamic/removed-stack-instance-dynamic.tfstack.hcl index ddc4bf3ca8..107b1cbd92 100644 --- a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/removed-stack-instance-dynamic/removed-stack-instance-dynamic.tfstack.hcl +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/removed-stack-instance-dynamic/removed-stack-instance-dynamic.tfstack.hcl @@ -5,6 +5,8 @@ required_providers { } } +provider "testing" "default" {} + variable "input" { type = map(string) default = {} @@ -15,6 +17,11 @@ variable "removed" { default = {} } +variable "removed-direct" { + type = set(string) + default = [] +} + stack "simple" { for_each = var.input @@ -29,6 +36,9 @@ stack "simple" { removed { for_each = var.removed + // This removed block targets the stack directly, and just tells it to + // remove all components in the stack. + from = stack.simple[each.key] source = "../valid" @@ -37,3 +47,19 @@ removed { input = each.value } } + +removed { + for_each = var.removed-direct + + // This removed block removes the component in the specified stack directly. + // This is okay as long as only a single component in the stack is being + // removed. If an entire stack is being removed, you should use the other + // approach. + + from = stack.simple[each.key].component.self + source = "../" + + providers = { + testing = provider.testing.default + } +}