Skip to content

Commit

Permalink
stacks: error when removing a component still in config (#35692)
Browse files Browse the repository at this point in the history
  • Loading branch information
liamcervante authored Sep 10, 2024
1 parent f844ba0 commit b22db51
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 1 deletion.
1 change: 0 additions & 1 deletion internal/stacks/stackruntime/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3147,7 +3147,6 @@ func TestApply_RemovedBlocks(t *testing.T) {
)

// TODO: Add tests for and implement the following cases:
// - Removed and component blocks that target the same instance.
// - Validate what happens when a removed block foreach evaluates to
// unknown.
// - Add a test for a removed block targeting state that has already been
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,24 @@ func (r *RemovedInstance) ModuleTreePlan(ctx context.Context) (*plans.Plan, tfdi
return doOnceWithDiags(ctx, &r.moduleTreePlan, r.main, func(ctx context.Context) (*plans.Plan, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics

component := r.main.Stack(ctx, r.Addr().Stack, PlanPhase).Component(ctx, r.Addr().Item.Component)
if component != nil {
insts, unknown := component.Instances(ctx, PlanPhase)
if !unknown {
if _, exists := insts[r.key]; exists {
// The instance we're planning to remove is also targeted
// by a component block. We won't remove it, and we'll
// report a diagnostic to that effect.
return nil, diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Cannot remove component instance",
Detail: fmt.Sprintf("The component instance %s is targeted by a component block and cannot be removed. The relevant component is defined at %s.", r.Addr(), component.Declaration(ctx).DeclRange.ToHCL()),
Subject: r.DeclRange(ctx),
})
}
}
}

known, unknown, moreDiags := EvalProviderValues(ctx, r.main, r.call.Config(ctx).config.ProviderConfigs, PlanPhase, r)
if moreDiags.HasErrors() {
// We won't actually add the diagnostics here, they should be
Expand Down
108 changes: 108 additions & 0 deletions internal/stacks/stackruntime/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4437,6 +4437,114 @@ func TestPlan_RemovedBlocks(t *testing.T) {
},
},
},
"duplicate component": {
source: filepath.Join("with-single-input", "removed-component-instance"),
initialState: stackstate.NewStateBuilder().
AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.self[\"a\"]")).
AddInputVariable("id", cty.StringVal("a")).
AddInputVariable("input", cty.StringVal("a"))).
AddResourceInstance(stackstate.NewResourceInstanceBuilder().
SetAddr(mustAbsResourceInstanceObject("component.self[\"a\"].testing_resource.data")).
SetProviderAddr(mustDefaultRootProvider("testing")).
SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: mustMarshalJSONAttrs(map[string]any{
"id": "a",
"value": "a",
}),
})).
Build(),
store: stacks_testing_provider.NewResourceStoreBuilder().
AddResource("a", cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("a"),
"value": cty.StringVal("a"),
})).
Build(),
inputs: map[string]cty.Value{
"input": cty.SetVal([]cty.Value{
cty.StringVal("a"),
}),
"removed": cty.SetVal([]cty.Value{
cty.StringVal("a"),
}),
},
wantPlanChanges: []stackplan.PlannedChange{
&stackplan.PlannedChangeApplyable{
Applyable: false, // No! The removed block is a duplicate of the component!
},
&stackplan.PlannedChangeComponentInstance{
Addr: mustAbsComponentInstance("component.self[\"a\"]"),
PlanComplete: true,
PlanApplyable: false, // no changes
Action: plans.Update,
PlannedInputValues: map[string]plans.DynamicValue{
"id": mustPlanDynamicValueDynamicType(cty.StringVal("a")),
"input": mustPlanDynamicValueDynamicType(cty.StringVal("a")),
},
PlannedInputValueMarks: map[string][]cty.PathValueMarks{
"input": nil,
"id": nil,
},
PlannedOutputValues: make(map[string]cty.Value),
PlannedCheckResults: &states.CheckResults{},
PlanTimestamp: fakePlanTimestamp,
},
&stackplan.PlannedChangeResourceInstancePlanned{
ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.self[\"a\"].testing_resource.data"),
ChangeSrc: &plans.ResourceInstanceChangeSrc{
Addr: mustAbsResourceInstance("testing_resource.data"),
PrevRunAddr: mustAbsResourceInstance("testing_resource.data"),
ChangeSrc: plans.ChangeSrc{
Action: plans.NoOp,
Before: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("a"),
"value": cty.StringVal("a"),
})),
After: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("a"),
"value": cty.StringVal("a"),
})),
},
ProviderAddr: mustDefaultRootProvider("testing"),
},
PriorStateSrc: &states.ResourceInstanceObjectSrc{
AttrsJSON: mustMarshalJSONAttrs(map[string]any{
"id": "a",
"value": "a",
}),
Dependencies: make([]addrs.ConfigResource, 0),
Status: states.ObjectReady,
},
ProviderConfigAddr: mustDefaultRootProvider("testing"),
Schema: stacks_testing_provider.TestingResourceSchema,
},
&stackplan.PlannedChangeHeader{
TerraformVersion: version.SemVer,
},
&stackplan.PlannedChangePlannedTimestamp{
PlannedTimestamp: fakePlanTimestamp,
},
&stackplan.PlannedChangeRootInputValue{
Addr: stackaddrs.InputVariable{Name: "input"},
Value: cty.SetVal([]cty.Value{
cty.StringVal("a"),
}),
},
&stackplan.PlannedChangeRootInputValue{
Addr: stackaddrs.InputVariable{Name: "removed"},
Value: cty.SetVal([]cty.Value{
cty.StringVal("a"),
}),
},
},
wantPlanDiags: []expectedDiagnostic{
{
severity: tfdiags.Error,
summary: "Cannot remove component instance",
detail: "The component instance component.self[\"a\"] 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/removed-component-instance/removed-component-instance.tfstack.hcl:18,1-17.",
},
},
},
}

for name, tc := range tcs {
Expand Down

0 comments on commit b22db51

Please sign in to comment.