Commit Graph

195 Commits

Author SHA1 Message Date
Łukasz Sierant 19cce931a8 Test case for changing backend hash during aborted state migration
Pulled and updated from
https://github.com/hashicorp/terraform/pull/26260
2021-11-02 10:20:37 -05:00
Billy Keyes 52ca30273f
command/format: improve list nested attribute rendering (#29827)
* command/format: fix list nested attr diff rendering

Previously, diffs only rendered correctly if all changed elements
appeared before all unchanged elements. Once an unchanged element was
found, all remaining elements were skipped. This usually led to the
output being an empty list with a weird amount of space between the
brackets.

* command/format: improve list nested attr rendering

This makes several changes that make diffs for lists clearer and more
consistent:

  * Separate items with brackets instead of only new lines. This better
    matches the input syntax and avoids confusion from the first and
    last brackets implying there is a single item.
  * Render an action symbol for each element of the list
  * Use the correct action symbol for new and deleted items
  * Fix the alignment of opening and closing brackets

I also refactored the structure so it is similar to the set and map
cases to minimize duplication of the new prints.

* Fix re-use of blockBodyDiffResult struct
2021-11-02 11:13:56 -04:00
Nick Fagerlund 02e62c9851
Cloud: Init without erroring when no workspaces match the `tags` (#29835)
Previously, `terraform init` was throwing an error if you configured the cloud
block with `tags` and there weren't any tagged workspaces yet. Confusing and
alienating, since that that's a fairly normal situation! Basically TFC was
handling an empty list of workspaces worse than other backends, because it
doesn't support an unnamed default workspace.

This commit catches that condition during `Meta.selectBackend()` and asks the
user to pick a name for their first tagged workspace. If they cancel out, we
still error, but if we know what name they want, we can handle it the same way
as a nonexistent workspace specified in `name` -- just pass it to
`Meta.SetWorkspace()`, and let the workspace get implicitly created when
`InitCommand.Run()` eventually calls `StateMgr()`.
2021-11-01 10:20:15 -07:00
Martin Atkins 94cbc8fb5d experiments: config_driven_move has concluded
Based on feedback during earlier alpha releases, we've decided to move
forward with the current design for the first phase of config-driven
refactoring.

Therefore here we've marked the experiment as concluded with no changes
to the most recent incarnation of the functionality. The other changes
here are all just updating test fixtures to no longer declare that they
are using experimental features.
2021-11-01 08:46:15 -07:00
Chris Arcand 511afcd43a command: Adjust skipping of resource counts for any remote implementation
When using the Terraform Cloud integration - like the 'remote'
backend - resource count output should be suppressed if those counts are
being rendered remotely. This generalizes this to the shared
BackendWithRemoteTerraformVersion interface.
2021-10-29 21:23:28 -05:00
James Bardin 81e709d185
Merge pull request #29825 from hashicorp/jbardin/no-panicwrap
Remove the use of panicwrap in the Terraform CLI
2021-10-29 14:41:05 -04:00
Martin Atkins f266d1ee82
"Add cloud integration option"
A more native integration for Terraform Cloud and its CLI-driven run workflow.

Instead of a backend, users declare a special block in the top-level terraform settings
block to configure Terraform Cloud, then run terraform init.

Full documentation will follow in later commits.
2021-10-28 18:30:01 -07:00
Chris Arcand 6dc1fed6d5 Clarify legacy Ui comments 2021-10-28 19:29:21 -05:00
Chris Arcand 14260a5b4c Adjust default workspace naming prompt
The previous version is a little overdramatic and somewhat accusatory.
Just lay it out how it is: TFC needs workspaces to be named.
2021-10-28 19:29:21 -05:00
Omar Ismail fc5863844c Cloud migration: ignore backend version check when empty worksapces 2021-10-28 19:29:19 -05:00
Barrett Clark f5366468b4 Cloud Backend reference migrating away from TFC 2021-10-28 19:29:19 -05:00
Chris Arcand 2c0294c7e3 Tweak configuration copy for TFC
This aligns more with the existing copy for backends
2021-10-28 19:29:19 -05:00
Omar Ismail 1ff7827416 Multi-state migration prompt fix 2021-10-28 19:29:17 -05:00
Barrett Clark cc6de251d8 Update init reconfigure error message
If you move from the remote backend to the cloud block you will see this
error message.
2021-10-28 19:29:17 -05:00
Barrett Clark e16c53b561 Simplify/Consolidate the cloud conditionals 2021-10-28 19:29:16 -05:00
Barrett Clark ab304d831f Found another path to backend init error
There are actually a few different ways to get to this message.

1. Blank state — no previous terraform applied. Start with a cloud block.
1. Implicit local — start with no backend specified. This actually goes
   through the same code execution path as the first scenario.
1. Explicit local — start with a backend local block that has been
   applied, then change from the local backend to a cloud block. This
   will recognize the state, and is a different path through the code in
   the meta backend.

This commit handles the last case. The messaging has also been tweaked.

End to end test included as well.
2021-10-28 19:29:16 -05:00
Barrett Clark d29532cfeb tfc apply before init
https://app.asana.com/0/1199201948575144/1201019450474843/f
2021-10-28 19:29:16 -05:00
Barrett Clark e49f271eb5 Update copy for terraform init with cloud block 2021-10-28 19:29:16 -05:00
Chris Arcand 09d5b70b7a command/meta_backend_migrate:Tweak multi-to-single To TFC copy 2021-10-28 19:29:15 -05:00
Omar Ismail dfb4609be2 Backend State Migration from cloud to cloud.
* Add test for tfc to tfc mgiration
* Fix old tests, and remove unused code.
2021-10-28 19:29:15 -05:00
Barrett Clark 93bfcff61a More Terraform Cloud UX changes
* Update e2e tests to specify Terraform Cloud
* Update more of the terraform command flow to specify Terraform Cloud
2021-10-28 19:29:14 -05:00
Barrett Clark 261a2b49d3 UX for terraform init that copies state to TFC 2021-10-28 19:29:14 -05:00
Barrett Clark 0cd9490838 PR Feedback: cleanup variable setter 2021-10-28 19:29:14 -05:00
Chris Arcand f8256f6634 Update ux for "terraform init [-reconfigure]" 2021-10-28 19:29:14 -05:00
Omar Ismail dc76bbee73 Backend State Migration: Add remote backend test 2021-10-28 19:29:14 -05:00
Omar Ismail 3fedd6898c Backend State Migration to `cloud`: Multiple Workspaces
* Handle when there are multiple workspaces migrating to cloud,
using both the cloud name strategy and cloud tags strategy.
* Add e2e tests
2021-10-28 19:29:13 -05:00
Barrett Clark edbc84420c Run apply -refresh-state instead of refresh
When a user runs `terraform refresh` we give them an error message that
tells them to run `terraform apply -refresh-state`. We could just run
that command for them, though. That is what this PR does.
2021-10-28 19:29:12 -05:00
Omar Ismail 55fc590904 Teraform Cloud Backend State Migration
* determining source or destination to cloud
* handling single to single state migrations to cloud,
using a name strategy or a tags strategy
* Add end-to-end tests for state migration.
2021-10-28 19:29:12 -05:00
Chris Arcand fccc873a3d Remove ability to declare a 'cloud' backend
With the alternative block introduced in 7bf9b2c7b, this removes the
ability to explicitly declare the 'cloud' backend. The literal backend
interface is an implementation detail and no longer a user-level
concept when using Terraform Cloud.
2021-10-28 19:29:09 -05:00
Chris Arcand a4c24e3147 Add cloud {} configuration block for Terraform Cloud
This is a replacement declaration for using Terraform Cloud as a remote
backend, leaving the literal backend as an implementation detail and not
a user-level concept.
2021-10-28 19:29:09 -05:00
James Bardin 9b62dd6901
Merge pull request #29648 from hashicorp/jbardin/tfproto6.1
tfproto version 6.1
2021-10-28 16:00:02 -04:00
James Bardin d03a037567 insert panic handlers 2021-10-28 11:51:39 -04:00
James Bardin 42742c173d remove wrapped streams and readline 2021-10-28 11:51:39 -04:00
James Bardin 622c4df14c remove the use of panicwrap
Stop using panicwrap, and execute terraform in the main process.
2021-10-28 11:51:39 -04:00
Alisdair McDiarmid ded4f1a0fd
Merge pull request #29805 from hashicorp/alisdair/fix-init-workspace-select-input-false
command/init: Fail if -input=false but required
2021-10-26 11:37:22 -04:00
Alisdair McDiarmid ecb98e1c43 command/init: Fail if -input=false but required
When running `terraform init` against a backend with multiple
workspaces, none of which are the currently indicated local workspace,
Terraform prompts the user to choose a workspace from the list. In
automation, using the `-input=false` argument should disable asking for
input, but previously would hang instead.
2021-10-25 15:08:10 -04:00
Alisdair McDiarmid 6d0521e6be
Merge pull request #29803 from hashicorp/alisdair/init-backend-provider-errors
cli: Fix backend init failure with deleted cache
2021-10-25 13:57:45 -04:00
Alisdair McDiarmid 22923f9873
Merge pull request #29793 from hashicorp/alisdair/fix-remote-backend-migrate-version-check
backend/remote: Fix version check when migrating
2021-10-25 12:50:27 -04:00
Alisdair McDiarmid d42d83572b cli: Fix backend init failure with deleted cache
When an explicit backend is configured with a configuration which has
not yet been initialized, running `terraform init` performs a state
migration to fetch the remotely stored state in order to operate on it.
Like the previous bug introduced by the recent provider diagnostics
change, this code path was not correctly configured to enable init mode
for the backend, which resulted in a fatal error during init when the
cache dir is deleted.

Setting the `Init` backend option allows this code path to continue
without error when first initializing the backend for state migration.
The new e2e test fails without this change.
2021-10-25 12:45:35 -04:00
Alisdair McDiarmid 1729431520 backend/remote: Fix version check when migrating
When migrating state to an existing Terraform Cloud workspace using the
remote backend, we check the remote version is compatible with the local
one by default.

This commit fixes two bugs in this code:

- If using the "name" strategy for the remote backend, the list of
  destination workspaces is empty. This resulted in no version checking
  of the remote workspace, and we fell back to the string equality
  check.
- The user-specified CLI flag `-ignore-remote-version` was not being
  applied for the state migration version checking.
2021-10-21 14:10:25 -04:00
Alisdair McDiarmid 39de3ebec7 cli: Fix init failure with deleted cache
The init command needs to initialize a backend, in order to access
state, in turn to derive provider requirements from state. The backend
initialization step requires building provider factories, which
previously would fail if a lockfile was present without a corresponding
local provider cache.

This commit ensures that in this situation only, errors with the
provider factories are temporarily ignored. This allows us to continue
to initialize the backend, fetch providers, and then report any errors
as necessary.
2021-10-21 08:44:26 -04:00
Alisdair McDiarmid 1190b95fe2 command/e2etest: Ensure init fixes missing cache
We test that a deleted provider cache results in an error when running
terraform plan, but previously did not test that running init (as
instructed) would resolve the issue. This (failing) e2e test adds that
step.
2021-10-21 08:41:01 -04:00
Martin Atkins 5b266dd5ca command: Remove the experimental "terraform add" command
We introduced this experiment to gather feedback, and the feedback we saw
led to us deciding to do another round of design work before we move
forward with something to meet this use-case.

In addition to being experimental, this has only been included in alpha
releases so far, and so on both counts it is not protected by the
Terraform v1.0 Compatibility Promises.
2021-10-20 06:42:47 -07:00
Alisdair McDiarmid cdd5ee6fb3
Merge pull request #29773 from hashicorp/alisdair/init-lock-flags
cli: Restore -lock and -lock-timeout init flags
2021-10-19 09:45:15 -04:00
Alisdair McDiarmid c587384dff cli: Restore -lock and -lock-timeout init flags
The -lock and -lock-timeout flags were removed prior to the release of
1.0 as they were thought to have no effect. This is not true in the case
of state migrations when changing backends. This commit restores these
flags, and adds test coverage for locking during backend state
migration.

Also update the help output describing other boolean flags, showing the
argument as the user would type it rather than the default behavior.
2021-10-19 09:32:30 -04:00
Alisdair McDiarmid fb58f9e6d2 cli: Fix flaky init cancel test
There is a race between the MockSource and ShutdownCh which sometimes
causes this test to fail. Add a HangingSource implementation of Source
which hangs until the context is cancelled, so that there is always time
for a user-initiated shutdown to trigger the cancellation code path
under test.
2021-10-19 09:10:49 -04:00
James Bardin ef3c98466d
Merge pull request #29755 from hashicorp/jbardin/first-plan-lineage
Check for stale plan with no state metadata
2021-10-14 13:31:30 -04:00
James Bardin 9c80574417 test planfile may need to have a specific lineage
In order to test applying a plan from an existing state, we need to be
able to inject the state meta into the planfile.
2021-10-13 17:28:14 -04:00
Martin Atkins bee7403f3e command/workspace_delete: Allow deleting a workspace with empty husks
Previously we would reject attempts to delete a workspace if its state
contained any resources at all, even if none of the resources had any
resource instance objects associated with it.

Nowadays there isn't any situation where the normal Terraform workflow
will leave behind resource husks, and so this isn't as problematic as it
might've been in the v0.12 era, but nonetheless what we actually care
about for this check is whether there might be any remote objects that
this state is tracking, and for that it's more precise to look for
non-nil resource instance objects, rather than whole resources.

This also includes some adjustments to our error messaging to give more
information about the problem and to use terminology more consistent with
how we currently talk about this situation in our documentation and
elsewhere in the UI.

We were also using the old State.HasResources method as part of some of
our tests. I considered preserving it to avoid changing the behavior of
those tests, but the new check seemed close enough to the intent of those
tests that it wasn't worth maintaining this method that wouldn't be used
in any main code anymore. I've therefore updated those tests to use
the new HasResourceInstanceObjects method instead.
2021-10-13 13:54:11 -07:00
Alisdair McDiarmid b9f3dab035 json-output: Release format version 1.0 2021-10-06 11:13:06 -04:00
Omar Ismail bea7e3ebce
Backend State Migration: change variable names from one/two to source/destination (#29699) 2021-10-05 16:36:50 -04:00
Martin Atkins 01b22f4b76 command/e2etest: TestProviderTampering
We have various mechanisms that aim to ensure that the installed provider
plugins are consistent with the lock file and that the lock file is
consistent with the provider requirements, and we do have existing unit
tests for them, but all of those cases mock our fake out at least part of
the process and in the past that's caused us to miss usability
regressions, where we still catch the error but do so at the wrong layer
and thus generate error message lacking useful additional context.

Here we'll add some new end-to-end tests to supplement the existing unit
tests, making sure things work as expected when we assemble the system
together as we would in a release. These tests cover a number of different
ways in which the plugin selections can grow inconsistent.

These new tests all run only when we're in a context where we're allowed
to access the network, because they exercise the real plugin installer
codepath. We could technically build this to use a local filesystem mirror
or other such override to avoid that, but the point here is to make sure
we see the expected behavior in the main case, and so it's worth the
small additional cost of downloading the null provider from the real
registry.
2021-10-05 10:59:59 -07:00
Martin Atkins d09510a8fb command: Early error message for missing cache entries of locked providers
In the original incarnation of Meta.providerFactories we were returning
into a Meta.contextOpts whose signature didn't allow it to return an
error directly, and so we had compromised by making the provider factory
functions themselves return errors once called.

Subsequent work made Meta.contextOpts need to return an error anyway, but
at the time we neglected to update our handling of the providerFactories
result, having it still defer the error handling until we finally
instantiate a provider.

Although that did ultimately get the expected result anyway, the error
ended up being reported from deep in the guts of a Terraform Core graph
walk, in whichever concurrently-visited graph node happened to try to
instantiate the plugin first. This meant that the exact phrasing of the
error message would vary between runs and the reporting codepath didn't
have enough context to given an actionable suggestion on how to proceed.

In this commit we make Meta.contextOpts pass through directly any error
that Meta.providerFactories produces, and then make Meta.providerFactories
produce a special error type so that Meta.Backend can ultimately return
a user-friendly diagnostic message containing a specific suggestion to
run "terraform init", along with a short explanation of what a provider
plugin is.

The reliance here on an implied contract between two functions that are
not directly connected in the callstack is non-ideal, and so hopefully
we'll revisit this further in future work on the overall architecture of
the CLI layer. To try to make this robust in the meantime though, I wrote
it to use the errors.As function to potentially unwrap a wrapped version
of our special error type, in case one of the intervening layers is
changed at some point to wrap the downstream error before returning it.
2021-10-05 10:59:59 -07:00
Martin Atkins df578afd7e backend/local: Check dependency lock consistency before any operations
In historical versions of Terraform the responsibility to check this was
inside the terraform.NewContext function, along with various other
assorted concerns that made that function particularly complicated.

More recently, we reduced the responsibility of the "terraform" package
only to instantiating particular named plugins, assuming that its caller
is responsible for selecting appropriate versions of any providers that
_are_ external. However, until this commit we were just assuming that
"terraform init" had correctly selected appropriate plugins and recorded
them in the lock file, and so nothing was dealing with the problem of
ensuring that there haven't been any changes to the lock file or config
since the most recent "terraform init" which would cause us to need to
re-evaluate those decisions.

Part of the game here is to slightly extend the role of the dependency
locks object to also carry information about a subset of provider
addresses whose lock entries we're intentionally disregarding as part of
the various little edge-case features we have for overridding providers:
dev_overrides, "unmanaged providers", and the testing overrides in our
own unit tests. This is an in-memory-only annotation, never included in
the serialized plan files on disk.

I had originally intended to create a new package to encapsulate all of
this plugin-selection logic, including both the version constraint
checking here and also the handling of the provider factory functions, but
as an interim step I've just made version constraint consistency checks
the responsibility of the backend/local package, which means that we'll
always catch problems as part of preparing for local operations, while
not imposing these additional checks on commands that _don't_ run local
operations, such as "terraform apply" when in remote operations mode.
2021-10-01 14:43:58 -07:00
Martin Atkins 6a98e4720c plans/planfile: Create takes most arguments via a struct type
Previously the planfile.Create function had accumulated probably already
too many positional arguments, and I'm intending to add another one in
a subsequent commit and so this is preparation to make the callsites more
readable (subjectively) and make it clearer how we can extend this
function's arguments to include further components in a plan file.

There's no difference in observable functionality here. This is just
passing the same set of arguments in a slightly different way.
2021-10-01 14:43:58 -07:00
Martin Atkins 8d193ad268 core: Simplify and centralize plugin availability checks
Historically the responsibility for making sure that all of the available
providers are of suitable versions and match the appropriate checksums has
been split rather inexplicably over multiple different layers, with some
of the checks happening as late as creating a terraform.Context.

We're gradually iterating towards making that all be handled in one place,
but in this step we're just cleaning up some old remnants from the
main "terraform" package, which is now no longer responsible for any
version or checksum verification and instead just assumes it's been
provided with suitable factory functions by its caller.

We do still have a pre-check here to make sure that we at least have a
factory function for each plugin the configuration seems to depend on,
because if we don't do that up front then it ends up getting caught
instead deep inside the Terraform runtime, often inside a concurrent
graph walk and thus it's not deterministic which codepath will happen to
catch it on a particular run.

As of this commit, this actually does leave some holes in our checks: the
command package is using the dependency lock file to make sure we have
exactly the provider packages we expect (exact versions and checksums),
which is the most crucial part, but we don't yet have any spot where
we make sure that the lock file is consistent with the current
configuration, and we are no longer preserving the provider checksums as
part of a saved plan.

Both of those will come in subsequent commits. While it's unusual to have
a series of commits that briefly subtracts functionality and then adds
back in equivalent functionality later, the lock file checking is the only
part that's crucial for security reasons, with everything else mainly just
being to give better feedback when folks seem to be using Terraform
incorrectly. The other bits are therefore mostly cosmetic and okay to be
absent briefly as we work towards a better design that is clearer about
where that responsibility belongs.
2021-10-01 14:43:58 -07:00
Zach Whaley c9a5fdb366
cliconfig: Fix error message about invalid credentials helper type 2021-09-29 13:36:59 -07:00
James Bardin ab0322e406 remove debugging println 2021-09-28 17:58:40 -04:00
James Bardin c2e0d265cf LoadModule now always returns the module
We don't need to load the configuration twice, since configload can
return the module for us.
2021-09-28 17:58:40 -04:00
James Bardin a53faf43f6 return partial config from LoadConfig with errors
LoadConfig should return any parsed configuration in order for the
caller to verify `required_version`.
2021-09-28 13:30:03 -04:00
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
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 9847eaa9cf remove usage of MinItems/MaxItems
MinItems and MaxItems are not used on nested types in the protocol, so
remove their usage in Terraform to prevent future confusion.
2021-09-24 12:26:00 -04: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
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
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
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 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 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 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
Alisdair McDiarmid d5b5407ccc format: Fix incorrect nesting of Color/Sprintf
Colorizing the result of an interpolated string can result in
incorrect output, if the values used to generate the string happen to
include color codes such as `[red]` or `[bold]`. Instead we should
always colorize the format string before calling functions like
`Sprintf`. This commit fixes all instances in this file.
2021-09-16 15:22:37 -04:00
Alisdair McDiarmid f0cf4235f9 cli: Refactor resource drift rendering 2021-09-16 15:22:37 -04:00
Martin Atkins 718fa3895f backend: Remove Operation.Parallelism field
The presence of this field was confusing because in practice the local
backend doesn't use it for anything and the remote backend was using it
only to return an error if it's set to anything other than the default,
under the assumption that it would always match ContextOpts.Parallelism.

The "command" package is the one actually responsible for handling this
option, and it does so by placing it into the partial ContextOpts which it
passes into the backend when preparing for a local operation. To make that
clearer, here we remove Operation.Parallelism and change the few uses of
it to refer to ContextOpts.Parallelism instead, so that everyone is
reading and writing this value from the same place.
2021-09-14 10:35:08 -07:00
James Bardin d0993b0e80 fix temp directory handling in some tests
Cleanup some more test fixtures to use t.TempDir

Use EvalSymlinks with temp dir paths to help with MacOS errors from
various terraform components.
2021-09-13 13:45:04 -04:00
Martin Atkins 65e0c448a0 workdir: Start of a new package for working directory state management
Thus far our various interactions with the bits of state we keep
associated with a working directory have all been implemented directly
inside the "command" package -- often in the huge command.Meta type -- and
not managed collectively via a single component.

There's too many little codepaths reading and writing from the working
directory and data directory to refactor it all in one step, but this is
an attempt at a first step towards a future where everything that reads
and writes from the current working directory would do so via an object
that encapsulates the implementation details and offers a high-level API
to read and write all of these session-persistent settings.

The design here continues our gradual path towards using a dependency
injection style where "package main" is solely responsible for directly
interacting with the OS command line, the OS environment, the OS working
directory, the stdio streams, and the CLI configuration, and then
communicating the resulting information to the rest of Terraform by wiring
together objects. It seems likely that eventually we'll have enough wiring
code in package main to justify a more explicit organization of that code,
but for this commit the new "workdir.Dir" object is just wired directly in
place of its predecessors, without any significant change of code
organization at that top layer.

This first commit focuses on the main files and directories we use to
find provider plugins, because a subsequent commit will lightly reorganize
the separation of concerns for plugin launching with a similar goal of
collecting all of the relevant logic together into one spot.
2021-09-10 14:56:49 -07:00
Martin Atkins 343279110a core: Graph walk loads plugin schemas opportunistically
Previously our graph walker expected to recieve a data structure
containing schemas for all of the provider and provisioner plugins used in
the configuration and state. That made sense back when
terraform.NewContext was responsible for loading all of the schemas before
taking any other action, but it no longer has that responsiblity.

Instead, we'll now make sure that the "contextPlugins" object reaches all
of the locations where we need schema -- many of which already had access
to that object anyway -- and then load the needed schemas just in time.

The contextPlugins object memoizes schema lookups, so we can safely call
it many times with the same provider address or provisioner type name and
know that it'll still only load each distinct plugin once per Context
object.

As of this commit, the Context.Schemas method is now a public interface
only and not used by logic in the "terraform" package at all. However,
that does leave us in a rather tenuous situation of relying on the fact
that all practical users of terraform.Context end up calling "Schemas" at
some point in order to verify that we have all of the expected versions
of plugins. That's a non-obvious implicit dependency, and so in subsequent
commits we'll gradually move all responsibility for verifying plugin
versions into the caller of terraform.NewContext, which'll heal a
long-standing architectural wart whereby the caller is responsible for
installing and locating the plugin executables but not for verifying that
what's installed is conforming to the current configuration and dependency
lock file.
2021-09-10 14:56:49 -07:00
Martin Atkins 80b3fcf93e core: Replace contextComponentFactory with contextPlugins
In the v0.12 timeframe we made contextComponentFactory an interface with
the expectation that we'd write mocks of it for tests, but in practice we
ended up just always using the same "basicComponentFactory" implementation
throughout.

In the interests of simplification then, here we replace that interface
and its sole implementation with a new concrete struct type
contextPlugins.

Along with the general benefit that this removes an unneeded indirection,
this also means that we can add additional methods to the struct type
without the usual restriction that interface types prefer to be small.
In particular, in a future commit I'm planning to add methods for loading
provider and provisioner schemas, working with the currently-unused new
fields this commit has included in contextPlugins, as compared to its
predecessor basicComponentFactory.
2021-09-10 14:56:49 -07:00
Alisdair McDiarmid cd29c3e5fd
Revert "json-output: Release format version 1.0" 2021-09-09 11:25:35 -04:00
Laura Pacilio a819d7db3a
Merge pull request #29502 from hashicorp/alisdair/json-format-version
json-output: Release format version 1.0
2021-09-09 11:06:58 -04:00
Alisdair McDiarmid 7f3a29b46e
Merge pull request #29523 from hashicorp/alisdair/moved-ui
command: Render "moved" annotations in plan UI
2021-09-07 14:28:11 -04:00
James Bardin 1ad0421e15
Merge pull request #29522 from hashicorp/jbardin/json-blocktoattr
allow json output to marshal ConfigModeAttr blocks
2021-09-07 11:32:21 -04:00
Alisdair McDiarmid 29999c1d6f command: Render "moved" annotations in plan UI
For resources which are planned to move, render the previous run address
as additional information in the plan UI. For the case of a move-only
resource (which otherwise is unchanged), we also render that as a
planned change, but without any corresponding action symbol.

If all changes in the plan are moves without changes, the plan is no
longer considered "empty". In this case, we skip rendering the action
symbols in the UI.
2021-09-03 17:44:07 -04:00
James Bardin ac2a870ea0 allow json output to marshal ConfigModeAttr blocks
In order to marshal config blocks using ConfigModeAttr, we need to
insert the fixup body to map hcl blocks to the attribute in the schema.
2021-09-03 13:53:52 -04:00
Alisdair McDiarmid b60f201eed json-output: Release format version 1.0 2021-09-01 15:15:18 -04:00
James Bardin 863963e7a6 de-linting 2021-09-01 11:36:21 -04:00
Martin Atkins 89b05050ec core: Functional-style API for terraform.Context
Previously terraform.Context was built in an unfortunate way where all of
the data was provided up front in terraform.NewContext and then mutated
directly by subsequent operations. That made the data flow hard to follow,
commonly leading to bugs, and also meant that we were forced to take
various actions too early in terraform.NewContext, rather than waiting
until a more appropriate time during an operation.

This (enormous) commit changes terraform.Context so that its fields are
broadly just unchanging data about the execution context (current
workspace name, available plugins, etc) whereas the main data Terraform
works with arrives via individual method arguments and is returned in
return values.

Specifically, this means that terraform.Context no longer "has-a" config,
state, and "planned changes", instead holding on to those only temporarily
during an operation. The caller is responsible for propagating the outcome
of one step into the next step so that the data flow between operations is
actually visible.

However, since that's a change to the main entry points in the "terraform"
package, this commit also touches every file in the codebase which
interacted with those APIs. Most of the noise here is in updating tests
to take the same actions using the new API style, but this also affects
the main-code callers in the backends and in the command package.

My goal here was to refactor without changing observable behavior, but in
practice there are a couple externally-visible behavior variations here
that seemed okay in service of the broader goal:
 - The "terraform graph" command is no longer hooked directly into the
   core graph builders, because that's no longer part of the public API.
   However, I did include a couple new Context functions whose contract
   is to produce a UI-oriented graph, and _for now_ those continue to
   return the physical graph we use for those operations. There's no
   exported API for generating the "validate" and "eval" graphs, because
   neither is particularly interesting in its own right, and so
   "terraform graph" no longer supports those graph types.
 - terraform.NewContext no longer has the responsibility for collecting
   all of the provider schemas up front. Instead, we wait until we need
   them. However, that means that some of our error messages now have a
   slightly different shape due to unwinding through a differently-shaped
   call stack. As of this commit we also end up reloading the schemas
   multiple times in some cases, which is functionally acceptable but
   likely represents a performance regression. I intend to rework this to
   use caching, but I'm saving that for a later commit because this one is
   big enough already.

The proximal reason for this change is to resolve the chicken/egg problem
whereby there was previously no single point where we could apply "moved"
statements to the previous run state before creating a plan. With this
change in place, we can now do that as part of Context.Plan, prior to
forking the input state into the three separate state artifacts we use
during planning.

However, this is at least the third project in a row where the previous
API design led to piling more functionality into terraform.NewContext and
then working around the incorrect order of operations that produces, so
I intend that by paying the cost/risk of this large diff now we can in
turn reduce the cost/risk of future projects that relate to our main
workflow actions.
2021-08-30 13:59:14 -07:00
Alisdair McDiarmid 40dd4292b8
Merge pull request #29438 from hashicorp/alisdair/init-force-copy-multiple-workspaces
command: Suppress prompt for init -force-copy
2021-08-20 15:21:43 -04:00
Alisdair McDiarmid 2762a940c0 command: Suppress prompt for init -force-copy
The -force-copy flag to init should automatically migrate state.
Previously this was not applied to one case: when migrating from
a backend with multiple workspaces to another backend supporting
multiple workspaces. I believe this was an oversight so this commit
fixes that.
2021-08-20 14:46:09 -04:00
Alisdair McDiarmid 70a4f7a6b6 command: Fix stale lock after running add 2021-08-20 13:22:11 -04:00
Martin Atkins 43d753e727 command: "terraform add" is experimental
We're aware of several quirks of this command's current design, which
result from some existing architectural limitations that we can't address
immediately.

However, we do still want to make this command available in its current
capacity as an incremental improvement, so as a compromise we'll document
it as experimental. Our intent here is to exclude it from the
Terraform 1.0 Compatibility Promises so that we can have the space to
continue to improve the design as other parts of the overall Terraform
system gain new capabilities.

We don't currently have any concrete plan for this command to be
stabilized and subject to compatibility promises. That decision will
follow from ongoing discussions with other teams whose systems may need to
change in order to support the final design of "terraform add".
2021-08-19 09:27:30 -07:00
James Bardin 68ed50616e handle null and unknown values in attr diffs
The code adopted from block diffs was not set to handle null and unknown
values, as those are not allowed for blocks.

We also revert the change to formatting nested object types as single
attributes, because the attribute formatter cannot handle sensitive
values from the schema. This presents some awkward syntax for diffs for
now, but should suffice until the entire formatter can be refactored to
better handle these new nested types.
2021-08-18 14:12:01 -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
James Bardin a94155d0ca
Merge pull request #29397 from hashicorp/jbardin/format-id-name-marks
unmark object ID or Name for formatting
2021-08-17 14:58:56 -04:00
James Bardin a48b024c0a unmark object ID or Name for formatting 2021-08-17 12:24:43 -04:00