Update `moved` semantics to allow moving a resource managed by an attribute or inline block to a separate resource

lorengordon commented 2 months ago

Terraform Version

1.9.5

Use Cases

The AWS provider recently approved a proposal for a pattern that will be used going forward to manage "exclusive relationships" between resources. The first implementation of this pattern was for aws_iam_role and their aws_iam_role_policy attachments.

I'm really excited about this new pattern, and would love to adopt it quickly. However, it introduces a dilemma for module authors. How do we refactor the module and minimize the migration pain for users? With the current features available in Terraform, module users would have to write import blocks for every attachment resource, which makes the refactoring backwards-incompatible and a major version bump.

Attempted Solutions

To demonstrate the problem, I attempted to migrate a module we author to use the new exclusive pattern with the aws_iam_role and aws_iam_role_policy resource. You can see that here, https://github.com/plus3it/terraform-aws-tardigrade-iam-principals/pull/205. There's also a discussion of the migration options on the aws provider repo, https://github.com/hashicorp/terraform-provider-aws/issues/39376#issuecomment-2363922272.

Proposal

I would like to propose an update for moved block semantics, allowing users to specify a from expression that uniquely identifies a resource managed by an inline block. Using aws_iam_role as an example, it might look like:

moved {
  from = aws_iam_role.example,inline_policy,<inline_policy_name>
  to   = aws_iam_role_policy.example_policy_name
}

Those semantics largely follow the idea of import requirements for the target resource type. For an inline_policy block on an aws_iam_role resource, we only need to know the inline policy name to map the state to the new resource.

It occurs to me that moved does not currently support interpolation, or expressions, or for_each, the way import does. Since these inline blocks do not have state addresses, I think this proposal would also require updates to extend that support to moved blocks. A complete example, to make my pr testing the migration to the exclusive resource simply a feature release, might look like this:

moved {
  for_each = toset(var.inline_policies[*].name)

  from = aws_iam_role.this,inline_policy,${each.value}
  to   = aws_iam_role_policy.this[each.value]
}

And then similarly for attachments of managed policies (once that "exclusive" resource is available), I'd write something like

moved {
  for_each = toset(var.managed_policy_arns)

  from = aws_iam_role.this,managed_policy_arns,${each.value}
  to   = aws_iam_role_policy_attachment.this[each.value]
}

References

crw commented 2 months ago

Thanks for this feature request! If you are viewing this issue and would like to indicate your interest, please use the 👍 reaction on the issue description to upvote this issue. We also welcome additional use case descriptions. Thanks again!

crw commented 2 months ago

Hi @lorengordon, just wanted to report back that we discussed this in triage today. This is something we have considered, but would require more research as to how to implement as the eventual solution will somewhat complicated. Just as an FYI.

lorengordon commented 2 months ago

Thanks for the update @crw. It occurred to me that I could reframe this as an import problem, also. We would need two things from an import block that it cannot do today:

Allow import in modules, instead of only from the root config.
Ability to indicate that import should not fail if the resource does not exist. Import the resource if it does exist, but if it does not then plan to create it as normal.

wmhartl commented 4 weeks ago

Thanks for the update @crw. It occurred to me that I could reframe this as an import problem, also. We would need two things from an import block that it cannot do today:
1. Allow `import` in modules, instead of only from the root config.

2. Ability to indicate that `import` should not fail if the resource does not exist. Import the resource if it does exist, but if it does not then plan to create it as normal.

This is far simpler and more valuable in my mind and would save hours of work for changes like the inline_policy deprecation. Thanks for posting this @lorengordon.

hashicorp / terraform