Verify e.g. ExactlyOneOf later so argument can be null when count = 0

[YakDriver]

SDK version

2.10.1

Use-cases

Use case is for evaluation order of ExactlyOneOf (et al.) to allow it to be a short circuit operand. Practitioners want to short circuit in cases where count = 0. However, current evaluation order prevents this from happening.

For example, currently, in the configuration below, if local.cidr is null, we don't want the test resource at all. Only if count > 0 do we want to verify that ExactlyOneOf three arguments (i.e., destination_cidr_block, destination_ipv6_cidr_block, or destination_prefix_list_id) is set:

resource "aws_route" "test" {
  count                  = local.cidr == null ? 0 : 2
  destination_cidr_block = local.cidr
}

The current evaluation order is roughly equivalent to this backwards Go statement. We're calling, e.g., diags.HasError(), before checking to see if diags is nil.

if diags.HasError() && diags != nil {
  ...
}

The way it should work is if count = 0, the resource short circuits. Similarly, if, e.g., diags is nil, we drop out of the && operation before the diags.HasError() call creates a nil pointer panic.

if diags != nil && diags.HasError() {
  ...
}

In order for this to work, diags.HasError() must be able to pass compile-time evaluation. Similarly, ExactlyOneOf could have a compile-time check to ensure syntactic correctness but not fully evaluate until the short circuit condition is passed at runtime.

As it is now, since count = 0 is not evaluating like a short circuit operator, practitioners are getting this error even when count will evaluate to 0:

% terraform plan
Error: Invalid combination of arguments
│ 
│   3: resource "aws_route" "example" {
│ 
│ "destination_prefix_list_id": one of `destination_cidr_block,destination_ipv6_cidr_block,destination_prefix_list_id` must be specified

Attempted Solutions

Prior to AWS Provider v4, we allowed these arguments (e.g., destination_cidr_block) to be empty strings, subverting the correct validation that exactly one of 3 arguments needed to be set to a non-empty value. Practitioners used the empty string as an out to avoid an error when count = 0.

With v4, we corrected the validation but broke some configurations. We published the change as breaking since configurations would need to use null instead of an empty string. Unfortunately, that also inadvertently took away the "out" that practitioners were using to allow the combination of destination_cidr_block = "" with count = 0 to pass validation.

Proposal

Evaluate ExactlyOneOf later in the process so that it is evaluated after count > 0 is determined.

References

• Remove Zero Values From Attribute Validations terraform-provider-aws#13943
• ec2/customer_gateway: Remove empty validation terraform-provider-aws#22926
• ec2/network_acl: Remove empty validation terraform-provider-aws#22928
• ec2/routes: Remove empty validation terraform-provider-aws#22931
• ec2: Remove remaining empty validation terraform-provider-aws#22948
• Remove last empty string validation terraform-provider-aws#22954
• aws_route.destination_cidr_block being evaluated in plan after 4.2.0 update - even if count evaluates to 0 terraform-provider-aws#23343

[bflad]

Very briefly --

My understanding of this situation is that providers (via the SDK) cannot determine the number of resource/data source instances during ValidateResourceConfig/ValidateDatasourceConfig as the count information is not passed across the protocol. While we could theoretically add that to the protocol, I think it is generally assumed that a statically analyzed "valid" configuration should not matter based on the number of instances.

If Terraform CLI should not be calling the Validate*Config RPC on these types of configurations, that would need to be changed upstream. (I believe it should be calling these RPCs in that situation because otherwise it could leave a potentially awkward situation where terraform validate can pass but terraform plan/apply wouldn't, but admittedly I don't have all that context loaded at the moment.)

One option on the provider-side is to skip the schema configuration of ExactlyOneOf and instead implement this validation logic via CustomizeDiff since that only gets called when there are resource instances.

Another option on the SDK-side could be changing this type of validation to only being performed during PlanResourceChange, ApplyResourceChange, or ReadDataSource RPCs, however I think we would need to think more about whether that is a reasonable change to globally decide, especially if we leave the other schema validation fields the same. Effectively it would lower the practitioner experience of the SDK functionality since many times this situation could/should be caught during configuration validation, which was the intended purpose of those schema fields.

[bflad]

Closing out as the above is satisfactory for now. 👍

[github-actions]

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.