Commit Graph

183 Commits

Author SHA1 Message Date
Martin Atkins
72dd14ca5c core: Do everything except the actual action for plans.NoOp
Previously we tried to early-exit before doing anything at all for any
no-op changes, but that means we also skip some ancillary steps like
evaluating any preconditions/postconditions.

Now we'll skip only the main action itself for plans.NoOp, and still run
through all of the other side-steps.

Since one of those other steps is emitting events through the hooks
interface, this means that now no-op actions are visible to hooks, whereas
before we always filtered them out before calling. I therefore added some
additional logic to the hooks to filter them out at the UI layer instead;
the decision for whether or not to report that we visited a particular
object and found no action required seems defensible as a UI-level concern
anyway.
2022-07-22 15:27:15 -07:00
Martin Atkins
9e277033bc core: Create apply graph nodes even for no-op "changes"
We previously would optimize away the graph nodes for any resource
instance without a real change pending, but that means we don't get an
opportunity to re-check any invariants associated with the instance, such
as preconditions and postconditions.

Other upstream changes during apply can potentially decide the outcome of
a condition even if the instance itself isn't being changed, so we do
still need to revisit these during apply or else we might skip running
certain checks altogether, if they yielded unknown results during planning
and then don't get run during apply.
2022-07-22 15:27:15 -07:00
Martin Atkins
3cd069900b core: Use DynamicExpand even for root module outputs
We previously had a special case in the graph transformer for output
values where it would directly create an individual output value node
instead of an "expand" node as we would do for output values in nested
modules.

While it's true that we do always know that expanding a root module
output value will always produce exactly one instance, treating this case
as special creates the risk of those two codepaths diverging in other
ways.

Instead, we'll let the expand node also deal with root modules and
minimize the special case only to how we look up any changes for the
output values, since the design of plans.Changes is a bit awkward and
requires us to ask the question differently for root module output values.
Otherwise, the behavior will now be consistent across all output values
regardless of module.
2022-07-22 15:25:22 -07:00
James Bardin
9487cfba28 add test for planned private data in destroy 2022-07-06 13:47:35 -04:00
James Bardin
acba1159e4 add provider metas to destroy plan 2022-07-06 13:47:35 -04:00
James Bardin
ccd2c7820a plan resource destroy
Call PlanResourceDestroy during a destroy plan.

This allows providers two new abilities:
- They can evaluate if the plan is valid, notifying users of any
  potential errors before an apply is started, which may not be able to
  complete.
- They can inspect and modify their private data during a destroy plan
  just like they can with an other plan operation.
2022-07-06 13:47:35 -04:00
James Bardin
e95bfe6d19 fix test mocks to behave when planning destroys 2022-07-06 13:47:35 -04:00
Liam Cervante
d876e68e2d
Fail global required_version check if it contains any prerelease fields (#31331)
* Fail global required_version check if it contains any prerelease fields

* go mod tidy

* Improve required_version prerelease not supported error string

* Add prerelease version constraint unit tests

* Fix side-effects by populating global diags too soon
2022-06-30 09:58:28 +01:00
Martin Atkins
90ea7b0bc5 tfdiags: Treat unknown-related or sensitive-related messages differently
By observing the sorts of questions people ask in the community, and the
ways they ask them, we've inferred that various different people have been
confused by Terraform reporting that a value won't be known until apply
or that a value is sensitive as part of an error message when that message
doesn't actually relate to the known-ness and sensitivity of any value.

Quite reasonably, someone who sees Terraform discussing an unfamiliar
concept like unknown values can assume that it must be somehow relevant to
the problem being discussed, and so in that sense Terraform's current
error messages are giving "too much information": information that isn't
actually helpful in understanding the problem being described, and in the
worst case is a distraction from understanding the problem being described.

With that in mind then, here we introduce an explicit annotation on
diagnostic objects that are directly talking about unknown values or
sensitive values, and then the diagnostic renderer will react to that to
avoid using the terminology "known only after apply" or "sensitive" in the
generated diagnostic annotations unless we're rendering a message that is
explicitly related to one of those topics.

This ends up being a bit of a cross-cutting concern because the code that
generates these diagnostics and the code that renders them are in separate
packages and are not directly aware of each other. With that in mind, the
logic for actually deciding for a particular diagnostic whether it's
flagged in one of these special ways lives inside the tfdiags package as
an intermediation point, which both the diagnostic generator (in the core
package) and the diagnostic renderer can both depend on.
2022-06-23 13:52:23 -07:00
James Bardin
77e6b622f8
Merge pull request #31283 from hashicorp/jbardin/plan-import
Use plan graph for importing resources
2022-06-23 13:26:59 -04:00
James Bardin
142ce15ed6 remove unused field and extra assignment 2022-06-23 11:47:01 -04:00
James Bardin
c43df009db add required attr to import test
Missing required attributes should not prevent importing
2022-06-22 13:30:44 -04:00
James Bardin
77cca0af7c remove import transformer 2022-06-20 16:11:02 -04:00
James Bardin
c002bab730 update test fixtures for better imports 2022-06-20 15:40:49 -04:00
James Bardin
e97ae28441 Combine all plan graphs, including import
Combine all plan-time graphs into a single graph builder, because
_everything is a plan_!

Convert the import graph to a plan graph. This should resolve a few edge
cases about things not being properly evaluated during import, and takes
a step towards being able to _plan_ an import.
2022-06-20 15:40:49 -04:00
Martin Atkins
fda0579537 Experiments supported only in alpha/dev builds
We originally introduced the idea of language experiments as a way to get
early feedback on not-yet-proven feature ideas, ideally as part of the
initial exploration of the solution space rather than only after a
solution has become relatively clear.

Unfortunately, our tradeoff of making them available in normal releases
behind an explicit opt-in in order to make it easier to participate in the
feedback process had the unintended side-effect of making it feel okay
to use experiments in production and endure the warnings they generate.
This in turn has made us reluctant to make use of the experiments feature
lest experiments become de-facto production features which we then feel
compelled to preserve even though we aren't yet ready to graduate them
to stable features.

In an attempt to tweak that compromise, here we make the availability of
experiments _at all_ a build-time flag which will not be set by default,
and therefore experiments will not be available in most release builds.

The intent (not yet implemented in this PR) is for our release process to
set this flag only when it knows it's building an alpha release or a
development snapshot not destined for release at all, which will therefore
allow us to still use the alpha releases as a vehicle for giving feedback
participants access to a feature (without needing to install a Go
toolchain) but will not encourage pretending that these features are
production-ready before they graduate from experimental.

Only language experiments have an explicit framework for dealing with them
which outlives any particular experiment, so most of the changes here are
to that generalized mechanism. However, the intent is that non-language
experiments, such as experimental CLI commands, would also in future
check Meta.AllowExperimentalFeatures and gate the use of those experiments
too, so that we can be consistent that experimental features will never
be available unless you explicitly choose to use an alpha release or
a custom build from source code.

Since there are already some experiments active at the time of this commit
which were not previously subject to this restriction, we'll pragmatically
leave those as exceptions that will remain generally available for now,
and so this new approach will apply only to new experiments started in the
future. Once those experiments have all concluded, we will be left with
no more exceptions unless we explicitly choose to make an exception for
some reason we've not imagined yet.

It's important that we be able to write tests that rely on experiments
either being available or not being available, so here we're using our
typical approach of making "package main" deal with the global setting
that applies to Terraform CLI executables while making the layers below
all support fine-grain selection of this behavior so that tests with
different needs can run concurrently without trampling on one another.

As a compromise, the integration tests in the terraform package will
run with experiments enabled _by default_ since we commonly need to
exercise experiments in those tests, but they can selectively opt-out
if they need to by overriding the loader setting back to false again.
2022-06-17 14:46:07 -07:00
Alisdair McDiarmid
479c71f93d
Merge pull request #31210 from hashicorp/update-optional-type-attributes
Edit type constraints docs for style and flow
2022-06-17 15:47:57 -04:00
Martin Atkins
dc5964f8a3 refactoring: Use addrs.Map for maps with addresses as keys
We introduced the addrs.UniqueKey and addrs.UniqueKeyer mechanics as part
of implementing the ValidateMoves and ApplyMoves functions, as a way to
better encapsulate the solution to the problem that lots of our address
types aren't comparable and so cannot be used directly as map keys.

However, exposing addrs.UniqueKey handling directly in the logic adds
various noise to the algorithms and, in particular, obscures the fact that
MoveResults.Changes and MoveResult.Blocked both have different map key
types.

Here then we'll use the new addrs.Map helper type, which encapsulates the
idea of a map from an addrs.UniqueKeyer type to an arbitrary value type,
using the unique keys as the map keys internally. This does unfortunately
mean that we lose the conventional Go map access syntax and have to use
a method-based API instead, but I (subjectively) think that's an okay
compromise in return for avoiding the need to keep track inline of which
addrs.UniqueKey values correspond with which real addresses.

This is intended as an entirely-mechanical change, with equivalent
behavior to what it replaced. If anything here is doing something
materially different than what it replaced then that's a mistake.
2022-06-16 07:03:36 -07:00
James Bardin
0d3d95486a
Merge pull request #31218 from hashicorp/jbardin/validate-provider-local-names
Validate duplicate provider local names in `required_providers`
2022-06-15 13:51:38 -04:00
James Bardin
f1ce3edcc5 copy dependency values when sorting
Expanded resource instances can initially share the same dependency
slice, so we must take care to not modify the array values when
checking the dependencies.

In the future we can convert these to a generic Set data type, as we
often need to compare for equality and take the union of multiple groups
of dependencies.
2022-06-14 11:09:27 -04:00
Alisdair McDiarmid
922de89be1 Conclude module variable optional attrs experiment 2022-06-13 12:27:21 -04:00
James Bardin
256b113990
Merge pull request #31176 from hashicorp/jbardin/plan-destroy-configure-provider
Configure providers during a destroy plan
2022-06-13 09:05:24 -04:00
James Bardin
d7238d510a skip already added provider nodes
We can skip providers which already have a node in the graph for their
type.
2022-06-10 10:37:58 -04:00
Alisdair McDiarmid
f8e2b3ada5
Merge pull request #31154 from hashicorp/alisdair/module-variable-optional-default
Add inline defaults to optional object attribute type constraints
2022-06-06 11:06:43 -04:00
James Bardin
619fa61f0b have mockProvider always check it was configured
We do this already for other calls, but skipped UpgradeResourceState
since it wasn't previously possible to configure during a destroy plan.
2022-06-01 16:16:20 -04:00
James Bardin
38d70a1c3d configure providers during destroy plan
Now that we can fully evaluate a provider configuration, make sure we
configure the provider before using it during a destroy plan.
2022-06-01 16:03:27 -04:00
James Bardin
93ff27227a
Merge pull request #31163 from hashicorp/jbardin/plan-destroy
Use plan graph builder for destroy
2022-06-01 15:37:13 -04:00
James Bardin
e36ee757a5 minor cleanup from review 2022-06-01 15:29:59 -04:00
Alisdair McDiarmid
5b0052cc36 core: Apply type defaults to module variables
Now that variables parse and retain a set of default values for
object attributes, we must apply the defaults during variable
evaluation. We do so immediately before type conversion, preprocessing
the given value so that conversion will receive the intended defaults as
appropriate.
2022-05-31 12:11:15 -04:00
Alisdair McDiarmid
a6587970d0
Merge pull request #30552 from gbataille/29156_do_not_log_sensitive_values
Fixes #29156: Failing sensitive variables values are not logged
2022-05-30 10:40:51 -04:00
James Bardin
ec476af655 test for configured destroy plan provider 2022-05-27 11:58:25 -04:00
James Bardin
8fed14fc59 use the PlanGraphBuilder for destroy
Rather than maintain a separate graph builder for destroy, use the
normal plan graph with some extra options. Utilize the same pattern as
the validate graph for now, where we take the normal plan graph builder
and inject a new concrete function for the destroy nodes.
2022-05-27 10:59:11 -04:00
James Bardin
77d13808d5 rename field to destroyPlan for consistency 2022-05-27 10:58:28 -04:00
James Bardin
6860596b68 don't add orphan nodes during destroy
All instances in state are being removed for destroy, so we can skip
checking for orphans. Because we want to use the normal plan graph, we
need to be able to still call this during destroy, so flag it off.
2022-05-27 10:56:40 -04:00
James Bardin
d6f0d1ea57 don't add config nodes during destroy plan
We want to use the normal plan graph for destroy, so we need to flag off
configuration for that process.
2022-05-27 10:55:38 -04:00
James Bardin
e3a6e1f6e8 remove unused fields from DestroyEdgeTransformer 2022-05-27 10:54:58 -04:00
James Bardin
b8e362d24c do not connect references _to_ destroyers either
Destroy nodes should never participate in references. These edges didn't
come up before, because we weren't building a complete graph including
all temporary values.
2022-05-27 10:50:01 -04:00
James Bardin
48309835b7 add unknown paths to diags for debugging
When a user reports a "Configuration contains unknown value" error,
there is no information on what might have been unknown during apply.
Add unknown attribute paths to the diagnostic message to provide some
more information when a reproduction may not be possible. Sine this is
one of those "should never happen" types of errors which will be
reported to the developers directly, we can leave the format as the raw
internal representation for simplicity.
2022-05-23 13:28:30 -04:00
Alisdair McDiarmid
6494aa0326 Include var declaration where possible 2022-05-13 17:09:38 -04:00
Martin Atkins
289bb60ce1 core: Defer on transitive dependencies for data resources with conditions
When a data resource is used for the purposes of verifying a condition
about an object managed elsewhere (e.g. if the managed resource doesn't
directly export all of the information required for the condition) it's
important that we defer the data resource read to the apply step if the
corresponding managed resource has any changes pending.

Typically we'd expect that to come "for free" but unfortunately we have
a pragmatic special case in our handling of data resources where we
normally defer to the apply step only if a _direct_ dependency of the data
resource has a change pending, and allow a plan-time read if there's
a pending change in an indirect dependency. This allowed us to preserve
some compatibility with the questionable historical behavior of always
reading data resources proactively unless the configuration contains
unknown values, since the arguably-more-correct behavior would've been a
regression for anyone who had been depending on that before.

Since preconditions and postconditions didn't exist until now, we are not
constrained in the same way by backward compatibility, and so we can adopt
the more correct behavior in the case where a data resource has conditions
specified. This does unfortunately make the handling of data resources
with conditions subtly inconsistent with those that don't, but this is
a better situation than the alternative where it would be easy to get into
a trapped situation where the remote system is invalid and it's impossible
to plan the change that would make it valid again because the conditions
evaluate too soon, prior to the fix being applied.
2022-05-11 11:01:38 -07:00
Martin Atkins
4cffff24b1 core: Report reason for deferring data read until apply
We have two different reasons why a data resource might be read only
during apply, rather than during planning as usual: the configuration
contains unknown values, or the data resource as a whole depends on a
managed resource which itself has a change pending.

However, we didn't previously distinguish these two in a way that allowed
the UI to describe the difference, and so we confusingly reported both
as "config refers to values not yet known", which in turn led to a number
of reasonable questions about why Terraform was claiming that but then
immediately below showing the configuration entirely known.

Now we'll use our existing "ActionReason" mechanism to tell the UI layer
which of the two reasons applies to a particular data resource instance.
The "dependency pending" situation tends to happen in conjunction with
"config unknown", so we'll prefer to refer that the configuration is
unknown if both are true.
2022-05-09 11:12:47 -07:00
Grégory Bataille
378ee6ac56
Fixes #29156: Failing sensitive variables values are not logged 2022-05-07 13:25:20 +02:00
htamakos
4cfb6bc893
communicator/ssh: Add support SSH over HTTP Proxy (#30274)
Terraform's remote-exec provision hangs out when it execs on HTTP Proxy bacause it dosen't support SSH over HTTP Proxy. This commits enables Terraform's remote-exec to support SSH over HTTP Proxy.

* adds `proxy_*` fields to `connection` which add configuration for a proxy host
* if `proxy_host` set, connect to that proxy host via CONNECT method, then make the SSH connection to `host` or `bastion_host`
2022-04-27 16:59:17 -04:00
James Bardin
d9af967a6e build a test apply graph during plan
The plan graph does not contain all the information necessary to detect
cycles which may happen when building the apply graph. Once we have more
information from the plan we can build the complete apply graph with all
individual instances to verify that the apply can begin without errors.
2022-04-22 15:54:01 -04:00
James Bardin
7da52d94f2
Merge pull request #30900 from hashicorp/jbardin/replace-triggered-by
Configurable instance replacement via lifecycle `replace_triggered_by`
2022-04-22 14:36:42 -04:00
James Bardin
0fe38fba6c prevent unsynchronized output changes access
The raw plan output changes were stored in the output exec node, when
they should have instead been fetch lazily through the context via the
synchronized ChangesSync value.
2022-04-20 14:45:58 -04:00
James Bardin
1e79682c24 minor fixes 2022-04-20 12:51:24 -04:00
James Bardin
e2fc9a19f5 use ResourceInstanceReplaceByTriggers
Set ResourceInstanceReplaceByTriggers in the change.
2022-04-20 09:17:10 -04:00
James Bardin
91121aa856 limit replace_triggered_by to same module instance
replace_triggered_by references are scoped to the current module, so we
need to filter changes for the current module instance. Rather than
creating a ConfigResource and filtering the result, make a
Changes.InstancesForAbsResource method to get only the AbsResource
changes.
2022-04-20 09:17:10 -04:00
James Bardin
fb6fcf783b Fix replace_triggered_by criteria
Only immediate changes to the resource are considered.
2022-04-20 09:17:10 -04:00