stacks: stacks destroy ordering should match Terraform (#35748)

* add tests that highlight known issues in the destroy mechanism

* stacks: stacks destroy ordering should match Terraform

internal/stacks/stackruntime/apply_destroy_test.go

@@ -940,6 +940,65 @@ func TestApplyDestroy(t *testing.T) {
 				},
 			},
 		},
+		"destroy-with-provider-req-and-removed": {
+			path: path.Join("auth-provider-w-data", "removed"),
+			state: stackstate.NewStateBuilder().
+				AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.load")).
+					AddDependent(mustAbsComponent("component.create")).
+					AddOutputValue("credentials", cty.StringVal("wrong"))). // must reload the credentials
+				AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.create")).
+					AddDependency(mustAbsComponent("component.load"))).
+				AddResourceInstance(stackstate.NewResourceInstanceBuilder().
+					SetAddr(mustAbsResourceInstanceObject("component.create.testing_resource.resource")).
+					SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{
+						AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{
+							"id":    "resource",
+							"value": nil,
+						}),
+						Status: states.ObjectReady,
+					}).
+					SetProviderAddr(mustDefaultRootProvider("testing"))).
+				Build(),
+			store: stacks_testing_provider.NewResourceStoreBuilder().AddResource("credentials", cty.ObjectVal(map[string]cty.Value{
+				"id": cty.StringVal("credentials"),
+				// we have the wrong value in state, so this correct value must
+				// be loaded for this test to work.
+				"value": cty.StringVal("authn"),
+			})).Build(),
+			mutators: []func(store *stacks_testing_provider.ResourceStore, testContext TestContext) TestContext{
+				func(store *stacks_testing_provider.ResourceStore, testContext TestContext) TestContext {
+					testContext.providers[addrs.NewDefaultProvider("testing")] = func() (providers.Interface, error) {
+						provider := stacks_testing_provider.NewProviderWithData(t, store)
+						provider.Authentication = "authn" // So we must reload the data source in order to authenticate.
+						return provider, nil
+					}
+					return testContext
+				},
+			},
+			cycles: []TestCycle{
+				{
+					planMode: plans.DestroyMode,
+					wantAppliedChanges: []stackstate.AppliedChange{
+						&stackstate.AppliedChangeComponentInstanceRemoved{
+							ComponentAddr:         mustAbsComponent("component.create"),
+							ComponentInstanceAddr: mustAbsComponentInstance("component.create"),
+						},
+						&stackstate.AppliedChangeResourceInstanceObject{
+							ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.create.testing_resource.resource"),
+							ProviderConfigAddr:         mustDefaultRootProvider("testing"),
+						},
+						&stackstate.AppliedChangeComponentInstanceRemoved{
+							ComponentAddr:         mustAbsComponent("component.load"),
+							ComponentInstanceAddr: mustAbsComponentInstance("component.load"),
+						},
+						&stackstate.AppliedChangeResourceInstanceObject{
+							ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.load.data.testing_data_source.credentials"),
+							ProviderConfigAddr:         mustDefaultRootProvider("testing"),
+						},
+					},
+				},
+			},
+		},
 	}
 	for name, tc := range tcs {
 		t.Run(name, func(t *testing.T) {

internal/stacks/stackruntime/apply_test.go

@@ -1144,6 +1144,70 @@ After applying this plan, Terraform will no longer manage these objects. You wil
 				},
 			},
 		},
+		"removed block with provider-to-component dep": {
+			path: path.Join("auth-provider-w-data", "removed"),
+			state: stackstate.NewStateBuilder().
+				AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.load")).
+					AddDependent(mustAbsComponent("component.create")).
+					AddOutputValue("credentials", cty.StringVal("wrong"))). // must reload the credentials
+				AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.create")).
+					AddDependency(mustAbsComponent("component.load"))).
+				AddResourceInstance(stackstate.NewResourceInstanceBuilder().
+					SetAddr(mustAbsResourceInstanceObject("component.create.testing_resource.resource")).
+					SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{
+						AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{
+							"id":    "resource",
+							"value": nil,
+						}),
+						Status: states.ObjectReady,
+					}).
+					SetProviderAddr(mustDefaultRootProvider("testing"))).
+				Build(),
+			store: stacks_testing_provider.NewResourceStoreBuilder().AddResource("credentials", cty.ObjectVal(map[string]cty.Value{
+				"id": cty.StringVal("credentials"),
+				// we have the wrong value in state, so this correct value must
+				// be loaded for this test to work.
+				"value": cty.StringVal("authn"),
+			})).Build(),
+			cycles: []TestCycle{
+				{
+					planMode: plans.NormalMode,
+					wantAppliedChanges: []stackstate.AppliedChange{
+						&stackstate.AppliedChangeComponentInstanceRemoved{
+							ComponentAddr:         mustAbsComponent("component.create"),
+							ComponentInstanceAddr: mustAbsComponentInstance("component.create"),
+						},
+						&stackstate.AppliedChangeResourceInstanceObject{
+							ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.create.testing_resource.resource"),
+							ProviderConfigAddr:         mustDefaultRootProvider("testing"),
+							NewStateSrc:                nil, // deleted
+						},
+						&stackstate.AppliedChangeComponentInstance{
+							ComponentAddr:         mustAbsComponent("component.load"),
+							ComponentInstanceAddr: mustAbsComponentInstance("component.load"),
+							OutputValues: map[addrs.OutputValue]cty.Value{
+								addrs.OutputValue{Name: "credentials"}: cty.StringVal("authn").Mark(marks.Sensitive),
+							},
+							InputVariables: make(map[addrs.InputVariable]cty.Value),
+							Dependents:     collections.NewSet(mustAbsComponent("component.create")),
+						},
+						&stackstate.AppliedChangeResourceInstanceObject{
+							ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.load.data.testing_data_source.credentials"),
+							NewStateSrc: &states.ResourceInstanceObjectSrc{
+								AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{
+									"id":    "credentials",
+									"value": "authn",
+								}),
+								AttrSensitivePaths: make([]cty.Path, 0),
+								Status:             states.ObjectReady,
+							},
+							ProviderConfigAddr: mustDefaultRootProvider("testing"),
+							Schema:             stacks_testing_provider.TestingDataSourceSchema,
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for name, tc := range tcs {
@@ -1168,7 +1232,9 @@ After applying this plan, Terraform will no longer manage these objects. You wil
 				config:    loadMainBundleConfigForTest(t, tc.path),
 				providers: map[addrs.Provider]providers.Factory{
 					addrs.NewDefaultProvider("testing"): func() (providers.Interface, error) {
-						return stacks_testing_provider.NewProviderWithData(t, store), nil
+						provider := stacks_testing_provider.NewProviderWithData(t, store)
+						provider.Authentication = "authn"
+						return provider, nil
 					},
 				},
 				dependencyLocks: *lock,

internal/stacks/stackruntime/internal/stackeval/component_instance.go

@@ -255,16 +255,12 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla
 							// doesn't exist so it's fine.
 							continue
 						}
-						depComponent := depStack.Component(ctx, depAddr.Item)
-						if depComponent == nil {
-							// again, the thing we need to wait to be deleted
-							// doesn't exist so it's fine.
-							continue
+						depComponent, depRemoved := depStack.ApplyableComponents(ctx, depAddr.Item)
+						if depComponent != nil && !depComponent.PlanIsComplete(ctx) {
+							opts.ExternalDependencyDeferred = true
+							break
 						}
-						if !depComponent.PlanIsComplete(ctx) {
-							// The other component couldn't be deleted in a single
-							// go, so to be safe we'll defer our deletions until
-							// the other one is complete.
+						if depRemoved != nil && !depRemoved.PlanIsComplete(ctx) {
 							opts.ExternalDependencyDeferred = true
 							break
 						}
@@ -299,26 +295,6 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla
 					opts.ExternalDependencyDeferred = true
 					break
 				}
-
-				// We're also going to look through any upstream components
-				// that are being removed to make sure they are removed first.
-				for _, depAddr := range c.PlanPrevDependents(ctx).Elems() {
-					depStack := c.main.Stack(ctx, depAddr.Stack, PlanPhase)
-					if depStack == nil {
-						break
-					}
-					depRemoved := depStack.Removed(ctx, depAddr.Item)
-					if depRemoved == nil {
-						break
-					}
-					if !depRemoved.PlanIsComplete(ctx) {
-						// The other component couldn't be deleted in a single
-						// go, so to be safe we'll defer our deletions until
-						// the other one is complete.
-						opts.ExternalDependencyDeferred = true
-						break
-					}
-				}
 			}
 
 			// The instance is also upstream deferred if the for_each value for

internal/stacks/stackruntime/internal/stackeval/main_apply.go

@@ -197,22 +197,11 @@ func ApplyPlan(ctx context.Context, config *stackconfig.Config, plan *stackplan.
 
 							// If we're being destroyed, then we're waiting for
 							// everything that depended on us anyway.
-							waitForRemoveds = collections.NewSet[stackaddrs.AbsComponent]()
+							waitForRemoveds = dependencyAddrs
 						} else {
 							// For all other actions, we must wait for our
 							// dependencies to finish applying their changes.
 							waitForComponents = dependencyAddrs
-
-							// If we're not being destroyed we might have some
-							// depdendents that are being destroyed, and we need
-							// to wait for them to finish before we can start.
-							waitForRemoveds = collections.NewSet[stackaddrs.AbsComponent]()
-							for _, dependent := range dependentAddrs.Elems() {
-								dependentStack := main.Stack(ctx, dependent.Stack, ApplyPhase)
-								if dependentStack.Removed(ctx, dependent.Item) != nil {
-									waitForRemoveds.Add(dependent)
-								}
-							}
 						}
 						if depCount := waitForComponents.Len(); depCount != 0 {
 							log.Printf("[TRACE] stackeval: %s waiting for its predecessors (%d) to complete", addr, depCount)

internal/stacks/stackruntime/internal/stackeval/removed_instance.go

@@ -120,16 +120,12 @@ func (r *RemovedInstance) ModuleTreePlan(ctx context.Context) (*plans.Plan, tfdi
 				// doesn't exist so it's fine.
 				break
 			}
-			depComponent := depStack.Component(ctx, depAddr.Item)
-			if depComponent == nil {
-				// again, the thing we need to wait to be deleted
-				// doesn't exist so it's fine.
+			depComponent, depRemoved := depStack.ApplyableComponents(ctx, depAddr.Item)
+			if depComponent != nil && !depComponent.PlanIsComplete(ctx) {
+				deferred = true
 				break
 			}
-			if !depComponent.PlanIsComplete(ctx) {
-				// The other component couldn't be deleted in a single
-				// go, so to be safe we'll defer our deletions until
-				// the other one is complete.
+			if depRemoved != nil && !depRemoved.PlanIsComplete(ctx) {
 				deferred = true
 				break
 			}