Skip to content

Conversation

dsa0x
Copy link
Member

@dsa0x dsa0x commented Sep 26, 2025

This fixes a bug where duplicate resource addresses are generated for the results of each instance of an expanded list resource. While AbsResourceInstance is indeed unique, the uniqueness key is not a valid hcl identifier.

Target Release

1.15.x

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@dsa0x dsa0x force-pushed the sams/query-genconfig-expanded-resource branch 2 times, most recently from 8a16803 to 53a7fd4 Compare September 26, 2025 09:23
@dsa0x dsa0x force-pushed the sams/query-genconfig-expanded-resource branch from 53a7fd4 to f9e4d0d Compare September 26, 2025 09:34
@dsa0x dsa0x added no-changelog-needed Add this to your PR if the change does not require a changelog entry 1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged and removed no-changelog-needed Add this to your PR if the change does not require a changelog entry labels Sep 26, 2025
@dsa0x dsa0x marked this pull request as ready for review September 26, 2025 10:14
@dsa0x dsa0x requested a review from a team as a code owner September 26, 2025 10:14
InstanceAddrs []addrs.AbsResourceInstance
}

func (t *ResourceCountTransformer) Transform(g *Graph) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is leaning on some very old code that probably should have been refactored out (notice it only mentions count, as if there's no other possibility 😆) I wonder if there's another was to get a generated value here without having to hide it in the NodeAbstractResourceInstance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config generation abstraction for instances from a list resource is genconfig.ResourceListElement. Since config generation is the only place where this index value has meaning, perhaps we can insert it there while iterating over the list states?

// override is set by the graph itself, just before this node executes.
override *configs.Override

// expansionEnum tracks the index of the resource instance within the resource.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't really track the index, it only happens to coincide with the order that the expansion addresses were generated by the Expander (the fact they are even sorted isn't guaranteed, it's just there to make consistent tests easier)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants