Commit Graph

119 Commits

Author SHA1 Message Date
James Bardin 931f459336 add a trouble test schema from the aws provider 2019-02-13 19:09:46 -05:00
James Bardin f932e11a50 create a test that removes a RequiresReplace path
One of the paths that triggers RequiresReplace does not apply to the
new value.
2019-02-12 11:48:36 -05:00
James Bardin 1ca7531cc7 allow implicit empty strings in lists
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.
2019-02-11 19:24:14 -05:00
James Bardin 1bfc27817e process state even after provider.Apply errors
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.
2019-02-11 15:41:07 -05:00
James Bardin be127725cc Additional tests with interpolated values 2019-02-07 20:23:39 -05:00
James Bardin 411df99f33 only force top-level id's back to unknown
Nested structures may have "id" fields, which should be treated
normally.
2019-02-05 16:16:08 -05:00
James Bardin 3b18dd7c01
Merge pull request #20224 from hashicorp/jbardin/sdk
SDK set fixes
2019-02-05 14:11:51 -05:00
James Bardin 79d1e0d7cf add failing test for multiple computed set elems 2019-02-05 12:08:17 -05:00
James Bardin 2e2374cfcb add failing test for set elements with custom diff
Adding a DiffSuppressFunc for set elements can cause them to be missed
in the set diff entirely.
2019-02-05 12:08:16 -05:00
Martin Atkins bdcac8792d plugin: Use correct schema when marshaling imported resource objects
Previously we were using the type name requested in the import to select
the schema, but a provider is free to return additional objects of other
types as part of an import result, and so it's important that we perform
schema selection separately for each returned object.

If we don't do this, we get confusing downstream errors where the
resulting object decodes to the wrong type and breaks various invariants
expected by Terraform Core.

The testResourceImportOther test in the test provider didn't catch this
previously because it happened to have an identical schema to the other
resource type being imported. Now the schema is changed and also there's
a computed attribute we can set as part of the refresh phase to make sure
we're completing the Read call properly during import. Refresh was working
correctly, but we didn't have any tests for it as part of the import flow.
2019-02-01 15:22:54 -08:00
James Bardin 89c1ba099f add computed set test with CustomizeDiff 2019-02-01 17:21:37 -05:00
James Bardin fa92e69e17 simplify test check 2019-01-30 14:55:04 -05:00
James Bardin 3b04b41250 fix RequiresNew in diff
With the new diff.Apply we can keep the diff mostly intact, but we need
turn off all RequiresNew flags so that the prior state is not removed
from the apply.
2019-01-30 14:55:04 -05:00
Martin Atkins 477da57a92 helper/plugin: Honor resource type overrides in import
One quirky aspect of our import feature is that we allow the importer to
produce additional resources alongside the one that was imported, such as
to create separate rules for each rule of an imported security group.

Providers need to be able to set the types of these other resources since
they may not match the "main" resource type. They do this by calling
ResourceData.SetType, which in turn sets InstanceState.Ephemeral.Type.

In our shims here we therefore need to copy that out into our new TypeName
field so that the new core import code can see it and create the right
type in the state.

Testing this required a minor change to the test harness to allow the
ImportStateCheck function to see the resource type.
2019-01-30 09:05:08 -08:00
James Bardin 775df57217 add more tests
verify that changes to defaults are detected
2019-01-23 20:03:10 -05:00
James Bardin 7dd0acc46b don't count empty containers in diff.Apply
If there were no matching keys, and there was no diff at all, don't set
a zero count for the container. Normally Providers can't reliably detect
empty vs unset here, but there are some cases that worked.
2019-01-23 19:34:11 -05:00
James Bardin 675d700a5f test for missing map entries 2019-01-23 17:04:17 -05:00
James Bardin 93d78c4ee7 disable broken import test for now 2019-01-22 18:10:12 -05:00
James Bardin f78b5045d0 add failing test for lost elements in list blocks
Modifying an element loses the modification, and other elements in a
TypeList.
2019-01-22 18:10:12 -05:00
James Bardin c045d3e6a3 disable known failing tests
We need these changes in master for testing, worry about these test
after.
2019-01-17 19:19:13 -05:00
James Bardin 4439a7dcf4 add tests for nested default values
Don't lose default values set within a nested block.
2019-01-17 18:51:18 -05:00
James Bardin 0d1252812b add more tests for a computed nested list and set 2019-01-15 11:55:02 -05:00
James Bardin 9b89f6ecc6 add tests for deprecated/removed attrs 2019-01-12 10:41:04 -05:00
James Bardin 3e3802c36f update existing test provider test 2019-01-10 13:08:54 -05:00
James Bardin f4fe6d6716 add tests with set hashes to the test provider
These are representative of things that real-world providers use in
tests.
2019-01-10 12:26:53 -05:00
James Bardin 7455bf2a55 provider tests for empty values
Add tests to make limited use of empty container values and empty
strings.
2019-01-08 16:26:22 -05:00
Martin Atkins 364d3ffc4a provider/test: Test for nested dynamic blocks
This is a HCL feature rather than a Terraform feature really, but we want
to make sure it keeps working consistently in future versions of Terraform
so this is a Terraform-flavored test for the block expansion behavior.

In particular, it tests that a nested dynamic block can access the parent
iterator, so that we won't regress #19543 in future.
2018-12-20 14:28:37 -08:00
Martin Atkins e39c69750c core: Specialized errors for incorrect indexes in resource reference
In prior versions of Terraform we permitted inconsistent use of indexes
in resource references, but in as of 0.12 the index usage must correlate
properly with whether "count" is set on the resource.

Since users are likely to have existing configurations with incorrect
usage, here we introduce some specialized error messages for situations
where we can detect such issues statically. This seems to cover all of the
common patterns we've seen in practice.

Some usage patterns will fall back on a less-helpful dynamic error here,
but no configurations coming from 0.11 can end up that way because 0.11
did not permit forms such as aws_instance.no_count[count.index].bar that
this validation would not be able to "see".

Our configuration upgrade tool also contains a fix for this already, but
it takes a more conservative approach of adding the index [1] rather than
[count.index] because it can't be sure (without human help) if correlation
of indices is what was intended.
2018-12-20 13:55:42 -08:00
James Bardin c70be3c328 failing tests when using resources with count
Two different tests failing around resourced with count
2018-12-17 12:15:43 -05:00
James Bardin 53ff35b9ca StateFunc tests 2018-12-03 18:03:45 -05:00
James Bardin f0e51aca1a test a nil computed value within an expression
Comparing a nil-computed value was returning unknown, preventing the
data source from being evaluated.
2018-11-28 17:37:58 -05:00
James Bardin 622f5cc6fb add test for computed map value
This ensures that a computed map can be correctly applied.
2018-11-27 08:54:15 -05:00
James Bardin c24a18d514 remove unnecessary computed flag
The "with_list" attr wasn't actually computed,

Make sure we read with the correct function.
2018-11-27 08:54:15 -05:00
James Bardin 15d2330918 computed value wasn't being set 2018-11-27 08:54:15 -05:00
James Bardin eddf676c1f add provider test with a nested list in a set
in some cases helper/schema misses the list counts.
2018-11-16 15:11:16 -05:00
James Bardin a681124301 verify DiffSuppresFunc behavior
Terraform used to provide empty diffs to the provider when calculating
`ignore_changes`, which would cause some DiffSuppressFunc to fail, as
can be seen in #18209.

Verify that this is no longer the case in 0.12
2018-11-16 11:17:23 -05:00
James Bardin 83317975fe add more tests with carious set combinations 2018-11-16 09:59:03 -05:00
James Bardin 71b55601ce new failing tests for nested sets 2018-11-16 09:59:03 -05:00
James Bardin d2bd41c260 add a nested set test 2018-11-13 18:53:02 -05:00
James Bardin c4d0be8a52 failing test for schemas with a single set attr
Resources with certain combinations of attributes in a nested single set
fail to perperly coerce their shimmed values.
2018-11-13 18:41:53 -05:00
James Bardin e91f381cc4 test case for optional bools in schema
Booleans in the legacy form were stored as strings, and can appear as
the incorrect type in the new type system.

Unset fields in sets also might show up erroneously in diffs, with
equal old and new values.
2018-11-01 16:19:03 -04:00
James Bardin 8212a6a9d0 add provider tests for force-new with a map
Adding and removing a single map that requires a new resource can cause
empty diffs, relying on the core proposed state values for destruction.
2018-10-31 13:42:28 -04:00
James Bardin 36cede09f7 add provider tests for SuppressDiffFunc 2018-10-30 14:53:38 -04:00
James Bardin 121c9c127f add timeout tests to the test provider 2018-10-30 13:14:08 -04:00
James Bardin 5303137b8c udpate test configs to work with hcl2
The last 2 broken tests will be hanlded later
2018-10-16 19:14:54 -07:00
Martin Atkins d11dd20bf3 builtin/providers/test: use new API for root module address
terraform.RootModulePath is no longer present, but
addrs.RootModuleInstance is equivalent to it.
2018-10-16 19:14:11 -07:00
Radek Simko 2974d63e75
Merge pull request #16588 from hashicorp/f-panic-on-invalid-rd-set
helper/schema: Opt-in panic on invalid ResourceData.Set
2017-11-08 19:17:46 +00:00
Radek Simko e93d64b18c
helper/schema: Opt-in panic on invalid ResourceData.Set 2017-11-08 10:05:11 +00:00
Chris Marchesi 5d5a670d69 provider/test: Added complex-ish list testing
Added a list SetNew test to try and reproduce issues testing diff
customization with the Nomad provider. We are running into "diffs didn't
match during apply", with the plan diff exhibiting a strange
off-by-one-type error in a list diff:

  datacenters.#:         "1" => "2"
  datacenters.0:         "dc1" => "dc2"
  datacenters.1:         "" => "dc3"
  datacenters.2:         "" => "dc3"

The test here does not reproduce that issue, unfortunately, but should
help pinpoint the root cause through elimination.
2017-11-01 14:25:32 -07:00
Chris Marchesi 529d7e6dae helper/schema: Review -> CustomizeDiff
Restoring the naming of this field in the resource back to
CustomizeDiff, as this is generally more descriptive of the process
that's happening, despite the lengthy name.
2017-11-01 14:25:32 -07:00
Chris Marchesi c6647a3bb7 helper/schema: CustomizeDiff -> Review
To keep with the current convention of most other schema.Resource
functional fields being fairly short, CustomizeDiff has been changed to
"Review". It would be "Diff", however it is already used by existing
functions in schema.Provider and schema.Resource.
2017-11-01 14:25:32 -07:00
Chris Marchesi 8af9610b87 helper/schema: Hook CustomizeDiffFunc into diff logic
It's alive! CustomizeDiff logic now has been inserted into the diff
process. The test_resource_with_custom_diff resource provides some basic
testing and a reference implementation.

There should now be plenty of test coverage for this feature via the
tests added for ResourceDiff, and the basic test added to the
schemaMap.Diff test, and the test resource, but more can be added to
test any specific case that comes up otherwise.
2017-11-01 14:25:32 -07:00
Martin Atkins f695e8b330 provider/test: allow test_resource to be imported
When working on the core import code, it's useful to have a zero-cost
local resource to work with for quick iteration.
2017-06-09 14:03:59 -07:00
Martin Atkins 410b60cb7f Stop requiring multi-vars (splats) to be in array brackets
Prior to Terraform 0.7, lists in Terraform were just a shallow abstraction
on top of strings with a magic delimiter between items. Wrapping a single
string in brackets in the configuration was Terraform's prompt that it
needed to split the string on that delimiter during interpolation.

In 0.7, when first-class lists were added, this convention was preserved
by flattening lists-of-lists by one level when they were encountered in
configuration. However, there was an oversight in that change where it
did not correctly handle the case where the inner list was unknown.

In #14135 we removed some code that was flattening partially-unknown lists
into fully-unknown (untyped) values. This inadvertently exposed the missed
case from the previous paragraph, causing issues for list-wrapped splat
expressions with unknown members. While this worked fine for resources,
due to some fixup done inside helper/schema, this did not work for other
interpolation contexts such as module blocks.

Various attempts to fix this up and restore the flattening behavior
selectively were unsuccessful, due to a proliferation of assumptions all
over the core code that would be too risky to change just to fix this bug.

This change, then, takes the different approach of removing the
requirement that splats be presented inside list brackets. This
requirement didn't make much sense anymore anyway, since no other
list-returning expression had this constraint and so the rest of Terraform
was already successfully dealing with both cases.

This leaves us with two different scenarios:

- For resource arguments, existing normalization code in helper/schema
  does its own flattening that preserves compatibility with the common
  practice of using bracketed splats. This change proves this with a test
  within the "test" provider that exercises the whole Terraform core and
  helper/schema stack that assigns bracketed splats to list and set
  attributes.

- For arguments in other blocks, such as in module callsites, the
  interpolator's own flattening behavior applies to known lists,
  preserving compatibility with configurations from before
  partially-computed splats were possible, but those wishing to use
  partially-computed splats are required to drop the surrounding brackets.
  This is less concerning because this scenario was introduced only in
  0.9.5, so the scope for breakage is limited to those who adopted this
  new feature quickly after upgrading.

As of this commit, the recommendation is to stop using brackets around
splats but the old form continues to be supported for backward
compatibility. In a future _major_ version of Terraform we will probably
phase out this legacy form to improve consistency, but for now both
forms are acceptable at the expense of some (pre-existing) weird behavior
when _actual_ lists-of-lists are used.

This addresses #14521 by officially adopting the suggested workaround of
dropping the brackets around the splat. However, it doesn't yet allow
passing of a partially-unknown list between modules: that still violates
assumptions in Terraform's core, so for the moment partially-unknown lists
work only within a _single_ interpolation expression, and cannot be
passed around between expressions. Until more holistic work is done to
improve Terraform's type handling, passing a partially-unknown splat
through to a module will result in a fully-unknown list emerging on
the other side, just as was the case before #14135; this change just
addresses the fact that this was failing with an error in 0.9.5.
2017-05-23 11:22:37 -07:00
Chris Marchesi f63ad1dbd1 providers/test: Add count resource <-> data source dep count scale tests
These tests cover the new refresh behaviour and would fail with "index
out of range" if the refresh graph is not expanded to take new resources
into account as well (scale out), or if it does not with expanded count
orphans in a way that makes sure they don't get interpolated when walked
(scale in).
2017-05-12 15:45:06 -07:00
Martin Atkins e67c359b2d provider/test: allow assigning a label to each instance
When testing the behavior of multiple provider instances (either aliases
or child module overrides) it's convenient to be able to label the
individual instances to determine which one is actually being used for
the purpose of making test assertions.
2017-05-11 10:52:51 -07:00
Chris Marchesi d41b806789 core: Restore CountBoundaryTransformer to apply, add/adjust tests
Moving the transformer wholesale looks like it broke some tests, with
some actually doing legit work in normalizing singular resources from a
foo.0 notation to just foo.

Adjusted the TestPlanGraphBuilder to account for the extra
meta.count-boundary nodes in the graph output now, as well as added
another context test that tests this case. It appears the issue happens
during validate, as this is where the state can be altered to a broken
state if things are not properly transformed in the plan graph.
2017-04-19 22:23:52 -07:00
Chris Marchesi 2802d319d2 core: Move CountBoundaryTransformer to the plan graph builder
This fixes interpolation issues on grandchild data sources that have
multiple instances (ie: counts). For example, baz depends on bar, which
depends on foo.

In this instance, after an initial TF run is done and state is saved,
the next refresh/plan is not properly transformed, and instead of the
graph/state coming through as data.x.bar.0, it comes through as
data.x.bar.  This breaks interpolations that rely on splat operators -
ie: data.x.bar.*.out.
2017-04-19 16:56:54 -07:00
Mitchell Hashimoto c6d0333dc0
flatmap: mark computed list as a computed value in Expand
Fixes #12183

The fix is in flatmap for this but the entire issue is a bit more
complex. Given a schema with a computed set, if you reference it like
this:

    lookup(attr[0], "field")

And "attr" contains a computed set within it, it would panic even though
"field" is available. There were a couple avenues I could've taken to
fix this:

1.) Any complex value containing any unknown value at any point is
entirely unknown.

2.) Only the specific part of the complex value is unknown.

I took route 2 so that the above works without any computed (since
"name" is not computed but something else is). This may actually have an
effect on other parts of Terraform configs, however those similar
configs would've simply crashed previously so it shouldn't break any
pre-existing configs.
2017-02-23 10:03:59 -08:00
Mitchell Hashimoto dd8ee38ba8
providers/test: additional testing via integration tests 2017-01-28 11:09:24 -08:00
James Bardin d1d6907640 Add a provider test for a list of maps
Interpolation of a map from a list of maps was not working. Add a
provider example test to cover this.
2016-12-16 10:36:26 -05:00
James Bardin 68ba2d6ff0 ResourceConfig.get should never return (nil, true)
Fixes a case where ResourceConfig.get inadvertently returns a nil value.

Add an integration test where assigning a map to a list via
interpolation would panic.
2016-11-18 16:24:40 -05:00
James Nugent fb150ef72f provider/test: Add test of data source count.index
This adds a unit test to the test provider that verifies count.index
behaves correctly. Although not ideal this is hard to implement as a
context test without changing around the (non helper/schema)
implementation of the x_data_source.
2016-09-03 13:58:30 -07:00
Paul Hinze 14cea95e86
terraform: another set of ignore_changes fixes
This set of changes addresses two bug scenarios:

(1) When an ignored change canceled a resource replacement, any
downstream resources referencing computer attributes on that resource
would get "diffs didn't match" errors. This happened because the
`EvalDiff` implementation was calling `state.MergeDiff(diff)` on the
unfiltered diff. Generally this is what you want, so that downstream
references catch the "incoming" values. When there's a potential for the
diff to change, thought, this results in problems w/ references.

Here we solve this by doing away with the separate `EvalNode` for
`ignore_changes` processing and integrating it into `EvalDiff`. This
allows us to only call `MergeDiff` with the final, filtered diff.

(2) When a resource had an ignored change but was still being replaced
anyways, the diff was being improperly filtered. This would cause
problems during apply when not all attributes were available to perform
the replacement.

We solve that by deferring actual attribute removal until after we've
decided that we do not have to replace the resource.
2016-07-08 16:48:23 -05:00
Paul Hinze 4a1b36ac0d
core: rerun resource validation before plan and apply
In #7170 we found two scenarios where the type checking done during the
`context.Validate()` graph walk was circumvented, and the subsequent
assumption of type safety in the provider's `Diff()` implementation
caused panics.

Both scenarios have to do with interpolations that reference Computed
values. The sentinel we use to indicate that a value is Computed does
not carry any type information with it yet.

That means that an incorrect reference to a list or a map in a string
attribute can "sneak through" validation only to crop up...

 1. ...during Plan for Data Source References
 2. ...during Apply for Resource references

In order to address this, we:

 * add high-level tests for each of these two scenarios in `provider/test`
 * add context-level tests for the same two scenarios in `terraform`
   (these tests proved _really_ tricky to write!)
 * place an `EvalValidateResource` just before `EvalDiff` and `EvalApply` to
   catch these errors
 * add some plumbing to `Plan()` and `Apply()` to return validation
   errors, which were previously only generated during `Validate()`
 * wrap unit-tests around `EvalValidateResource`
 * add an `IgnoreWarnings` option to `EvalValidateResource` to prevent
   active warnings from halting execution on the second-pass validation

Eventually, we might be able to attach type information to Computed
values, which would allow for these errors to be caught earlier. For
now, this solution keeps us safe from panics and raises the proper
errors to the user.

Fixes #7170
2016-07-01 13:12:57 -05:00
James Nugent 75ef7ab636 provider/test: Add more variants of maps
This commit adds a binary for the test provider, and adds a variety of
different types of map to the schema.
2016-06-09 10:49:49 +01:00
Paul Hinze b4df304b47
helper/schema: Normalize bools to "true"/"false" in diffs
For a long time now, the diff logic has relied on the behavior of
`mapstructure.WeakDecode` to determine how various primitives are
converted into strings.  The `schema.DiffString` function is used for
all primitive field types: TypeBool, TypeInt, TypeFloat, and TypeString.

The `mapstructure` library's string representation of booleans is "0"
and "1", which differs from `strconv.FormatBool`'s "false" and "true"
(which is used in writing out boolean fields to the state).

Because of this difference, diffs have long had the potential for
cosmetically odd but semantically neutral output like:

    "true" => "1"
    "false" => "0"

So long as `mapstructure.Decode` or `strconv.ParseBool` are used to
interpret these strings, there's no functional problem.

We had our first clear functional problem with #6005 and friends, where
users noticed diffs like the above showing up unexpectedly and causing
troubles when `ignore_changes` was in play.

This particular bug occurs down in Terraform core's EvalIgnoreChanges.
There, the diff is modified to account for ignored attributes, and
special logic attempts to handle properly the situation where the
ignored attribute was going to trigger a resource replacement. That
logic relies on the string representations of the Old and New fields in
the diff to be the same so that it filters properly.

So therefore, we now get a bug when a diff includes `Old: "0", New:
"false"` since the strings do not match, and `ignore_changes` is not
properly handled.

Here, we introduce `TypeBool`-specific normalizing into `finalizeDiff`.
I spiked out a full `diffBool` function, but figuring out which pieces
of `diffString` to duplicate there got hairy. This seemed like a simpler
and more direct solution.

Fixes #6005 (and potentially others!)
2016-05-05 09:00:58 -05:00
Paul Hinze f480ae3430 core: Fix issues with ignore_changes
The ignore_changes diff filter was stripping out attributes on Create
but the diff was still making it down to the provider, so Create would
end up missing attributes, causing a full failure if any required
attributes were being ignored.

In addition, any changes that required a replacement of the resource
were causing problems with `ignore_chages`, which didn't properly filter
out the replacement when the triggering attributes were filtered out.

Refs #5627
2016-03-21 14:20:36 -05:00
Paul Hinze c3e27b3e0a provider/test: a test provider
Here we also introduce a `test` provider meant as an aid to exposing
via automated tests issues involving interactions between
`helper/schema` and Terraform core.

This has been helpful so far in diagnosing `ignore_changes` problems,
and I imagine it will be helpful in other contexts as well.

We'll have to be careful to prevent the `test` provider from becoming a
dumping ground for poorly specified tests that have a clear home
elsewhere. But for bug exposure I think it's useful to have.
2016-03-21 08:59:54 -05:00