We were using the wrong cty operation to access map members, causing a
panic whenever a prior value was present for a resource type with a nested
block backed by a map value.
In an earlier commit we changed objchange.ProposedNewObject so that the
task of populating unknown values for attributes not known during apply
is the responsibility of the provider's PlanResourceChange method, rather
than being handled automatically.
However, we were also using objchange.ProposedNewObject to construct the
placeholder new object for a deferred data resource read, and so we
inadvertently broke that deferral behavior. Here we restore the old
behavior by introducing a new function objchange.PlannedDataResourceObject
which is a specialized version of objchange.ProposedNewObject that
includes the forced behavior of populating unknown values, because the
provider gets no opportunity to customize a deferred read.
TestContext2Plan_createBeforeDestroy_depends_datasource required some
updates here because its implementation of PlanResourceChange was not
handling the insertion of the unknown value for attribute "computed".
The other changes here are just in an attempt to make the flow of this
test more obvious, by clarifying that it is simulating a -refresh=false
run, which effectively forces a deferred read since we skip the eager
read that would normally happen in the refresh step.
Previously we would construct a proposed new state with unknown values in
place of any not-set-in-config computed attributes, trying to save the
provider a little work in specifying that itself.
Unfortunately that turns out to be problematic because it conflates two
concerns: attributes can be explicitly set in configuration to an unknown
value, in which case the final result of that unknown overrides any
default value the provider might normally populate.
In other words, this allows the provider to recognize in the proposed new
state the difference between an Optional+Computed attribute being set to
unknown in the config vs not being set in the config at all.
The provider now has the responsibility to replace these proposed null
values with unknown values during PlanResourceChange if it expects to
select a value during the apply step. It may also populate a known value
if the final result can be predicted at plan time, as is the case for
constant defaults specified in the provider code.
This change comes from a realization that from core's perspective the
helper/schema ideas of zero values, explicit default values, and
customizediff tweaks are all just examples of "defaults", and by allowing
the provider to see during plan whether these attributes are being
explicitly set in configuration and thus decide whether the default will
be provided immediately during plan or deferred until apply.
ProposedNewObject intentionally replaces a null prior with an unknown
prior in order to easily fill in unknown values where they "show through"
under values not set explicitly in config, but it was failing to handle
that situation when dealing with nested blocks that are backed by sets.
incoming values
Addresses an odd state where the priorV of an object to be changed is
known but null.
While this situation should not happen, it seemed prudent to ensure that
core is resilient to providers sending incorrect values (which might
also occur with manually edited state).
We need to make the collection itself be a tuple or object rather than
list or map in this case, since otherwise all of the elements of the
collection are constrained to be of the same type and that isn't the
intent of a provider indicating that it accepts any type.
This produces a "proposed new state", which already has prior computed
values propagated into it (since that behavior is standard for all
resource types) but could be customized further by the provider to make
the "_planned_ new state".
In the process of implementing this it became clear that our configschema
DecoderSpec behavior is incorrect, since it's producing list values for
NestingList and map values for NestingMap. While that seems like it should
be right, we should actually be using tuple and object types respectively
to allow each block to have a different runtime type in situations where
an attribute is given the type cty.DynamicPseudoType. That's not fixed
here, and so without a further fix list and map blocks will panic here.
The DecoderSpec implementation will be fixed in a subsequent commit.
Martin Atkins