Commit Graph

505 Commits

Author SHA1 Message Date
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
Megan Bang 9b9b26a3cd update error message for invalid json 2021-10-13 13:51:07 -05:00
James Bardin 87e852c832
Merge pull request #29741 from hashicorp/jbardin/test-fix
fix test fixture with multiple provider instances
2021-10-13 09:53:02 -04:00
James Bardin 73b1263a86 wrap multiple provider creations into a factory fn
When a test uses multiple instances of the same provider, we may need to
have separate objects to prevent overwriting of the MockProvider state.
Create a completely new MockProvider in each factory function call
rather than re-using the original provider value.
2021-10-12 17:47:50 -04:00
Martin Atkins 965c0f3f91 build: Run staticcheck with "go run"
Running the tool this way ensures that we'll always run the version
selected by our go.mod file, rather than whatever happened to be available
in $GOPATH/bin on the system where we're running this.

This change caused some contexts to now be using a newer version of
staticcheck with additional checks, and so this commit also includes some
changes to quiet the new warnings without any change in overall behavior.
2021-10-12 11:42:17 -07:00
Martin Atkins d76759a6a9 configs/configload: snapshotDir must be used via pointer
A snapshotDir tracks its current position as part of its state, so we need
to use it via pointer rather than value so that Readdirnames can actually
update that position, or else we'll just get stuck at position zero.

In practice this wasn't hurting anything because we only call Readdir once
on our snapshots, to read the whole directory at once. Still nice to fix
to avoid a gotcha for future maintenence, though.
2021-10-12 11:42:17 -07:00
James Bardin 903ae5edfd fix test fixtures with multiple providers
Allow these to share the same backing MockProvider.
2021-10-12 14:34:34 -04:00
James Bardin 656f03b250 fix test fixture had the instance in the wrong mod
Make the state match the fixture config. The old test was not
technically invalid, but because it caused multiple instances of the
provider to be created, they were backed by the same MockProvider value
resulting in the `*Called` fields interfering.
2021-10-12 13:53:02 -04:00
James Bardin ef8f1b3e38
Merge pull request #29734 from hashicorp/jbardin/missing-change
Check for nil change during apply
2021-10-12 12:22:51 -04:00
Martin Atkins 2e6b6e9a6b go.mod: go get google.golang.org/protobuf@v1.27.1 2021-10-11 16:37:39 -07:00
James Bardin 3f4b680f1c Check for nil change during apply
Because NodeAbstractResourceInstance.readDiff can return a nil change,
we must check for that in all callers.
2021-10-08 16:46:29 -04:00
James Bardin 3c942ee3ac
Merge pull request #29720 from hashicorp/jbardin/destroy-plan-deposed
don't refresh deposed instances during the destroy plan
2021-10-08 16:23:59 -04:00
James Bardin a036109bc1 add comment about when we call ConfigureProvider 2021-10-08 15:23:36 -04:00
megan07 7dda3366a6
Update internal/backend/remote-state/gcs/backend.go
Co-authored-by: appilon <apilon@hashicorp.com>
2021-10-08 10:02:05 -05:00
James Bardin 03f71c2f06 fixup tests for MockProvider changes
Resetting the *Called fields and enforcing configuration broke a few
tests.
2021-10-08 08:42:06 -04:00
Megan Bang 81201d69a3 check valid json 2021-10-07 16:33:21 -05:00
Megan Bang 348c761bea add better error if credentials are invalid 2021-10-07 16:28:47 -05:00
James Bardin 22b400b8de skip refreshing deposed during destroy plan
The destroy plan should not require a configured provider (the complete
configuration is not evaluated, so they cannot be configured).

Deposed instances were being refreshed during the destroy plan, because
this instance type is only ever destroyed and shares the same
implementation between plan and walkPlanDestroy. Skip refreshing during
walkPlanDestroy.
2021-10-07 16:51:48 -04:00
James Bardin ad9944e523 test that providers are configured for calls
Have the MockProvider ensure that Configure is always called before any
methods that may require a configured provider.

Ensure the MockProvider *Called fields are zeroed out when re-using the
provider instance.
2021-10-07 16:48:56 -04: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
James Bardin aece887a85
Merge pull request #29701 from hashicorp/jbardin/proposed-new-null-objs
objchange: fix ProposedNew from null objects
2021-10-05 13:09:49 -04:00
James Bardin 58d85fcc2e add test for planing unknown data source values 2021-10-05 12:31:23 -04:00
James Bardin 18d354223e objchange: fix ProposedNew from null objects
The codepath for AllAttributesNull was not correct for any nested object
types with collections, and should create single null values for the
correct NestingMode rather than a single object with null attributes.

Since there is no reason to descend into nested object types to create
nullv alues, we can drop the AllAttributesNull function altogether and
create null values as needed during ProposedNew.

The corresponding AllBlockAttributesNull was only called internally in 1
location, and simply delegated to schema.EmptyValue. We can reduce the
package surface area by dropping that function too and calling
EmptyValue directly.
2021-10-04 15:03:25 -04:00
James Bardin c68422d125
Merge pull request #29682 from hashicorp/jbardin/less-data-depends_on
Refine data depends_on dependency checks
2021-10-04 11:39:21 -04: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 702413702c plans/planfile: Include dependency locks in saved plan files
We recently removed the legacy way we used to track the SHA256 hashes of
individual provider executables as part of a plans.Plan, because these
days we want to track the checksums of entire provider packages rather
than just the executable.

In order to achieve that new goal, we can save a copy of the dependency
lock information inside the plan file. This follows our existing precedent
of using exactly the same serialization formats we'd normally use for
this information, and thus we can reuse the existing models and
serializers and be confident we won't lose any detail in the round-trip.

As of this commit there's not yet anything actually making use of this
mechanism. In a subsequent commit we'll teach the main callers that write
and read plan files to include and expect (respectively) dependency
information, verifying that the available providers still match by the
time we're applying the plan.
2021-10-01 14:43:58 -07:00
Martin Atkins 3f85591998 depsfile: SaveLocksToBytes function
In a future commit we'll use this to include dependency lock information
in full fidelity inside a saved plan file.
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
James Bardin 618e9cf8ec test for unexpected data reads 2021-09-30 17:13:33 -04:00
James Bardin 016463ea9c don't check all ancestors for data depends_on
Only depends_on ancestors for transitive dependencies when we're not
pointed directly at a resource. We can't be much more precise here,
since in order to maintain our guarantee that data sources will wait for
explicit dependencies, if those dependencies happen to be a module,
output, or variable, we have to find some upstream managed resource in
order to check for a planned change.
2021-09-30 16:43:09 -04:00
Zach Whaley c9a5fdb366
cliconfig: Fix error message about invalid credentials helper type 2021-09-29 13:36:59 -07:00
James Bardin f93f16824c
Merge pull request #29665 from hashicorp/jbardin/required_version
Check required_version as early as possible
2021-09-29 08:18:20 -04: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
Martin Atkins f60d55d6ad core: Emit only one warning for move collisions in destroy-plan mode
Our current implementation of destroy planning includes secretly running a
normal plan first, in order to get its effect of refreshing the state.

Previously our warning about colliding moves would betray that
implementation detail because we'd return it from both of our planning
operations here and thus show the message twice. That would also have
happened in theory for any other warnings emitted by both plan operations,
but it's the move collision warning that made it immediately visible.

We'll now only return warnings from the initial plan if we're also
returning errors from that plan, and thus the warnings of both plans can
never mix together into the same diags and thus we'll avoid duplicating
any warnings.

This does mean that we'd lose any warnings which might hypothetically
emerge only from the hidden normal plan and not from the subsequent
destroy plan, but we'll accept that as an okay tradeoff here because those
warnings are likely to not be super relevant to the destroy case anyway,
or else we'd emit them from the destroy-plan walk too.
2021-09-27 15:46:36 -07: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 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 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