If a set element is nil in validateConfigNulls, we don't want to
append that element to the diagnostic path, since it doesn't offer any
useful info to the user.
The helper/schema diff process loses empty strings, causing them to show
up as unset (null) during apply. Besides failing to show as set by
GetOk, the absence of the value also triggers the schema to insert a
default value again during apply.
It would also be be preferable if the defaults weren't re-evaluated
again during ApplyResourceChange, but that would require a more invasive
patch to the field readers, and ensuring the empty string is stored in
the plan should block the default.
When upgrading from a flatmap state, unset blocks would not exist in the
state, while they will represented as empty in the new cty.Value. This
will cause an unexpected diff in the first plan after upgrade. This
situation may normally be applied with no impact, but some providers may
have unexpected behavior, and if the attributes force replacement it may
require manual alteration of the state to complete the upgrade.
Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/8828
Prior to Terraform 0.12, providers were not required to implement the `MigrateState` function when increasing the `SchemaVersion` above `0`. Effectively there is no flatmap state difference between version 0 and defined `SchemaVersion` or lowest `StateUpgrader` `Version` (whichever is lowest) when `MigrateState` is undefined, so here we remove the error and increase the schema version automatically.
When handling the json state in UpgradeResourceState, the schema
must be what core uses, because that is the schema used for
encoding/decoding the json state.
When converting from flatmap to json state, the legacy schema will be
used to decode the flatmap to a cty value, but the resulting json will
be encoded using the CoreConfigSchema to match what core expects.
When normalizing the state during read, if the resource was previously
imported, most nil-able values will be nil, and we need to prefer the
values returned by the latest Read operation. This didn't come up
before, because Read is usually working with a state create by plan and
Apply which has already shaped the state with the expected empty values.
Having the src value preferred only during Apply better follows the
intent of this function, which should allow Read to return whatever
values it deems necessary. Since Read and Plan use the same
normalization logic, the implied Read before plan should take care of any
perpetual diffs.
Terraform core would previously ignore unexpected attributes found in
the state, but since we now need to encode/decode the state according
the schema, the attributes must match the schema.
On any state upgrade, remove attributes no longer present in the schema
from the state. The only change this requires from providers is that
going forward removal of attribute is considered a schema change, and
requires an increment of the SchemaVersion in order to trigger the
removal of the attributes from state.
Make sure values removed from a map during apply are not copied into the
new map. The broken test is no longer valid in this case, and the
updated diff.Apply should prevent the case it used to cover.
A list-like attribute containing null values will present a list to
helper/schema with nils, which can cause panics. Since null values were
not possible in configuration before HCL2 and not supported by the
legacy SDK, return an error to the user.
It turns out that collections containing only unknowns could be lost,
meaning there wasn't a direct correlation between the unknown and null
value which would have otherwise been restored.
The legacy diff process inserts unknown values into an optional+computed
map. Fix these up in post-plan normalization process, by looking for
known strings that were changed to unknown.
This method was added early on when the diff was being applied as the
legacy code would have done, which is no longer the case. Everything
that normalizeFlatmapContainers does should be covered by the
combination of the initial diff.Apply and the normalizeNullValues on the
final cty.Value.
As we've improved the cty.Value normalization, we need to remove
normalization procedures from the flatmap handling. Keeping the empty
containers in the flatmap will prevent unexpected nils from being added
to some schema configurations
Providers were not strict (and were not forced to be) about customizing
the diff when a computed attribute needed to be updated during apply.
The fix we have in place to prevent loss of information during the
helper/schema apply process would add in single missing value back in.
The first place this was caught was when we attempt to fix up the
flatmapped attributes. The 1->0 count error is now better handled by our
cty.Value normalization step, so we can remove the special apply case
here altogether
The next place is in normalizeNullValues, and since the intent was to
re-insert missing zero-value lists and sets, adding a check for a length
of 0 protects us from adding in extra elements.
The new test fixture emulated common provider behavior of re-computing
values without customizing the diff. Since we can work around it, and
core will provider appropriate warnings, the shims should try to
maintain the legacy behavior.
The new normalization should make preventing those changes unnecessary,
and will also prevent extra empty elements from being added when
resources are refreshed.
cty.Value.AsValueMap can return nil if called on an empty map or object.
The logic above was dealing with that case for maps, but object types
were falling through into this codepath and panicking when trying to
assign a new key into the nil dstMap.
This also includes a bonus fix where we were calling ty.ElementType in
a switch case that accepts object types. Object types don't have a single
element type, so we can't call ElementType on those (that also panics)
but we _can_ use the type of the value we selected from src to construct
our placeholder null value.
While copyMissingValues was meant to re-insert empty values that were
null after apply, it turns out plan is sometimes not predictable as
well.
normalizeNullValue is meant to fix up any null/empty transitions between
to values, and be useful during plan as well. For plan the function only
concerns itself with individual, known values, and skips sets entirely.
The result of running with plan == true is that only changes between
empty and null collections should be fixed.
This adds unexpected values in some cases, and since the case this
handles is only within set objects, we'll deal woth this when tackling
the sets themselves.
Zero values and empty containers can be lost during the shimming
process, and during the provider's Apply step.
If we have known zero value containers and primitives in the source,
which appear as null values in the destination, we copy over the zero
value. Sets (and lists to an extent) are more difficult, since there
before and after indexes may not correlate. In that case we take the
entire container if it's wholly known, expecting the provider to have
correctly handled the value.
Errors were being ignore with the intention that they would be caught
later in validation, but it turns out we nee dto catch those earlier.
The legacy schemas also allowed providers to set and empty string for a
bool value, which we need to handle here, since it's not being handled
from user input like a normal config value.
When normalizing flatmapped containers, compare the attributes to the
prior state and preserve pre-existing zero-length or unknown values. A
zero-length value that was previously unknown is preserved as a
zero-length value, as that may have been computed as such by the
provider.
The main significant change here is that the package name for the proto
definition is "tfplugin5", which is important because this name is part
of the wire protocol for references to types defined in our package.
Along with that, we also move the generated package into "internal" to
make it explicit that importing the generated Go package from elsewhere is
not the right approach for externally-implemented SDKs, which should
instead vendor the proto definition they are using and generate their
own stubs to ensure that the wire protocol is the only hard dependency
between Terraform Core and plugins.
After this is merged, any provider binaries built against our
helper/schema package will need to be rebuilt so that they use the new
"tfplugin5" package name instead of "proto".
In a future commit we will include more elaborate and organized
documentation on how an external codebase might make use of our RPC
interface definition to implement an SDK, but the primary concern here
is to ensure we have the right wire package name before release.
In order to prevent mismatched states between read/plan/apply, we need
to ensure that the attributes are generated consistently each time.
Because of the various ways in which helper/schema and the hcl2 shims
interpret empty values, the only way to ensure consistency is to always
remove them altogether.
Resource timeouts were a separate config block, but did not exist in the
resource schema. Insert any defined timeouts when generating the
configshema.Block so that the fields can be accepted and validated by
core.
While the schema Diff fucntion returns a nil diff when creating an empty
(except for id) resource, the Apply function expects the diff to be
initialized and ampty.
PlanResourceChange isn't returning the diff, but rather it is returning
the destired state. If the propsed state results in a nil diff, then,
the propsed state is what should be returned.
Make sure Meta fields are not nil, as the schema package expects those
to be initialised.
The new helper/plugin package contains the grpc servers for handling the
new plugin protocol
The GRPCProviderServer and GRPCProvisionerServer handle the grpc plugin
protocol, and convert the requests to the legacy schema.Provider and
schema.Provisioner methods.
James Bardin