Commit Graph

28912 Commits

Author SHA1 Message Date
James Bardin 625e768678 make sure required_version is checked before diags
We must ensure that the terraform required_version is checked as early
as possible, so that new configuration constructs don't cause init to
fail without indicating the version is incompatible.

The loadConfig call before the earlyconfig parsing seems to be unneeded,
and we can delay that to de-tangle it from installing the modules which
may have their own constraints.

TODO: it seems that loadConfig should be able to handle returning the
version constraints in the same manner as loadSingleModule.
2021-09-28 13:30:03 -04:00
James Bardin 372814e49a
Merge pull request #29659 from hashicorp/jbardin/really-nested-within
refactoring: exhaustive NestedWithin checks
2021-09-27 12:55:04 -04:00
James Bardin cac1f5c264 refactoring: exhaustive NestedWithin checks
When checking dependencies between statements, we need to check all
combinations of To and From addresses.
2021-09-27 12:48:17 -04:00
Alisdair McDiarmid a742d7ee88
Merge pull request #29649 from hashicorp/alisdair/json-ui-exhaustiveness
json-output: Add change reasons to explain deletes
2021-09-24 14:02:19 -04:00
Alisdair McDiarmid b699391d04 json-output: Add change reasons to explain deletes
The extra feedback information for why resource instance deletion is
planned is now included in the streaming JSON UI output.

We also add an explicit case for no-op actions to switch statements in
this package to ensure exhaustiveness, for future linting.
2021-09-24 13:17:49 -04:00
James Bardin 8ce2254ad6
Merge pull request #29647 from hashicorp/jbardin/test-temp-cleanup
temp path clean for some backend tests
2021-09-24 12:27:53 -04:00
James Bardin c8cd0b1e74
Merge pull request #29624 from hashicorp/jbardin/no-block-to-attr
skip FixUpBlockAttrs when we can detect types from a new SDK
2021-09-24 11:01:53 -04:00
Alisdair McDiarmid 50fac6afd8
Merge pull request #29645 from hashicorp/alisdair/verify-remote-terraform-version-update
backend/remote: Support interop from 0.14 to 1.1
2021-09-24 10:07:09 -04:00
Alisdair McDiarmid 57318ef561 backend/remote: Support interop from 0.14 to 1.1
The previous conservative guarantee that we would not make backwards
incompatible changes to the state file format until at least Terraform
1.1 can now be extended. Terraform 0.14 through 1.1 will be able to
interoperably use state files, so we can update the remote backend
version compatibility check accordingly.
2021-09-24 09:26:09 -04:00
Alisdair McDiarmid f1e9d88ddc
Merge pull request #29640 from hashicorp/alisdair/fix-refresh-only-with-orphans
core: Fix refresh-only interaction with orphans
2021-09-24 09:25:46 -04:00
Martin Atkins 5d620303eb
Update CHANGELOG.md 2021-09-23 14:47:40 -07:00
Martin Atkins 0f76e3a4e1
Update CHANGELOG.md 2021-09-23 14:47:01 -07:00
Martin Atkins d97ef10bb8 core: Don't return other errors if move statements are invalid
Because our validation rules depend on some dynamic results produced by
actually running the plan, we deal with moves in a "backwards" order where
we try to apply them first -- ignoring anything strange we might find --
and then validate the original statements only after planning.

An unfortunate consequence of that approach is that when the move
statements are invalid it's likely that move execution will not fully
complete, and so the generated plan is likely to be incorrect and might
well include errors resulting from the unresolved moves.

To mitigate that, here we let any move validation errors supersede all
other diagnostics that the plan phase might've generated, in the hope that
it'll help the user focus on fixing the incorrect move statements without
creating confusing by reporting errors that only appeared as a quick of
how Terraform worked around the invalid move statements earlier.
2021-09-23 14:37:08 -07:00
Martin Atkins 1bff623fd9 core: Report a warning if any moves get blocked
In most cases Terraform will be able to automatically fully resolve all
of the pending move statements before creating a plan, but there are some
edge cases where we can end up wanting to move one object to a location
where another object is already declared.

One relatively-obvious example is if someone uses "terraform state mv" in
order to create a set of resource instance bindings that could never have
arising in normal Terraform use.

A less obvious example arises from the interactions between moves at
different levels of granularity. If we are both moving a module to a new
address and moving a resource into an instance of the new module at the
same time, the old module might well have already had a resource of the
same name and so the resource move will be unresolvable.

In these situations Terraform will move the objects as far as possible,
but because it's never valid for a move "from" address to still be
declared in the configuration Terraform will inevitably always plan to
destroy the objects that didn't find a final home. To give some additional
explanation for that result, here we'll add a warning which describes
what happened.

This is not a particularly actionable warning because we don't really
have enough information to guess what the user intended, but we do at
least prompt that they might be able to use the "terraform state" family
of subcommands to repair the ambiguous situation before planning, if they
want a different result than what Terraform proposed.
2021-09-23 14:37:08 -07:00
Martin Atkins 04f9e7148c command/format: Include deletion reasons in plan report
The core runtime is now able to specify a reason for some situations when
Terraform plans to delete a resource instance.

This commit makes that information visible in the human-oriented UI. A
previous commit already made the underlying data informing these new hints
visible as part of the machine-oriented (JSON) plan output.

This also removes the bold formatting from the existing "has moved to"
hints, because subjectively it seemed like the result was emphasizing too
many parts of the output and thus somewhat defeating the benefit of the
emphasis in trying to create additional visual hierarchy for sighted users
running Terraform in a terminal. Now only the first line containing the
main action statement will be in bold, and all of the parenthesized
follow-up notes will be unformatted.
2021-09-23 14:37:08 -07:00
Martin Atkins a1a713cf28 core: Report ActionReasons when we plan to delete "orphans"
There are a few different reasons why a resource instance tracked in the
prior state might be considered an "orphan", but previously we reported
them all identically in the planned changes.

In order to help users understand the reason for a surprising planned
delete, we'll now try to specify an additional reason for the planned
deletion, covering all of the main reasons why that could happen.

This commit only introduces the new detail to the plans.Changes result,
though it also incidentally exposes it as part of the JSON plan result
in order to keep that working without returning errors in these new
cases. We'll expose this information in the human-oriented UI output in
a subsequent commit.
2021-09-23 14:37:08 -07:00
Martin Atkins 7b99861b1c refactoring: Don't implicitly move for resources with for_each
Our previous rule for implicitly moving from IntKey(0) to NoKey would
apply that move even when the current resource configuration uses
for_each, because we were only considering whether "count" were set.

Previously this was relatively harmless because the resource instance in
question would end up planned for deletion anyway: neither an IntKey nor
a NoKey are valid keys for for_each.

Now that we're going to be announcing these moves explicitly in the UI,
it would be confusing to see Terraform report that IntKey moved to NoKey
in a situation where the config changed from count to for_each, so to
address that we'll only generate the implied statement if neither
repetition argument is set.
2021-09-23 14:37:08 -07:00
James Bardin 9c078c27cf temp path clean for some backend tests 2021-09-23 17:16:33 -04:00
Alisdair McDiarmid ceb580ec40 core: Fix refresh-only interaction with orphans
When planning in refresh-only mode, we must not remove orphaned
resources due to changed count or for_each values from the planned
state. This was previously happening because we failed to pass through
the plan's skip-plan-changes flag to the instance orphan node.
2021-09-23 16:38:08 -04:00
Chris Arcand db6ca866fc
Merge pull request #29627 from hashicorp/auto-select-single-workspace
Auto select single workspace when necessary
2021-09-23 11:09:09 -05:00
Chris Arcand 171cdbbf93 command: Clean up testInputResponseMap before failing on unused answers
If you don't, the unused answers will persist in the package-level var
and bleed in to other tests.
2021-09-22 16:03:11 -05:00
Chris Arcand 60bc7aa05d command: Auto-select single workspace if necessary
When initializing a backend, if the currently selected workspace does
not exist, the user is prompted to select from the list of workspaces
the backend provides.

Instead, we should automatically select the only workspace available
_if_ that's all that's there.

Although with being a nice bit of polish, this enables future
improvments with Terraform Cloud in potentially removing the implicit
depenency on always using the 'default' workspace when the current
configuration is mapped to a single TFC workspace.
2021-09-22 16:03:11 -05:00
James Bardin 8706a18c4b refine the skipFixup heuristic
We can also rule out some attribute types as indicating something other
than the legacy SDK.

- Tuple types were not generated at all.
- There were no single objects types, the convention was to use a block
  list or set of length 1.
- Maps of objects were not possible to generate, since named blocks were
  not implemented.
- Nested collections were not supported, but when they were generated they
  would have primitive types.
2021-09-22 16:29:50 -04:00
hc-github-team-tf-core cd1d30ea95 Cleanup after v1.1.0-alpha20210922 release 2021-09-22 18:12:02 +00:00
hc-github-team-tf-core d14bbbb6f2
Release v1.1.0-alpha20210922 2021-09-22 17:49:31 +00:00
Martin Atkins 3f1c15c792
Upgrade to Go 1.17.1 2021-09-22 10:31:31 -07:00
James Bardin 6b4e73af48 skip the blocktoattr fixup with nested types
If structural types are being used, we can be assured that the legacy
SDK SchemaConfigModeAttr is not being used, and the fixup is not needed.

This prevents inadvertent mapping of blocks to structural attributes,
and allows us to skip the fixup overhead when possible.
2021-09-22 12:17:20 -04:00
Martin Atkins 83f0376673 refactoring: ApplyMoves new return type
When we originally stubbed ApplyMoves we didn't know yet how exactly we'd
be using the result, so we made it a double-indexed map allowing looking
up moves in both directions.

However, in practice we only actually need to look up old addresses by new
addresses, and so this commit first removes the double indexing so that
each move is only represented by one element in the map.

We also need to describe situations where a move was blocked, because in
a future commit we'll generate some warnings in those cases. Therefore
ApplyMoves now returns a MoveResults object which contains both a map of
changes and a map of blocks. The map of blocks isn't used yet as of this
commit, but we'll use it in a later commit to produce warnings within
the "terraform" package.
2021-09-22 09:01:10 -07:00
Martin Atkins d054102d38 addrs: AbsResource.UniqueKey distinct from AbsResourceInstance.UniqueKey
The whole point of UniqueKey is to deal with the fact that we have some
distinct address types which have an identical string representation, but
unfortunately that fact caused us to not notice that we'd incorrectly
made AbsResource.UniqueKey return a no-key instance UniqueKey instead of
its own distinct unique key type.
2021-09-22 09:01:10 -07:00
Chris Arcand 08a86b6c13
Merge pull request #29621 from hashicorp/error-on-unused-test-answers
command: Ensure all answers were used in command.testInputResponseMap
2021-09-22 09:42:21 -05:00
Chris Arcand 8684a85e26 command: Ensure all answers were used in command.testInputResponseMap
Remove answers from testInputResponse as they are given, and raise an
error during cleanup if any answers remain unused.

This enables tests to ensure that the expected mock answers are actually
used in a test; previously, an entire branch of code including an input
sequence could be omitted and the test(s) would not fail.

The only test that had unused answers in this map is one leftover from
legacy state migrations, a prompt that was removed in
7c93b2e5e6
2021-09-21 22:26:16 -05:00
Paddy 4fe75bead3
Remove panic debugging information (#29512)
It was out of date.

Fixes #1113.
2021-09-21 05:45:04 -07:00
Laura Pacilio 8f09e27597
Merge pull request #29567 from drasko95/patch-1
Fix a documentation typo
2021-09-20 17:24:31 -04:00
Alisdair McDiarmid 5386d6c5b8
Merge pull request #29603 from hashicorp/alisdair/moved-json-output
json-output: Add support for config-driven move to plan JSON and streaming JSON UI
2021-09-20 16:49:11 -04:00
Alisdair McDiarmid b59b057591 json-output: Config-driven move support in JSON UI
Add previous address information to the `planned_change` and
`resource_drift` messages for the streaming JSON UI output of plan and
apply operations.

Here we also add a "move" action value to the `change` object of these
messages, to represent a move-only operation.

As part of this work we also simplify this code to use the plan's
DriftedResources values instead of recomputing the drift from state.
2021-09-20 15:25:23 -04:00
Alisdair McDiarmid 78c4a8c461 json-output: Previous address for resource changes
Configuration-driven moves are represented in the plan file by setting
the resource's `PrevRunAddr` to a different value than its `Addr`. For
JSON plan output, we here add a new field to resource changes,
`previous_address`, which is present and non-empty only if the resource
is planned to be moved.

Like the CLI UI, refresh-only plans will include move-only changes in
the resource drift JSON output. In normal plan mode, these are elided to
avoid redundancy with planned changes.
2021-09-20 15:25:23 -04:00
Martin Atkins 78705f4f10
Update CHANGELOG.md 2021-09-20 11:29:24 -07:00
Martin Atkins f0034beb33 core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary
Going back a long time we've had a special magic behavior which tries to
recognize a situation where a module author either added or removed the
"count" argument from a resource that already has instances, and to
silently rename the zeroth or no-key instance so that we don't plan to
destroy and recreate the associated object.

Now we have a more general idea of "move statements", and specifically
the idea of "implied" move statements which replicates the same heuristic
we used to use for this behavior, we can treat this magic renaming rule as
just another "move statement", special only in that Terraform generates it
automatically rather than it being written out explicitly in the
configuration.

In return for wiring that in, we can now remove altogether the
NodeCountBoundary graph node type and its associated graph transformer,
CountBoundaryTransformer. We handle moves as a preprocessing step before
building the plan graph, so we no longer need to include any special nodes
in the graph to deal with that situation.

The test updates here are mainly for the graph builders themselves, to
acknowledge that indeed we're no longer inserting the NodeCountBoundary
vertices. The vertices that NodeCountBoundary previously depended on now
become dependencies of the special "root" vertex, although in many cases
here we don't see that explicitly because of the transitive reduction
algorithm, which notices when there's already an equivalent indirect
dependency chain and removes the redundant edge.

We already have plenty of test coverage for these "count boundary" cases
in the context tests whose names start with TestContext2Plan_count and
TestContext2Apply_resourceCount, all of which continued to pass here
without any modification and so are not visible in the diff. The test
functions particularly relevant to this situation are:
 - TestContext2Plan_countIncreaseFromNotSet
 - TestContext2Plan_countDecreaseToOne
 - TestContext2Plan_countOneIndex
 - TestContext2Apply_countDecreaseToOneCorrupted

The last of those in particular deals with the situation where we have
both a no-key instance _and_ a zero-key instance in the prior state, which
is interesting here because to exercises an intentional interaction
between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves,
where we intentionally generate an implied move statement that produces
a collision and then expect ApplyMoves to deal with it in the same way as
it would deal with all other collisions, and thus ensure we handle both
the explicit and implied collisions in the same way.

This does affect some UI-level tests, because a nice side-effect of this
new treatment of this old feature is that we can now report explicitly
in the UI that we're assigning new addresses to these objects, whereas
before we just said nothing and hoped the user would just guess what had
happened and why they therefore weren't seeing a diff.

The backend/local plan tests actually had a pre-existing bug where they
were using a state with a different instance key than the config called
for but getting away with it because we'd previously silently fix it up.
That's still fixed up, but now done with an explicit mention in the UI
and so I made the state consistent with the configuration here so that the
tests would be able to recognize _real_ differences where present, as
opposed to the errant difference caused by that inconsistency.
2021-09-20 09:06:22 -07:00
Martin Atkins ee9e346039 refactoring: ApplyMoves skips moving when destination address occupied
Per our rule that the content of the state can never make a move statement
invalid, our behavior for two objects trying to occupy the same address
will be to just ignore that and let the object already at the address
take priority.

For the moment this is silent from an end-user perspective and appears
only in our internal logs. However, I'm hoping that our future planned
adjustment to the interface of this function will include some way to
allow reporting these collisions in some end-user-visible way, either as
a separate warning per collision or as a single warning that collects
together all of the collisions into a single message somehow.

This situation can arise both because the previous run state already
contained an object at the target address of a move and because more than
one move ends up trying to target the same location. In the latter case,
which one "wins" is decided by our depth-first traversal order, which is
in turn derived from our chaining and nesting rules and is therefore
arbitrary but deterministic.
2021-09-20 09:06:22 -07:00
Martin Atkins ef5a1c9cfe refactoring: ImpliedMoveStatements function
This new function complements the existing function FindMoveStatements
by potentially generating additional "implied" move statements that aren't
written explicit in the configuration but that we'll infer by comparing
the configuration and te previous run state.

The goal here is to infer only enough to replicate the effect of the
"count boundary fixup" graph node (terraform.NodeCountBoundary) that we
currently use to deal with this concern of preserving the zero-instance
when switching between "count" and not "count".

This is just dead code for now. A subsequent commit will introduce this
into the "terraform" package while also removing
terraform.NodeCountBoundary, thus achieving the same effect as before but
in a way that'll get reported in the UI as a move, using the same language
that we'd use for an explicit move statement.
2021-09-20 09:06:22 -07:00
Martin Atkins 7f99a8802e addrs: MoveEndpointInModule.SelectsResource
This is similar to the existing SelectsModule method, returning true if
the reciever selects either a particular resource as a whole or any of the
instances of that resource.

We don't need this test in the normal case, but we will need it in a
subsequent commit when we'll be possibly generating _implied_ move
statements between instances of resources, but only if there aren't
explicit move statements mentioning those resources already.
2021-09-20 09:06:22 -07:00
Alisdair McDiarmid 2b6a1be18f
Merge pull request #29597 from hashicorp/alisdair/move-refresh-ui
cli: Improved plan UI for move-only changes
2021-09-17 14:54:39 -04:00
Alisdair McDiarmid 638784b195 cli: Omit move-only drift, except for refresh-only
The set of drifted resources now includes move-only changes, where the
object value is identical but a move has been executed. In normal
operation, we previousl displayed these moves twice: once as part of
drift output, and once as part of planned changes.

As of this commit we omit move-only changes from drift display, except
for refresh-only plans. This fixes the redundant output.
2021-09-17 14:47:00 -04:00
Alisdair McDiarmid 61b2d8e3fe core: Plan drift includes move-only changes
Previously, drifted resources included only updates and deletes. To
correctly display the full changes which would result as part of a
refresh-only apply, the drifted resources must also include move-only
changes.
2021-09-17 14:47:00 -04:00
Alisdair McDiarmid 9a7bbdab6f Fix terraform add test failure due to bad merge 2021-09-17 14:46:44 -04:00
Alisdair McDiarmid d425c26d77
Merge pull request #29589 from hashicorp/alisdair/planfile-drifted-resources
core: Compute resource drift during plan phase, store in plan file
2021-09-17 14:23:04 -04:00
Alisdair McDiarmid 1f57b7a8bd
Merge pull request #29235 from magodo/terraform_add_output_append
`terraform add`: `-out` option append to existing config & optionally check resource existance
2021-09-17 11:19:55 -04:00
Laura Pacilio dfbef12a6c
Merge pull request #29598 from hashicorp/laura-add-mrui-to-sidebar
Add Machine-Readable UI to sidebar
2021-09-16 17:30:37 -04:00
Laura Pacilio a8e5b6a4ad Fix alphabetical order of sidebar 2021-09-16 17:11:31 -04:00
Laura Pacilio 4d1baaceab Add Machine-Readable UI to sidebar and add hyphen :-) 2021-09-16 17:06:52 -04:00