* prep for processing the structured run output
* undo unwanted change to a json key
* Add skeleton functions and API for refactored renderer
* goimports
* Fix documentation of the RenderOpts struct
* Add README explaining implementation details for renderer and plans for future expansion
* Update internal/command/jsonformat/README.md
Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>
* address comments
Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Make writing a plan file the default. We already create plans which have
no changes so the plan result would need to be checked in automation, so
having plans with errors should not pose a problem.
If we find workflows which cannot handle a plan that can't be applied,
we can reevaluate the need for a specialized flag. In the meantime, it
feels more logical that the plan output would always describe the result
of the plan, even if that included errors.
This is a prototype of how the CLI layer might make use of Terraform
Core's ability to produce a partial plan if it encounters an error during
planning, with two new situations:
- When using local CLI workflow, Terraform will show the partial plan
before showing any errors.
- "terraform plan" has a new option -always-out=..., which is similar to
the existing -out=... but additionally instructs Terraform to produce
a plan file even if the plan is incomplete due to errors. This means
that the plan can still be inspected by external UI implementations.
This is just a prototype to explore how these parts might fit together.
It's not a complete implementation and so should not be shipped. In
particular, it doesn't include any mention of a plan being incomplete in
the "terraform show -json" output or in the "terraform plan -json" output,
both of which would be required for a complete solution.
When we originally introduced the trust-on-first-use checksum locking
mechanism in v0.14, we had to make some tricky decisions about how it
should interact with the pre-existing optional read-through global cache
of provider packages:
The global cache essentially conflicts with the checksum locking because
if the needed provider is already in the cache then Terraform skips
installing the provider from upstream and therefore misses the opportunity
to capture the signed checksums published by the provider developer. We
can't use the signed checksums to verify a cache entry because the origin
registry protocol is still using the legacy ziphash scheme and that is
only usable for the original zipped provider packages and not for the
unpacked-layout cache directory. Therefore we decided to prioritize the
existing cache directory behavior at the expense of the lock file behavior,
making Terraform produce an incomplete lock file in that case.
Now that we've had some real-world experience with the lock file mechanism,
we can see that the chosen compromise was not ideal because it causes
"terraform init" to behave significantly differently in its lock file
update behavior depending on whether or not a particular provider is
already cached. By robbing Terraform of its opportunity to fetch the
official checksums, Terraform must generate a lock file that is inherently
non-portable, which is problematic for any team which works with the same
Terraform configuration on multiple different platforms.
This change addresses that problem by essentially flipping the decision so
that we'll prioritize the lock file behavior over the provider cache
behavior. Now a global cache entry is eligible for use if and only if the
lock file already contains a checksum that matches the cache entry. This
means that the first time a particular configuration sees a new provider
it will always be fetched from the configured installation source
(typically the origin registry) and record the checksums from that source.
On subsequent installs of the same provider version already locked,
Terraform will then consider the cache entry to be eligible and skip
re-downloading the same package.
This intentionally makes the global cache mechanism subordinate to the
lock file mechanism: the lock file must be populated in order for the
global cache to be effective. For those who have many separate
configurations which all refer to the same provider version, they will
need to re-download the provider once for each configuration in order to
gather the information needed to populate the lock file, whereas before
they would have only downloaded it for the _first_ configuration using
that provider.
This should therefore remove the most significant cause of folks ending
up with incomplete lock files that don't work for colleagues using other
platforms, and the expense of bypassing the cache for the first use of
each new package with each new configuration. This tradeoff seems
reasonable because otherwise such users would inevitably need to run
"terraform providers lock" separately anyway, and that command _always_
bypasses the cache. Although this change does decrease the hit rate of the
cache, if we subtract the never-cached downloads caused by
"terraform providers lock" then this is a net benefit overall, and does
the right thing by default without the need to run a separate command.
Previously, there was mixed usage of "(sensitive)" and "(sensitive value)" and even though it was more common to see "(sensitive)", the thought is that it's a value we are hiding rather than describing something already shown.
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.
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.
