110 Commits (paddy_module_attribution_tmp)

Author	SHA1	Message	Date
Paddy Carver	7d14463660	Update in response to review. ... So thanks to review, we have a few changes all rolled up in one here, to account for a design change. First, the codebase has marched on while we wrote this, meaning we need to refer to providers as addrs.Provider, not as strings. This is for the best, but is a lot of replacing through the codebase. Second, some methods were missing from the provider-side GRPC implementations, so we added those. Third, it turns out that attaching the meta configs to resources in transform_provider wouldn't actually work, because resources aren't guaranteed to be in the same module as the provider config powering them. So we need to attach them in transform_attach_config_resource, not in transform_provider, so they'll be on the right side of the module boundary. Unfortunately, this means we're going to have to stop passing around the provider's provider_meta specifically, and are going to have to pass the entire block, because we don't have the provider data we need when we attach the config to filter out just what we need. We now separate them out in the Eval methods. This, unfortunately, also causes us issues, because we don't have that info already when we call EvalValidate, which means we're either going to need to pipe new info through to it, or we're not going to be able to validate the schema pre-plan-time. There's still a recommendation for an end-to-end test to do, but I'm hopeful.	6 years ago
Paddy Carver	300b054f12	OK but run the tests this time. ... Fix some panics because I didn't run _all_ the tess, like a dolt.	6 years ago
Paddy Carver	f3a950de5b	Make ProviderMeta available in the SDK.	6 years ago
Paddy Carver	546b58befc	First implementation of module attribution. ... A pass at implementing module attribution ("provider_meta blocks in the terraform block", whatever, it's for module attribution). The general idea is that we're adding a new ProviderMeta struct to our configs package, which is then added to Files, Modules, and parsed as part of the terraform block. The configs.Config type then makes that available to the terraform.ProviderTransformer type, which is already scoped by module. It picks out the ProviderMeta for the provider we need in its Transform method, and checks to see if the node it's working on implements the terraform.GraphNodeAttachProviderMetaConfigs interface, which is a new interface we're adding. It has only one method: AttachProviderMetaConfigs, which is passed a *configs.ProviderMeta. We implement this on the terraform.NodeAbstractResource type, storing the passed *configs.ProviderMeta in a new property on that struct. That struct is eventually made available to the terraform.NodeApplyableResourceInstance type, and its method evalTreeManagedResources, which creates an EvalApply type. The EvalApply type gets a ProviderMeta type, which is set to the *configs.ProviderMeta obtained from the terraform.NodeAbstractResource available to it. This means that EvalApply now has the raw config values the user entered for the provider_meta block, but we also need a schema; providers can define their own schema for the provider_meta block, and we need that schema to know how to translate what is in the config into a cty.Value, which is what we eventually want here. Fortunately, EvalApply already has access to the provider's schema; we can augment the GetSchema RPC call to pass back the ProviderMeta schema (added to helper/schema.Provider) in addition to resource, data source, and provider schemas. This makes it available, eventually, to EvalApply. Now that EvalApply has both the config values and the schema, we can parse them into a cty.Value. Finally, we augment the ApplyResourceChange RPC call with a ProviderMeta field, which can be populated by EvalApply with the parsed cty.Value, which successfully makes the config values available to the provider. This still needs tests written for it to ensure it works, but no current tests are broken on it and it compiles, which is reassuring.	6 years ago
James Bardin	49439d02d1	sanitize provisioner output strings ... The grpc protocol requires strings to be valid utf8, but because provisioners often don't have control over the command output, invalid utf8 sequences can make it into the response causing grpc transport errors. Replace all invalid utf sequences with the standard utf replacement character in the provisioner output. The code is a direct copy from the go1.13 std library, and can be replaced with strings.ToValidUTF8 once it's available.	6 years ago
Alex Pilon	4bf43efcfd	move hcl2shim package to configs	7 years ago
James Bardin	75602df5ef	Revert "Remove removed attribute from applied state" ... This reverts commit 2e2a363052.	7 years ago
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	f362198467	Merge pull request #21806 from hashicorp/jbardin/lost-empty-strings ... prevent an empty string from being lost	7 years ago
James Bardin	9365a2d97d	private and timeout handling in grpc_provider ... Load private data for read, so the resource can get it's configured timeouts if they exist. Ensure PlanResourceChange returns the saved private data when there is an empty diff. Handle the timeout decoding into Meta in the PlanResourceChange, so that it's always there for later operations.	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	8ae31aa2db	normalize empty blocks during import ... Like Upgrade, Import is another case where we don't have the context of the configuration and need to ensure missing blocks are normalized.	7 years ago
James Bardin	e6ee78555a	Merge pull request #21721 from hashicorp/jbardin/remove-new-removed ... Remove removed attribute from applied state	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
James Bardin	2e2a363052	Remove removed attribute from applied state ... When a Diff contains a NewRemoved attribute (which would have been null in the planned state), the final value is often the "zero" value string for the type, which the provider itself still applies to the state. Rather than risking a change of behavior in helper/schema by fixing the inconsistency, we'll remove the NewRemoved attributes after apply to prevent further issues resulting from the change in planned value.	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	a056b84cdd	add delete timeout test ... The timeout value is still not being persisted. While it doesn't fix this issue, make sure to always return Private during Plan.	7 years ago
James Bardin	dcab82e897	send and receive Private through ReadResource ... Send Private data blob through ReadResource as well. This will allow for extra flexibility for future providers that may want to pass data out of band through to their resource Read functions.	7 years ago
James Bardin	c9e1d26c25	remove the legacy schema access ... Having removed the methods, it is straightforward to mechanically update this file to get rid of all references to the "legacy schema". There is now only one config schema type to deal with in the sdk.	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
Martin Atkins	88e76fa9ef	configs/configschema: Introduce the NestingGroup mode for blocks ... In study of existing providers we've found a pattern we werent previously accounting for of using a nested block type to represent a group of arguments that relate to a particular feature that is always enabled but where it improves configuration readability to group all of its settings together in a nested block. The existing NestingSingle was not a good fit for this because it is designed under the assumption that the presence or absence of the block has some significance in enabling or disabling the relevant feature, and so for these always-active cases we'd generate a misleading plan where the settings for the feature appear totally absent, rather than showing the default values that will be selected. NestingGroup is, therefore, a slight variation of NestingSingle where presence vs. absence of the block is not distinguishable (it's never null) and instead its contents are treated as unset when the block is absent. This then in turn causes any default values associated with the nested arguments to be honored and displayed in the plan whenever the block is not explicitly configured. The current SDK cannot activate this mode, but that's okay because its "legacy type system" opt-out flag allows it to force a block to be processed in this way anyway. We're adding this now so that we can introduce the feature in a future SDK without causing a breaking change to the protocol, since the set of possible block nesting modes is not extensible.	7 years ago
James Bardin	af8115dc9b	removing the ~ set flag is no longer needed ... The computed set sigil ~ should no longer appear in the diffs, because the config will be cleaned before generating the diff.	7 years ago
James Bardin	a3d58665ad	use LegacyResourceSchema ... rather than the previous .CoreConfigSchemaForShimming	7 years ago
James Bardin	c5023c7702	cleanup after AsSingle removal	7 years ago
James Bardin	1a9c06d0f5	Revert "helper/schema: Implementation of the AsSingle mechanism" ... This reverts commit 1987a92386.	7 years ago
James Bardin	7b67105407	don't strip new-computeds from plan diffs ... Stripping these was a patch for some provider behavior which was fixed in other ways, and is no longer needed. Removing this allows us to implement correct CusomizeDiffFuncs in providers so that they can mark fields with empty values as computed during a plan.	7 years ago
James Bardin	e024960c74	Revert "filter nulls when shimming a config" ... This reverts commit 97bde5467c.	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	009df443f7	restore lost unknowns during a planned update. ... Because schema.ResourceDiff can't differentiate between unknown values and new computed values, unknowns can be lost during an update. If a planned value converted an unknown to a null, restore the unknown so that it can be correctly replaced in the final plan.	7 years ago
Martin Atkins	135121562e	helper/plugin: Implement Schema.SkipCoreTypeCheck ... The previous commit added this flag but did not implement it. Here we implement it by adjusting the shape of schema we return to Terraform Core to mark the attribute as untyped and then ensure that gets handled correctly on the SDK side.	7 years ago
Martin Atkins	1987a92386	helper/schema: Implementation of the AsSingle mechanism ... The previous commit added a new flag to schema.Schema which is documented to make a list with MaxItems: 1 be presented to Terraform Core as a single value instead, giving a way to switch to non-list nested resources without it being a breaking change for Terraform v0.11 users as long as it's done prior to a provider's first v0.12-compatible release. This is the implementation of that mechanism. It's intentionally implemented as a suite of extra fixups rather than direct modifications to existing shim code because we want to ensure that this has no effect whatsoever on the result of a resource type that _isn't_ using AsSingle. Although there is some small unit test coverage of the fixup steps here, the primary testing for this is in the test provider since the integration of all of these fixup steps in the correct order is the more important result than any of the intermediate fixup steps.	7 years ago
James Bardin	e080706e2e	treat normalization during ReadResource like Plan ... This will allow resources to return an unexpected change to set blocks and attributes, otherwise we could mask these changes during normalization. Change the "plan" argument in normalizeNullValues to "preferDst" to more accurately describe what the option is doing, since it no longer applies only to PlanResourceChange.	7 years ago
James Bardin	6ecf9b143b	we can normalize nulls in Read again ... This should be the final change from removing the flatmap normalization. Since we're no longer trying to a consistent zero or null value in the flatmap config, rather we're trying to maintain the previously applied value, ReadResource also needs to apply the normalizeNullValues step in order to prevent unexpected diffs.	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	6cdf9ff566	Revert "normalize all objects read from the provider" ... This reverts commit 209a0a460a.	7 years ago
James Bardin	209a0a460a	normalize all objects read from the provider ... Use objchange.NormalizeObjectFromLegacySDK to ensure that all objects returned from the provider match what is expected based on the configuration according to the schemas.	7 years ago
James Bardin	3600f59bb7	Merge pull request #20525 from hashicorp/jbardin/extra-set-value ... remove the partially-known ~ set sigil in diffs	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	47604c36c8	remove the partially-known ~ set sigil in diffs ... The NewExtra values are stored outside the diff from plan, and the original keys may not contain the ~ prefix. Adding the NewExtra back into the diff with the mismatched key was causing an entire new set element to be populated. Since this symbol isn't used to apply the diff in helper/schema, we can simply strip them out.	7 years ago
James Bardin	33d5ddf291	remove empty timeouts blocks in copyTimeoutValues ... The hcl2shims will always add in the timeouts block, because there's no way to differentiate a null single block from an empty one in the flatmapped state. Since we are only concerned with keeping the prior timeouts value, always set the new value to null, and then copy over the prior value if it exists.	7 years ago
James Bardin	49230f8198	existing fields cannot become computed during plan ... Fields with no change can only become computed during initial creation.	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
James Bardin	f9b62cb5fe	Merge pull request #20335 from hashicorp/jbardin/diff-apply ... Diff apply needs to check for both types of containers keys	7 years ago