Add a new ChangeReason, ReasonDeleteBecauseNoMoveTarget, to provide better
information in cases where a planned deletion is due to moving a resource to
a target not in configuration.
Consider a case in which a resource instance exists in state at address A, and
the user adds a moved block to move A to address B. Whether by the user's
intention or not, address B does not exist in configuration.
Terraform combines the move from A to B, and the lack of configuration for B,
into a single delete action for the (previously nonexistent) entity B.
Prior to this commit, the Terraform plan will report that resource B will be
destroyed because it does not exist in configuration, without explicitly
connecting this to the move.
This commit provides the user an additional clue as to what has happened, in a
case in which Terraform has elided a user's action and inaction into one
potentially destructive change.
This is a clumsy way to do this, but a pragmatic way to inform potential
consumers that this part of the format is not yet finalized without having
to read the docs to see our warning about that.
We need to get some practical experience with a few different consumers
making use of this format before we can be confident that it's designed
appropriately. We're not _expecting_ to break it, but we'd like to leave
the opportunity open in case we quickly learn that there's something
non-ideal about this design.
Previously we were attempting to infer the checkable object address kind
of a given address by whether it included "output" in the position where
a resource type name would otherwise go.
That was already potentially risky because we've historically not
prevented a resource type named "output", and it's also a
forward-compatibility hazard in case we introduce additional object kinds
with entirely-new addressing schemes in future.
Given that, we'll instead always be explicit about what kind of address
we're storing in a wire or file format, so that we can make sure to always
use the intended parser when reading an address back into memory, or
return an error if we encounter a kind we're not familiar with.
This is a new-shaped representation of check results which follows the
two-tiered structure of static objects and dynamic instances of objects,
thereby allowing consumers to see which checkable objects exist in the
configuration even if a dynamic evaluation error prevented actually
expanding them all to determine their declared instances.
Eventually we'll include this in the state too, but this initially adds it
only to the plan in order to replace the now-deprecated experimental
conditions result that was present but undocumented in Terraform v1.2.
In an earlier commit we changed the states.CheckResults model to
explicitly model the config object vs. dynamic checkable object hierarchy,
but neglected to update the logic in Terraform Core to take that into
account when propagating the object expansion decisions from the plan
phase to the apply phase. That meant that we were incorrectly classifying
zero-instance resources always as having an unknown number of instances,
rather than possibly being known to have zero instances.
This now follows the two-level heirarchy of the data structure, which has
the nice side-effect that we can remove some of the special-case methods
from checks.State that we were using to bulk-load data: the data is now
shaped in the appropriate way to reload the data using the same method
the plan phase would've used to record the results in the first place.
This allows us to retain check results from one run into the next, so that
we can react to status changes between runs and potentially report e.g.
that a previously-failing check has now been fixed, or that a
previously-failing check is "still failing" so that an operator can get
a hint as to whether a problem is something they've just introduced or if
it was already an active problem before they made a change.
A significant goal of the design changes around checks in earlier commits
(with the introduction of package "checks") was to allow us to
differentiate between a configuration object that we didn't expand at all
due to an upstream error, which has _unknown_ check status, and a
configuration object that expanded to zero dynamic objects, which
therefore has a _passing_ check status.
However, our initial lowering of checks.State into states.CheckResults
stayed with the older model of just recording each leaf check in isolation,
without any tracking of the containers.
This commit therefore lightly reworks our representation of check results
in the state and plan with two main goals:
- The results are grouped by the static configuration object they came
from, and we capture an aggregate status for each of those so that
we can differentiate an unknown aggregate result from a passing
aggregate result which has zero dynamic associated objects.
- The granularity of results is whole checkable objects rather than
individual checks, because checkable objects have durable addresses
between runs, but individual checks for an object are more of a
syntactic convenience to make it easier for module authors to declare
many independent conditions that each have their own error messages.
Since v1.2 exposed some details of our checks model into the JSON plan
output there are some unanswered questions here about how we can shift to
reporting in the two-level heirarchy described above. For now I've
preserved structural compatibility but not semantic compatibility: any
parser that was written against that format should still function but will
now see fewer results. We'll revisit this in a later commit and consider
other structures and what to do about our compatibility constraint on the
v1.2 structure.
Otherwise though, this is an internal-only change which preserves all of
the existing main behaviors of conditions as before, and just gets us
ready to build user-facing features in terms of this new structure.
The "checks" package is an expansion what we previously called
plans.Conditions to accommodate a new requirement that we be able to track
which checks we're expecting to run even if we don't actually get around
to running them, which will be helpful when we start using checks as part
of our module testing story because test reporting tools appreciate there
being a relatively consistent set of test cases from one run to the next.
So far this should be essentially a no-op change from an external
functionality standpoint, aside from some minor adjustments to how we
report some of the error and warning cases from condition evaluation in
light of the fact that the "checks" package can now track errors as a
different outcome than a failure of a valid check.
As is often the case with anything which changes what we track
in the EvalContext and persist between plan and apply, Terraform Core is
pretty brittle and so this had knock-on effects elsewhere too. Again, the
goal is for these changes to not create any material externally-visible
difference, and just to accommodate the new assumption that there will
always be a "checks" object available for tracking during a graph walk.
The checks.Checks type aims to encapsulate keeping track of check results
during a run and then reporting on them afterwards even if the run was
aborted early for some reason.
The intended model here is that each new run starts with an entirely fresh
checks.Checks, with all of the statuses therefore initially unknown, and
gradually populates the check results as we walk the graph in Terraform
Core. This means that even if we don't complete the run due to an error
or due to targeting options we'll still report anything we didn't visit
yet as unknown.
This commit only includes the modeling of checks in the checks package.
For now this is just dead code, and we'll wire it in to Terraform Core in
subsequent commits.
We previously added methods like this for some of the other types in this
package, including Local in this same file, but apparently haven't needed
these two yet.
Our existing addrs.Checkable represents a particular (possibly-dynamic)
object that can have checks associated with it.
This new addrs.ConfigCheckable represents static configuration objects
that can potentially generate addrs.Checkable objects.
The idea here is to allow us to predict from the configuration a set of
potential checkable object containers and then dynamically associate
the dynamic checkable objects with them as we make progress with planning.
This is intended for our integration of checks into the "terraform test"
testing harness, to be used instead of the weirdo builtin provider we were
using as a placeholder before we had first-class syntax for checks.
Test reporting tools find it helpful for there to be a consistent set of
test cases from one run to the next so that they can report on trends over
multiple runs, and so our ConfigCheckable addresses will serve as the
relatively-static "test case" that we'll then associate the dynamic checks
with, so that we can still talk about objects in the test result report
even if we end up not reaching them due to an upstream conditution failure.
protoc-gen-go generates non-style-compliant import directives, but since
those files are just generated anyway we don't need to worry too much
about what style they are in: the style that protoc-gen-go generates is
the canonical style for these ones.
The importscheck script now uses "go run" to run this program and so
the Go toolchain will install it automatically using the version number
of golang.org/x/tools specified in our go.mod file.
All four of these scripts have #! lines calling for them to run in bash,
and they are all marked as executable, so there's no harm in running them
directly and it makes it clearer that we're running them in the shell
specified on the #! line, rather than with whatever "sh' happens to be
on the current system.
(Note that this _was_ still correctly using the #! lines before, because
it's `sh -c` rather than just `sh`, but nonetheless that old way seemed
needlessly confusing.)
When we run many of these together e.g. in the checks.yaml GitHub Actions
workflow, it's hard to tell from the output exactly which command is
producing which subset of the output.
To help clarify that, we'll ask make to print out the command line it's
running before it runs it.
At the risk of a little bit of hidden spooky action at a distance, this
will slightly change the behavior of the "goimports check" to compare
against the base branch of a PR rather than to origin/main if we happen
to find one of the environment variables that GitHub Actions sets
automatically in its runners. This is targeting our "checks.yml" workflow
in particular.
The intention here is to avoid misreporting files that haven't actually
changed when a PR is targeting a branch other than the main branch, such
as directly targeting a historical release branch.
We'll still run against origin/main when we're not running in GitHub
Actions, since that's _typically_ the correct branch to use for new
work, even if it will eventually get backported to a release branch.
The previous implementation of this check tried to accumulate all of the
changed files into a single big string and then run goimports once with
all of them, but that approach ran into problems for changesets of a
certain (platform-specific) size due to limits on maximum command line
length.
This new version instead uses bash arrays and runs goimports separately
for each of the files which appear to have changed relative to the
base branch. This is likely to be slower to complete for changesets that
have many different changed files, but it's better for it to be slow than
to return an error and fail to check some of the files.
This is a complement to "timestamp" and "timeadd" which allows
establishing the ordering of two different timestamps while taking into
account their timezone offsets, which isn't otherwise possible using the
existing primitives in the Terraform language.
