Previously we expected clients to provide an inline raw prior state to
PlanStackChanges and an inline raw plan to ApplyStackChanges, which was
a simpler design but meant that we might end up generating a state or plan
that's too large to be submitted in a single gRPC request, which would then
be difficult to resolve.
Instead we'll offer separate RPC functions for loading raw state and plan
using a gRPC streaming approach, which better mirrors the streaming
approach we use to _emit_ these artifacts. Although we don't actually need
this benefit right now, this makes it possible in principle for a client
that's running PlanStackChanges to feed back the raw planned actions
concurrently into OpenPlan and thus avoid buffering the whole plan on the
client side at all.
This required resolving the pre-existing FIXME about the inconsistency
where stackeval wants a raw plan for apply but expects the caller to
have dealt with loading the prior state for planning. Here it's resolved
in the direction of the caller (rpcapi) always being responsible for
loading both artifacts, because that means we can continue supporting the
old inline approach for a while without that complexity having to infect
the lower layers.
Ideally we should remove the legacy approach before this API becomes
constrained by compatibility promises, but I've preserved the old API
for now to give us some flexibility in when we update the existing
clients of this API to use the new approach.
Co-authored-by: Martin Atkins <mart@degeneration.co.uk>
In the very first implementation of "sensitive values" we were
unfortunately not disciplined about separating the idea of "marked value"
from the idea of "sensitive value" (where the latter is a subset of the
former). The first implementation just assumed that any marking whatsoever
meant "sensitive".
We later improved that by adding the marks package and the marks.Sensitive
value to standardize on the representation of "sensitive value" as being
a value marked with _that specific mark_.
However, we did not perform a thorough review of all of the mark-handling
codepaths to make sure they all agreed on that definition. In particular,
the state and plan models were both designed as if they supported arbitrary
marks but then in practice marks other than marks.Sensitive would be
handled in various inconsistent ways: dropped entirely, or interpreted as
if marks.Sensitive, and possibly do so inconsistently when a value is
used only in memory vs. round-tripped through a wire/file format.
The goal of this commit is to resolve those oddities so that there are now
two possible situations:
- General mark handling: some codepaths genuinely handle marks
generically, by transporting them from input value to output value in
a way consistent with how cty itself deals with marks. This is the
ideal case because it means we can add new marks in future and assume
these codepaths will handle them correctly without any further
modifications.
- Sensitive-only mark preservation: the codepaths that interact with our
wire protocols and file formats typically have only specialized support
for sensitive values in particular, and lack support for any other
marks. Those codepaths are now subject to a new rule where they must
return an error if asked to deal with any other mark, so that if we
introduce new marks in future we'll be forced either to define how we'll
avoid those markings reaching the file/wire formats or extend the
file/wire formats to support the new marks.
Some new helper functions in package marks are intended to standardize how
we deal with the "sensitive values only" situations, in the hope that
this will make it easier to keep things consistent as the codebase evolves
in future.
In practice the modules runtime only ever uses marks.Sensitive as a mark
today, so all of these checks are effectively covering "should never
happen" cases. The only other mark Terraform uses is an implementation
detail of "terraform console" and does not interact with any of the
codepaths that only support sensitive values in particular.
Because we treat dependency edges as reversed when a component instance
is being destroyed, the final result (an object representing output values)
for a component instance being destroyed must not depend on anything else
in the evaluation graph, or else we'd cause a promise self-reference as
the downstream component tries to configure itself based on our outputs.
As a special case then, for a component instance being destroyed we take
the planned output values directly from the plan, relying on the fact that
the plan phase sets them to the prior state output values in that case,
and therefore the result for such a component is available immediately
without blocking on any other expression evaluation during the apply phase.
This combines with several previous commits to create a first pass at
handling ordering correctly when planning and applying a full destroy.
This commit also incorporates some fixes and improvements to stackeval's
apply-time testing helpers, which had some quirks and bugs when first
added in a recent commit. One of those problems also revealed that the
raw state loader was not resilient to a buggy caller setting a state
entry to nil instead of removing it altogether, and that mistake seems
relatively easy to make (as I did here in the test helper) so we'll
tolerate it to make it possible to recover if such a bug does end up
occurring in real code too.
In earlier commits we stubbed out some testing utilities as part of adding
some unit tests, but we didn't end up using all of them in the initial
round and it turned out that some of them were incorrect or incomplete.
Now we'll improve the test utilities, this time with a focus mainly on
integration-type tests, because we'll start adding those in the near
future.
Specifically:
- utilities for rendering diagnostics, mainly in situations where they
are unexpected and thus we just want to see the full information as
part of the test results
- helper functions for concisely asserting that there should be no
diagnostics at all, or that there should be no error diagnostics
- the existing helper for faking up a prior state based on hard-coded raw
state messages was incorrect in that it was trying to use statekeys.Key
values as map keys, and those values are not typically comparable.
Instead, we should use the string representations that are designed
to be used as unique map keys.
- the testPlan function, and the lower-level testPlanOutput it's built
with, help to encapsulate the asynchronous event-emitting behavior of
Main.PlanAll into a "normal-looking" function that just blocks until
it's done and then returns all of its results. This automatically
round-trips the planned changes through serialization and
deserialization as we would between a normal plan and apply phase, and
so the calling test can make its assertions against the applyable plan
object instead of the raw component parts it's made from.
Although we can often use the presence of resource instances in a
component instance as an implication of the component instance's existence,
it's possible (albeit rare) for a component instance to have no resources
in it at all, and thus it would be ambiguous whether it exists or not.
Now we'll track the existence of the component instances themselves as
extra objects in both the raw state and the state description. Along with
their existence we'll also track a snapshot of the output values as they
were at the most recent apply, which for now is primarily for external
consumption but we'll also include them in the raw state in case it ends up
being useful for something down the road.
Add additional fields to the AppliedChange#ResourceInstance
We're adding Resource Mode, Resource Type and Provider Address to the AppliedChange's ResourceInstance
The main entry point here assumes the caller is providing serialized state
objects wrapped in anypb.Any messages.
That's an inconvenient representation for hand-writing in tests, so the
new function LoadFromDirectProto allows skipping the deserialization steps
and instead has the caller write the wanted values as if they had already
been parsed/unmarshaled.
This comment was noting that an earlier stub of this function was only
returning an external facing "state description" object and not including
the raw state for use when creating future plans.
That was subsequently fixed -- this now returns both description and raw
forms -- but the comment stuck around.
Previously our strategy for reporting the "applied change" results for
resource instance objects was to iterate over the final state and emit
an object for each resource instance we found in there.
That's not a sufficient solution though, because if any objects are deleted
as part of applying the plan then they won't be in the final state and so
we won't know to tell the caller that they should drop the relevant
objects from the stack-level state.
Instead, we'll keep track of all of the resource instance objects that
could potentially be affected by applying a plan and then use that set to
decide which "applied change" objects to emit. If one of the affected
objects is not present in the final state then we'll assume it was deleted
and so command the caller to delete its records of that object from both
the raw state and the state description.
With this in place, it's now possible to create a destroy-mode plan and
then apply it to end up with no resource instances at all. The stack
runtime itself is still missing handling of destroying its own objects
like components and embedded stacks, so this isn't a complete solution but
it does at least allow properly cleaning up the records of objects that
exist in remote systems, so it's now easier to clean up a development
environment with real infrastructure backing it.
We need to retain the prior state (in a form that Terraform Core's modules
runtime can accept) between the plan and apply phases because Terraform
uses it to verify that the provider's "final plan" is consistent with
the initial plan.
We previously tried to do this by reusing the "old" value from the planned
change, but that's not really possible in practice because of the
historical implementation detail that Terraform wants state information
in a JSON format and plan information in MessagePack.
This also contains the beginnings of handling the "discarding" of
unrecognized keys from the state data structures, although we'll probably
need to do more in that area later since this is just the bare minimum.
Previously we incorrectly assumed that resource instances only have one
"current" object each. However, resource instances can also have deposed
objects which we must also keep track of.
This is a breaking change to the rpcapi since we're now using a new
message type for a resource instance _object_ address in several places.
That breakage is intentional here because at the time of writing this
commit the rpcapi is not yet in any stable release and we want to force
updating our existing internal client to properly handle deposed objects
too.
This gets us ever closer to being able to preserve resource instance
objects from one run to the next. In subsequent commits we'll make use of
the "LoadFromProto" option to load the prior state during the planning
phase and take it into account when we're deciding what actions to
propose.
This is a sketch of the overall structure of the prior state decoder and
the model type it populates.
Before we can complete this we'll need to slightly rework how the apply
phase emits the raw events that this is consuming, and in particular to
change the raw state representation to be JSON-based to match with how
Terraform Core expects to receive it once reloaded. That will follow in
later commits.
Previously we just had a stub that only generated basic external
description objects. Now we'll also emit "raw state" objects for these,
and have some initial support for deposed objects too.
Our model for state for a stack involves a set of objects that is each
identified by an opaque string key. Although those keys are opaque to
callers of Terraform Core, we will actually be using them for some meaning
in Terraform Core itself, since that will avoid redundantly storing the
same information both in the key and in the object associated with the
key.
This therefore aims to encapsulate the generation and parsing of these
keys to help ensure we'll always use them consistently.
Previously we just had this just stubbed out to always return an empty
object because we didn't have the schema information available to
transcode the JSON-encoded state data into the form our RPC API wants to
produce.
An earlier commit added the schema information we need, so we can now
transcode just in time to produce the protocol buffers serialization of
the applied change description.
This is still otherwise just a stub implementation. We'll still need to
deal with emitting the "raw" representation of this state for use in
future plans, and make sure we're properly handling deposed objects, in
future commits.
Unfortunately for historical reasons the "terraform" package produces
state and plan artifacts that have dynamic data elements pre-encoded in
the formats that Terraform CLI's traditional plan and state formats use.
Since Stacks uses different plan and state models, and exclusively uses
MessagePack encoding for dynamic values in both, we'll sometimes need to
transcode from one format to another when generating and decoding the
stacks-oriented representations.
Transcoding between these serialization formats requires access to the
relevant schema because the formats themselves each have an infoset that
is only a subset of Terraform's type system. Therefore we'll now annotate
the PlannedChange and AppliedChange objects that include
provider-specific data types with the schema required to transcode them.
This commit does not yet actually arrange to make use of those schemas.
That will follow in later commits.
We'd like to start developing some clients for this part of the RPC API
concurrently with remaining work here in Terraform Core, and so this is
a bare-minimum implementation of emitting "applied change" events for
resource instances just so there's something for client developers to test
against.
There are various things wrong with this, including but not limited to:
- It always reports that the new state for a resource instance is an
empty object, rather than including the correct updated object.
- It only supports "current" resource instance objects, and ignores
deposed ones.
- It just uses the resource instance address alone as the key for the
state description map, which is fine for now when we only have one
kind of description object anyway but will not be sufficient for a
real implementation that needs to emit various different kinds of
object.
We'll need to rework most of this in future commits, but this is hopefully
sufficient to start implementing and testing API clients, at least to some
extend, despite the crudity of the results.
This is a first pass at actually driving the apply phase through to
completion, using the ChangeExec function to schedule the real change
operations from the plan and then, just like for planning, a visit to
each participating object to ask it to check itself and report the results
of any changes it has made.
Many of our objects don't have any external side-effects of their own and
so just need to check their results are still valid after the apply phase
has replaced unknown values with known values. For those we can mostly
just share the same logic between plan and apply aside from asking for
ApplyPhase instead of PlanPhase.
ComponentInstance and OutputValue are the two objects whose treatment
differs the most between plan and apply, because both of those need to
emit externally-visible "applied change" objects to explain their
side-effects to the caller.
The apply-walk driver has a lot of behavior in common with the existing
plan-walk driver. For this commit we're just accepting that and having
two very similar pieces of code that differ only in some leaf details.
In a future commit we might try to generalize that so we can share more
logic between the two, but only if we can do that without making the code
significantly harder to read.
