hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.71k stars 9.55k forks source link

Give better feedback when two "moved" blocks target the same resource instance address #33671

Open KenSpur opened 1 year ago

KenSpur commented 1 year ago

Terraform Version

Terraform v1.5.5
on windows_amd64

Terraform Configuration Files

code to reproduce issue

Debug Output

Should be easy to recreate using the provided code .

│ Error: Ambiguous move statements │ │ on module\main.tf line 23: │ 23: moved { │ │ A statement at main.tf:19,1 declared that azurerm_resource_group.rg moved to module.resource_group.azurerm_resource_group.moved, but this statement instead declares that module.resource_group.azurerm_resource_group.main moved there. │ │ Each resource can have moved from only one source resource.

Expected Behavior

The azurerm_resource_group should be successfully moved from the root module to the child module without any errors. This process should not hinder the consumption of child modules that make use of the moved statement.

Actual Behavior

Received the following error:

│ Error: Ambiguous move statements │ │ on module\main.tf line 18: │ 18: moved { │ │ A statement at main.tf:19,1 declared that azurerm_resource_group.rg moved to module.resource_group.azurerm_resource_group.moved, but this statement instead declares that module.resource_group.azurerm_resource_group.main moved there. │ │ Each resource can have moved from only one source resource.

Steps to Reproduce

1.terraform init 2.terraform apply

Additional Context

While the provided example uses a local module, the same issue arises when consuming externally hosted modules.

References

No response

apparentlymart commented 1 year ago

Hi @KenSpur! Thanks for reporting this.

Based on your code example, I think Terraform is working as intended here. Terraform requires an unambiguous graph of moves so that the validity of the move statements can be decided entirely by the configuration, and conversely so that it isn't possible to get yourself into a trap where you can't proceed because the state contains two resource instances that have apparently moved to the same address.

You can get a similar effect to what you described here by exploiting the "chaining rule" for moves: if the to of one move statement matches the from of another then Terraform will bind a resource instance from either of the from addresses to the final to address. In your case, that would mean changing the main.tf move statement to the following:

moved {
  from = azurerm_resource_group.rg
  to   = module.resource_group.azurerm_resource_group.main
}

Such a declaration, combined with the existing moved block in the child module, creates several possible situations, handled in the following preference order:

  1. State has no objects bound to any of the addresses, in which case both of the moved blocks are inert.
  2. State already has an object bound to module.resource_group.azurerm_resource_group.moved, in which case both of the moved blocks are inert.
  3. State already has an object bound to module.resource_group.azurerm_resource_group.main, in which case the root module moved block is inert and the existing object will be rebound to module.resource_group.azurerm_resource_group.main.
  4. State already has an object bound to top-level azurerm_resource_group.rg, in which case both moved blocks are active and that object gets rebound to module.resource_group.azurerm_resource_group.moved

As a general rule, if the source address for an inert moved block has an object bound to it then that object will be planned for destruction. That is Terraform's last resort for dealing with an unresolvable situation, and so chaining rule allows you to describe which of multiple conflicting objects should be kept. If you don't have multiple conflicting objects then the single object will end up bound to the final address either way.

Since Terraform was working as designed here, I'm going to relabel this as an enhancement. My initial idea for what this enhancement represents is to make this error message give a direct suggestion about using the chaining rule -- that is, a more concise version of what I described above -- and also to consider adding a section about this rarer situation to the refactoring documentation.

Thanks again!

KenSpur commented 1 year ago

Hey @apparentlymart,

Thank you for the detailed response. I appreciate the insight into Terraform's design considerations regarding the moved statement.

From the scenarios you outlined, I gather that chaining is an integral part of managing the state transitions effectively. With this in mind, would it be accurate to infer that the best practice, especially when refactoring or working with modules, is to always bind to the initial from address in a move chain?

apparentlymart commented 1 year ago

In your particular situation there is only one possible valid chain, because the root module can declare that an object has moved into its child but the child cannot declare that the object has moved from its parent.

If this were all happening in the same module then you as the author would need to make a decision about which object should "win" in the edge case where there are multiple objects bound to addresses in the chain. For example:

moved {
  from = null_resource.a
  to   = null_resource.b
}

moved {
  from = null_resource.b
  to   = null_resource.c
}

resource "null_resource" "c" {
}

In the above situation, if the prior state has objects bound to both null_resource.a and null_resource.b then per the chaining rule it will be the object that was bound to b that will get moved to c and preserved; the object bound to a will be proposed for deletion, because there is no existing vacant address for it to be rebound to.

This mechanism is primarily intended to deal with situations where a module has been refactored multiple times and so existing users of the module could potentially be upgrading over multiple refactors at once and therefore need Terraform to follow the chain to reach the module author's intended end state. Things get a little more tricky when refactoring within a module interacts with refactoring between modules, as is true in your case, but the same rule applies: the object that's nearest to the end of the chain will be preserved, and any others earlier in the chain will be proposed for destroy.

In practice multiple objects in the chain is rare when using Terraform in a conventional way, but the moved mechanism is designed to still produce an unambiguous result even when the input is "strange", which can be caused by less common operations such as terraform state mv that were performed earlier.

KenSpur commented 1 year ago

@apparentlymart

Thank you for the detailed explanation. Based on your guidance, I've updated my code using the null_resource to create a working example. I appreciate your assistance in clarifying this for me!