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

Route53 import private zone with multiple vpc #5317

Conversation

julienduchesne
Copy link
Contributor

Fixes #892

Changes proposed in this pull request:

  • Adds an import handler for the route53 zone resource. This handler will create zone association resources when there are more than one VPC associated to a route53 zone.
  • Allow the modification of the route53 zone resource vpc_id attribute without forcing a new resource. During import, the VPC ids can end up in the wrong resources (Ex: a vpc_id that you want in the main resource might go in the association resource and vice versa).

Output from acceptance testing:

TESTARGS='-run=TestAccAWSRoute53Zone_importBasic'  make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSRoute53Zone_importBasic -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAWSRoute53Zone_importBasic
--- PASS: TestAccAWSRoute53Zone_importBasic (80.99s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       81.002s

TESTARGS='-run=TestAccAWSRoute53Zone_importTwoVpcs'  make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSRoute53Zone_importTwoVpcs -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
        === RUN   TestAccAWSRoute53Zone_importTwoVpcs
--- PASS: TestAccAWSRoute53Zone_importTwoVpcs (214.95s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       214.970s

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Jul 25, 2018
@julienduchesne julienduchesne changed the title Route53 import zone multiple vpc Route53 import private zone with multiple vpc Jul 25, 2018
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. service/route53 Issues and PRs that pertain to the route53 service. labels Jul 25, 2018
@bflad
Copy link
Contributor

bflad commented Jul 25, 2018

Allow the modification of the route53 zone resource vpc_id attribute without forcing a new resource. During import, the VPC ids can end up in the wrong resources (Ex: a vpc_id that you want in the main resource might go in the association resource and vice versa).

Should we deprecate the vpc_id argument on the aws_route53_zone resource? If multiple associations are possible and the associations are indeterminately ordered, it sounds like we should remove support for the ambiguous argument.

@julienduchesne
Copy link
Contributor Author

The issue is that you need at least one vpc when you create a private zone. I have another branch where I tried to deprecate vpc_id using multiple vpc blocks inside the aws_route53_zone but if we handle it that way, we also deprecate the possibility to create zone_associations using a count

@bflad
Copy link
Contributor

bflad commented Jul 25, 2018

Ah, bummer! I was being optimistic that they would remove that restriction. 😖

aws_route53_zone.main: InvalidInput: When you're creating a private hosted zone (when you specify true for PrivateZone), you must also specify values for VPCId and VPCRegion.

@julienduchesne
Copy link
Contributor Author

julienduchesne commented Jul 25, 2018

Oh, I just saw that Terraform 0.12 will support iteration on blocks https://www.hashicorp.com/blog/hashicorp-terraform-0-12-preview-for-and-for-each. Here's what I would do then:

  1. Support vpc_id and vpc_region as is. With the deprecated attribute.
  2. Implement a new vpc block so that you can add as many vpcs as you want in a zone
  3. Using the zone_association resource will only work with vpc_id and it will have to be removed at the same time as vpc_id

Like this:

resource "aws_route53_zone" "main" {
  name = "test132.test.com."

  vpc {
    id     = "vpc-xxxxx"
    region = "us-west-2"
  }

  vpc {
    id = "vpc-xxxxx"
  }
}

Works?

@bflad
Copy link
Contributor

bflad commented Jul 25, 2018

We'll also want to look at this in the context of this issue: #384 (PR #2005) -- we'll need to be sure that you can build a Terraform configuration with the proper ordering on a single apply to support cross-account authorizations. I think we'd still need to support a separate association resource in that case to get the [create zone] -> [cross-account authorize] -> [create association] ordering right, but we cannot suggest everyone to use ignore_changes = ["vpc"] in the zone for that scenario. 😕

It'd be so nice if the API was easier to work with! 😄

@julienduchesne
Copy link
Contributor Author

Argh, it might do more harm than good then. At least with this PR, we can change the vpc_id and vpc_region without recreating the zone entirely.

@ghost ghost added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Oct 5, 2018
@bflad bflad closed this in #6299 Oct 30, 2018
@bflad
Copy link
Contributor

bflad commented Oct 30, 2018

Hi @julienduchesne 👋 Sorry for the long silence here, but good news is that in version 1.42.0 we will have support for this feature.

We are opting to go with the following setup, similar to what you outlined above:

  • Support multiple vpc configuration blocks in aws_route53_zone and remove restriction for multiple association import
  • Deprecate vpc_id and vpc_region arguments in aws_route53_zone in favor of vpc
  • Document that aws_route53_zone_association usage is discouraged unless necessary, including information about the ignore_changes workaround.

Hopefully sometime after Terraform 0.12 has settled we can start to think about better solutions for these problems more globally in Terraform core (configuration blocks as separate items for Terraform graph ordering).

Thanks for your hard work as usual!

@julienduchesne julienduchesne deleted the route53-import-zone-multiple-vpc branch January 18, 2019 16:11
@ghost
Copy link

ghost commented Apr 1, 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 Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/route53 Issues and PRs that pertain to the route53 service. size/L 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
Development

Successfully merging this pull request may close these issues.

Can't import private Route53 Hosted Zone with more than one VPC association
2 participants