Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactoring: Implied move statements can be cross-package #30333

Merged
merged 1 commit into from
Jan 11, 2022

Conversation

apparentlymart
Copy link
Contributor

Terraform uses "implied" move statements to represent the situation where it automatically handles a switch from count to no-count on a resource. Because that situation requires targeting only a specific resource instance inside a specific module instance, implied move statements are always presented as if they had been declared in the root module and then traversed through the exact module instance path to reach the target resource.

However, that means they can potentially cross a module package boundary, if the changed resource belongs to an external module. Normally we prohibit that to avoid the root module depending on implementation details of the called module, but Terraform generates these implied statements based only on information in the called module and so there's no need to apply that same restriction to implied move statements, which will always have source and destination addresses belonging to the same module instance.

This change therefore fixes a misbehavior where Terraform would reject an attempt to switch from no-count to count in a called module, where previously the author of the calling configuration had no recourse to fix it because the change has actually happened upstream.

This fixes #30326. The function which generates the implied move statements already checks that they get generated with Implied: true set, and so the new test here just forces that to be true in order to see how the validation function reacts to it.

Terraform uses "implied" move statements to represent the situation where
it automatically handles a switch from count to no-count on a resource.
Because that situation requires targeting only a specific resource
instance inside a specific module instance, implied move statements are
always presented as if they had been declared in the root module and then
traversed through the exact module instance path to reach the target
resource.

However, that means they can potentially cross a module package boundary,
if the changed resource belongs to an external module. Normally we
prohibit that to avoid the root module depending on implementation details
of the called module, but Terraform generates these implied statements
based only on information in the called module and so there's no need to
apply that same restriction to implied move statements, which will always
have source and destination addresses belonging to the same module
instance.

This change therefore fixes a misbehavior where Terraform would reject
an attempt to switch from no-count to count in a called module, where
previously the author of the calling configuration had no recourse to fix
it because the change has actually happened upstream.
@apparentlymart apparentlymart added bug config 1.1-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged labels Jan 10, 2022
@apparentlymart apparentlymart added this to the v1.1.4 milestone Jan 10, 2022
@apparentlymart apparentlymart requested a review from a team January 10, 2022 23:41
@apparentlymart apparentlymart self-assigned this Jan 10, 2022
@github-actions
Copy link
Contributor

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 19, 2022
@xiehan xiehan deleted the b-move-implied-cross-package branch July 22, 2024 12:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.1-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged bug config
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect "Cross-package move statement" for move implied by changing "count" in an external module
2 participants