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

Modify defaults for parent from empty strings to null. #53

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

theherk
Copy link

@theherk theherk commented May 24, 2024

Due to, it seems some changes to the aws_route53_zone data source in provider version 5.51.0, it seems using empty strings here causes an issue where the following error is received.

│ Error: Invalid combination of arguments
│
│   with
module.cloudfront.module.cloudfront_distribution.module.route53_alias.data.aws_route53_zone.default[0],
│   on
.terraform/modules/cloudfront.cloudfront_distribution.route53_alias/main.tf
line 4, in data "aws_route53_zone" "default":
│    4:   name         = var.parent_zone_name
│
│ "name": only one of `name,zone_id` can be specified, but
`name,zone_id`
│ were specified.

By using null in place of empty strings here, the validation seems to pass without issue. This can be validated by trying to use this module with 5.51.0, and finding that no matter which is provided both are interpreted as having been provided, triggering the error.

history of data source
file

Due to, it seems some changes to the `aws_route53_zone` data source in
provider version 5.51.0, it seems using empty strings here causes an
issue where the following error is received.

```
│ Error: Invalid combination of arguments
│
│   with
module.cloudfront.module.cloudfront_distribution.module.route53_alias.data.aws_route53_zone.default[0],
│   on
.terraform/modules/cloudfront.cloudfront_distribution.route53_alias/main.tf
line 4, in data "aws_route53_zone" "default":
│    4:   name         = var.parent_zone_name
│
│ "name": only one of `name,zone_id` can be specified, but
`name,zone_id`
│ were specified.
```

By using null in place of empty strings here, the validation seems to
pass without issue. This can be validated by trying to use this module
with 5.51.0, and finding that no matter which is provided both are
interpreted as having been provided, triggering the error.

[history of data source
file](https://github.com/hashicorp/terraform-provider-aws/commits/0750cf2ce20f2b1cc6144176ce33593ba53b0220/internal/service/route53/zone_data_source.go)
@theherk theherk requested review from a team as code owners May 24, 2024 14:46
@theherk theherk requested review from hans-d and joe-niland May 24, 2024 14:46
@mergify mergify bot added the triage Needs triage label May 24, 2024
@yuekui
Copy link

yuekui commented May 24, 2024

This should already be fixed by hashicorp/terraform-provider-aws#37686
in terraform-provider-aws v5.51.1

@theherk
Copy link
Author

theherk commented May 25, 2024

Sounds good, @yuekui. Glad to hear it. I still think providing empty strings in lieu of not providing a value when the desired intent is no value, is an odd choice in languages with nulls; evil as they are. 😅 If this is merged, cool; if closed directly, that's sensible too.

@kevcube
Copy link

kevcube commented Jun 6, 2024

/terratest

@kevcube
Copy link

kevcube commented Jun 6, 2024

/terratest

@kevcube kevcube enabled auto-merge (squash) June 6, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants