Commit Graph

550 Commits

Author SHA1 Message Date
Brian Flad 3d908f56aa
helper/schema: Add deprecation to ResourceData.UnsafeSetFieldRaw
This functionality is no longer supported in Terraform 0.12 and above.
2019-02-13 22:12:10 -05:00
James Bardin 8be864c1c7 don't allow computed set elems to be equal
If set elements are computed, we can't be certain that they are actually
equal. Catch identical computed set hashes when they are added to the
set, and alter the set key slightly to keep the set counts correct.

In previous versions the interpolation string would be included in the
set, and different string values would cause the set to hash
differently, so this is change is only activated for the new protocol.
2019-02-05 12:08:17 -05:00
James Bardin 55b4307767 add proto5 feature flag
Add feature flag to allow special proto 5 behavior in helper/schema.
This is Meant to be used as a last resort for shim-related bugs.
2019-02-05 12:08:16 -05:00
James Bardin 81a4e705b1 DiffSuppressFunc should noop diffs in sets
Sets rely on diffs being complete for all elements, even when they are
unchanged. When encountering a DiffSuppressFunc inside a set the diffs
were being dropped entirely, possible causing set elements to be lost.
2019-02-05 12:08:16 -05:00
James Bardin 9cf8f48239 decode legacy timeouts
The new decoder is more precise, and unpacks the timeout block into a
single map, which ResourceTimeout.ConfigDecode was updated to handle.
We however still need to work with legacy versions of terraform, with
the old decoder.
2019-01-30 16:10:17 -05:00
Paul Tyng bb9ae50279
Copy TF version to helper/schema provider 2019-01-28 14:38:49 -05:00
Martin Atkins ae0be75ae0 helper/schema: TypeMap of Resource is actually of TypeString
Historically helper/schema did not support non-primitive map attributes
because they cannot be represented unambiguously in flatmap. When we
initially implemented CoreConfigSchema here we mapped that situation to
a nested block of mode NestingMap, even though that'd never worked until
now, assuming that it'd be harmless because providers wouldn't be using
it.

It turns out that some providers are, in fact, incorrectly populating
a TypeMap schema with Elem: &schema.Resource, apparently under the false
assumption that it would constrain the keys allowed in the map. In
practice, helper/schema has just been ignoring this and treating such
attributes as map of string. (#20076)

In order to preserve the behavior of these existing incorrectly-specified
attribute definitions, here we mimic the helper/schema behavior by
presenting as an attribute of type map(string).

These attributes have also been shown in some documentation as nested
blocks (with no equals sign), so that'll need to be fixed in user
configurations as they upgrade to Terraform 0.12. However, the existing
upgrade tool rules will take care of that as a natural consequence of the
name being indicated as an attribute in the schema, rather than as a block
type.

This fixes #20076.
2019-01-25 14:12:58 -08:00
James Bardin bc5eecd7f2 make sure id really gets set in SetId
SetId needs to overwrite the newState as well, since the internal calls
to DataSource.Id() will override the set attribute.
2019-01-10 20:28:11 -05:00
Martin Atkins 06acc3f6c8 helper/schema: Skip validation of unknown values
With the introduction of explicit "null" in 0.12 it's possible for a value
that is unknown during plan to become a known null during apply, so we
need to slightly weaken our validation rules to accommodate that, in
particular skipping the validation of conflicting attributes if the result
could potentially be valid after the unknown values become known.

This change is in the codepath that is common to both 0.12 and 0.11
callers, but that's safe because 0.11 re-runs validation during the apply
step and so will still catch problems here, albeit in the apply step
rather than in the plan step, thus matching the 0.12 behavior. This new
behavior is a superset of the old in the sense that everything that was
valid before is still valid.

The implementation here also causes us to skip all other validation for
an attribute whose value is unknown. Most of the downstream validation
functions handle this directly anyway, but again this doesn't add any new
failure cases, and should clean up some of the rough edges we've seen with
unknown values in 0.11 once people upgrade to 0.12-compatible providers.
Any issues we now short-circuit during planning will still be caught
during apply.

While working on this I found that the existing "Not a list" test was not
actually testing the correct behavior, so this also includes a tweak to
that to ensure that it really is checking the "should be a list" path
rather than the "cannot be set" codepath it was inadvertently testing
before.
2019-01-04 14:46:47 -08:00
James Bardin 8ab5698e2a
Merge pull request #19587 from hashicorp/jbardin/safe-appends
don't modify argument slices
2018-12-10 15:10:02 -05:00
James Bardin 3d6ec09a83
Merge pull request #19552 from olindata/bugfix/setting-sets-in-list
helper/schema: Fix setting a set in a list caused error
2018-12-10 12:25:23 -05:00
James Bardin b5de50c0a2 don't modify argument slices
There were a couple spots where argument slices weren't being copied
before `append` was called, which could possibly modify the caller's
slice data.
2018-12-10 11:59:27 -05:00
Farid Neshat 44a45b7332 helper/schema: Fix setting a set in a list
The added test in this commit, without the fix, will make d.Set return
the following error:

`Invalid address to set: []string{"ports", "0", "set"}`

This was due to the fact that setSet in feild_writer_map tried to
convert a slice into a set by creating a temp set schema and calling
writeField on that with the address(`[]string{"ports", "0", "set"}"` in
this case). However the temp schema was only for the set and not the
whole schema as seen in the address so, it should have been `[]string{"set"}"`
so it would align with the schema.

This commits adds another variable there(tempAddr) which will only
contain the last entry of the address that would be the set key, which
would match the created schema

This commit potentially fixes the problem described in #16331
2018-12-05 10:09:54 +01: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
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
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 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 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
Radek Simko 0cbf745e5a
helper/schema: Avoid erroring out on undefined timeouts 2018-11-07 15:38:58 +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 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 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 d50a152f8b check for a nil diff in simpleDiff 2018-10-19 14:25:20 -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 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
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
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