Skip to content

Commit 669e8ff

Browse files
liamcervanteliamcervante
authored
stacks: skip full plan/apply cycles when deleting empty state (#35831)
1 parent 0be94d4 commit 669e8ff

File tree

10 files changed

+496
-11
lines changed

10 files changed

+496
-11
lines changed

‎internal/stacks/stackruntime/apply_destroy_test.go

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,193 @@ func TestApplyDestroy(t *testing.T) {
999999
},
10001000
},
10011001
},
1002+
"empty-destroy-with-data-source": {
1003+
path: path.Join("with-data-source", "dependent"),
1004+
cycles: []TestCycle{
1005+
{
1006+
planMode: plans.DestroyMode,
1007+
planInputs: map[string]cty.Value{
1008+
"id": cty.StringVal("foo"),
1009+
},
1010+
// deliberately empty, as we expect no changes from an
1011+
// empty state.
1012+
wantAppliedChanges: []stackstate.AppliedChange{
1013+
&stackstate.AppliedChangeComponentInstanceRemoved{
1014+
ComponentAddr: mustAbsComponent("component.data"),
1015+
ComponentInstanceAddr: mustAbsComponentInstance("component.data"),
1016+
},
1017+
&stackstate.AppliedChangeComponentInstanceRemoved{
1018+
ComponentAddr: mustAbsComponent("component.self"),
1019+
ComponentInstanceAddr: mustAbsComponentInstance("component.self"),
1020+
},
1021+
&stackstate.AppliedChangeInputVariable{
1022+
Addr: mustStackInputVariable("id"),
1023+
},
1024+
},
1025+
},
1026+
},
1027+
},
1028+
"partial destroy recovery": {
1029+
path: "component-chain",
1030+
description: "this test simulates a partial destroy recovery",
1031+
state: stackstate.NewStateBuilder().
1032+
// we only have data for the first component, indicating that
1033+
// the second and third components were destroyed but not the
1034+
// first one for some reason
1035+
AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.one")).
1036+
AddDependent(mustAbsComponent("component.two")).
1037+
AddInputVariable("id", cty.StringVal("one")).
1038+
AddInputVariable("value", cty.StringVal("foo")).
1039+
AddOutputValue("value", cty.StringVal("foo"))).
1040+
AddResourceInstance(stackstate.NewResourceInstanceBuilder().
1041+
SetAddr(mustAbsResourceInstanceObject("component.one.testing_resource.data")).
1042+
SetProviderAddr(mustDefaultRootProvider("testing")).
1043+
SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{
1044+
AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{
1045+
"id": "one",
1046+
"value": "foo",
1047+
}),
1048+
Status: states.ObjectReady,
1049+
})).
1050+
AddInput("value", cty.StringVal("foo")).
1051+
AddOutput("value", cty.StringVal("foo")).
1052+
Build(),
1053+
store: stacks_testing_provider.NewResourceStoreBuilder().
1054+
AddResource("one", cty.ObjectVal(map[string]cty.Value{
1055+
"id": cty.StringVal("one"),
1056+
"value": cty.StringVal("foo"),
1057+
})).
1058+
Build(),
1059+
cycles: []TestCycle{
1060+
{
1061+
planMode: plans.DestroyMode,
1062+
planInputs: map[string]cty.Value{
1063+
"value": cty.StringVal("foo"),
1064+
},
1065+
wantPlannedChanges: []stackplan.PlannedChange{
1066+
&stackplan.PlannedChangeApplyable{
1067+
Applyable: true,
1068+
},
1069+
&stackplan.PlannedChangeComponentInstance{
1070+
Addr: mustAbsComponentInstance("component.one"),
1071+
Action: plans.Delete,
1072+
Mode: plans.DestroyMode,
1073+
PlanComplete: true,
1074+
PlanApplyable: true,
1075+
PlannedInputValues: map[string]plans.DynamicValue{
1076+
"id": mustPlanDynamicValueDynamicType(cty.StringVal("one")),
1077+
"value": mustPlanDynamicValueDynamicType(cty.StringVal("foo")),
1078+
},
1079+
PlannedInputValueMarks: map[string][]cty.PathValueMarks{
1080+
"id": nil,
1081+
"value": nil,
1082+
},
1083+
PlannedOutputValues: map[string]cty.Value{
1084+
"value": cty.StringVal("foo"),
1085+
},
1086+
PlannedCheckResults: &states.CheckResults{},
1087+
PlanTimestamp: fakePlanTimestamp,
1088+
},
1089+
&stackplan.PlannedChangeResourceInstancePlanned{
1090+
ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.one.testing_resource.data"),
1091+
ChangeSrc: &plans.ResourceInstanceChangeSrc{
1092+
Addr: mustAbsResourceInstance("testing_resource.data"),
1093+
PrevRunAddr: mustAbsResourceInstance("testing_resource.data"),
1094+
ProviderAddr: mustDefaultRootProvider("testing"),
1095+
ChangeSrc: plans.ChangeSrc{
1096+
Action: plans.Delete,
1097+
Before: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{
1098+
"id": cty.StringVal("one"),
1099+
"value": cty.StringVal("foo"),
1100+
})),
1101+
After: mustPlanDynamicValue(cty.NullVal(cty.Object(map[string]cty.Type{
1102+
"id": cty.String,
1103+
"value": cty.String,
1104+
}))),
1105+
},
1106+
},
1107+
PriorStateSrc: &states.ResourceInstanceObjectSrc{
1108+
AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{
1109+
"id": "one",
1110+
"value": "foo",
1111+
}),
1112+
Status: states.ObjectReady,
1113+
Dependencies: make([]addrs.ConfigResource, 0),
1114+
},
1115+
ProviderConfigAddr: mustDefaultRootProvider("testing"),
1116+
Schema: stacks_testing_provider.TestingResourceSchema,
1117+
},
1118+
&stackplan.PlannedChangeComponentInstance{
1119+
Addr: mustAbsComponentInstance("component.three"),
1120+
Action: plans.Delete,
1121+
Mode: plans.DestroyMode,
1122+
PlanComplete: true,
1123+
PlanApplyable: true,
1124+
RequiredComponents: collections.NewSet(mustAbsComponent("component.two")),
1125+
PlannedOutputValues: map[string]cty.Value{
1126+
"value": cty.DynamicVal,
1127+
},
1128+
PlanTimestamp: fakePlanTimestamp,
1129+
},
1130+
&stackplan.PlannedChangeComponentInstance{
1131+
Addr: mustAbsComponentInstance("component.two"),
1132+
Action: plans.Delete,
1133+
Mode: plans.DestroyMode,
1134+
PlanComplete: true,
1135+
PlanApplyable: true,
1136+
RequiredComponents: collections.NewSet(mustAbsComponent("component.one")),
1137+
PlannedOutputValues: map[string]cty.Value{
1138+
"value": cty.DynamicVal,
1139+
},
1140+
PlanTimestamp: fakePlanTimestamp,
1141+
},
1142+
&stackplan.PlannedChangeHeader{
1143+
TerraformVersion: version.SemVer,
1144+
},
1145+
&stackplan.PlannedChangeOutputValue{
1146+
Addr: mustStackOutputValue("value"),
1147+
Action: plans.Delete,
1148+
Before: cty.StringVal("foo"),
1149+
After: cty.NullVal(cty.String),
1150+
},
1151+
&stackplan.PlannedChangePlannedTimestamp{
1152+
PlannedTimestamp: fakePlanTimestamp,
1153+
},
1154+
&stackplan.PlannedChangeRootInputValue{
1155+
Addr: mustStackInputVariable("value"),
1156+
Action: plans.NoOp,
1157+
Before: cty.StringVal("foo"),
1158+
After: cty.StringVal("foo"),
1159+
DeleteOnApply: true,
1160+
},
1161+
},
1162+
wantAppliedChanges: []stackstate.AppliedChange{
1163+
&stackstate.AppliedChangeComponentInstanceRemoved{
1164+
ComponentAddr: mustAbsComponent("component.one"),
1165+
ComponentInstanceAddr: mustAbsComponentInstance("component.one"),
1166+
},
1167+
&stackstate.AppliedChangeResourceInstanceObject{
1168+
ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.one.testing_resource.data"),
1169+
ProviderConfigAddr: mustDefaultRootProvider("testing"),
1170+
},
1171+
&stackstate.AppliedChangeComponentInstanceRemoved{
1172+
ComponentAddr: mustAbsComponent("component.three"),
1173+
ComponentInstanceAddr: mustAbsComponentInstance("component.three"),
1174+
},
1175+
&stackstate.AppliedChangeComponentInstanceRemoved{
1176+
ComponentAddr: mustAbsComponent("component.two"),
1177+
ComponentInstanceAddr: mustAbsComponentInstance("component.two"),
1178+
},
1179+
&stackstate.AppliedChangeOutputValue{
1180+
Addr: mustStackOutputValue("value"),
1181+
},
1182+
&stackstate.AppliedChangeInputVariable{
1183+
Addr: mustStackInputVariable("value"),
1184+
},
1185+
},
1186+
},
1187+
},
1188+
},
10021189
}
10031190
for name, tc := range tcs {
10041191
t.Run(name, func(t *testing.T) {

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,26 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla
208208

209209
mode := c.main.PlanningOpts().PlanningMode
210210
if mode == plans.DestroyMode {
211+
212+
if !c.main.PlanPrevState().HasComponentInstance(c.Addr()) {
213+
// If the component instance doesn't exist in the previous
214+
// state at all, then we don't need to do anything.
215+
//
216+
// This means the component instance was added to the config
217+
// and never applied, or that it was previously destroyed
218+
// via an earlier destroy operation.
219+
//
220+
// Return a dummy plan:
221+
return &plans.Plan{
222+
UIMode: plans.DestroyMode,
223+
Complete: true,
224+
Applyable: true,
225+
Errored: false,
226+
Timestamp: c.main.PlanTimestamp(),
227+
Changes: plans.NewChangesSrc(), // no changes
228+
}, nil
229+
}
230+
211231
// If we are destroying, then we are going to do the refresh
212232
// and destroy plan in two separate stages. This helps resolves
213233
// cycles within the dependency graph, as anything requiring
@@ -331,6 +351,18 @@ func (c *ComponentInstance) ApplyModuleTreePlan(ctx context.Context, plan *plans
331351
panic("called ApplyModuleTreePlan with an evaluator not instantiated for applying")
332352
}
333353

354+
if plan.UIMode == plans.DestroyMode && plan.Changes.Empty() {
355+
stackPlan := c.main.PlanBeingApplied().Components.Get(c.Addr())
356+
357+
// If we're destroying and there's nothing to destroy, then we can
358+
// consider this a no-op.
359+
return &ComponentInstanceApplyResult{
360+
FinalState: plan.PriorState, // after refresh
361+
AffectedResourceInstanceObjects: resourceInstanceObjectsAffectedByStackPlan(stackPlan),
362+
Complete: true,
363+
}, diags
364+
}
365+
334366
// This is the result to return along with any errors that prevent us from
335367
// even starting the modules runtime apply phase. It reports that nothing
336368
// changed at all.

‎internal/stacks/stackruntime/plan_test.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,108 @@ func TestPlan_invalid(t *testing.T) {
174174
}
175175
}
176176

177+
// TestPlan uses a generic framework for running plan integration tests
178+
// against Stacks. Generally, new tests should be added into this function
179+
// rather than copying the large amount of duplicate code from the other
180+
// tests in this file.
181+
//
182+
// If you are editing other tests in this file, please consider moving them
183+
// into this test function so they can reuse the shared setup and boilerplate
184+
// code managing the boring parts of the test.
185+
func TestPlan(t *testing.T) {
186+
fakePlanTimestamp, err := time.Parse(time.RFC3339, "1991-08-25T20:57:08Z")
187+
if err != nil {
188+
t.Fatal(err)
189+
}
190+
191+
tcs := map[string]struct {
192+
path string
193+
state *stackstate.State
194+
store *stacks_testing_provider.ResourceStore
195+
cycle TestCycle
196+
}{
197+
"empty-destroy-with-data-source": {
198+
path: path.Join("with-data-source", "dependent"),
199+
cycle: TestCycle{
200+
planMode: plans.DestroyMode,
201+
planInputs: map[string]cty.Value{
202+
"id": cty.StringVal("foo"),
203+
},
204+
wantPlannedChanges: []stackplan.PlannedChange{
205+
&stackplan.PlannedChangeApplyable{
206+
Applyable: true,
207+
},
208+
&stackplan.PlannedChangeComponentInstance{
209+
Addr: mustAbsComponentInstance("component.data"),
210+
PlanApplyable: true,
211+
PlanComplete: true,
212+
Action: plans.Delete,
213+
Mode: plans.DestroyMode,
214+
RequiredComponents: collections.NewSet(mustAbsComponent("component.self")),
215+
PlannedOutputValues: make(map[string]cty.Value),
216+
PlanTimestamp: fakePlanTimestamp,
217+
},
218+
&stackplan.PlannedChangeComponentInstance{
219+
Addr: mustAbsComponentInstance("component.self"),
220+
PlanComplete: true,
221+
PlanApplyable: true,
222+
Action: plans.Delete,
223+
Mode: plans.DestroyMode,
224+
PlannedOutputValues: map[string]cty.Value{
225+
"id": cty.NullVal(cty.DynamicPseudoType),
226+
},
227+
PlanTimestamp: fakePlanTimestamp,
228+
},
229+
&stackplan.PlannedChangeHeader{
230+
TerraformVersion: version.SemVer,
231+
},
232+
&stackplan.PlannedChangePlannedTimestamp{
233+
PlannedTimestamp: fakePlanTimestamp,
234+
},
235+
&stackplan.PlannedChangeRootInputValue{
236+
Addr: mustStackInputVariable("id"),
237+
Action: plans.Create,
238+
Before: cty.NullVal(cty.DynamicPseudoType),
239+
After: cty.StringVal("foo"),
240+
DeleteOnApply: true,
241+
},
242+
},
243+
},
244+
},
245+
}
246+
for name, tc := range tcs {
247+
t.Run(name, func(t *testing.T) {
248+
ctx := context.Background()
249+
250+
lock := depsfile.NewLocks()
251+
lock.SetProvider(
252+
addrs.NewDefaultProvider("testing"),
253+
providerreqs.MustParseVersion("0.0.0"),
254+
providerreqs.MustParseVersionConstraints("=0.0.0"),
255+
providerreqs.PreferredHashes([]providerreqs.Hash{}),
256+
)
257+
258+
store := tc.store
259+
if store == nil {
260+
store = stacks_testing_provider.NewResourceStore()
261+
}
262+
263+
testContext := TestContext{
264+
timestamp: &fakePlanTimestamp,
265+
config: loadMainBundleConfigForTest(t, tc.path),
266+
providers: map[addrs.Provider]providers.Factory{
267+
addrs.NewDefaultProvider("testing"): func() (providers.Interface, error) {
268+
return stacks_testing_provider.NewProviderWithData(t, store), nil
269+
},
270+
},
271+
dependencyLocks: *lock,
272+
}
273+
274+
testContext.Plan(t, ctx, tc.state, tc.cycle)
275+
})
276+
}
277+
}
278+
177279
func TestPlanWithMissingInputVariable(t *testing.T) {
178280
ctx := context.Background()
179281
cfg := loadMainBundleConfigForTest(t, "plan-undeclared-variable-in-component")

‎internal/stacks/stackruntime/testdata/mainbundle/test/auth-provider-w-data/auth-provider-w-data.tfstack.hcl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ provider "testing" "main" {}
1010

1111
provider "testing" "credentialed" {
1212
config {
13+
require_auth = true
1314
authentication = component.load.credentials
1415
}
1516
}

‎internal/stacks/stackruntime/testdata/mainbundle/test/auth-provider-w-data/removed/removed.tfstack.hcl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ provider "testing" "main" {}
1010

1111
provider "testing" "credentialed" {
1212
config {
13+
require_auth = true
1314
authentication = component.load.credentials
1415
}
1516
}

‎internal/stacks/stackruntime/testdata/mainbundle/test/component-chain/component-chain.tf

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
terraform {
2+
required_providers {
3+
testing = {
4+
source = "hashicorp/testing"
5+
version = "0.1.0"
6+
}
7+
}
8+
}
9+
10+
11+
variable "id" {
12+
type = string
13+
default = null
14+
nullable = true # We'll generate an ID if none provided.
15+
}
16+
17+
variable "value" {
18+
type = string
19+
}
20+
21+
resource "testing_resource" "data" {
22+
id = var.id
23+
value = var.value
24+
}
25+
26+
output "value" {
27+
value = testing_resource.data.value
28+
}

0 commit comments

Comments
 (0)