Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

helper/schema: ConfigMode field in *Schema #20626

Merged
merged 2 commits into from
Mar 12, 2019

Conversation

apparentlymart
Copy link
Contributor

@apparentlymart apparentlymart commented Mar 9, 2019

This allows a provider developer slightly more control over how an SDK schema is mapped into the Terraform configuration language, overriding some default assumptions.

ConfigMode overrides the default assumption that a schema with an Elem of type *Resource is to be mapped to configuration as a nested block, allowing mapping as an attribute containing an object type instead.

This should rarely be needed, but it can be useful in situations where the provider was assuming attribute-like behaviors in 0.11 (such as ability to explicitly assign an empty list) which were possible because 0.11 didn't enforce the use of block syntax for nested resources. For some real examples, see #20505.

Since this uses the attribute syntax, this'll work best for SDK schema attributes that already had plural names, suggesting that they were representing a single attribute containing multiple objects:

  thingies = [
    {
      foo = "bar"
    },
  ]
  thingies = []

Back-compatibility with Terraform 0.11 may require this to be used with singular-named attributes in situations where prevailing usage was inconsistent about whether a particular name was an attribute or a block, which is less-ideal but can be used in a pinch to avoid or defer a breaking change:

  thingy = [
    {
      foo = "bar"
    },
    {
      baz = "boop"
    },
  ]

Along with the explicit forcing of attribute mode in config, this also includes an implicit rule that Computed-only (that is, not also Optional) attributes are always represented as attributes, since it is confusing and strange to use the configuration-only idea of nested block syntax for an attribute that can't be specified in configuration anyway. This results in better UX in the terraform plan output because the exact structure of the result can be shown, rather than confusing the user by showing the result as a nested block diff when no such nested block was present.

These behaviors only apply when a provider is being used with Terraform v0.12 or later. They are ignored altogether in Terraform v0.11 mode, since attribute vs. block syntax is not enforced in v0.11 anyway. We are adding this primarily to allow the v0.12 version of a resource type schema to be specified to match the prevailing usage of it in existing configurations, in situations where the default mapping to v0.12 concepts is not appropriate.

This is split into two commits because the first commit will need to be cherry-picked onto the v0.11 branch to enable the downgrade-and-acctest regression testing strategy for ensuring that provider logic continues to behave correctly with v0.11 as future changes are made. I will do that as soon as this is merged, so approval of this PR also implies approval to place the first commit on the v0.11 branch (just to avoid a pointness second review round-trip.)

@apparentlymart apparentlymart added this to the v0.12.0 milestone Mar 9, 2019
@apparentlymart apparentlymart requested a review from a team March 9, 2019 00:41
This allows a provider developer slightly more control over how an SDK
schema is mapped into the Terraform configuration language, overriding
some default assumptions.

ConfigMode overrides the default assumption that a schema with
an Elem of type *Resource is to be mapped to configuration as a nested
block, allowing mapping as an attribute containing an object type instead.

These behaviors only apply when a provider is being used with Terraform
v0.12 or later. They are ignored altogether in Terraform v0.11 mode, to
preserve compatibility. We are adding these primarily to allow the v0.12
version of a resource type schema to be specified to match the prevailing
usage of it in existing configurations, in situations where the default
mapping to v0.12 concepts is not appropriate.

This commit adds only the fields themselves and the InternalValidate rules
for them. A subsequent commit for Terraform v0.12 will add the behavior
as part of the protocol version 5 shim layer.
This makes some slight adjustments to the shape of the schema we
present to Terraform Core without affecting how it is consumed by the
SDK and thus the provider. This mechanism is designed specifically to
avoid changing how the schema is interpreted by the SDK itself or by the
provider, so that prior behavior can be preserved in Terraform v0.11 mode.

This also includes a new rule that Computed-only (i.e. not also Optional)
schemas _always_ map to attributes, because that is a better mapping of
the intent: they are object values to be used in expressions. Nested
blocks conceptually represent nested objects that are in some sense
independent of what they are embedded in, and so they cannot themselves be
computed.
@apparentlymart apparentlymart force-pushed the f-sdk-subresource-as-attr branch from 555b9f1 to adcbae8 Compare March 11, 2019 21:04
@apparentlymart apparentlymart merged commit 4de0b33 into master Mar 12, 2019
bflad added a commit to hashicorp/terraform-provider-aws that referenced this pull request Apr 3, 2019
…e150daa0

Required for new `ConfigMode` schema parameter.

References:

* hashicorp/terraform#20505
* hashicorp/terraform#20626

Updated via:

```
go get github.com/hashicorp/terraform@44702fa6c163391fe7295b6004a06cefe150daa0
go mod tidy
go mod vendor
```
bflad added a commit to hashicorp/terraform-provider-aws that referenced this pull request Apr 3, 2019
…ess configuration block attributes

References:

* hashicorp/terraform#20505
* hashicorp/terraform#20626

In Terraform 0.12, behaviors with configuration blocks are more explicit to allow configuration language improvements and remove ambiguities that required various undocumented workarounds in Terraform 0.11 and prior. As a consequence, configuration blocks that are marked `Optional: true` and `Computed: true` will no longer support explicitly zero-ing out the configuration without special implementation.

The `ConfigMode` schema parameter on the configuration block attribute allows the schema to override certain behaviors. In particular, setting `ConfigMode: schema.SchemaConfigModeAttr` will allow practitioners to continue setting the configuration block attribute to an empty list (in the semantic sense, e.g. `attr = []`), keeping this prior behavior in Terraform 0.12. This parameter does not have any effect with Terraform 0.11. This solution may be temporary or revisited in the future.

Previous output from Terraform 0.11:

```
--- PASS: TestAccAWSNetworkAcl_Egress_ConfigMode (53.47s)
--- PASS: TestAccAWSNetworkAcl_Ingress_ConfigMode (51.81s)
```

Previous output from Terraform 0.12:

```
--- FAIL: TestAccAWSNetworkAcl_Egress_ConfigMode (38.74s)
    testing.go:568: Step 4 error: config is invalid: Unsupported argument: An argument named "egress" is not expected here. Did you mean "ingress"?

--- FAIL: TestAccAWSNetworkAcl_Ingress_ConfigMode (38.89s)
    testing.go:568: Step 4 error: config is invalid: Unsupported argument: An argument named "ingress" is not expected here. Did you mean to define a block of type "ingress"?
```

Output from Terraform 0.11:

```
--- PASS: TestAccAWSNetworkAcl_Egress_ConfigMode (52.88s)
--- PASS: TestAccAWSNetworkAcl_Ingress_ConfigMode (52.70s)
```

Output from Terraform 0.12:

```
--- PASS: TestAccAWSNetworkAcl_Egress_ConfigMode (52.43s)
--- PASS: TestAccAWSNetworkAcl_Ingress_ConfigMode (53.60s)
```
bflad added a commit to hashicorp/terraform-provider-aws that referenced this pull request Apr 4, 2019
…ngress configuration block attributes

References:

* hashicorp/terraform#20505
* hashicorp/terraform#20626

In Terraform 0.12, behaviors with configuration blocks are more explicit to allow configuration language improvements and remove ambiguities that required various undocumented workarounds in Terraform 0.11 and prior. As a consequence, configuration blocks that are marked `Optional: true` and `Computed: true` will no longer support explicitly zero-ing out the configuration without special implementation.

The `ConfigMode` schema parameter on the configuration block attribute allows the schema to override certain behaviors. In particular, setting `ConfigMode: schema.SchemaConfigModeAttr` will allow practitioners to continue setting the configuration block attribute to an empty list (in the semantic sense, e.g. `attr = []`), keeping this prior behavior in Terraform 0.12. This parameter does not have any effect with Terraform 0.11. This solution may be temporary or revisited in the future.

Previous output from Terraform 0.11:

```
--- PASS: TestAccAWSSecurityGroup_Egress_ConfigMode (50.28s)
--- PASS: TestAccAWSSecurityGroup_Ingress_ConfigMode (49.85s)
```

Previous output from Terraform 0.12:

```
--- FAIL: TestAccAWSSecurityGroup_Egress_ConfigMode (31.74s)
    testing.go:568: Step 2 error: config is invalid: Unsupported argument: An argument named "egress" is not expected here. Did you mean to define a block of type "egress"?

--- FAIL: TestAccAWSSecurityGroup_Ingress_ConfigMode (31.94s)
    testing.go:568: Step 2 error: config is invalid: Unsupported argument: An argument named "ingress" is not expected here. Did you mean to define a block of type "ingress"?
```

Output from Terraform 0.11:

```
--- PASS: TestAccAWSSecurityGroup_Egress_ConfigMode (52.27s)
--- PASS: TestAccAWSSecurityGroup_Ingress_ConfigMode (53.55s)
```

Output from Terraform 0.12:

```
--- PASS: TestAccAWSSecurityGroup_Egress_ConfigMode (57.20s)
--- PASS: TestAccAWSSecurityGroup_Ingress_ConfigMode (55.80s)
```
bflad added a commit to hashicorp/terraform-provider-aws that referenced this pull request Apr 4, 2019
…tion block attributes

References:

* hashicorp/terraform#20505
* hashicorp/terraform#20626

In Terraform 0.12, behaviors with configuration blocks are more explicit to allow configuration language improvements and remove ambiguities that required various undocumented workarounds in Terraform 0.11 and prior. As a consequence, configuration blocks that are marked `Optional: true` and `Computed: true` will no longer support explicitly zero-ing out the configuration without special implementation.

The `ConfigMode` schema parameter on the configuration block attribute allows the schema to override certain behaviors. In particular, setting `ConfigMode: schema.SchemaConfigModeAttr` will allow practitioners to continue setting the configuration block attribute to an empty list (in the semantic sense, e.g. `attr = []`), keeping this prior behavior in Terraform 0.12. This parameter does not have any effect with Terraform 0.11. This solution may be temporary or revisited in the future.

Previous output from Terraform 0.11:

```
--- PASS: TestAccAWSRouteTable_Route_ConfigMode (66.53s)
```

Previous output from Terraform 0.12:

```
--- FAIL: TestAccAWSRouteTable_Route_ConfigMode (36.11s)
    testing.go:568: Step 2 error: config is invalid: Unsupported argument: An argument named "route" is not expected here. Did you mean to define a block of type "route"?
```

Output from Terraform 0.11:

```
--- PASS: TestAccAWSRouteTable_Route_ConfigMode (67.27s)
```

Output from Terraform 0.12:

```
--- PASS: TestAccAWSRouteTable_Route_ConfigMode (69.03s)
```
bflad added a commit to hashicorp/terraform-provider-aws that referenced this pull request Apr 6, 2019
…ion block attribute

References:

* hashicorp/terraform#20505
* hashicorp/terraform#20626

In Terraform 0.12, behaviors with configuration blocks are more explicit to allow configuration language improvements and remove ambiguities that required various undocumented workarounds in Terraform 0.11 and prior. As a consequence, configuration blocks that are marked `Optional: true` and `Computed: true` will no longer support explicitly zero-ing out the configuration without special implementation.

The `ConfigMode` schema parameter on the configuration block attribute allows the schema to override certain behaviors. In particular, setting `ConfigMode: schema.SchemaConfigModeAttr` will allow practitioners to continue setting the configuration block attribute to an empty list (in the semantic sense, e.g. `attr = []`), keeping this prior behavior in Terraform 0.12. This parameter does not have any effect with Terraform 0.11. This solution may be temporary or revisited in the future.

Previous output from Terraform 0.11:

```
--- PASS: TestAccAWSEMRCluster_Step_ConfigMode (828.01s)
```

Previous output from Terraform 0.12:

```
--- FAIL: TestAccAWSEMRCluster_Step_ConfigMode (390.69s)
    testing.go:568: Step 4 error: config is invalid: Unsupported argument: An argument named "step" is not expected here. Did you mean to define a block of type "step"?
```

Output from Terraform 0.11:

```
--- PASS: TestAccAWSEMRCluster_Step_ConfigMode (764.03s)
```

Output from Terraform 0.12:

```
--- PASS: TestAccAWSEMRCluster_Step_ConfigMode (833.49s)
```
minamijoyo added a commit to minamijoyo/tfschema that referenced this pull request May 21, 2019
Fixes #9

Terraform v0.12 introduced a new `SchemaConfigModeAttr` feature.
Most attributes have simple types, but if `SchemaConfigModeAttr` is set for
an attribute, it is syntactically NestedBlock but semantically interpreted
as an Attribute. In this case, Attribute has a complex data type. It is
reasonable to use the same notation as the type annotation in HCL2 to
represent the correct data type. However, it seems that HCL2 has a type
annotation parser but no writer, so we implement it by ourselves.

See also:
- #9
- hashicorp/terraform-provider-aws#8187
- hashicorp/terraform#20626
- https://www.terraform.io/docs/configuration/types.html

This is a breaking change caused by the upstream,
so I think it is impossible to maintain tfschema backward compatibility.
I tried to be as consistent as possible.
@pselle pselle deleted the f-sdk-subresource-as-attr branch June 25, 2019 16:34
@ghost
Copy link

ghost commented Jul 24, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jul 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants