Commit Graph

933 Commits

Author SHA1 Message Date
James Bardin ac63d2995f
Merge pull request #19559 from hashicorp/jbardin/resource-test-shim
don't add numeric indexes to resources with a count of 0
2018-12-05 17:34:30 -05:00
James Bardin 7d296f752c don't add numeric indexes to resources with a count of 0 2018-12-05 13:41:53 -05:00
Brian Flad 1e81a3e7fa
helper/schema: Always propagate NewComputed for previously zero value primative type attributes
When the following conditions were met:
* Schema attribute with a primative type (e.g. Type: TypeString) and Computed: true
* Old state of attribute set to zero value for type (e.g. "")
* Old state ID of resource set to non-empty (e.g. existing resource)

Attempting to use CustomizeDiff with SetNewComputed() would result in the difference  previously being discarded. This update ensures that previous zero values or resource existence does not influence the propagation of the computed update.

Previously:

```
--- FAIL: TestSetNewComputed (0.00s)
    --- FAIL: TestSetNewComputed/NewComputed_should_always_propagate (0.00s)
        resource_diff_test.go:684: Expected (*terraform.InstanceDiff)(0xc00051cea0)({
             mu: (sync.Mutex) {
              state: (int32) 0,
              sema: (uint32) 0
             },
             Attributes: (map[string]*terraform.ResourceAttrDiff) (len=1) {
              (string) (len=3) "foo": (*terraform.ResourceAttrDiff)(0xc0003dcec0)({
               Old: (string) "",
               New: (string) "",
               NewComputed: (bool) true,
               NewRemoved: (bool) false,
               NewExtra: (interface {}) <nil>,
               RequiresNew: (bool) false,
               Sensitive: (bool) false,
               Type: (terraform.DiffAttrType) 0
              })
             },
             Destroy: (bool) false,
             DestroyDeposed: (bool) false,
             DestroyTainted: (bool) false,
             Meta: (map[string]interface {}) <nil>
            })
            , got (*terraform.InstanceDiff)(0xc00051ce80)({
             mu: (sync.Mutex) {
              state: (int32) 0,
              sema: (uint32) 0
             },
             Attributes: (map[string]*terraform.ResourceAttrDiff) {
             },
             Destroy: (bool) false,
             DestroyDeposed: (bool) false,
             DestroyTainted: (bool) false,
             Meta: (map[string]interface {}) <nil>
            })

--- FAIL: TestSchemaMap_Diff (0.01s)
    --- FAIL: TestSchemaMap_Diff/79-NewComputed_should_always_propagate_with_CustomizeDiff (0.00s)
        schema_test.go:3289: expected:
            *terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{"foo":*terraform.ResourceAttrDiff{Old:"", New:"", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, Meta:map[string]interface {}(nil)}

            got:
            <nil>

FAIL
FAIL  github.com/hashicorp/terraform/helper/schema  0.825s
```
2018-12-04 22:48:30 -05:00
James Bardin 924b97238f Handle StateFuncs in provider shim
Any state modifying functions can only be run once during the plan-apply
cycle. When regenerating the Diff during ApplyResourceChange, strip out
all StateFunc and CustomizeDiff functions from the schema.

Thew NewExtra diff field was where config data that was modified by a
StateFunc was stored, and needs to be maintained between plan and apply.

During PlanResourceChange, store any NewExtra data from the Diff in the
PlannedPrivate data, and re-insert the NewExtra data into the Diff
generated during ApplyResourceChange.
2018-12-03 18:12:02 -05:00
James Bardin 3dacdba678
Merge pull request #19521 from hashicorp/jbardin/prepare-provider-config
catch conversion errors in PrepareProviderConfig
2018-11-30 15:32:56 -05:00
James Bardin 5f9b189fcf catch conversion errors in PrepareProviderConfig
Errors were being ignore with the intention that they would be caught
later in validation, but it turns out we nee dto catch those earlier.

The legacy schemas also allowed providers to set and empty string for a
bool value, which we need to handle here, since it's not being handled
from user input like a normal config value.
2018-11-30 14:51:52 -05:00
Martin Atkins 72e279e6b2 providers: Consistently use int64 for schema versions
The rest of Terraform is still using uint64 for this in various spots, but
we'll update that gradually later. We use int64 here because that matches
what's used in our protobuf definition, and unsigned integers are not
portable across all of the protobuf target languages anyway.
2018-11-30 11:22:39 -08:00
James Bardin 6daf4989d4
Merge pull request #19475 from hashicorp/jbardin/computed-containers
Shim computed containers
2018-11-27 09:15:23 -05:00
James Bardin 6f4d86094f preserve possible zero values when normalizing
When normalizing flatmapped containers, compare the attributes to the
prior state and preserve pre-existing zero-length or unknown values. A
zero-length value that was previously unknown is preserved as a
zero-length value, as that may have been computed as such by the
provider.
2018-11-27 08:54:15 -05:00
Martin Atkins 58fa38b89a helper/schema: Update docs for PromoteSingle
This is no longer effective and should not be used in any new schema.
2018-11-26 17:11:34 -08:00
Martin Atkins 3d54af9c09 helper/schema: Better mimic some undocumented behaviors in Core schema
Since the SDK's schema system conflates attributes and nested blocks, it's
possible to state some nonsensical schema situations such as:

- A nested block is both optional but has MinItems > 0
- A nested block is entirely computed but has MinItems or MaxItems set

Both of these weird situations are handled here in the same way that the
existing helper/schema validation code would've handled them: by
effectively disabling the MinItems/MaxItems checks where they would've
been ignored before.

the MinItems/MaxItems
2018-11-26 17:11:34 -08:00
Martin Atkins 37da625ee9 helper/schema: Tell Core attribute is optional if set conditionally
The SDK has a mechanism that effectively makes it possible to declare an
attribute as being _conditionally_ required, which is not a concept that
Terraform Core is aware of.

Since this mechanism is in practice only used for a small UX improvement
in prompting for these values interactively when the environment variable
is not set, we avoid here introducing all of this complexity into the
plugin protocol by just having the provider selectively modify its schema
if it detects that such an attribute might be set dynamically.

This then prevents Terraform Core from validating the presence of the
argument or prompting for a new value for it, allowing the null value to
pass through into the provider so that the default value can be generated
again dynamically.

This is a kinda-kludgey solution which we're accepting here because the
alternative would be a much-more-complex two-pass decode operation within
Core itself, and that doesn't seem worth it.

This fixes #19139.
2018-11-26 17:11:34 -08:00
James Bardin f375691819 add missing key-value from test 2018-11-19 18:58:29 -05:00
Martin Atkins 4fe9632f09 plugin: Establish our current plugin protocol as version 5
The main significant change here is that the package name for the proto
definition is "tfplugin5", which is important because this name is part
of the wire protocol for references to types defined in our package.

Along with that, we also move the generated package into "internal" to
make it explicit that importing the generated Go package from elsewhere is
not the right approach for externally-implemented SDKs, which should
instead vendor the proto definition they are using and generate their
own stubs to ensure that the wire protocol is the only hard dependency
between Terraform Core and plugins.

After this is merged, any provider binaries built against our
helper/schema package will need to be rebuilt so that they use the new
"tfplugin5" package name instead of "proto".

In a future commit we will include more elaborate and organized
documentation on how an external codebase might make use of our RPC
interface definition to implement an SDK, but the primary concern here
is to ensure we have the right wire package name before release.
2018-11-19 09:56:41 -08:00
James Bardin e95f2b586e another test case in helper/plugin 2018-11-16 15:12:16 -05:00
James Bardin 3716db3865
Merge pull request #19384 from hashicorp/jbardin/nested-sets
New Attribute and Diff handling in shims
2018-11-16 11:55:41 -05:00
James Bardin 89b2c6f21e comment fixes 2018-11-16 11:24:14 -05:00
James Bardin 17ecda53b5 strip empty containers from flatmap attributes
In order to prevent mismatched states between read/plan/apply, we need
to ensure that the attributes are generated consistently each time.
Because of the various ways in which helper/schema and the hcl2 shims
interpret empty values, the only way to ensure consistency is to always
remove them altogether.
2018-11-16 09:59:03 -05:00
James Bardin 21dfa56766 use ShimInstanceStateFromValue in DiffFromValues
This makes sure the diff is generated with the matching set ids from
helper/schema.

Update the tests to add ID fields to the state, which will exists in
practice, since any state traversing through the shims will have the ID
inserted.
2018-11-16 09:59:03 -05:00
James Bardin 34766ca666 use the new InstanceState shim 2018-11-16 09:59:03 -05:00
James Bardin df04e2e7a6 move InstanceState shim into schema.Resource
This was the resource can rebuild the flatmapped state using the
schema and ResourceData, providing us the the correct set key values.
2018-11-16 09:59:03 -05:00
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