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

d/aws_route53_zone: support attribute-only search #39671

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

isometry
Copy link

@isometry isometry commented Oct 10, 2024

Description

When operating at scale and building re-usable modules, requiring a Hosted Zone's name can introduce undesirable inter-module coupling. This change enables aws_route53_zone data source to retrieve a Hosted Zone based solely on the filter attributes specified (i.e. private_zone, vpc_id and tags) without having to specify the expected Hosted Zone name.

The change should be 100% backwards compatible, and also introduces a handful of minor optimisations to improve performance and simplify logic.

Output from Acceptance Testing

% make testacc TESTS=TestAccRoute53ZoneDataSource PKG=route53 GO_VER=go
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/route53/... -v -count 1 -parallel 20 -run='TestAccRoute53ZoneDataSource'  -timeout 360m
2024/10/11 12:45:41 Initializing Terraform AWS Provider...
=== RUN   TestAccRoute53ZoneDataSource_id
=== PAUSE TestAccRoute53ZoneDataSource_id
=== RUN   TestAccRoute53ZoneDataSource_name
=== PAUSE TestAccRoute53ZoneDataSource_name
=== RUN   TestAccRoute53ZoneDataSource_name_idEmptyString
=== PAUSE TestAccRoute53ZoneDataSource_name_idEmptyString
=== RUN   TestAccRoute53ZoneDataSource_tags
=== PAUSE TestAccRoute53ZoneDataSource_tags
=== RUN   TestAccRoute53ZoneDataSource_tagsOnly
=== PAUSE TestAccRoute53ZoneDataSource_tagsOnly
=== RUN   TestAccRoute53ZoneDataSource_vpc
=== PAUSE TestAccRoute53ZoneDataSource_vpc
=== RUN   TestAccRoute53ZoneDataSource_serviceDiscovery
=== PAUSE TestAccRoute53ZoneDataSource_serviceDiscovery
=== CONT  TestAccRoute53ZoneDataSource_id
=== CONT  TestAccRoute53ZoneDataSource_serviceDiscovery
=== CONT  TestAccRoute53ZoneDataSource_tags
=== CONT  TestAccRoute53ZoneDataSource_name_idEmptyString
=== CONT  TestAccRoute53ZoneDataSource_name
=== CONT  TestAccRoute53ZoneDataSource_vpc
=== CONT  TestAccRoute53ZoneDataSource_tagsOnly
--- PASS: TestAccRoute53ZoneDataSource_name_idEmptyString (99.95s)
--- PASS: TestAccRoute53ZoneDataSource_name (101.68s)
--- PASS: TestAccRoute53ZoneDataSource_tagsOnly (106.87s)
--- PASS: TestAccRoute53ZoneDataSource_id (109.63s)
--- PASS: TestAccRoute53ZoneDataSource_serviceDiscovery (109.88s)
--- PASS: TestAccRoute53ZoneDataSource_tags (143.98s)
--- PASS: TestAccRoute53ZoneDataSource_vpc (153.27s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/route53    160.243s

@isometry isometry requested a review from a team as a code owner October 10, 2024 15:37
Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • 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.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/route53 Issues and PRs that pertain to the route53 service. needs-triage Waiting for first response or review from a maintainer. labels Oct 10, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome @isometry 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@isometry isometry force-pushed the feature/data-route53-zone-by-tags branch from daeb318 to 0ef5920 Compare October 10, 2024 16:24
* Enable pure attribute-based route53 hosted zone lookup without having
  to specify the zone name.
@isometry isometry force-pushed the feature/data-route53-zone-by-tags branch from 0ef5920 to 559b54a Compare October 11, 2024 10:49
@isometry
Copy link
Author

isometry commented Oct 11, 2024

I've updated in-place with a further optimisation for the case where a zone_id is specified to avoid pagination when direct lookup is possible. This cuts 7%/~12s off the time to run acceptance tests.

@justinretzolk
Copy link
Member

Related #27851
Related #38186
Related #31703

@justinretzolk justinretzolk added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Oct 11, 2024
@isometry
Copy link
Author

isometry commented Oct 11, 2024

Related #27851

Quite… this change enables the following at decent efficiency (O(n^2) => O(n)):

data "aws_route53_zones" "all" {}

data "aws_route53_zone" "map" {
  for_each = toset(data.aws_route53_zones.all.ids)
  zone_id  = each.key
}

output "all_zones" {
  value = data.aws_route53_zone.map
}

Copy link
Contributor

@bateller bateller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/route53 Issues and PRs that pertain to the route53 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants