Commit Graph

1060 Commits

Author SHA1 Message Date
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.
2019-05-14 18:12:57 -04:00
James Bardin c8a2f3840b remove SkipCoreTypeCheck
This experiment is no longer needed for handling computed blocks, since
the legacy SDK can't reasonably handle Dynamic types, we need to remove
this before the final release.

Remove LegacySchema functions as well, since handling SkipCoreTypeCheck
was the only thing left they were handling.
2019-05-14 18:05:30 -04:00
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.
2019-05-14 17:59:45 -04:00
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.
2019-05-13 19:04:25 -04:00
Radek Simko 8a6d1d62b6
stringer: Regenerate files with latest version 2019-05-13 15:34:27 +01:00
James Bardin d495fb5b35
Merge pull request #21274 from hashicorp/jbardin/validate-integers
validate integers when using protoV5
2019-05-11 09:50:36 -04:00
James Bardin 6bc36d3321 validate integers when using protoV5
The new type system only has a Number type, but helper schema
differentiates between Int and Float values. Verify that a new config
value is an integer during Validate, because the existing WeakDecode
validation will decode a float value into an integer while the config
FieldReader will attempt to parse the float exactly.

Since we're limiting this to protoV5, we can be certain that any valid
config value will be converted to an `int` type by the shims. The only
case where an integral float value will appear is if the integer is out
of range for the systems `int` type, but we also need to prevent that
anyway since it would fail to read in the same manner.
2019-05-11 09:34:28 -04:00
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.
2019-05-10 18:05:41 -04:00
James Bardin 02781206ff
Merge pull request #21223 from hashicorp/jbardin/unknown-computed
restrict the ComputedKeys usage to containers
2019-05-07 08:36:47 -04:00
Brian Flad 00a76d019a
Merge pull request #18614 from craigatgoogle/float-validation
Added FloatBetween validation function.
2019-05-06 17:56:56 -07:00
James Bardin 7075bc9a4d restrict the ComputedKeys usage to containers
Computed primitive values must see the UnknownConfigValue or they are
assumed to be unchanged. Restrict the usage of the protov5 ComputedKeys
to containers.
2019-05-06 19:19:10 -04:00
James Bardin 133d3d7971 check for computed values in the config
First check the ComputedValues field in the config when reading config
field, so that we can detect if there is an unknown value in a
container. Since maps, lists and sets are verified to exist by looking
for a "length" first, an unknown config value in the config is ignored.
2019-05-02 14:08:40 -07:00
James Bardin 9a2a6d14bd Stop grpc server when running ACC tests
The grpc server does not shutdown when the listener is closed. Since
tests aren't run through go-plugin, which has a separate RPC Shutdown
channel to stop the server, we need to track and stop the server
directly.
2019-04-29 14:14:04 -04:00
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.
2019-04-23 12:59:30 -04:00
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.
2019-04-23 12:49:58 -04:00
James Bardin 67395306e1 delete unknown values from apply config altogether
removeConfigUnknowns need to remove the value completely from the config
map. Removing this value allows GetOk and GetOkExists to indicate if the
value was set in the config in the case of an Optional+Computed
attribute.
2019-04-22 18:06:26 -04:00
Alex Somesan 275e0a53f2
Merge pull request #20851 from hashicorp/doc-validatefunc-maps
Document that ValidateFunc works on maps.
2019-04-17 20:45:15 +02:00
Alex Somesan 01a169c66f make fmt 2019-04-17 19:58:06 +02:00
Alex Somesan 0395c3ac58
Re-phrase docs to enumerate types which support ValidateFunc 2019-04-17 18:58:58 +02:00
Martin Atkins 861a2ebf26 helper/schema: Use a more targeted shim for nested set diff applying
We previously attempted to make the special diff apply behavior for nested
sets of objects work with attribute mode by totally discarding attribute
mode for all shims.

In practice, that is too broad a solution: there are lots of other shimming
behaviors that we _don't_ want when attribute mode is enabled. In
particular, we need to make sure that the difference between null and
empty can be seen in configuration.

As a compromise then, we will give all of the shims access to the real
ConfigMode and then do a more specialized fixup within the diff-apply
logic: we'll construct a synthetic nested block schema and then use that
to run our existing logic to deal with nested sets of objects, while
using the previous behavior in all other cases.

In effect, this means that the special new behavior only applies when the
provider uses the opt-in ConfigMode setting on a particular attribute,
and thus this change has much less risk of causing broad, unintended
regressions elsewhere.
2019-04-17 07:47:31 -07:00
Martin Atkins bd1a215580 helper/resource: Ignore Removed attributes for ImportStateVerify
Due to the lossiness of our legacy models for diff and state, shimming a
diff and then creating a state from it produces a different result than
shimming a state directly. That means that ImportStateVerify no longer
works as expected if there are any Computed attributes in the schema where
d.Set isn't called during Read.

Fixing that for every case would require some risky changes to the shim
behavior, so we're instead going to ask provider developers to address it
by adding `d.Set` calls where needed, since that is the contract for
"Computed" anyway -- a default value should be produced during Create, and
thus by extension during Import.

However, since a common situation where this occurs is attributes marked
as "Removed", since all of the code that deals with them has generally
been deleted, we'll avoid problems in that case here by treating Removed
attributes as ignored for the purposes of ImportStateVerify.

This required exporting some functionality that was formerly unexported
in helper/schema, but it's a relatively harmless schema introspection
function so shouldn't be a big deal to export it.
2019-04-16 11:14:49 -07:00
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.
2019-04-10 14:53:52 -07:00
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.
2019-04-10 09:39:45 -04:00
James Bardin 5f52aba3ae Remove unknown value strings from apply diffs
The synthetic config value used to create the Apply diff should contain
no unknown config values. Any remaining UnknownConfigValues were due to
that being used as a placeholder for values yet to be computed, and
these should be marked NewComputed in the diff.
2019-04-10 09:34:39 -04:00
James Bardin 3ec93710fc PromoteSingle is used in 0.11 mode 2019-04-08 17:12:39 -04:00
James Bardin a3d58665ad use LegacyResourceSchema
rather than the previous .CoreConfigSchemaForShimming
2019-04-08 16:45:35 -04:00
James Bardin 8730d99309 LegacyResourceSchema to remove 0.12 features
This allows us to call CoreConfigSchema and return something that looks
like the original schema.
2019-04-08 16:45:35 -04:00
James Bardin c5023c7702 cleanup after AsSingle removal 2019-04-08 16:45:35 -04:00
James Bardin 1a9c06d0f5 Revert "helper/schema: Implementation of the AsSingle mechanism"
This reverts commit 1987a92386.
2019-04-08 16:45:35 -04:00
James Bardin 4dbe6add77 Revert "helper/schema: Schema.AsSingle flag"
This reverts commit 4c0c74571de9c96ad2902ccf4af962ec495cd5d4.
2019-04-08 16:45:35 -04:00
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.
2019-04-03 17:37:58 -04:00
James Bardin 51ddc554f5
Merge pull request #20909 from hashicorp/jbardin/validate-config-nulls
Validate configuration shims for nulls in lists
2019-04-03 14:06:22 -04:00
James Bardin e024960c74 Revert "filter nulls when shimming a config"
This reverts commit 97bde5467c.
2019-04-03 13:52:56 -04:00
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.
2019-04-03 11:10:24 -04:00
Brian Flad a8e3787afc
helper/schema: Prevent setSet() panic with typed nil
References:

* https://github.com/hashicorp/terraform/issues/14418
* v0.9.5 (original bug report): a59ee0b30e/helper/schema/field_writer_map.go (L311)
* v0.11.12 (Terraform AWS Provider discovery): 057286e522/helper/schema/field_writer_map.go (L343)

When creating flatten functions in Terraform Providers that return *schema.Set, its possible to return a typed `nil`, e.g.

```go
func flattenHeaders(h *cloudfront.Headers) *schema.Set {
	if h.Items != nil {
		return schema.NewSet(schema.HashString, flattenStringList(h.Items))
	}
	return nil
}
```

This previously could cause a panic, e.g.

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x1881911]

goroutine 1325 [running]:
github.com/hashicorp/terraform/helper/schema.(*MapFieldWriter).setSet(0xc00054bf00, 0xc00073efa0, 0x5, 0x5, 0x5828140, 0x0, 0xc0002cea50, 0xc000e996a8, 0xc001026e40)
	/Users/bflad/go/pkg/mod/github.com/hashicorp/terraform@v0.11.12/helper/schema/field_writer_map.go:343 +0x211
```

Here we catch the typed `nil` and return an empty list flatmap result instead. Unit testing result prior to code update:

```
--- FAIL: TestMapFieldWriter (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
  panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x1777cdc]

goroutine 913 [running]:
testing.tRunner.func1(0xc00045b800)
  /usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:830 +0x392
panic(0x192cf20, 0x2267ca0)
  /usr/local/Cellar/go/1.12.1/libexec/src/runtime/panic.go:522 +0x1b5
github.com/hashicorp/terraform/helper/schema.(*MapFieldWriter).setSet(0xc0004648a0, 0xc0004408d0, 0x1, 0x1, 0x19e3de0, 0x0, 0xc00045c600, 0x30, 0x19e0080)
  /Users/bflad/src/github.com/hashicorp/terraform/helper/schema/field_writer_map.go:344 +0x68c
github.com/hashicorp/terraform/helper/schema.(*MapFieldWriter).set(0xc0004648a0, 0xc0004408d0, 0x1, 0x1, 0x19e3de0, 0x0, 0x1, 0x18)
  /Users/bflad/src/github.com/hashicorp/terraform/helper/schema/field_writer_map.go:107 +0x28b
github.com/hashicorp/terraform/helper/schema.(*MapFieldWriter).WriteField(0xc0004648a0, 0xc0004408d0, 0x1, 0x1, 0x19e3de0, 0x0, 0x0, 0x0)
  /Users/bflad/src/github.com/hashicorp/terraform/helper/schema/field_writer_map.go:89 +0x504
github.com/hashicorp/terraform/helper/schema.TestMapFieldWriter(0xc00045b800)
  /Users/bflad/src/github.com/hashicorp/terraform/helper/schema/field_writer_map_test.go:337 +0x2ddd
testing.tRunner(0xc00045b800, 0x1a44f90)
  /usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:865 +0xc0
created by testing.(*T).Run
  /usr/local/Cellar/go/1.12.1/libexec/src/testing/testing.go:916 +0x35a
```
2019-04-01 20:10:32 -04:00
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.
2019-03-29 14:54:54 -04:00
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.
2019-03-29 13:56:43 -04:00
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.
2019-03-29 13:56:43 -04:00
Alex Somesan ef681e527d Rephrase for clarity 2019-03-29 16:53:13 +01:00
Alex Somesan adebc65d95 Document that ValidateFunc works on maps. 2019-03-27 20:10:46 +01:00
James Bardin bb62aba651 add (forces new resource) to provider test diffs
Add the (forces new resource) annotation to the diff output for provider
tests failures when we can. This helps providers narrow down what might
be triggering changes when encountering test failures with the new SDK.
2019-03-22 15:30:51 -04:00
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.
2019-03-21 15:19:59 -07:00
Martin Atkins 7f860dc83e helper/schema: Schema.SkipCoreTypeCheck flag
When running in v0.12-and-higher mode, this will cause the SDK to report
the type of the attribute as "any", effectively skipping type checking
on the Core side altogether and checking only in the SDK and provider
code.

The practical impact of this is to restore the v0.11-style checking
behavior of allowing object values to be missing certain attributes as
long as they are marked as optional in the schema. The SDK can do this
because it uses a unified schema model for both object values and nested
blocks, while Terraform Core only supports the idea of "optional" when
talking about attributes in nested blocks.

This is a continuation of the pile of workarounds that also includes
the ConfigMode and AsSingle fields, allowing providers to selectively opt
out of new v0.12 behaviors in situations where they conflict with
decisions made in the design of the providers in our old world where
Terraform Core delegated _all_ validation to providers.

This is designed as an opt-in so that we can limit its impact only to
specific cases where it's needed and minimize the risk of regressions
elsewhere. Providers should use this sparingly only in situations where
prevailing usage disagrees with the new expectations of Terraform Core in
v0.12.

This commit only adds the flag, and does not implement any behavior for it
yet. That means this commit can exist in both the v0.11 and v0.12
codebases, allowing for API compatibility. A subsequent commit for v0.12
(not included in v0.11) will then implement this behavior.
2019-03-21 15:19:59 -07:00
Justin Downing 1e32ae243c grammatical updates to comments and docs (#20195) 2019-03-21 14:05:41 -07:00
Martin Atkins 35df450dc0 helper/resource: Preserve provider address when shimming to old state
It's important to preserve the provider address because during the destroy
phase of provider tests we'll use the references in the state to determine
which providers are required, and so without this attempts to override
the provider using the "provider" meta-argument can cause failures at
destroy time when the wrong provider gets selected.

(This is particularly acute in the google-beta provider tests because that
provider is _always_ used with provider = "google-beta" to override the
default behavior of using the normal "google" provider.)
2019-03-19 15:08:46 -07:00
Paul Tyng ea7e922007
oxford comma 2019-03-18 14:16:20 -04:00
Paul Tyng ec9450a262
Fix limitations on Elem for TypeMap 2019-03-18 14:15:25 -04:00
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.
2019-03-14 15:36:15 -07:00
Martin Atkins 1c8150428f helper/schema: Schema.AsSingle flag
This setting indicates that an attribute defined as TypeList or TypeSet
should be presented to Terraform Core as a single value instead when
running in Terraform v0.12 or later. It has no effect for Terraform v0.10
or v0.11.

This commit just introduces the setting without any associated behavior,
so it can be included in both the v0.12 and v0.11 branches. A subsequent
commit only to the v0.12 branch will introduce the behavior as part of
the protocol version 5 shims.
2019-03-14 15:36:15 -07:00
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.
2019-03-13 19:14:17 -04:00