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 Resource: aws_route53_vpc_association_authorization #14215

Merged
merged 11 commits into from
Aug 6, 2020
Merged

New Resource: aws_route53_vpc_association_authorization #14215

merged 11 commits into from
Aug 6, 2020

Conversation

gazoakley
Copy link
Contributor

@gazoakley gazoakley commented Jul 16, 2020 •
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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

This PR builds on awesome prior work by @RyanJarv (I've retained the original commits for posterity) and adds support for the new ListHostedZonesByVPC API function - this allows Terraform to check if the resource is present or not.

Closes #384
Closes #12362

Release note for CHANGELOG:

NOTES:

* resource/aws_route53_zone_association: The addition of cross-account zone association support required the use of new `ListHostedZonesByVPC` API call. Restrictive IAM permissions for Terraform may require updates. [GH-14103]

FEATURES

* **New Resource**: `aws_route53_vpc_association_authorization` [GH-14103]

ENHANCEMENTS

* resource/aws_route53_zone_association: Cross-account zone associations can now be created in conjunction with the new `aws_route53_vpc_association_authorization` resource [GH-14103]

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSRoute53VpcAssociationAuthorization_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSRoute53VpcAssociationAuthorization_ -timeout 120m
=== RUN   TestAccAWSRoute53VpcAssociationAuthorization_basic
=== PAUSE TestAccAWSRoute53VpcAssociationAuthorization_basic
=== CONT  TestAccAWSRoute53VpcAssociationAuthorization_basic
--- PASS: TestAccAWSRoute53VpcAssociationAuthorization_basic (156.50s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	160.265s

@gazoakley gazoakley requested a review from a team July 16, 2020 20:22
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. 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. needs-triage Waiting for first response or review from a maintainer. documentation Introduces or discusses updates to documentation. labels Jul 16, 2020
@bflad bflad added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Jul 16, 2020
@bflad bflad added this to the v3.1.0 milestone Jul 16, 2020
@bflad
Copy link
Contributor

bflad commented Jul 16, 2020

Marking this for right after we complete our work for 3.0 in the next week or two, although review cycles might make it slightly after 3.1 for actual release. 👍

@gazoakley gazoakley force-pushed the f-route53-vpc-association-authorization branch from 68df933 to 0a8d5bd Compare July 16, 2020 21:09
@mbevc1
Copy link

mbevc1 commented Jul 30, 2020

Is this going to make it to 2.x as well?

@RyanJarv
Copy link
Contributor

@gazoakley appreciate you picking this up. Looking over this now and looks good so far, will dig into it more shortly though.

@RyanJarv
Copy link
Contributor

I think there may be some stuff in the docs that may be obsolete with this change. Probably worth looking over that briefly as well.

@bflad bflad self-assigned this Jul 31, 2020
bflad
bflad suggested changes Jul 31, 2020
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Hi @gazoakley 👋 Thank you for continuing this work. This is looking good so far. I left some initial feedback below. Please reach out with any questions or if you do not have time to implement the items.

aws/resource_aws_route53_vpc_association_authorization.go Outdated Show resolved Hide resolved
aws/resource_aws_route53_vpc_association_authorization.go Show resolved Hide resolved
aws/resource_aws_route53_vpc_association_authorization.go Show resolved Hide resolved
aws/resource_aws_route53_vpc_association_authorization.go Outdated Show resolved Hide resolved
aws/resource_aws_route53_vpc_association_authorization.go Outdated Show resolved Hide resolved
aws/resource_aws_route53_vpc_association_authorization.go Outdated Show resolved Hide resolved
aws/resource_aws_route53_vpc_association_authorization.go Outdated Show resolved Hide resolved
aws/resource_aws_route53_vpc_association_authorization_test.go Outdated Show resolved Hide resolved
aws/resource_aws_route53_zone_association.go Show resolved Hide resolved
website/docs/r/route53_vpc_association_authorization.html.markdown Outdated Show resolved Hide resolved
@RyanJarv
Copy link
Contributor

RyanJarv commented Aug 1, 2020 •
edited
Loading

Just fyi made a rebased version of this branch here, mostly just because that's easier to work with locally (been running into issues where some changes don't seem to show UI, so trying to not rely on that for now). No real difference other than that though.

https://github.com/RyanJarv/terraform-provider-aws/tree/jarv/cross-account-hostedzone-association-cleanup4

@gazoakley gazoakley requested a review from bflad August 1, 2020 12:21
bflad added a commit that referenced this pull request Aug 6, 2020
@bflad
service/route53: Finalize cross-account VPC association handling
fb151fe 
Reference: #10333
Reference: #13527
Reference: #13826
Reference: #14215

Changes:

* Add disappears testing to aws_route53_vpc_association_authorization.
* Ensure aws_route53_vpc_association_authorization example shows implicit ordering for downstream aws_route53_zone_association usage
* Fix aws_route53_zone_association VPC Region handling by including it in the ID, falling back to the attribute state, or falling back to the provider region. Document additional import handling.
* Add aws_route53_zone_association error handling to remove resource from state.
* Standardize aws_route53_zone_association disappears testing.

Output from acceptance testing:

```
--- PASS: TestAccAWSRoute53VpcAssociationAuthorization_disappears (110.26s)
--- PASS: TestAccAWSRoute53VpcAssociationAuthorization_basic (113.19s)

--- PASS: TestAccAWSRoute53ZoneAssociation_CrossAccount (211.48s)
--- PASS: TestAccAWSRoute53ZoneAssociation_CrossRegion (275.34s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_Zone (280.22s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears (281.72s)
--- PASS: TestAccAWSRoute53ZoneAssociation_disappears_VPC (281.85s)
--- PASS: TestAccAWSRoute53ZoneAssociation_basic (283.83s)
```
@bflad bflad merged commit 22aee4f into hashicorp:master Aug 6, 2020
@bflad
Copy link
Contributor

bflad commented Aug 6, 2020

Thanks so much @gazoakley 👍 I was able to resolve the remaining test failures with some additional error and VPC region attribute handling.

bflad added a commit that referenced this pull request Aug 6, 2020
@bflad
Update CHANGELOG for #14215
d4bb1be
@ghost
Copy link

ghost commented Aug 7, 2020

This has been released in version 3.1.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!

@bashoKa bashoKa mentioned this pull request Aug 25, 2020
Closed
@ghost
Copy link

ghost commented Sep 6, 2020

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 Sep 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bflad bflad Awaiting requested review from bflad

Assignees

@bflad bflad

Labels
documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. 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
v3.1.0
Development

Successfully merging this pull request may close these issues.

AWS Route53 of other account Feature Request: Add support for CreateVPCAssociationAuthorization AWS API
4 participants
@gazoakley @bflad @mbevc1 @RyanJarv