Remove Zero Values From Attribute Validations

Question

Remove Zero Values From Attribute Validations

Closed this issue 3 years ago · 5 comments

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Prior to Terraform 0.12, the Terraform configuration language did not have the concept of null, where the attribute and its value could be seen as completely unconfigured to the provider. As a workaround in these older versions of Terraform, the provider occasionally accepted the zero-values for attribute types (e.g. "" for schema.TypeString, 0 for schema.TypeInt) from the configuration in the associated schema validation (ValidateFunc) and ignored passing these values through to the underlying API calls (e.g. using d.GetOk() or d.Get() and conditionalizing based on zero value). For example:

# Example Terraform 0.11 and earlier workaround for lack of null support
variable "destination" {
  default = "10.0.0.0/8"
}

variable "destination_type" {
  default = "ipv4"
}

resource "aws_route" "example" {
  destination_cidr_block      = "${var.destination_type == "ipv4" ? var.destination : ""}"
  destination_ipv6_cidr_block = "${var.destination_type == "ipv6" ? var.destination : ""}"
  route_table_id              = "${aws_route_table.example.id}"
  vpc_peering_connection_id   = "${aws_vpc_peering_connection.example.id}"
}

In terms of the schema implementation, this required the validation to accept either the zero value or an actual valid value. In this example:

"destination_cidr_block": {
	Type:     schema.TypeString,
	Optional: true,
	ForceNew: true,
	ValidateFunc: validation.Any(
		validation.StringIsEmpty,
		validateIpv4CIDRNetworkAddress,
	),
},

"destination_ipv6_cidr_block": {
	Type:     schema.TypeString,
	Optional: true,
	ForceNew: true,
	ValidateFunc: validation.Any(
		validation.StringIsEmpty,
		validateIpv6CIDRNetworkAddress,
	),
	DiffSuppressFunc: suppressEqualCIDRBlockDiffs,
},

Other than the slight extra implementation required, this does have an additional downsides:

The validation cannot make decisions based purely on an attribute being configured or not. For example, the above cannot implement the proper ConflictsWith validation since it represents a breaking change (nor ExactlyOneOf or RequiredWhen if it was appropriate).
Other future schema validation enhancements will also likely be based on values being unconfigured when necessary
Future schema types that support nullable values in addition to zero values will show differences between an unconfigured value and the zero value for practitioners.

Given that Terraform AWS Provider 3.0.0 and later will only support Terraform 0.12, we should consider fixing and enhancing these validations in a major version release to no longer support the workarounds required in Terraform 0.11 and earlier.

In the above case as the final breaking change:

"destination_cidr_block": {
	Type:     schema.TypeString,
	Optional: true,
	ForceNew: true,
	ValidateFunc: validateIpv4CIDRNetworkAddress,
	ConflictsWith: []string{"destination_cidr_block"},
},

"destination_ipv6_cidr_block": {
	Type:     schema.TypeString,
	Optional: true,
	ForceNew: true,
	ValidateFunc: validateIpv6CIDRNetworkAddress,
	ConflictsWith: []string{"destination_cidr_block"},
	DiffSuppressFunc: suppressEqualCIDRBlockDiffs,
},

Affected Resource(s)

TBD

References

List to be fully filled in the future:

Answer 1 · 2020-06-25T17:29:30.000Z

Another downside of the additional validation for an empty string is that the error message for a failed validation can be confusing, for example:

Error: aws_route_table.test: "10.4.0.1/16" is not a valid IPv4 CIDR block; did you mean "10.4.0.0/16"?



Error: aws_route_table.test: expected "route.0.cidr_block" to be an empty string: got 10.4.0.1/16

Answer 2 · 2021-01-21T06:26:42.000Z

One major function this affects is validateArn, which is a valid change but it has a wide blast radius. It would fix confusing bugs though, especially with data sources that accept the arn as an argument such as: #10524

Answer 3 · 2021-01-28T15:24:29.000Z

One (minor) benefit of removing the zero value ("") for the aws_route destination_cidr_block and destination_ipv6_cidr_block attributes is that we can add an ExactlyOneOf check to ensure that one and only one route destination attribute is specified.

Answer 4 · 2022-02-10T18:16:05.000Z

This functionality has been released in v4.0.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Answer 5 · 2022-05-14T02:41:21.000Z

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.