deferred actions: allow unknown foreach attributes within import blocks #35311
Conversation
|
|
||
| var typeName, name string | ||
| switch tt := remain[0].(type) { | ||
| case hcl.TraverseRoot: |
There was a problem hiding this comment.
Above should ensure that this is a TraverseRoot, and would also panic with RootName() if it weren't.
There was a problem hiding this comment.
For a resource, this would definitely be TraverseRoot here. But we can (in theory) also handle data sources here. For a data source, the TraverseRoot is the data part of the traversal, and we skipped over that above meaning we would be processing a TraverseAttr here.
We're not actually processing data sources in this function yet, as the only user is the import functionality which can't target data sources, but I think it's okay to still support both here.
There was a problem hiding this comment.
Thanks. Now I see a key point we need to address, managed resources and data resources shouldn't be handled that differently. A managed resource can be prefixed with resource. just like a data source is prefixed with data.. I just checked though and while reference parsing works, the target addr parsing code was never updated, but we should probably start taking this into account to clean things up though.
There was a problem hiding this comment.
I'll put a follow up PR together that fixes this both here and in the target parsing function.
| switch tt := remain[0].(type) { | ||
| case hcl.TraverseRoot: | ||
| typeName = tt.Name | ||
| case hcl.TraverseAttr: |
There was a problem hiding this comment.
I don't think we could have a TraverseAttr here, since there is only one resource address following a module, and we turned that into a TraverseRoot
There was a problem hiding this comment.
As in the other comment, for a data source this would be TraverseAttr as we consumed the TraverseRoot when processing the data entry.
|
|
||
| var typeName, name string | ||
| switch tt := remain[0].(type) { | ||
| case hcl.TraverseRoot: |
There was a problem hiding this comment.
Thanks. Now I see a key point we need to address, managed resources and data resources shouldn't be handled that differently. A managed resource can be prefixed with resource. just like a data source is prefixed with data.. I just checked though and while reference parsing works, the target addr parsing code was never updated, but we should probably start taking this into account to clean things up though.
liamcervante
force-pushed
the
liamcervante/deferred-actions/unknown-import-to
branch
from
60bf04c
to
8fd3bfa
Compare
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
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. |
This PR adds support for unknown
for_eachvalues withinimportblocks when deferred actions is enabled.This can result in the
toargument being essentially unknown. To combat this we parse thetoargument as a partial resource, and defer all resources that it might refer to once the unknown values are resolved.Any resources that are already in state and might be targeted by a partial
toaddress are not deferred. The behaviour of the import block when targeting a resource already in state is simply a no-op. So, when the unknowntois eventually resolved and would point to a resource already in state it doesn't matter.This builds on the work in the previous PR allowing unknown
idvalues: #35300. The way we mark a resource as being deferred due to an import block is to simply mark the target ID value as being unknown.