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.

Sign up for GitHub

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

Jump to bottom

New data sources: aws_route53_resolver_rule and aws_route53_resolver_rules #9085

Closed
wants to merge 8 commits into from
Closed

New data sources: aws_route53_resolver_rule and aws_route53_resolver_rules #9085

wants to merge 8 commits into from

Conversation

ewbankkit
Copy link
Contributor

@ewbankkit ewbankkit commented Jun 21, 2019 •
edited
Loading

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #8508.

Release note for CHANGELOG:

- New Data Source:  aws_route53_resolver_rule
- New Data Source:  aws_route53_resolver_rules

Output from acceptance testing:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccDataSourceAwsRoute53ResolverRule_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccDataSourceAwsRoute53ResolverRule_ -timeout 120m
=== RUN   TestAccDataSourceAwsRoute53ResolverRule_basic
=== PAUSE TestAccDataSourceAwsRoute53ResolverRule_basic
=== RUN   TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
=== PAUSE TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
=== CONT  TestAccDataSourceAwsRoute53ResolverRule_basic
=== CONT  TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_basic (41.30s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags (252.57s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	252.604s
$ make testacc TEST=./aws/ TESTARGS='-run=TestAccDataSourceAwsRoute53ResolverRules_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccDataSourceAwsRoute53ResolverRules_ -timeout 120m
=== RUN   TestAccDataSourceAwsRoute53ResolverRules_basic
=== PAUSE TestAccDataSourceAwsRoute53ResolverRules_basic
=== RUN   TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
=== PAUSE TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
=== CONT  TestAccDataSourceAwsRoute53ResolverRules_basic
=== CONT  TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_basic (20.62s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId (257.43s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	257.496s

@ewbankkit ewbankkit requested a review from a team June 21, 2019 21:42
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/route53 Issues and PRs that pertain to the route53 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Jun 21, 2019
@ewbankkit
Copy link
Contributor Author

Related:

@ewbankkit
Copy link
Contributor Author

ewbankkit commented Jun 21, 2019 •
edited
Loading

TODO:

  • Add aws_route53_resolver_rules data source

@ewbankkit ewbankkit force-pushed the issue-8508 branch 2 times, most recently from e374d41 to 4d07c52 Compare June 27, 2019 15:06
@ewbankkit ewbankkit changed the title [WIP] New data sources: aws_route53_resolver_rule and aws_route53_resolver_rules New data sources: aws_route53_resolver_rule and aws_route53_resolver_rules Jun 27, 2019
@ewbankkit
Copy link
Contributor Author

Removed WIP.
Ready for review.

@ewbankkit
Copy link
Contributor Author

Rebased to fix merge conflict.

@maryelizbeth maryelizbeth added enhancement Requests to existing resources that expand the functionality or scope. new-data-source Introduces a new data source. and removed enhancement Requests to existing resources that expand the functionality or scope. labels Aug 19, 2019
@maryelizbeth maryelizbeth requested a review from nywilken August 19, 2019 19:36
@nywilken nywilken self-assigned this Aug 22, 2019
nywilken
nywilken reviewed Aug 22, 2019
View reviewed changes
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

Hi @ewbankkit thanks for putting together these data sources. This is looking good. I left a few minor suggestions along with some output around a failing test case that only occurs when running all data source tests in parallel. Please take a look and let me know if you have any questions or if you no longer have bandwidth to work on this PR.

aws/data_source_aws_route53_resolver_rule.go Outdated Show resolved Hide resolved
aws/data_source_aws_route53_resolver_rules.go Show resolved Hide resolved
website/docs/d/route53_resolver_rules.html.markdown Outdated Show resolved Hide resolved
website/docs/d/route53_resolver_rules.html.markdown Outdated Show resolved Hide resolved
aws/data_source_aws_route53_resolver_rule.go Outdated Show resolved Hide resolved
aws/data_source_aws_route53_resolver_rules_test.go Outdated Show resolved Hide resolved
aws/data_source_aws_route53_resolver_rules.go Outdated Show resolved Hide resolved
@ewbankkit
Copy link
Contributor Author

Re-ran acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccDataSourceAwsRoute53ResolverRule_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccDataSourceAwsRoute53ResolverRule_ -timeout 120m
=== RUN   TestAccDataSourceAwsRoute53ResolverRule_basic
=== PAUSE TestAccDataSourceAwsRoute53ResolverRule_basic
=== RUN   TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
=== PAUSE TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
=== CONT  TestAccDataSourceAwsRoute53ResolverRule_basic
=== CONT  TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_basic (41.96s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags (274.83s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	274.855s
$ make testacc TEST=./aws/ TESTARGS='-run=TestAccDataSourceAwsRoute53ResolverRules_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccDataSourceAwsRoute53ResolverRules_ -timeout 120m
=== RUN   TestAccDataSourceAwsRoute53ResolverRules_basic
=== PAUSE TestAccDataSourceAwsRoute53ResolverRules_basic
=== RUN   TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
=== PAUSE TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
=== CONT  TestAccDataSourceAwsRoute53ResolverRules_basic
=== CONT  TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_basic (16.98s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId (428.15s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	428.186s

@ewbankkit
Copy link
Contributor Author

@nywilken All review comments addressed.

@nywilken
Copy link
Contributor

nywilken commented Sep 6, 2019

hi @ewbankkit sorry for the delayed turn around on this PR. I was still running into issues when running the tests for both data sources in parallel.

make testacc TEST=./aws/ TESTARGS='-run=TestAccDataSourceAwsRoute53ResolverRule'

I went ahead and fixed the test by changing the route53 resolver rule type in one of the test configurations to get it passing. I will merge this in and we can look to revisit the test if you think it no longer reflects your initial intent.

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccDataSourceAwsRoute53ResolverRule -timeout 120m
=== RUN   TestAccDataSourceAwsRoute53ResolverRule_basic
=== PAUSE TestAccDataSourceAwsRoute53ResolverRule_basic
=== RUN   TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
=== PAUSE TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
=== RUN   TestAccDataSourceAwsRoute53ResolverRules_basic
=== PAUSE TestAccDataSourceAwsRoute53ResolverRules_basic
=== RUN   TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
=== PAUSE TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
=== CONT  TestAccDataSourceAwsRoute53ResolverRule_basic
=== CONT  TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId
=== CONT  TestAccDataSourceAwsRoute53ResolverRules_basic
=== CONT  TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_basic (18.07s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_basic (42.41s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRule_ResolverEndpointIdWithTags (220.82s)
--- PASS: TestAccDataSourceAwsRoute53ResolverRules_ResolverEndpointId (220.96s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       221.002s

@nywilken
Copy link
Contributor

nywilken commented Sep 6, 2019

Merged manually with additional commits.

@nywilken nywilken closed this Sep 6, 2019
@nywilken nywilken added this to the v2.28.0 milestone Sep 6, 2019
@ewbankkit ewbankkit deleted the issue-8508 branch September 6, 2019 20:46
mikecook referenced this pull request Sep 9, 2019
@nywilken
update CHANGELOG for #9805
b4efb0b
@ghost
Copy link

ghost commented Sep 12, 2019

This has been released in version 2.28.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 for triage. Thanks!

@ewbankkit ewbankkit mentioned this pull request Oct 1, 2019
Closed
@ghost
Copy link

ghost commented Nov 1, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nywilken nywilken nywilken left review comments

Assignees

@nywilken nywilken

Labels
documentation Introduces or discusses updates to documentation. new-data-source Introduces a new data source. provider Pertains to the provider itself, rather than any interaction with AWS. service/route53 Issues and PRs that pertain to the route53 service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Milestone
v2.28.0
Development

Successfully merging this pull request may close these issues.

Feature Request: aws_route53_resolver_rule data source
3 participants
@ewbankkit @nywilken @maryelizbeth