Commit Graph

1012 Commits

Author SHA1 Message Date
James Bardin b872491baa incremental progress towards applying diffs 2018-11-16 09:58:42 -05:00
James Bardin ce5d7ff6d0 spelling 2018-11-13 18:41:53 -05:00
Dana Hoffman 7edbb3c8bf return state even if cfg is invalid 2018-11-12 21:49:30 -05:00
Radek Simko 0cbf745e5a
helper/schema: Avoid erroring out on undefined timeouts 2018-11-07 15:38:58 +00:00
Radek Simko 7eae051a16
Merge pull request #19286 from hashicorp/radeksimko/b-timeouts-parsing-fix
helper/schema: Fix timeout parsing during Provider.Diff
2018-11-06 11:18:31 +00:00
Radek Simko 1cb8f1df80
helper/schema: Fix timeout parsing in ResourceTimeout.ConfigDecode 2018-11-05 12:42:12 +00:00
Radek Simko 82a77f9bb5
helper/schema: Add test for invalid timeout value 2018-11-05 12:42:12 +00:00
Radek Simko 2fe3f16cb3
helper/schema: Return error on invalid timeout type 2018-11-05 12:42:11 +00:00
Radek Simko 186a6dcc38
helper/schema: Add test for wrong timeout type 2018-11-05 12:42:11 +00:00
James Bardin ae1f93a24f
Merge pull request #19236 from hashicorp/jbardin/resource-tests
skip resource tests for now
2018-11-01 11:52:50 -04:00
James Bardin a5ef403dfd skip resource tests for now
These aren't going to be fixed in the immediate future, and are
preventing the CI tests from being helpful.
2018-10-31 14:17:23 -04:00
James Bardin 718a3c400a fix state variable name 2018-10-31 13:43:50 -04:00
James Bardin e0ea2a5d06 if there is no plan diff, prefer the prior state
The prior state may contain customizations made by the provider. If
there is no prior state, then take the proposed state.
2018-10-30 15:58:00 -04:00
James Bardin f153720a36 add checks for timeouts attributes and blocks
Don't overwrite anything the provider defined, in order to maintain
existing behavior.

Change strings to pre-defined constants
2018-10-30 14:16:44 -04:00
James Bardin e38a5a769d copy timouts into plan and apply state
helper/schema will remove "timeouts" from the config, and stash them in
the diff.Meta map. Terraform sees "timeouts" as a regular config block,
so needs them to be present in the state in order to not show a diff.

Have the GRPCProviderServer shim copy all timeout values into any state
it returns to provide consistent diffs in core.
2018-10-30 13:14:08 -04:00
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.
2018-10-30 13:14:08 -04:00
James Bardin dbaf347392
Merge pull request #19136 from hashicorp/jbardin/simple-diff
panic fixes
2018-10-19 17:06:37 -04:00
Brian Flad 62bf23850b
Merge pull request #19122 from hashicorp/f-helper-validation-Any
helper/validation: Add Any() SchemaValidateFunc
2018-10-19 15:11:33 -04:00
James Bardin d50a152f8b check for a nil diff in simpleDiff 2018-10-19 14:25:20 -04:00
Brian Flad 46804080aa
helper/validation: Add Any() SchemaValidateFunc
`Any()` allows any single passing validation of multiple `SchemaValidateFunc` to pass validation to cover cases where a standard validation function does not cover the functionality or to make error messaging simpler.

Example provider usage:

```go
ValidateFunc: validation.Any(
  validation.IntAtLeast(42),
  validation.IntAtMost(5),
),
```
2018-10-18 20:58:53 -04:00
James Bardin ff4e81cc2b add old values when computing the new InstanceDiff
This was previously done in the RequiresNew code, which is skipped in
new style provider.
2018-10-18 20:05:33 -04:00
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.
2018-10-18 12:45:55 -04:00
James Bardin e077c9ce95 Insert default values into provider config
Add any top-level default attributes from the provider schema into Null
config values.
2018-10-18 11:40:47 -04:00
James Bardin a3ac49b3fb GRPCProviderServer and PrepareProviderconfig
Update the server side of the plugin to match the new method signature.
2018-10-18 08:48:55 -04:00
Sander van Harmelen 48ef7ecfa6 Updates after running `make fmt` with Go v1.11.1 2018-10-17 14:11:08 -07:00
Brian Flad 17ac9a5756
helper/validation: Add All() and IntInSlice() SchemaValidateFunc
`All()` combines the outputs of multiple `SchemaValidateFunc`, to reduce the usage of custom validation functions that implement standard validation functions.

Example provider usage:

```go
ValidateFunc: validation.All(
  StringLenBetween(5, 42),
  StringMatch(regexp.MustCompile(`[a-zA-Z0-9]+`), "value must be alphanumeric"),
),
```

`IntInSlice()` is the `int` equivalent of `StringInSlice()`

Example provider usage:

```go
ValidateFunc: validation.IntInSlice([]int{30, 60, 120})
```

Output from unit testing:

```
$ make test TEST=./helper/validation
==> Checking that code complies with gofmt requirements...
go generate ./...
2018/10/17 14:16:03 Generated command/internal_plugin_list.go
go list ./helper/validation | xargs -t -n4 go test  -timeout=2m -parallel=4
go test -timeout=2m -parallel=4 github.com/hashicorp/terraform/helper/validation
ok  	github.com/hashicorp/terraform/helper/validation	1.106s
```
2018-10-17 14:22:29 -04:00
James Bardin 1ab96f42b7 fail nonfunctional resource tests
The helper/resource unit tests will panic, because they were using the
legacy terraform.MockResourceProvider, which doesn't have the same
internals required by the new GRPC shims.

Fail these tests for now, and a new test provider will need to be made
out of a schema.Provider instance.
2018-10-17 12:51:07 -04:00
James Bardin 38163f2b37 use SimpleDiff and set "id" as RequiresReplace
Use the new SimpleDiff method of the provider so that the diff isn't
altered by ForceNew attributes.

Always set an "id" as RequiresReplace so core knows an instance will be
replaced, even if all ForceNew attributes are filtered out due to
ignore_changes.
2018-10-16 19:14:54 -07:00
James Bardin 46b4c27dbe create a SimpleDiff for the new provider shims
Terraform now handles any actual "diffing" of resource, and the existing
Diff functions are only used to shim the schema.Provider to the new
methods. Since terraform is handling what used to be the Diff, the
provider now should not modify the diff based on RequiresNew due to it
interfering with the ignore_changes handling.
2018-10-16 19:14:11 -07:00
James Bardin 0d4d572c39 start work on helper/resource test fixtures
The helper resource tests won't pass for now, as they use a
terraform.MockProvider which can't be used in the schema.Provider shims.
2018-10-16 19:14:11 -07:00
James Bardin f8b1a3c7a4 make sure apply can properly destroy
We need to ensure that a destroyed value is returned as such from apply
2018-10-16 19:14:11 -07:00
James Bardin 52c0032aed update provisioners for multiple processes
The "internal" provisioners are still run in a separate process, and
need to be updated to restart on each walk.
2018-10-16 19:14:11 -07:00
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.
2018-10-16 19:14:11 -07:00
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.
2018-10-16 19:14:11 -07:00
Radek Simko d93b462e9c helper/plugin: don't panic in ReadDataSource State 2018-10-16 19:14:11 -07:00
Martin Atkins 1908aff476 helper/resource: Fix duplicated function testConfig
An earlier change introduced a new function testConfig to the main code
for this package, which conflicted with a function of the same name in
the test code.

Here we rename the function from the test code, allowing for the more
generally-named testConfig to be the one in the main code.
2018-10-16 19:14:11 -07:00
Radek Simko 2de0903538 Fix data source bug 2018-10-16 19:14:11 -07:00
Martin Atkins 33151f5011 core: Move StateValueFromInstanceState shim from helper/schema
This one doesn't depend on any helper/schema specific bits and it'll also
be useful for the shims in our mock provider in core.
2018-10-16 19:14:11 -07:00
Martin Atkins 76d11f44cc core: Move some of the helper/schema shims so provider mock can use them
The old names are now wrappers around these new functions.
2018-10-16 19:14:11 -07:00
James Bardin a87470cc15 resource ids must always have a value
The "id" field is assumed to always exist, and must have a valid value.
Set "id" to unknown when planning new resource changes to indicate that
it will be computed.
2018-10-16 19:14:11 -07:00
Martin Atkins 97da905c6e helper/plugin: ReadResource to deal with missing remote object 2018-10-16 19:14:11 -07:00
Martin Atkins a7342de274 core: Properly handle no-op changes in plan
Previously we just left these out of the plan altogether, but in the new
plan types we intentionally include change information for every resource
instance, even if no changes are actually planned, to allow alternative
plan file viewers to show what isn't changing as well as what is.
2018-10-16 19:14:11 -07:00
Martin Atkins c27f900d92 helper/plugin: Don't panic while preparing response in ApplyResourceChange 2018-10-16 19:14:11 -07:00
Martin Atkins 686018ae12 helper/plugin: don't panic in PlanResourceChange PlannedState 2018-10-16 19:14:11 -07:00
Martin Atkins 44bc7519a6 terraform: More wiring in of new provider types
This doesn't actually work yet, but it builds and then panics in a pretty
satisfying way.
2018-10-16 19:12:54 -07:00
James Bardin c07ce1cd4b move conversion functions into separate package
Managing which function need to be shared between the terraform plugin
and the helper plugin without creating cycles was becoming difficult.
Move all functions related to converting between terraform and proto
type into plugin/convert.
2018-10-16 19:11:09 -07:00
Martin Atkins a3403f2766 terraform: Ugly huge change to weave in new State and Plan types
Due to how often the state and plan types are referenced throughout
Terraform, there isn't a great way to switch them out gradually. As a
consequence, this huge commit gets us from the old world to a _compilable_
new world, but still has a large number of known test failures due to
key functionality being stubbed out.

The stubs here are for anything that interacts with providers, since we
now need to do the follow-up work to similarly replace the old
terraform.ResourceProvider interface with its replacement in the new
"providers" package. That work, along with work to fix the remaining
failing tests, will follow in subsequent commits.

The aim here was to replace all references to terraform.State and its
downstream types with states.State, terraform.Plan with plans.Plan,
state.State with statemgr.State, and switch to the new implementations of
the state and plan file formats. However, due to the number of times those
types are used, this also ended up affecting numerous other parts of core
such as terraform.Hook, the backend.Backend interface, and most of the CLI
commands.

Just as with 5861dbf3fc49b19587a31816eb06f511ab861bb4 before, I apologize
in advance to the person who inevitably just found this huge commit while
spelunking through the commit history.
2018-10-16 19:11:09 -07:00
James Bardin 7188d5cbfe add grpc shims to helper/plugin
Add shim functions to automatically wrap plugins in the new GRPC
implementations.
2018-10-16 18:58:49 -07:00
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.
2018-10-16 18:58:49 -07:00
James Bardin b88410984b legacy provider needs to handle StateUpgraders
In order to not require state migrations to be supported in both
MigrateState and StateUpgraders, the legacy provider codepath needs to
handle the StateUpgraders transparently during Refresh.
2018-10-16 18:56:50 -07:00
James Bardin dcaf5aa262 add some of the shims used for the grpc provider
This adds some of the required shim functions to the schema package.
While this further bloats the already huge package, adding the helpers
here was significantly less disruptive than refactoring types into
separate packages to prevent import cycles.

The majority of tests here are directly adapted from existing schema
tests to provide as many known good values to the shims as possible.
2018-10-16 18:56:50 -07:00
James Bardin 7d24936507 updates to teh StateUpgraders
Fix documentation.
Require StateUpgraders to be add in order, and test in validation.
2018-10-16 18:53:51 -07:00
James Bardin 0c33b26e04 StateUpgrade redux
It turns out that state upgrades need to be handled differently since
providers are going to be backwards compatible. This means that new
state upgrades may still be stored in the flatmap format when used wih
terraform 0.11. Because we can't account for the specific version which
could produce a legacy state, all future state upgrades need to record
the schema types for decoding.

Rather than defining a single Upgrade function for states, we now have a
list of functions, each of which handle upgrading a specific version to
the next. In practice this isn't much different from the way many
resources implement upgrades themselves, with a separate function for
each version dispatched from the MigrateState function. The only added
burden is the recording of the schema type, and we intend to supply
tools and helper function to prevent the need to copy the entire
existing schema in all cases.
2018-10-16 18:53:51 -07:00
James Bardin 9eef5e3f91 implement UpgradeState for schema.Resource
This is the provider-side UpgradeState implementation for a particular
resource. This new function will be called to upgrade a saved state with
an old schema version to the current schema.

UpgradeState also requires a record of the last schema and version that
could have been stored as a flatmapped state. If the stored state is in
the legacy flatmap format, this will allow the provider to properly
decode the flatmapped state into the expected structure for the new json
encoded state. If the stored state's version is below that of the
LegacySchema.Version value, it will first be processed by the legacy
MigrateState function.
2018-10-16 18:53:51 -07:00
James Bardin bcc8be7400 add schema.InternalMap
This exposes the internal schemaMap for use by the new provider shims.
2018-10-16 18:53:51 -07:00
James Bardin 0120d53baf only add "id" to top-level resources
Make sure we only add "id" to the top-level resource, since Resource is
also used for nested blocks.
2018-10-16 18:53:51 -07:00
James Bardin 15ccf2dda5 use a custom comparer for cty.Type
Make sure we also compare cty.Types in `cmp.Equal`, even though they
contain unexported fields.
2018-10-16 18:50:57 -07:00
James Bardin 3112b707be SetId should set the attribute as well
The update protocol shims will also check for this this, but eventually
"id" will only be a normal attribute, and we shouldn't have to special
case this.
2018-10-16 18:50:57 -07:00
James Bardin befa81c74f add implicit "id" to resource attribute schemas
When converting a legacy schemaMap to a configschema, we need to add
"id" as a required attribute to top-level resources if it's not
declared.

The "id" field will be required to interoperate with the legacy helper
schema, since the presence of an id was used to indicate the existence
of a resource.
2018-10-16 18:50:57 -07:00
Martin Atkins 479c6b2466 move "configschema" from "config" to "configs"
The "config" package is no longer used and will be removed as part
of the 0.12 release cleanup. Since configschema is part of the
"new world" of configuration modelling, it makes more sense for
it to live as a subdirectory of the newer "configs" package.
2018-10-16 18:50:29 -07:00
Martin Atkins 3c10a3b213 helper/schema: Tolerate incorrectly-specified collection elems
We historically tolerated this, so we need to tolerate it here too in
order to work correctly with existing provider code.
2018-10-16 18:49:20 -07:00
Martin Atkins c937c06a03 terraform: ugly huge change to weave in new HCL2-oriented types
Due to how deeply the configuration types go into Terraform Core, there
isn't a great way to switch out to HCL2 gradually. As a consequence, this
huge commit gets us from the old state to a _compilable_ new state, but
does not yet attempt to fix any tests and has a number of known missing
parts and bugs. We will continue to iterate on this in forthcoming
commits, heading back towards passing tests and making Terraform
fully-functional again.

The three main goals here are:
- Use the configuration models from the "configs" package instead of the
  older models in the "config" package, which is now deprecated and
  preserved only to help us write our migration tool.
- Do expression inspection and evaluation using the functionality of the
  new "lang" package, instead of the Interpolator type and related
  functionality in the main "terraform" package.
- Represent addresses of various objects using types in the addrs package,
  rather than hand-constructed strings. This is not critical to support
  the above, but was a big help during the implementation of these other
  points since it made it much more explicit what kind of address is
  expected in each context.

Since our new packages are built to accommodate some future planned
features that are not yet implemented (e.g. the "for_each" argument on
resources, "count"/"for_each" on modules), and since there's still a fair
amount of functionality still using old-style APIs, there is a moderate
amount of shimming here to connect new assumptions with old, hopefully in
a way that makes it easier to find and eliminate these shims later.

I apologize in advance to the person who inevitably just found this huge
commit while spelunking through the commit history.
2018-10-16 18:46:46 -07:00
Martin Atkins 5782357c28 backend: Update interface and implementations for new config loader
The new config loader requires some steps to happen in a different
order, particularly in regard to knowing the schema in order to
decode the configuration.

Here we lean directly on the configschema package, rather than
on helper/schema.Backend as before, because it's generally
sufficient for our needs here and this prepares us for the
helper/schema package later moving out into its own repository
to seed a "plugin SDK".
2018-10-16 18:39:12 -07:00
Martin Atkins 1fb714ea3b helper/schema: CoreConfigSchema method for Backend
This is just like the method of the same name on Resource, adapting the
helper/schema model of schema into Terraform Core's idea of schema.
2018-10-16 18:24:47 -07:00
Martin Atkins d6c6f8852c configschema: include description in schema
We will need access to this information in order to render interactive
input prompts, and it will also be useful in returning schema information
to external tools such as text editors that have autocomplete-like
functionality.
2018-10-16 18:24:47 -07:00
Paddy 35d82b0555
Merge pull request #18795 from hashicorp/paddy_diff_nested_checkKey
Allow for nested fields with checkKey on ResourceDiffs.
2018-09-26 14:21:28 -07:00
Paddy Carver 2eadc8f625 Don't allow sub-blocks for SetNew.
You can't set individual items in lists, you can only set the list as a
whole, so we shouldn't allow sub-blocks to SetNew or SetNewComputed.
2018-09-26 12:38:38 -07:00
Paddy Carver 393c32624a Add tests to ensure clearing sub-blocks works. 2018-09-20 10:52:36 -07:00
Paul Hinze 470ab3869f
helper/pathorcontents: Skip one test when root
One of the tests in this package doesn't work when the tests are run as
root - like inside of a Docker container. The test is still useful to
specify `pathorcontents` behavior when a file is not readable, so it's
better to skip than just delete it. See linked issue for further
disussion.

Closes #7707
2018-09-19 11:38:51 -05:00
Kristin Laemmert 813b5fd27f
Merge pull request #18825 from sean-/master-make-dev-drift
build: Updates required to bump golang version to 1.11
2018-09-17 09:00:04 -07:00
Brian Flad a47583d3a8
Merge pull request #18688 from hashicorp/f-resource-paralleltest
helper/resource: Add ParallelTest() function to allow opt-in acceptance testing concurrency with t.Parallel()
2018-09-13 11:25:28 -04:00
Sean Chittenden d749420a25
Fix drift caused from gofmt when running make dev and go 1.11.
A fresh checkout of `origin/master` does not build atm using the `dev`
target because `master` has not been formatted using `gofmt` from Go
1.11 (tis has been the case for a while if you've been running devel).

None of the drift in question is especially new but now that Go 1.11
has been released and gofmt's formatting guidelines have been updated,
it would be *really* nice if the code in `master` reflected the current
tooling in order to avoid having to fight this drift locally.

* 8mo: https://github.com/hashicorp/terraform/blame/master/backend/remote-state/s3/backend_test.go#L260-L261
* 6mo: https://github.com/hashicorp/terraform/blame/master/builtin/provisioners/chef/linux_provisioner_test.go#L124
* 1yr: 7cfeffe36b/command/init.go (L75-L76)
* 12d: 7cfeffe36b/command/meta_backend_test.go (L1437)
* 2yr: 7cfeffe36b/helper/schema/resource_timeout_test.go (L26)
* 4yr: 7cfeffe36b/helper/schema/schema_test.go (L2059)
* 1yr: 7cfeffe36b/plugin/discovery/get_test.go (L151)
2018-09-09 10:18:08 -07:00
Paddy Carver 7ee4339555 Allow for nested fields with checkKey on ResourceDiffs.
When checking if a ResourceDiff key is valid, allow for keys that exist
on sub-blocks. This allows things like diff.Clear to be called on
sub-block fields, instead of just on top-level fields.
2018-09-05 17:29:03 -07:00
Sander van Harmelen 3e935c846f terraform/terraform_remote_state: accept complex configs
The `remote` backend config contains an attribute that is defined as a `*schema.Set`, but currently only `string` values are accepted as the `config` attribute is defined as a `schema.TypeMap`.

Additionally the `b.Validate()` method wasn’t called to prevent a possible panic in case of unexpected configurations being passed to `b.Configure()`.

This commit is a bit of a hack to be able to support this in the 0.11 series. The 0.12 series will have proper support, so when merging 0.12 this should be reverted again.
2018-08-29 20:21:47 +02:00
Paul Tyng 74eae6825a
Fix typo in comment 2018-08-16 16:29:29 -04:00
Brian Flad 798162125b helper/resource: Add ParallelTest() function to allow opt-in acceptance testing concurrency with t.Parallel()
While this initial implementation is a very simple wrapper function, implementing this in the helper/resource package provides some downstream benefits:
* Provides a standard interface for plugin developers to enable parallel acceptance testing
* Existing plugins can simply convert resource.Test to resource.ParallelTest references (as appropriate) to enable the functionality, rather than worrying about additional line(s) to each acceptance test function or TestCase
* Potential enhancements to ParallelTest (e.g. adding an environment variable to skip enabling the behavior) are consistently propagated
2018-08-15 15:00:27 -04:00
Dana Hoffman 40225061b4 pretty-print json in http requests/responses 2018-08-09 12:27:58 -07:00
Chris Marchesi 67182d06d7
Merge pull request #18125 from hashicorp/f-testing-taint
helper/resource: Add ability to pre-taint resources
2018-08-01 09:31:07 -07:00
Paddy Carver b7d19e40f8 Deprecated -> DeprecationMessage.
At the Enablement team's request, change from using `Deprecated` to
`DeprecationMessage`, as it's a string value, not a boolean.
2018-06-20 11:21:46 -07:00
Paddy Carver 0dd2408d65 Export resource deprecation.
We already had the functionality to make resources deprecated, which was
used when migrating resources to data sources, but the functionality was
unexported, so only the schema package could do it. Now it's exported,
meaning providers can mark entire resources as deprecated. I also added
a test in hopefully-the-right place?
2018-06-19 12:02:55 -07:00
Chris Marchesi 3505769600
helper/resource: Add ability to pre-taint resources
This adds the Taint field to the acceptance testing framework, allowing
the ability to pre-taint resources at the beginning of a particular
TestStep. This can be useful for when an explicit ForceNew is required
for a specific resource for troubleshooting things like diff mismatches,
etc.

The field accepts resource addresses as a list of strings. To keep
things simple for the time being, only addresses in the root module are
accepted. If we ever want to expand this past that, I'd be almost
inclined to add some facilities to the core terraform package to help
translate actual module resource addresses (ie:
module.foo.module.bar.some_resource.baz) into the correct state, versus
the current convention in some acceptance testing facilities that take
the module address as a list of strings (ie: []string{"root", "foo",
"bar"}).
2018-05-25 07:52:49 -07:00
kt a182a53108
Add a HashInt helper/schema function 2018-04-27 00:04:41 -07:00
Chris Marchesi d7048cb640
helper/schema: ResourceDiff ForceNew attribute correctness
A couple of bugs have been discovered in ResourceDiff.ForceNew:

* NewRemoved is not preserved when a diff for a key is already present.
This is because the second diff that happens after customization
performs a second getChange on not just state and config, but also on
the pre-existing diff. This results in Exists == true, meaning nil is
never returned as a new value.
* ForceNew was doing the work of adding the key to the list of changed
keys by doing a full SetNew on the existing value. This has a side
effect of fetching zero values from what were otherwise undefined values
and creating diffs for these values where there should not have been
(example: "" => "0").

This update fixes these scenarios by:

* Adding a new private function to check the existing diff for
NewRemoved keys. This is included in the check on new values in
diffChange.
* Keys that have been flagged as ForceNew (or parent keys of lists and
sets that have been flagged as ForceNew) are now maintained in a
separate map. UpdatedKeys now returns the results of both of these maps,
but otherwise these keys are ignored by ResourceDiff.
* Pursuant the above, values are no longer pushed into the newDiff
writer by ForceNew. This prevents the zero value problem, and makes for
a cleaner implementation where the provider has to "manually" SetNew to
update the appropriate values in the writer. It also prevents
non-computed keys from winding up in the diff, which ResourceDiff
normally blocks by design.

There are also a couple of tests for cases that should never come up
right now involving Optional/Computed values and NewRemoved, for which
explanations are given in annotations of each test. These are here to
guard against future regressions.
2018-04-08 07:50:41 -07:00
James Bardin 61eae050ab
Merge pull request #17588 from creack/creack/fix-deepcopy-type
Fix issue with deepcopy returning wrong type causing panic
2018-04-05 09:24:08 -04:00
Chris Marchesi db5ed51703
Merge pull request #17667 from hashicorp/f-resource-diff-getokexists-computed
helper/schema: Add Computed to ResourceDiff, expose GetOkExists
2018-04-02 07:32:04 -07:00
Adam Lewandowski 56a330c533 Remove attribute values from ConflictsWith validation message 2018-03-30 07:53:59 -04:00
Chris Marchesi fdfe89ed1d
helper/schema: Computed -> NewValueKnown, add comments to GetOkExists
Added some more context to GetOkExists, moved Computed to NewValueKnown
to accommodate some changes that will be coming up in HCL2 that may make
"Computed" less intuitive of a function name, and updated the docs for
NewValueKnown as well.
2018-03-29 15:18:35 -07:00
Chris Marchesi 274b933077
helper/schema: Add Computed to ResourceDiff, expose GetOkExists
This adds a new method to ResourceDiff: Computed, which exposes the
computed read result field to ResourceDiff. In the context of
customizing the diff, this is important as interpolated and otherwise
computed values will show up in the diff as blank, with no way of
determining if the value is actually blank or if it's a computed value
not available at diff customization time. Currently assumptions need to
be made on this, but this does not help in validation scenarios where
one needs to differentiate between an actual blank value and a value
that will be available later.

This is exposed for the most part via NewComputed in the diff, but the
tests cover both the config reader as well (with no diff, even though
this should not come up in normal operation) and also the newDiff reader
when someone sets a new value using SetNew and SetNewComputed.

This commit also exposes GetOkExists. The tests were mostly pulled from
ResourceData but a few were added to ensure that config was being
properly covered as well, in addition to covering SetNew and
SetNewComputed.
2018-03-29 12:04:36 -07:00
Adit Sarfaty dfa623250a helper/validation: IP address and IP address range validation helpers 2018-03-29 09:41:59 -07:00
James Bardin 798df9dafa make sure ResourceData timeouts are always set
Return the global default timeout if the ResourceData timeouts are nil.

Set the timeouts from the Resource when calling Resource.Data, so that
the config values are always available.
2018-03-22 15:10:43 -04:00
Brian Flad 077b828902
Merge pull request #17484 from hashicorp/f-validation-ValidateRFC3339TimeString
validation: Add ValidateRFC3339TimeString
2018-03-15 16:22:04 -04:00
Brian Flad 994a78dbc7 validation: Return "invalid RFC3339 timestamp" in ValidateRFC3339TimeString 2018-03-15 12:31:31 -04:00
Guillaume J. Charmes a4f941368b
Fix issue with deepcopy returning wrong type causing panic
Signed-off-by: Guillaume J. Charmes <gcharmes@magicleap.com>
2018-03-14 19:34:23 -04:00
James McGill 035d56409f helper/schema: handle TypeMap elem consistently with other collection types
For historical reasons, the handling of element types for maps is inconsistent with other collection types.

Here we begin a multi-step process to make it consistent, starting by supporting both the "consistent" form of using a schema.Schema and an existing erroneous form of using a schema.Type directly. In subsequent commits we will phase out the erroneous form and require the schema.Schema approach, the same as we do for TypeList and TypeSet.
2018-03-14 14:50:41 -07:00
Radek Simko f6c3e40439
Merge pull request #17463 from hashicorp/b-helper-r-diff-nested-keys
helper/schema: Allow ResourceDiff.ForceNew on nested fields (avoid crash)
2018-03-14 18:53:44 +00:00
Chase 02ff60d083
Fixing small typo in resource/wait.go 2018-03-14 17:33:03 +01:00
Paul Tyng 528cbecfce
Make failure message more explicit 2018-03-10 21:53:54 -05:00
Paul Tyng 707d7febe3
Use constants in test 2018-03-10 20:56:13 -05:00
Paul Tyng f18e4f2921
Export a const for validation methods 2018-03-10 12:40:37 -05:00
Brian Flad ea64b24cd9 validation: Add ValidateRFC3339TimeString 2018-03-01 23:19:07 -05:00
Radek Simko ccf8a31cbb
helper/schema: Allow ResourceDiff.ForceNew on nested fields (avoid crash) 2018-03-01 15:51:01 +00:00
Stuart Auld 7d1f594f54 helper/resource: don't panic if a requested module does not exist 2018-02-16 11:32:49 -08:00
Stuart Auld df9446f490 helper/resource: allow tests to opt out of temporary file cleanup
This is rarely needed, but sometimes tests need to create temporary files as part of their operation. This should be used sparingly, since it prevents the pro-active cleanup of the temporary working directory.
2018-02-16 11:24:01 -08:00
Radek Simko 7af1c2b3a4
helper/schema: Prevent crash on removal of computed field in CustomizeDiff 2018-02-01 12:05:22 +00:00
Stefan Schmidt ee6c8a777a s/repalce/replace 2018-01-23 17:41:03 +01:00
Gauthier Wallet 1b3f5fcbfb helper/structure: More cases for NormalizeJsonString tests
In terraform-providers/terraform-provider-aws#2935, we have been cleaning code
duplication by benefiting from the "NormalizeJsonString" present in the "structure" helper.

It appears that tests in the AWS provider are covering more use-cases,
which are added in this work.
2018-01-17 15:53:38 -08:00
James Bardin d91dc1a13e unused variable
vet can now catch these in closures
2017-12-26 14:32:57 -05:00
James Bardin 4b49a323c3 go fmt
slight change to go fmt coming in 0.10
2017-12-26 13:26:38 -05:00
James Bardin 0df8da59f7 add FIXMEs
This new codepath with the getDiff "customzed" return value, along with
the associated test need to be removed as soon as we can support unset
fields from the config, so we don't continue to carry this broken
behavior forward any longer than needed.
2017-12-20 08:51:00 -05:00
Chris Marchesi cb5ce1d35e
helper/schema: Extend diffChange and bubble up customized values
This extends the internal diffChange method so that ResourceDiff's
implementation of it can report back whether or not the value came from
a customized diff.

This is an effort to work to preserve the pre-ResourceDiff behaviour
that ignores the diff for computed keys when the old value was populated
but the new value wasn't - this behaviour is actually being depended on
by users that are using it to exploit using zero values in modules. This
should allow both scenarios to co-exist by shifting the NewComputed
exemption over to exempting values that come from diff customization.
2017-12-19 16:06:57 -08:00
James Bardin 643ef4334f revert the change that broke the test case
This reverts one of the changes from 6a4f7b0, which broke empty strings
being seen as unset for computed values.

This breaks a number of other tests, and is only an intermediate change
for evaluating other solutions.
2017-12-19 16:14:07 -05:00
James Bardin 7a8a443994 add failing test case
This case should be expected to fail with the current diff algorithm,
but the existing behavior was widely relied upon so we need to roll this
back until there is a representable nil value.
2017-12-19 15:14:58 -05:00
Martin Atkins c647b22d97 helper/customdiff: Helper functions for CustomizeDiff
The CustomizeDiff functionality in helper/schema is powerful, but directly
writing single CustomizeDiff functions can obscure the intent when a
number of different, orthogonal diff-customization behaviors are required.

This new library provides some building blocks that aim to allow a more
declarative form of CustomizeDiff implementation, by composing a number of
smaller operations. For example:

     &schema.Resource{
         // ...
         CustomizeDiff: customdiff.All(
             customdiff.ValidateChange("size", func (old, new, meta interface{}) error {
                 // If we are increasing "size" then the new value must be
                 // a multiple of the old value.
                 if new.(int) <= old.(int) {
                     return nil
                 }
                 if (new.(int) % old.(int)) != 0 {
                     return fmt.Errorf("new size value must be an integer multiple of old value %d", old.(int))
                 }
                 return nil
             }),
             customdiff.ForceNewIfChange("size", func (old, new, meta interface{}) bool {
                 // "size" can only increase in-place, so we must create a new resource
                 // if it is decreased.
                 return new.(int) < old.(int)
             }),
             customdiff.ComputedIf("version_id", func (d *schema.ResourceDiff, meta interface{}) bool {
                 // Any change to "content" causes a new "version_id" to be allocated.
                 return d.HasChange("content")
             }),
         ),
     }

The goal is to allow the various separate operations to be quickly seen
and to ensure that each of them runs independently of the others. These
functions all create closures on the call parameters, so the result is
still just a normal CustomizeDiffFunc and so the helpers in this package
can be combined with hand-written functions as needed.

As we get more experience writing CustomizeDiff functions we may wish to
expand the repertoire of functions here in future; this initial set
attempts to cover some common cases we've seen so far. We may also
investigate some helper functions that are entirely declarative and so
don't take callback functions at all, but want to learn what the relevant
use-cases are before going in too deep here.
2017-12-18 10:38:20 -08:00
Stuart Auld 1d1984771b helper/resource: test check helpers for resources in non-root modules 2017-12-18 05:58:10 -08:00
Chris Marchesi 85119304b3
Merge pull request #16895 from hashicorp/b-resource-test-expecterror
helper/resource: Fail tests with no error that have ExpectError set
2017-12-11 14:18:55 -08:00
Chris Marchesi 3f8dad30c9
helper/resource: Fail tests with no error that have ExpectError set
Looks like while we were checking errors correctly when ExpectError was
set, we weren't checking for the *absence* of an error, which is should
be checked as well (no error is still not the error we are looking for).

Added a few more tests for ExpectError as well.
2017-12-11 14:05:40 -08:00
Rob Campbell 5daeee5f6d Update various files for new version of "stringer"
The latest version of stringer now uses strconv instead of fmt.
2017-12-11 13:26:29 -08:00
Chris Marchesi 8e57c4361c
Merge pull request #16774 from hashicorp/f-validation-stringmatch
helper/validation: Add StringMatch
2017-12-04 16:47:23 -08:00
Chris Marchesi 5da62a44f4
helper/validation: Fix comment on StringMatch
To match the current implementation.
2017-12-01 10:52:25 -08:00
Martin Atkins ba0514106a return tfdiags.Diagnostics from validation methods
Validation is the best time to return detailed diagnostics
to the user since we're much more likely to have source
location information, etc than we are in later operations.

This change doesn't actually add any detail to the messages
yet, but it changes the interface so that we can gradually
introduce more detailed diagnostics over time.

While here there are some minor adjustments to some of the
messages to improve their consistency with terminology we
use elsewhere.
2017-11-28 11:15:29 -08:00
Chris Marchesi e33aa9dc2e
helper/validation: Add StringMatch
StringMatch returns a validation function that can be used to match a
string against a regular expression. This can be used for simple
substring validations or more complex validation scenarios. Optionally,
an error message can be returned so that the user is returned a better
error message other than that their field did not match a regular
expression that they might not be able to understand.
2017-11-27 17:16:15 -08: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
Nándor István Krácser e5cc8af7f3 helper/resource: fix ungrammatical doc comment in StateChangeConf 2017-11-08 09:43:36 -08:00
Radek Simko e93d64b18c
helper/schema: Opt-in panic on invalid ResourceData.Set 2017-11-08 10:05:11 +00:00
Chris Marchesi 3fa11e456b
helper/schema: Clear existing map/set/list contents before overwriting
There are situations where one may need to write to a set, list, or map
more than once per single TF operation (apply/refresh/etc). In these
cases, further writes using Set (example: d.Set("some_set", newSet))
currently create unstable results in the set writer (the name of the
writer layer that holds the data set by these calls) because old keys
are not being cleared out first.

This bug is most visible when using sets. Example: First write to set
writes elements that have been hashed at 10 and 20, and the second write
writes elements that have been hashed at 30 and 40. While the set length
has been correctly set at 2, since a set is basically a map (as is the
entire map writer) and map results are non-deterministic, reads to this
set will now deliver unstable results in a random but predictable
fashion as the map results are delivered to the caller non-deterministic
- sometimes you may correctly get 30 and 40, but sometimes you may get
10 and 20, or even 10 and 30, etc.

This problem propagates to state which is even more damaging as unstable
results are set to state where they become part of the permanent data
set going forward.

The problem also applies to lists and maps. This is probably more of an
issue with maps as a map can contain any key/value combination and hence
there is no predictable pattern where keys would be overwritten with
default or zero values. This is contrary to complex lists, which has
this problem as well, but since lists are deterministic and the length
of a list properly gets updated during the overwrite, the problem is
masked by the fact that a read will only read to the boundary of the
list, skipping any bad data that may still be available due past the
list boundary.

This update clears the child contents of any set, list, or map before
beginning a new write to address this issue. Tests are included for all
three data types.
2017-11-05 12:04:23 -08:00
Chris Marchesi 0c0ae3ca7c helper/schema: CustomizeDiff allowed on writable resources only
This keeps CustomizeDiff from being defined on data sources, where it
would be useless. We just catch this in InternalValidate like the rest
of the CRUD functions that are not used in data sources.
2017-11-01 15:42:24 -07:00
Chris Marchesi 2c541e8c97 helper/schema: Add behaviour detail to various custom diff comments
Added some more detailed comments to CustomizeDiff's comments. The new
comments detail how CustomizeDiff will be called in the event of
different scenarios like creating a new resource, diffing an existing
one, diffing an existing resource that has a change that requires a new
resource, and destroy/tainted resources.

Also added similar detail to ForceNew in ResourceDiff.

This should help mitigate any confusion that may come up when using
CustomizeDiff, especially in the ForceNew scenario when the second run
happens with no state.
2017-11-01 15:42:24 -07:00
Chris Marchesi f5bdbc0911 helper/schema: Simplify setDiff, remove exported SetDiff mentions
setDiff does not make use of its new parameter anymore, so it has been
removed. Also, as there is no more SetDiff (exported) function, mentions
of that have been removed from comments too.
2017-11-01 14:25:32 -07:00
Chris Marchesi 3ac0cdf0c0 helper/schema: Better ResourceDiff schema key validation
This fixes nil pointer issues that could come up if an invalid key was
referenced (ie: not one in the schema). Also ships a helper validation
function to streamline things.
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 09e2109ff8 helper/schema: Resouce.Diff no longer ResourceProvider API compatible
The old comments said that this interface was API compatible with
terraform.ResourceProvider's Diff method - with the addition of passing
down meta to it, this is no longer the case.

Not too sure if this is really a big deal - schema.Resource never fully
implemented terraform.ResourceProvider, from what I can see, and the
path from Provdier.Diff to Resource.Diff is still pretty clear. Just
wanted to remove an outdated comment.
2017-11-01 14:25:32 -07:00
Chris Marchesi 9625830980 helper/schema: Move computedKeys init to init function
This simplifies the new value initialization and future-proofs it
against more complex initialization functionality.
2017-11-01 14:25:32 -07:00
Chris Marchesi f0aafe4d67 helper/schema: Restore new value for complex set test 2017-11-01 14:25:32 -07:00
Chris Marchesi 3444549d60 helper/schema: Move computed key reader logic to childAddrOf
This will make this check safer and will remove the risk of it passing
on keys with a similar prefix.
2017-11-01 14:25:32 -07:00
Chris Marchesi 36aa63b338 helper/schema: Guard against out of range on childAddrOf
This could panic if we sent a parent that was actually longer than the
child (should almost never come up, but the guard will make it safe
anyway).

Also fixed a slice style lint warning in UpdatedKeys.
2017-11-01 14:25:32 -07:00
Chris Marchesi 931b0e1441 helper/schema: Remove exported SetDiff method
The consensus is that it's a generally better idea to move setting the
functionality of old values completely to the refresh/read process,
hence it's moot to have this function around anymore. This also means we
don't need the old value reader/writer anymore, which simplifies things.
2017-11-01 14:25:32 -07:00
Chris Marchesi ad98471559 helper/schema: Correct some comments
Removed some outdated comments for SetNew, and normalized the computed
key note for SetNew, SetNewComputed, and SetDiff.
2017-11-01 14:25:32 -07:00
Chris Marchesi 8a7c9a6f02 helper/schema: frist -> first 2017-11-01 14:25:32 -07:00
Chris Marchesi 7d5f9ed6b1 helper/schema: NewComputed values should be nil
When working on this initially, I think I thought that since NewComputed
values in the diff were empty strings, that it was using the zero value.
After review, it doesn't seem like this is the case - so I have adjusted
NewComputed to pass nil values. There is also a guard now that keeps the
new value writer from accepting computed fields with non-nil values.
2017-11-01 14:25:32 -07:00
Chris Marchesi 6f422d8c44 helper/schema: Remove unused log line
Meant to remove this before finalizing the PR. :P
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 f7e42728b6 helper/schema: Remove unused Destroy check for CustomizeDiff
Both Destroy and DestroyDeposed are not propagated down the diff stack,
meaning that there is no way we can tell at this point if an instance is
being destroyed or deposed, so this check would never be used.

In this regard, Destroy never runs a diff down the stack at all, and a
deposition check is not run until *after* the provider's diff function
is called. To answer this question and close it off, we could either
determine if a resource is deposed earlier, and propagate that down, or
treat deposed resources like full destroy nodes, and not diff them at
all (but rather making a diff with the only thing in it being
DestroyDeposed flagged).
2017-11-01 14:25:32 -07:00
Chris Marchesi fa1fc2ca8e helper/schema: CustomizeDiff invocation test
Just one more test to check to make sure that CustomizeDiff is called
on resource level.
2017-11-01 14:25:32 -07:00
Chris Marchesi ee769188d7 helper/schema: More CustomizeDiff test cases
Added a few more test cases for CustomizeDiff, caught another error in
the process. I think this is ready for review now, and possibly some
real-world testing of the waters by way of porting some resources that
would benefit from the feature.
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
Chris Marchesi 8126ee8401 helper/schema: Fix supplied config to tests 2017-11-01 14:25:32 -07:00
Chris Marchesi 196d7e63fe helper/schema: Drop ClearAll from ResourceDiff
As the interface has been specced out for ResourceDiff, the only diff
operation that can function on a non-computed key is ForceNew, and that
is only if a diff exists for that key. As such, allowing the user to
clear keys that cannot be operated on afterwards does not really make
much sense.

Will re-add this function if it is determined it's needed after all on
review.
2017-11-01 14:25:32 -07:00
Chris Marchesi 22220fd0f7 helper/schema: Final set of ResourceDiff tests, bug fixes
Final set of coverage for ResourceDiff and bug fixes to correct issues
the test cases brought up.

Will be dropping ClearAll in next commit, as with how this interface is
intended to be used, it does not really make much sense.
2017-11-01 14:25:32 -07:00
Chris Marchesi a5fc664ea6 helper/schema: Add schemaMap.DeepCopy
This provides a deep copy of a schemaMap, which will be needed for the
diff customization process as ResourceDiff will be able to flag fields
as ForceNew - we don't want to affect the source schema.
2017-11-01 14:25:32 -07:00
Chris Marchesi 6a4f7b0dce helper/schema: Don't ignore diffs for certain NewComputed keys
In diffString, removed values are marked if the old value is non-nil and
the new value is nil, regardless of if the new value is marked as a
computed value. With the new diff override behaviour, this can lead to
issues where a nil attribute may get inserted into the diff if the new
value has been marked as computed (as the value will be marked as
missing, but computed).

This propagates into finalizeDiff in some respects because even if
NewComputed is manually set in the diff that gets passed in, it is
ignored, and the schema becomes the only source of truth. Since our new
diff behaviour is mainly designed to be supported on computed keys only,
it stands to reason that we there will be a time where we want to set a
diff as NewComputed on a key, and see that change in the diff.

These two small changes makes that happen. No regressions in tests have
been observed via this change, so it seems safe and non-invasive.
2017-11-01 14:25:32 -07:00