Commit Graph

16 Commits

Author SHA1 Message Date
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
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
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
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
Alisdair McDiarmid
f0cf4235f9 cli: Refactor resource drift rendering 2021-09-16 15:22:37 -04:00
Alisdair McDiarmid
cd29c3e5fd
Revert "json-output: Release format version 1.0" 2021-09-09 11:25:35 -04:00
Alisdair McDiarmid
b60f201eed json-output: Release format version 1.0 2021-09-01 15:15:18 -04:00
Martin Atkins
383bbdeebc Upgrade to Go 1.17
This includes the addition of the new "//go:build" comment form in addition
to the legacy "// +build" notation, as produced by gofmt to ensure
consistent behavior between Go versions. The new directives are all
equivalent to what was present before, so there's no change in behavior.

Go 1.17 continues to use the Unicode 13 tables as in Go 1.16, so this
upgrade does not require also upgrading our Unicode-related dependencies.

This upgrade includes the following breaking changes which will also
appear as breaking changes for Terraform users, but that are consistent
with the Terraform v1.0 compatibility promises.

- On MacOS, Terraform now requires macOS 10.13 High Sierra or later.

This upgrade also includes the following breaking changes which will
appear as breaking changes for Terraform users that are inconsistent with
our compatibility promises, but have justified exceptions as follows:

- cidrsubnet, cidrhost, and cidrnetmask will now reject IPv4 CIDR
  addresses whose decimal components have leading zeros, where previously
  they would just silently ignore those leading zeros.

  This is a security-motivated exception to our compatibility promises,
  because some external systems interpret zero-prefixed octets as octal
  numbers rather than decimal, and thus the previous lenient parsing could
  lead to a different interpretation of the address between systems, and
  thus potentially allow bypassing policy when configuring firewall rules
  etc.

This upgrade also includes the following breaking changes which could
_potentially_ appear as breaking changes for Terraform users, but that do
not in practice for the reasons given:

- The Go net/url package no longer allows query strings with pairs
  separated by semicolons instead of ampersands. This primarily affects
  HTTP servers written in Go, and Terraform includes a special temporary
  HTTP server as part of its implementation of OAuth for "terraform login",
  but that server only needs to accept URLs created by Terraform itself
  and Terraform does not generate any URLs that would be rejected.
2021-08-17 15:20:05 -07:00
Kristin Laemmert
096010600d
terraform: use hcl.MergeBodies instead of configs.MergeBodies for pro… (#29000)
* terraform: use hcl.MergeBodies instead of configs.MergeBodies for provider configuration

Previously, Terraform would return an error if the user supplied provider configuration via interactive input iff the configuration provided on the command line was missing any required attributes - even if those attributes were already set in config.

That error came from configs.MergeBody, which was designed for overriding valid configuration. It expects that the first ("base") body has all required attributes. However in the case of interactive input for provider configuration, it is perfectly valid if either or both bodies are missing required attributes, as long as the final body has all required attributes. hcl.MergeBodies works very similarly to configs.MergeBodies, with a key difference being that it only checks that all required attributes are present after the two bodies are merged.

I've updated the existing test to use interactive input vars and a schema with all required attributes. This test failed before switching from configs.MergeBodies to hcl.MergeBodies.

* add a command package test that shows that we can still have providers with dynamic configuration + required + interactive input merging

This test failed when buildProviderConfig still used configs.MergeBodies instead of hcl.MergeBodies
2021-06-25 08:48:47 -04:00
Alisdair McDiarmid
3326ab7dae json-output: Omit unchanged resource_drift entries
Previously, if any resources were found to have drifted, the JSON plan
output would include a drift entry for every resource in state. This
commit aligns the JSON plan output with the CLI UI, and only includes
those resources where the old value does not equal the new value---i.e.
drift has been detected.

Also fixes a bug where the "address" field was missing from the drift
output, and adds some test coverage.
2021-06-17 15:09:16 -04:00
Kristin Laemmert
583859e510
commands: terraform add (#28874)
* command: new command, terraform add, generates resource templates

terraform add ADDRESS generates a resource configuration template with all required (and optionally optional) attributes set to null. This can optionally also pre-populate nonsesitive attributes with values from an existing resource of the same type in state (sensitive vals will be populated with null and a comment indicating sensitivity)

* website: terraform add documentation
2021-06-17 12:08:37 -04:00
Kristin Laemmert
329585d07d
jsonconfig: properly unwind and enumerate references (#28884)
The "references" included in the expression representation now properly unwrap for each traversal step, to match what was documented.
2021-06-14 09:22:22 -04:00
Kristin Laemmert
ac03d35997
jsonplan and jsonstate: include sensitive_values in state representations (#28889)
* jsonplan and jsonstate: include sensitive_values in state representations

A sensitive_values field has been added to the resource in state and planned values which is a map of all sensitive attributes with the values set to true.

It wasn't entirely clear to me if the values in state would suffice, or if we also need to consult the schema - I believe that this is sufficient for state files written since v0.15, and if that's incorrect or insufficient, I'll add in the provider schema check as well.

I also updated the documentation, and, since we've considered this before, bumped the FormatVersions for both jsonstate and jsonplan.
2021-06-14 09:19:13 -04:00
Kristin Laemmert
649095c602
providers subcommand tests (#28744)
* getproviders ParsePlatform: add check for invalid platform strings with too many parts

The existing logic would not catch things like a platform string containing multiple underscores. I've added an explicit check for exactly 2 parts and some basic tests to prove it.

* command/providers-lock: add tests

This commit adds some simple tests for the providers lock command. While adding this test I noticed that there was a mis-copied error message, so I replaced that with a more specific message. I also added .terraform.lock.hcl to our gitignore for hopefully obvious reasons.

getproviders.ParsePlatform: use parts in place of slice range, since it's available

* command: Providers mirror tests

The providers mirror command is already well tested in e2e tests, so this includes only the most absolutely basic test case.
2021-05-19 12:56:16 -04:00
Martin Atkins
f40800b3a4 Move states/ to internal/states/
This is part of a general effort to move all of Terraform's non-library
package surface under internal in order to reinforce that these are for
internal use within Terraform only.

If you were previously importing packages under this prefix into an
external codebase, you could pin to an earlier release tag as an interim
solution until you've make a plan to achieve the same functionality some
other way.
2021-05-17 14:09:07 -07:00
Martin Atkins
ffe056bacb Move command/ to internal/command/
This is part of a general effort to move all of Terraform's non-library
package surface under internal in order to reinforce that these are for
internal use within Terraform only.

If you were previously importing packages under this prefix into an
external codebase, you could pin to an earlier release tag as an interim
solution until you've make a plan to achieve the same functionality some
other way.
2021-05-17 14:09:07 -07:00