Previously we tried to handle externally-configured providers by treating
them mostly like normal provider configurations except quietly skipping
the configuration step when visiting them.
However, that wasn't sufficient because the runtime would still try to
decode a placeholder empty configuration to send into the InitProvider
method, which failed in cases where the provider had at least one required
configuration argument.
Instead then, we'll use a separate graph node type to represent
externally-configured providers, which allows us to use a much simpler
implementation for them and also, as a bonus, makes it considerably easier
to confirm that they are being treated as expected when reading the trace
log output, because we include the concrete node types in the graph dumps
and because we'll generate different trace messages when visiting those
nodes.
Altogether this avoids misleading error messages being returned when using
a provider with at least one required configuration argument through the
stacks runtime.
Previously this external providers concept had been tested exclusively
through the stacks runtime integration tests and not directly inside
"package terraform", and so this also introduces an explicit test for this
that's closer to the implementation, so we can inspect the behavior a
little more closely.
PlanOpts should contain only static options which are known before
planning. Previous to this commit, ctx.plan was modifying the opts
object, which, since it is passed via pointer, could be shared by other
ctx.plan calls happening concurrently, producing weird behaviour.
Such weird behaviour was only noticeable during tests, in which it is
common to pass a pointer to DefaultPlanOpts. When one test that modified
opts.forgetResources was run at the same time as another test,
DefaultPlanOpts.forgetResources would be populated when it shouldn't be.
The real Terraform binary does not do this and ctx.plan is only called
once per run.
One fix for the race condition would be to have ctx.plan take a deep copy of the
entire PlanOpts object which it can modify without touching the caller's
object. Due to the multiple nested data structures in PlanOpts this
would be a large amount of code, with a lot of DeepCopy methods it would
be easy to miss when adding struct fields.
Instead we just remove ImportTargets, forgetResources, and forgetModules
from PlanOpts, since they did not belong there in the first place. These
slices must be populated dynamically during planning once the entire
config tree is available and passed to the graph builder.
Currently, Terraform will only run properly signed versions of the experimental
cloud plugin that were downloaded from a TFC instance that provides the
appropriate service. That obstructs development on new cloud plugin features!
Our internal teams will need a "dev override" capability, like what we offer
provider authors.
However, unlike with providers, we don't have to integrate this into a
heterogeneous mix of sources for mirroring and caching a wide range of binaries.
There's only one cloud plugin, HashiCorp controls it, and end users should never
need to override the location of the binary for non-development reasons.
Thus, we have the luxury of being quite a bit stupider in how we handle the
override signal. Instead of adding it to the CLI config file schema, we'll just
use a single environment variable whose value is the path to an alternate
binary.
Enter: `TF_CLOUD_PLUGIN_DEV_OVERRIDE`.
By convention, Terraform Cloud workspaces connected to a VCS repo should should
only apply configs that come from the designated branch, to ensure that version
control remains the source of truth for that infrastructure.
The cloud version of `terraform apply` already includes a guardrail for that,
but we forgot to include an equivalent one for `terraform plan` when performing
saved plans (which can be applied later, unlike a purely speculative plan).
We suspect that TFC actually needs some additional formal access controls around
assigning configuration versions to workspaces, as everyone tends to be
surprised to learn that the API doesn't actually prevent this. But in the
meantime, what's good for the goose (apply command) is good for the gander (plan
command), so I'm copying over the check to voluntarily enforce the VCS workflow
for plans that can be applied.
* Add logic to collect variables for terrafrom test
* Add tests for test variable collection
* Update the test variable collection implementation
* Update internal/backend/local/test.go
Co-authored-by: Liam Cervante <liam.cervante@hashicorp.com>
* Update internal/backend/local/test.go
Co-authored-by: Liam Cervante <liam.cervante@hashicorp.com>
* Move test variables into var file
* resolve diff from cross-branch switch
* go fmt
---------
Co-authored-by: Liam Cervante <liam.cervante@hashicorp.com>
Previously "terraform console" always evaluated in a kinda strange context
where resource instance data comes from the prior state, but other derived
of values end up being calculated dynamically based on the current
configuration, which is okay for simple cases but can be confusing if the
configuration has changed significantly since the most recent apply, or
if there haven't yet been any applied changes.
Now we'll allow an optional new mode where Terraform runs the normal plan
phase (as if running "terraform plan") and then uses the resulting
_planned state_ as the basis for evaluation, allowing evaluation against
a partial approximation of what the world ought to look like if these
changes were applied, without having to actually apply them first.
As with the previous use of the eval walk, it's possible that an erroneous
situation will still produce a partial evaluation scope, and so the
console still allows evaluation against that scope but with a caveat that
it might produce unexpected results. In practice this can be useful for
debugging situations like when unknown values block planning of an object,
to allow inspection of the values that are contributing to that blocked
planning operation even though the plan is not yet complete.
The initial implementation of the module testing language included some
elements deeply embedded inside "package terraform", which is primarily
concerned with the traditional Terraform language, which we might now call
the "module language" to distinguish it from the test language and the
stacks language.
That was a reasonable design decision due to the modules runtime needing
to evaluate some expressions in a similar way to how the modules language
does it, but unfortunately it caused the test language runtime to depend
a little too closely on some current implementation details of how the
modules runtime deals with expression evaluation, and those details are
about to change as we teach Terraform to be able to deal with unknown
values in new places, as part of resolving this issue:
https://github.com/hashicorp/terraform/issues/30937
This commit therefore refactors the test runtime a little so that it
doesn't intrude so deeply into the module runtime's business. Instead of
directly extending the module runtime's expression evaluator, we'll
instead call into it in a similar way to how the "terraform console"
command does, by wrapping the evaluation data produced by the module
runtime to extend it with the only two special things that the test
runtime needs: references to the output values of previous runs, and
references to test-only variables that aren't declared as part of the
module under test.
This effectively means that the test runtime can now evaluate a superset
of the expressions that would be valid in the global scope of the module
under test using only the public API of the modules runtime, with no
testing-specific evaluation logic embedded inside the modules runtime.
This new strategy is also somewhat similar to how the stacks runtime calls
into the modules runtime to evaluate components, but the stacks runtime
only actually cares about output values and so doesn't need a full
evaluation scope. This means the result here is a little more symmetrical
with the approach we took in stacks, which will hopefully allow experience
working on one to translate better to the other.
We have a design goal of reusing the same design patterns for all of our
"condition-like" configuration constructs across all of Terraform's
languages, including custom conditions, validation rules, and check
blocks.
Now that we have the stacks language and the test language in addition to
what I guess we'll now call the "modules language", we'll eventually want
to hoist much of the handling of such blocks into a language-agnostic
place where all three languages can use it.
This is a very meagre start towards that goal, just factoring out the one
function for evaluating and validating the "error_message" argument
associated with a condition. I've factored it out into package "lang"
because that is currently the closest thing we have to a language-agnostic
place for this.
It's not _quite_ right because lang.Scope is quite specific to the modules
language, but we're already importing things from this package in both the
test runtime and the stacks runtime and so it's a good enough place until
in future we find a more complete design to represent the common elements
across all three languages.
This is a similar idea to terraform.Context.Eval, but rather than using
the special eval-only walk (which just reads data from the latest state
snapshot) we can now produce an evaluation scope based on the result of
either a plan or an apply operation, evaluating against either the planned
state or the new state respectively.
This will allow (in future commits) some code reuse for the test harness
and also the option to use "terraform console" to evaluate expressions
against planned state rather than only against prior state. However, as
of this commit there are not yet any direct callers of these new methods
except for the two new test cases.
Previously a SyncState became completely invalid after a call to Close,
but that's a little too strict because SyncState objects live on as part
of any returned lang.Scope objects that a caller might want to use for
(read-only) expression evaluation.
As a pragmatic compromise then, we'll instead continue to allow read access
through the SyncState object after it's closed, which is safe as long as
the caller does not try to perform those reads concurrently with its own
modifications to the underlying state.
In Terraform 1.7 we'll begin phasing out the legacy credential chain evaluation order
by defaulting use_legacy_workflow to false and deprecating the argument. This will
bring the default behavior into alignment with the AWS SDKs and CLI. Practitioners
can still preserve the legacy behavior by setting this argument to `true`. In Terraform
1.8 this argument will be removed, and the S3 backend will always use the default
credential chain evaluation order from the AWS SDK for Go.
Some time ago we ruled that Terraform Core would make a best effort to
return a partial plan describing the subset of actions that were
successfully proposed before encountering an error, which Terraform CLI
and Terraform Cloud then rely on to present some additional context to
support the associated error messages.
This change aims to improve that effort by making a distinction between
the failure of the planning operation itself vs. something else in the
configuration ruling that the successfully-created plan is unacceptable
for some separate reason.
In that case, it's helpful to still include that proposed change in the
plan, because the planning step itself succeeded and these problems are in
a sense "between" the planning operations, blocking any downstream work
from starting.
In particular, this change arranges for a failed prevent_destroy check or
a failed postcondition check to still include the planning result that
they were checked against, which then allows the UI the option of
displaying that planned action alongside the error describing why it was
unacceptable.
This doesn't include any Terraform CLI UI changes, but the UI layer is
already built to show partial plans returned alongside errors and so as
of this change the additional planned changes are already included. It'll
be up to future maintainers of Terraform CLI to decide whether and how
to refine that output, but the existing behavior is sufficient for now.
This method was returning diagnostics wrapped inside an error, and all
of its callers were then immediately unwrapping it and using the
diagnostics inside anyway.
To make that clearer, we'll just return diagnostics directly.
(It's likely that this was originally written as a function that returned
an error before we even _had_ the concept of diagnostics, and over time
we've gradually migrated all of the callers to be ready to accept
diagnostics but neglected to actually update the function itself.)
This was inadvertently trying to compare the string representation of an
addrs.AbsResource with the string representation of an
addrs.ConfigResource, making the two not match whenever the containing
module has count or for_each set.
The effects of this were pretty subtle:
- During normal apply the whole-resource state updates were not
necessarily happening before the instances got evaluated, but we got
away with it most of the time because the provider configuration for
a resource rarely changes and because we were redundantly
re-calculating the for_each/count for every instance and thus not
relying on the expander's idea of the instance expansion.
- During destroy-time apply the pruning transformer was pruning the
expansion nodes out altogether due to them having no dependents, and
so we were not updating the state and expander at all in that case.
This now correctly ensures that the expander node runs first and so gets
a chance to update the resource's selected provider configuration and the
expander's sense of the resource expansion before we visit any
instance-specific code that relies on those.
This also switches to using `addrs.Map` instead of a direct Go map because
that allows the Go compiler to to type-check that we're using the address
types consistently, thereby reducing the risk of regressing this again
in the future.
There are unfortunately no unit tests here because (as usual) our graph
transformation process resists unit testing as a result of all of the
transformers being subtly interdependent. However, this didn't regress
any existing tests and I manually inspected the graphs of the
configurations that caused me to spot this in the first place to verify
that they are now being constructed correctly.
Recently we've introduced the idea of the expansion of a particular module
or resource not yet being known, which has inadvertently introduced some
ambiguity about what "unknown" means in this package: "unknown" previously
represented "not registered yet", which is different than having
explicitly registered that we don't know.
To remove that ambiguity, we'll now use the term "unregistered" to talk
about the "not registered yet" situation, and reserve "unknown" for when
we know that we definitely don't know.
This is only an internal naming change within the package and doesn't
affect any exported API.
We shouldn't need this in normal code because the graph walk should ensure
that values are always registered before we read them, but this will be
useful in test code to avoid panicking in situations where the graph walk
doesn't behave correctly and we _do_ end up reading and writing in the
wrong order.
Currently we have a slightly different treatment for each of the different
kinds of named values, which makes it hard to generalize the
implementations of their very similar behaviors.
This package should over time take ownership of the problem of tracking
all three kinds of named value during a graph walk, with result data
structures that use this information (such as state and plan objects)
copying the relevant values out of here only once the graph walk is
complete.
Nothing is using this yet, though. Some retrofitting of this into
Terraform Core will follow in subsequent commits.
This logic was previously incorrect for any case where the number of
expanded steps was zero, causing the string representation to begin with
a dot when it ought not to.
* terraform: remove redundant code
NodeDestroyResourceInstance is never instantiated with a DeposedKey of anything other than states.NotDeposed, so the deleted code is never run. Deposed objects get a NodeDestroyDeposedResourceInstanceObject instead.
* tfdiags: add helper func
* configs: introduce removed block type
* terraform: add forget action
* renderer: render forget actions
* terraform: deposed objects can be forgotten
Deposed objects encountered during planning spawn
NodePlanDeposedResourceInstanceObject, which previously generated a
destroy change. Now it will generate a forget change if the deposed
object is a forget target, and a destroy change otherwise.
The apply graph gains a new node type,
NodeForgetDeposedResourceInstanceObject, whose execution simply removes
the object from the state.
* configs: add RemoveTarget address type
* terraform: modules can be forgotten
* terraform: error if removed obj still in config
* tests: better error on restore state fail
* Update CHANGELOG.md
