RequiresReplace paths with IndexSteps that have been added or removed
may fail to apply against one of the two state values. Only error out if
the path cannot be applied to both values.
Now that we're actually verifying correct behavior of providers during
plan and apply, our mock providers need to behave like real providers,
properly propagating any configured values through the plan and into the
final state.
For most of these it was simpler to just switch over to using the newer
PlanResourceChangeFn mock interface, away from the legacy DiffFn approach,
because then we can just return the ProposedNewState verbatim because our
schema for these tests does not require any default values to be
populated.
Prior to Terraform 0.12 there were certain behaviors we expected from
providers that were actually just details of the SDK and not part of the
enforced contract.
For 0.12 we're now codifying some of these behaviors explicitly via safety
checks in core, thus ensuring that all future providers will behave in a
consistent way that users can rely on.
Unfortunately, due to the hand-written nature of the mock provider
implementations we use in tests, they have been getting away with some
unusual behaviors that don't match our usual expectations, and our safety
checks now detect those as incorrect behaviors.
To address this, we make the minimal changes to each test to ensure that
its mock provider behaves in a consistent way, which requires that values
set in config be represented correctly in the plan and ultimately saved
in the new state, without any changes along the way. In particular, the
common testDiffFn implementation has historically used a number of special
hidden attributes to trigger special behaviors, and our new rules require
that these special settings propagate properly through the plan and into
the state.
Since these error messages get printed in Terraform's output and we
encourage users to share them as part of bug reports, we should avoid
including sensitive information in them to reduce the risk of accidental
exposure.
Due to the inprecision of our shimming from the legacy SDK type system to
the new Terraform Core type system, the legacy SDK produces a number of
inconsistencies that produce only minor quirky behavior or broken
edge-cases. To retain compatibility with those existing weird behaviors,
the legacy SDK opts out of our safety checks.
The intent here is to allow existing providers to continue to do their
previous unsafe behaviors for now, accepting that this will allow certain
quirky bugs from previous releases to persist, and then gradually migrate
away from the legacy SDK and remove this opt-out on a per-resource basis
over time.
As with the apply-time safety check opt-out, this is reserved only for
the legacy SDK and must not be used in any new SDK implementations. We
still include any inconsistencies as warnings in the logs as an aid to
anyone debugging weird behavior, so that they can see situations where
blame may be misplaced in the user-visible error messages.
We've allowed the legacy SDK an opt-out from the post-apply safety checks,
but previously we produced only a generic warning message in that case.
Now instead we'll still run the safety checks, but report the results in
the logs instead of as error diagnostics.
This should allow developers who are debugging strange interactions
between buggy legacy providers to get better insight into what's going
on upstream in order to help explain what's going on when these problems
inevitably get caught by other downstream safety checks when trying to
make use of these invalid results.
We've been gradually adding safety checks of this sort throughout the
lifecycle to help ensure that buggy providers can't introduce
hard-to-diagnose downstream failures and misbehavior. This completes the
set by verifying during plan time that the provider has produced a plan
that actually achieves the goals defined in the configuration.
In particular, this catches the situation where a provider may incorrectly
override a value explicitly set in configuration, which avoids creating
confusion by betraying the reasonable user expectation that referencing an
explicitly-defined attribute will produce exactly the value shown in
configuration.
The helper/schema handling of lists loses empty string values, but
retains the correct count. Only re-count the values if the count is
missing entirely, and allow our shims to re-populate the zero values.
* command/jsonplan:
- add variables to plan output
- print known planned values for resources
Previously, resource attribute values were only displayed if the values
were wholly known. Now we will filter the unknown values out of the
change and print the known values.
* command/jsonstate: added depends_on and tainted fields
* command/show: update tests to reflect added fields
Terraform core expects a sane state even when the provider returns an
error. Make sure at the prior state is always the default value to
return, and then alway attempt to process any state returned by
provider.Apply.
This was changed in the single attribute test cases, but the AttrPair
test is used a lot for data source. As far as tests are concerned, 0 and
unset should be treated equally for flatmapped collections.
Completing our set of provider result safety-check functions,
AssertPlanValid checks a result from a provider's PlanResourceChange to
make sure it doesn't propose a change that is not valid within the user
model of Terraform.
Specifically, it forbids the provider from planning a value that
contradicts what the user gave in configuration, which is important to
ensure that making a reference to an attribute elsewhere in the
configuration will produce exactly the given result, as users reasonably
expect.
Providers _are_ allowed (and, in fact, required) to make changes to
any Computed attribute values declared in the schema in order to fill in
the default values that the provider has generated. Later checks during
the apply phase will ensure that the provider remains true to these
planned values, to ensure that Terraform can keep its promise of doing
exactly what was planned or presenting an error explaining why not.
Previously, configupgrade would panic if it encountered a HEREDOC. For
the time being, we will simply print out the HEREDOC as-is.
Unfortunately, we discovered that terraform 0.11's version of HCL
allowed for HEREDOCs with the termination delimiter inline (instead of
on a newline, which is technically correct). Since 0.12configupgrade
needs to be bug-compatible with terraform 0.11, we must roll back to the
same version of HCL used in terraform 0.11.
This changes the contract for `PlanResourceChange` so that the provider is now responsible
for populating all default values during plan, including inserting any unknown values for
defaults it will fill in at apply time.
Objects with DynamicPseudoType attributes can't be coerced within a map
if a concrete type is set. Change the Value type used to an Object when
there is a type mismatch.
We've changed the contract for PlanResourceChange to now require the
provider to populate any default values (including unknowns) it wants to
set for computed arguments, so our mock provider here now needs to be a
little more complex to deal with that.
This fixes several of the tests in this package. A minor change to
TestLocal_applyEmptyDirDestroy was required to make it properly configure
the mock provider so PlanResourceChange can access the schema.
We now require a provider to populate all of its defaults -- including
unknown value placeholders -- during PlanResourceChange. That means the
mock provider for testing "terraform show -json" must now manage the
population of the computed "id" attribute during plan.
To make this logic a little easier, we also change the ApplyResourceChange
implementation to fill in a non-null id, since that makes it easier for
the mock PlanResourceChange to recognize when it needs to populate that
default value during an update.
Check attributes on null objects, and fill in unknowns. If we're
evaluating the object, it either means we are at the top level, or a
NestingSingle block was present, and in either case we need to treat the
attributes as null rather than the entire object.
Switch on the block types rather than Nesting, so we don't need add any
logic to change between List/Tuple or Map/Object when DynamicPseudoType
is involved.
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.
Now that ProposedNewState uses null to represent Computed attributes not
set in the configuration, the provider must fill in the unknown value for
"computed" in its plan result.
It seems that this test was incorrectly updated during our bulk-fix after
integrating the HCL2 work, but it didn't really matter because the
ReadDataSource function isn't called in the happy path anyway. But to
make the intent clearer here, we also now make ReadDataSource return an
error if it is called, making it explicit that no call is expected.
Data resources do not have a plan/apply distinction, so it is never valid
for a data resource to produce unknown values in its result object.
Unknown values in the data resource _config_ cause us to postpone the read
altogether, so a data source never receives unknown values as input and
therefore may never produce unknown values as output.
James Bardin