33 Commits (5e2b11657e82b7a7e239ff695c68795d883a4b4e)

Author	SHA1	Message	Date
James Bardin	d4b0788854	don't append nil elements to a diagnostic cty.Path ... 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.	7 years ago
James Bardin	4dcda04d96	prevent an empty string from being lost ... 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.	7 years ago
James Bardin	dbe22181ae	Ensure all object attrs & empty blocks in upgrade ... 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.	7 years ago
Brian Flad	94078f9029	helper/plugin: Allow missing MigrateState for provider flatmap state upgrades ... 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.	7 years ago
James Bardin	ec65fb960d	sdk: use core schema for json state upgrade ... 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.	7 years ago
James Bardin	cf61a689eb	only hold back empty container changes in apply ... 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.	7 years ago
James Bardin	b7ff04f1b6	remove extra attributes from state during upgrade ... 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.	7 years ago
James Bardin	2fe0f9376a	change "preferDst" to "apply" in normalizeNullVals ... Inverting this and renaming it makes it align with the current use of that boolean argument.	7 years ago
James Bardin	0696cf7245	don't retain removed map values ... 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.	7 years ago
James Bardin	f5395bd98a	validate null values in shimmed configs ... 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.	7 years ago
James Bardin	64c76be804	fixup lost collections containing unknowns ... 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.	7 years ago
James Bardin	86e30add98	fix unknowns added to maps by schemaMap ... 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.	7 years ago
James Bardin	11ec3a420e	remove normalizeFlatmapContainers ... 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.	7 years ago
James Bardin	9d4bb6ec14	stop removing empty flatmap containers ... 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	7 years ago
James Bardin	2b4d030a69	don't re-add removed list values even when planned ... 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.	7 years ago
James Bardin	9a39af5047	1->0 set changes non longer should happen in Read ... The new normalization should make preventing those changes unnecessary, and will also prevent extra empty elements from being added when resources are refreshed.	7 years ago
Martin Atkins	fedbd6c3b8	helper/plugin: fix panic with empty objects in normalizeNullValues ... 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.	7 years ago
James Bardin	ba081f5de4	change copyMissingValues to normalizeNullValues ... 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.	7 years ago
James Bardin	4f691c5988	don't replace null strings with empty strings ... 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.	7 years ago
James Bardin	b55ec74c27	add copyMissingValues for normalizing shimmed Vals ... 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.	7 years ago
James Bardin	8300d65539	don't strip sets with count 1 when normalizing ... normalizeFlatmapContainers should retain sets with a count of 1, and convert sets with a count of 0 if they were 1 before the Apply step.	7 years ago
James Bardin	5f9b189fcf	catch conversion errors in PrepareProviderConfig ... 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.	8 years ago
James Bardin	6f4d86094f	preserve possible zero values when normalizing ... 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.	8 years ago
James Bardin	f375691819	add missing key-value from test	8 years ago
Martin Atkins	4fe9632f09	plugin: Establish our current plugin protocol as version 5 ... 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.	8 years ago
James Bardin	e95f2b586e	another test case in helper/plugin	8 years ago
James Bardin	17ecda53b5	strip empty containers from flatmap attributes ... 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.	8 years ago
James Bardin	6dad121e70	insert resource timeouts into the config schema ... 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.	8 years ago
James Bardin	a8f75bc554	don't set defaults for deprecated or removed ... These may still have defaults set, even if they are not intended to be used.	8 years ago
James Bardin	e077c9ce95	Insert default values into provider config ... Add any top-level default attributes from the provider schema into Null config values.	8 years ago
James Bardin	caf74a218d	initialize empty diff in apply ... 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.	8 years ago
James Bardin	0b8c38207a	a plan with no diff should return proposed state ... 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.	8 years ago
James Bardin	63dcdbe948	helper/plugin package for grpc servers ... 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.	8 years ago