Do not add orphan nodes for deposed instances

Resource instances with no current object in state should not have
orphan nodes added to the graph, as deposed objects are handled
separately. This was previously handled correctly for the non-expanded
case, but expanded resources were missing the appropriate check for a
current object.

Also update the comment in the non-expanded case to hopefully clarify
that we're checking for the presence of a current object, not the
absence of any deposed objects. An instance may have both a current
object and zero or more deposed objects in some circumstances, and if
so, we still want an orphan node to be added if the instance is not in
configuration.
This commit is contained in:
Alisdair McDiarmid 2023-02-10 16:25:11 -05:00
parent d7d8a2262c
commit 7ecb0b8ffb
3 changed files with 73 additions and 2 deletions

View File

@ -33,7 +33,13 @@ func (t *OrphanResourceInstanceCountTransformer) Transform(g *Graph) error {
// number of instances of a single resource ought to always be small in any
// reasonable Terraform configuration.
Have:
for key := range rs.Instances {
for key, inst := range rs.Instances {
// Instances which have no current objects (only one or more
// deposed objects) will be taken care of separately
if inst.Current == nil {
continue
}
thisAddr := rs.Addr.Instance(key)
for _, wantAddr := range t.InstanceAddrs {
if wantAddr.Equal(thisAddr) {

View File

@ -161,6 +161,66 @@ func TestOrphanResourceCountTransformer_oneIndex(t *testing.T) {
}
}
func TestOrphanResourceCountTransformer_deposed(t *testing.T) {
state := states.NewState()
root := state.RootModule()
root.SetResourceInstanceCurrent(
mustResourceInstanceAddr("aws_instance.web").Resource,
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"foo"}`),
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
)
root.SetResourceInstanceCurrent(
mustResourceInstanceAddr("aws_instance.foo[0]").Resource,
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"foo"}`),
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
)
root.SetResourceInstanceCurrent(
mustResourceInstanceAddr("aws_instance.foo[1]").Resource,
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"foo"}`),
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
)
root.SetResourceInstanceDeposed(
mustResourceInstanceAddr("aws_instance.foo[2]").Resource,
states.NewDeposedKey(),
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"foo"}`),
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
)
g := Graph{Path: addrs.RootModuleInstance}
{
tf := &OrphanResourceInstanceCountTransformer{
Concrete: testOrphanResourceConcreteFunc,
Addr: addrs.RootModuleInstance.Resource(
addrs.ManagedResourceMode, "aws_instance", "foo",
),
InstanceAddrs: []addrs.AbsResourceInstance{mustResourceInstanceAddr("aws_instance.foo[0]")},
State: state,
}
if err := tf.Transform(&g); err != nil {
t.Fatalf("err: %s", err)
}
}
actual := strings.TrimSpace(g.String())
expected := strings.TrimSpace(testTransformOrphanResourceCountDeposedStr)
if actual != expected {
t.Fatalf("bad:\n\n%s", actual)
}
}
// When converting from a NoEach mode to an EachMap via a switch to for_each,
// an edge is necessary to ensure that the map-key'd instances
// are evaluated after the NoKey resource, because the final instance evaluated
@ -236,6 +296,10 @@ const testTransformOrphanResourceCountOneIndexStr = `
aws_instance.foo[1] (orphan)
`
const testTransformOrphanResourceCountDeposedStr = `
aws_instance.foo[1] (orphan)
`
const testTransformOrphanResourceForEachStr = `
aws_instance.foo (orphan)
aws_instance.foo["bar"] (orphan)

View File

@ -87,7 +87,8 @@ func (t *OrphanResourceInstanceTransformer) transform(g *Graph, ms *states.Modul
}
for key, inst := range rs.Instances {
// deposed instances will be taken care of separately
// Instances which have no current objects (only one or more
// deposed objects) will be taken care of separately
if inst.Current == nil {
continue
}