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

command: Fix various issues in the "terraform state ..." subcommands #20719

Merged
merged 1 commit into from
Mar 18, 2019

Conversation

apparentlymart
Copy link
Contributor

In earlier refactoring we updated these commands to support the new address and state types, but attempted to partially retain the old-style StateFilter abstraction that originally lived in the Terraform package, even though that was no longer being used for any other functionality.

Unfortunately the adaptation of the existing filtering to the new types wasn't exact and so these commands ended up having a few bugs that were not covered by the existing tests.

Since the old StateFilter behavior was the source of various misbehavior anyway, here it's removed altogether and replaced with some simpler functions in the state_meta.go file that are tailored to the use-cases of these sub-commands.

As well as just generally behaving more consistently with the other parts of Terraform that use the new resource address types, this commit fixes the following bugs:

  • A resource address of aws_instance.foo would previously match an resource of that type and name in any module, which disagreed with the expected interpretation elsewhere of meaning a single resource in the root module. (This was actually a pre-existing bug in 0.11 too; fixes terraform state rm foo.bar also removes module.baz.foo.bar #19692)

  • The terraform state mv command was not supporting moves from a single resource address to an indexed address and vice-versa, because the old logic didn't need to make that distinction while they are two separate address types in the new logic. Now we allow resources that do not have count/for_each to be treated as if they are instances for the purposes of this command, which is a better match for likely user intent and for the old behavior.

  • terraform state mv was not fully protecting against moving between resource types or resource modes. (Fixes Moving a resource drops the "data." prefix from data source #18978, and probably several others.)

Finally, we also clean up a little some of the usage output from these commands, which hasn't been updated for some time and so had both some stale information and some inaccurate terminology.

In earlier refactoring we updated these commands to support the new
address and state types, but attempted to partially retain the old-style
"StateFilter" abstraction that originally lived in the Terraform package,
even though that was no longer being used for any other functionality.

Unfortunately the adaptation of the existing filtering to the new types
wasn't exact and so these commands ended up having a few bugs that were
not covered by the existing tests.

Since the old StateFilter behavior was the source of various misbehavior
anyway, here it's removed altogether and replaced with some simpler
functions in the state_meta.go file that are tailored to the use-cases of
these sub-commands.

As well as just generally behaving more consistently with the other
parts of Terraform that use the new resource address types, this commit
fixes the following bugs:

- A resource address of aws_instance.foo would previously match an
  resource of that type and name in any module, which disagreed with the
  expected interpretation elsewhere of meaning a single resource in the
  root module.

- The "terraform state mv" command was not supporting moves from a single
  resource address to an indexed address and vice-versa, because the old
  logic didn't need to make that distinction while they are two separate
  address types in the new logic. Now we allow resources that do not have
  count/for_each to be treated as if they are instances for the purposes
  of this command, which is a better match for likely user intent and for
  the old behavior.

Finally, we also clean up a little some of the usage output from these
commands, which hasn't been updated for some time and so had both some
stale information and some inaccurate terminology.
@apparentlymart apparentlymart added this to the v0.12.0 milestone Mar 16, 2019
@apparentlymart apparentlymart self-assigned this Mar 16, 2019
@apparentlymart apparentlymart requested a review from a team March 16, 2019 02:59
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The ability to rm an entire module is wicked cool.

@apparentlymart apparentlymart merged commit c39905e into master Mar 18, 2019
@pselle pselle deleted the b-cmd-state-filtering branch June 25, 2019 16:34
@ghost
Copy link

ghost commented Jul 24, 2019

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 ghost locked and limited conversation to collaborators Jul 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants