Allow moving instances to new resources #23582
Conversation
| )) | ||
| c.showDiagnostics(diags) | ||
| return 1 | ||
| fromEachMode = eachModeForInstanceKey(addrTo.Resource.Key) | ||
| } | ||
|
|
||
| resourceAddr := addrTo.ContainingResource() |
There was a problem hiding this comment.
Do we need to do anything special here for the situation where the target resource address is in a module that doesn't exist in the state yet? I don't recall if SyncWrapper().SetResourceMeta handles automatically creating the module shell in that case; if it does, I think it could help to note that specifically in a comment here to aid future maintainers.
There was a problem hiding this comment.
Nope, moving to a new module already works correctly. I'm going to double check that we have that in a test, or update this one to cover it.
| )) | ||
| c.showDiagnostics(diags) | ||
| return 1 | ||
| fromEachMode = eachModeForInstanceKey(addrTo.Resource.Key) |
There was a problem hiding this comment.
Could/should we use the configuration as a safety check here to make sure the user is asking for something sensible? By which I mean, if there's no configuration block at the target resource at all then maybe we still fail, and likewise if the set-ness of the count or for_each of that resource disagrees with the selected each mode we could fail that too.
That would help make sure that a typo doesn't lead to moving the instance somewhere that would cause it to be immediately destroyed as an "orphan" on the next apply. That'd be consistent with our rule that you can't terraform import into a resource that isn't present in the configuration, so maybe there's some existing logic over there we could borrow and generalize to ensure they both follow the same rules.
There was a problem hiding this comment.
We actually don't handle config or prevent any of these sorts of things at all at the moment, so I think that's a question for a much larger change. Seeing how the normal workflow is to verify the change with a plan, I don't think typos will be too disastrous. Allowing individual instances to be moved also makes fixing any typos much easier as well.
If a state mv target happens to be a resource that doesn't exist, allow the creation of the new resource inferring the EachMode from the target address.
jbardin
force-pushed
the
jbardin/state-mv
branch
from
c0471b2 to
a5cb36b
Compare
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
If a state mv target happens to be a resource that doesn't exist, allow
the creation of the new resource inferring the EachMode from the target
address.
Fixes #22301
Fixes #23569
Fixes #23497