query: generate unique resource identifiers for results of expanded list resources#37681
Conversation
dsa0x
force-pushed
the
sams/query-genconfig-expanded-resource
branch
2 times, most recently
from
8a16803 to
53a7fd4
Compare
dsa0x
force-pushed
the
sams/query-genconfig-expanded-resource
branch
from
53a7fd4 to
f9e4d0d
Compare
dsa0x
added
no-changelog-needed
| InstanceAddrs []addrs.AbsResourceInstance | ||
| } | ||
|
|
||
| func (t *ResourceCountTransformer) Transform(g *Graph) error { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Oh right 😄 . I think we could at least rename that to ResourceExpansionTransformer. From the call-sites, i believe it is used by both count and for_each.
| // 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. |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Yeah it really isnt the the index, but the enum. Having looked that the Expander closely, we could even get this value from it directly without having to store it with with NodeResourcePlanInstance.
On the note of sorting, even if the result of ExpandedResource is not guaranteed to be sorted, we can sort the addresses and get a valid enum ourselves. Right now, I have created a method that relies on ExpandedResource in 364f395, but if that method is not sorted, we can still sort the resource in the new method.
dsa0x
force-pushed
the
sams/query-genconfig-expanded-resource
branch
from
4dc4eb1 to
364f395
Compare
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
2 participants
This fixes a bug where duplicate resource addresses are generated for the results of each instance of an expanded list resource. While
AbsResourceInstanceis indeed unique, the uniqueness key is not a valid hcl identifier, so we are using the enum of the list resource instance as an additional key in the address name.Target Release
1.15.x
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry