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

AWS/Route53Zone - create private hosted zone associated with VPC. #1526

Merged
merged 18 commits into from
May 14, 2015

Conversation

johnrengelman
Copy link
Contributor

Add ability to create a private Hosted Zone that is associated to a VPC.
Currently only supports a single VPC at creation time.

Future question is how to support multiple VPCs. Amazon uses a Associate/Disassociate request, however this is problematic since you must specify one (and only one) VPC during creation.

Alternatively, we could model it as a VPC Set and and use the first entry as the entry for creating and then associate the others.

@johnrengelman
Copy link
Contributor Author

Provides support for #1503

@johnrengelman
Copy link
Contributor Author

This is missing pieces for read/update to sync the VPC list for the hosted zone. Probably need to answer the above question and add support before this gets merged.

@phinze
Copy link
Contributor

phinze commented Apr 15, 2015

Hey thanks for this!

Future question is how to support multiple VPCs. Amazon uses a Associate/Disassociate request, however this is problematic since you must specify one (and only one) VPC during creation.

Our strategy with other similar scenarios has been to model the "attachment" or "association" as a resource itself. If I'm understanding the R53 API here correctly, this is a little awkward because you can specify a single VPC at zone creation time, but then N others later.

aws_vpc "foo" {
  cidr_block = "10.0.0.0/16"
}
aws_vpc "bar" {
  cidr_block = "10.1.0.0/16"
}
aws_route53_zone "main" {
  vpc_id = "${aws_vpc.foo.id}"
  vpc_region = "us-east-1"
}
/* proposed additional resource */
aws_route53_zone_vpc_association "main-bar" {
  zone_id = "${aws_route_53_zone.main.id}"
  vpc_id = "${aws_vpc.bar.id}"
}

Having a top-level resource makes the CRUD operations very straightforward:

  • Create: AssociateVPCWithHostedZone
  • Delete: DisassociateVPCWithHostedZone
  • Read: GetHostedZone and walk through VPCs

For a minute I thought we could just ignore the VPC fields during zone creation so we could model everything with the separate resources, but alas, those fields are what trigger a private zone vs a public one. 😞

So the sadness is that because of this asymmetry in the R53 API, the VPC you specify in the Zone fields will always be the "root" VPC for the zone from Terraform's perspective.

Let me know if this all makes sense to you, and what your take is.

Thanks again for putting the time and effort in here! 😀

@johnrengelman
Copy link
Contributor Author

I was thinking it would be easier to represent it like this:

aws_vpc "foo" {
  cidr_block = "10.0.0.0/16"
}
aws_vpc "bar" {
  cidr_block = "10.1.0.0/16"
}
aws_route53_zone "main" {
  vpc {
    id = "${aws_vpc.foo.id}"
  }
  vpc {
    id = "${aws_vpc.bar.id}"
    region = "us-east-1"
  }
}

as for the "zone_association" part...this is especially useful if the route53 zone is created outside of Terraform and you are just trying to attach your VPC to it. Would it be possibly to do something like collapse the zone_association into the aws_route53_zone resource (as a vpc entry) if it exists in the graph, otherwise treat it as a standalone resource?

@johnrengelman
Copy link
Contributor Author

Ha, I just realized there is an existing PR adding this same support (#1159). I've cleaned up some of this code based on that.
However both PRs have the same issue in that they don't correctly handle the VPC association on a read. I think the only way to do this is (Based on the Route53 API - http://docs.aws.amazon.com/Route53/latest/APIReference/API-get-hosted-zone-private.html) is to model the VPC associations on the HostedZone resource itself.

Would it be possible to collapse a resource "aws_route53_hosted_zone_assocation" into a resource "aws_route53_hosted_zone" if it exists in Terraform?

In other words:

resource "aws_route53_hosted_zone"  "test"{
   name ="test.com"
   vpc {
      id = "123abc"
   }
}

resource "aws_route53_hosted_zone_association" "foo" {
   hosted_zone = "${aws_route53_hosted_zone.test.id"
   vpc_id = "456xyz"
}

would be equivalent to

resource "aws_route53_hosted_zone" "test" {
  name = "test"
  vpc {
    id = "123abc"
  }
  vpc {
    id = "456xyz"
  }
}

?

@c4milo
Copy link
Contributor

c4milo commented Apr 24, 2015

It seems we have two contending PRs for this. Which one is it going to be merged?

@johnrengelman
Copy link
Contributor Author

The problem is that there is an outstanding question about how to implement this. The route53 API in this area doesn't map well to Terraform.

@c4milo
Copy link
Contributor

c4milo commented Apr 24, 2015

To be consistent with Route53 API, I would personally prefer:

resource "aws_route53_hosted_zone"  "test"{
   name ="test.com"
   vpc {
      id = "123abc"
   }
}

resource "aws_route53_hosted_zone_association" "foo" {
   hosted_zone = "${aws_route53_hosted_zone.test.id"
   vpc_id = "456xyz"
}

It is also consistent with the way other resources are defined in Terraform.

@johnrengelman
Copy link
Contributor Author

Unfortunately that is not consistent with the Route53 APIs because all associated VPCs are returned on the DescribePrivateHostedZone API call. So it would be difficult to distinguish which one was attached as part of HostedZone creation and which as an association.

It's legal to do this in Route53:

  1. Create HostedZone "test" w/ VPC "123abc"
  2. Associate VPC "456xyz" to HostedZone "test"
  3. Deassociate VPC "123abc" from HostedZone "test"

There's no notion in AWS which VPC was used to create the Private HostedZone.

@c4milo
Copy link
Contributor

c4milo commented Apr 24, 2015

oh I see, that's a bummer! Thanks for explaining further @johnrengelman.

@jonhatalla
Copy link

Perhaps limiting the association of a private hosted zone to a single VPC might mitigate 'what's legal in route53' versus 'what's maintainable/consise in terraform'.
In your situation, this would be the 'terraform process...

  1. Create HostedZone "test" w/ VPC "123abc"
  2. Create new HostedZone "test" w/VPC "456xyz"
  3. Deassociate VPC "123abc" from first HostedZone "test"
  4. delete first hosted zone 'test'

@johnrengelman
Copy link
Contributor Author

I guess we could model everything as an association instead of a hosted_zone and associations.
I think there is still a problem here because what if you are not managing the hosted zone with Terraform but you want to manage the association? You could then potentially delete the hosted zone even though terraform isn't managing it (since removing the last association would in effect delete the zone). I still think the only way this can work well is if the association code can check if the hosted zone itself is being managed by terraform. That way if the hosted zone is not managed by Terraform then you treat the associations as individual items, otherwise it collapses everything into a single HostedZone resource and manage it from there.

@jonhatalla
Copy link

Yeah, Totally agree that everything should be managed by with Terraform (creation, destruction, associations).

@c4milo
Copy link
Contributor

c4milo commented Apr 29, 2015

@johnrengelman I would do something similar to what @phinze suggested. Using AWS's default VPC for the initial creation of the zone, then swapping that with associations. I did something similar for DHCP Options Set but upon removing association: #1721

@catsby
Copy link
Contributor

catsby commented May 5, 2015

Hey @johnrengelman – have you been able to review this thread and given any thought to breaking the association out into it's own resource? We feel that's the best way forward here.

@catsby catsby added the waiting-response An issue/pull request is waiting for a response from the community label May 5, 2015
@johnrengelman
Copy link
Contributor Author

Yeah. I think that's right way to go. I'll try and re-work this PR in the next few days.

@kendawg2
Copy link

kendawg2 commented May 8, 2015

This runs the plan OK, but on apply it always fails somewhere in the creation process. It actually creates the zone as a private zone as expected, but throws an error and never adds the RRs to the zone.

aws_route53_zone.primary: Error: 1 error(s) occurred:

  • unexpected EOF
    Error applying plan:

1 error(s) occurred:

  • 1 error(s) occurred:
  • 1 error(s) occurred:
  • unexpected EOF

And then pretty much dumps after that. A destroy does not remove the zone as expected.

I know this is WIP so this may be expected.

@kendawg2
Copy link

kendawg2 commented May 8, 2015

@johnrengelman @catsby @phinze Not trying to add complexity here, but the basic functionality of being able to create a simple private hosted zone for a particular plan is probably the 99% use case. Being able to later associate an existing private zone is something needed but I would view as additive and a separate effort (one appears underway #1827. I believe these are complimentary efforts which can stand alone. I really don't like using the default VPC for initial association because you will have to disassociate it immediately so others can be created later. Particularly with DHCP options. It also can't cleanly destroy without impacting other VPC's a particular plan has no knowledge of. Just my $.02 worth.

Simply adding a VPC and region should automatically create a private vs public zone. If it isn't created with a VPC it will be public and there is no way to make it private after that (which I see as a much rarer case). So when the zone is created, it needs to have a VPC associated. #1159 addressed this fairly elegantly as does this code (they are similar). #1827 extends that to add an already existing private zone to another VPC (or at least should do that).

@johnrengelman
Copy link
Contributor Author

@kendawg2 Agreed. I didn't plan on using the default VPC because there are couple issues around that: 1) how do you identify the "default" VPC, 2) What do you do if someone deletes the "default" VPC.

I agree the association support can come after this since they are building on top of this PR.
In that case I'll run the tests and work out the bugs there.

@pmoust
Copy link
Contributor

pmoust commented May 8, 2015

@johnrengelman I cherry picked your commit in #1827 (also changing the vpc_region check a bit).

@johnrengelman
Copy link
Contributor Author

Yeah, I ran into the same issues with the vpc_region check and fix it. I'm rebasing my commits now and am going to add a couple more tests.
Would you be interested in just submitting your additions back to my branch and then we can bundle them up in 1 PR?

@pmoust
Copy link
Contributor

pmoust commented May 8, 2015

I don't mind at all, feel free to pull. I just want this implemented, reviewed and merged so I can move along :>

@johnrengelman
Copy link
Contributor Author

Great. I just rebased my commit and it looks like there are conflicts with the new nameserver changes that went in (of course the route53 API deviates here as well). Once I get those worked out and all my tests passing, then I'm pull your changes in.

@johnrengelman
Copy link
Contributor Author

@catsby I just rebased this to the current master. It was just the Changelog that was conflicting.

@phinze
Copy link
Contributor

phinze commented May 14, 2015

@johnrengelman ah yeah we usually keep changelog out of PRs for that reason - the core team generally takes care of the changelog post-merge 👍

@johnrengelman
Copy link
Contributor Author

Noted for future. Thanks.

@catsby
Copy link
Contributor

catsby commented May 14, 2015

It works!

vpcs

Thanks all for the hard work in designing and getting this working. Clicking the button now.

catsby added a commit that referenced this pull request May 14, 2015
AWS/Route53Zone - create private hosted zone associated with VPC.
@catsby catsby merged commit b3a4965 into hashicorp:master May 14, 2015
@saulshanabrook
Copy link

@johnrengelman To make a private route53 zone, can I just do something like this?

resource "aws_route53_zone" "private" {
  name = "${var.private_route_name}"
  vpc_id = "${aws_vpc.default.id}"
  tags {
    Version = "2"
  }
}

Or do I need an aws_route53_zone_association?

@kendawg2
Copy link

@saulshanabrook you only need the association after the zone is created in order to associate with additional VPC's. Your example will work for creating it and associating it with that VPC.

@saulshanabrook
Copy link

@kendawg2 Thanks!

@johnrengelman johnrengelman deleted the route53_private_hosted_zone branch September 28, 2015 19:21
@CaptTofu
Copy link

CaptTofu commented Sep 30, 2016

Hi all, I know this is a year later, but how do I create two zones -- one for forwared dns records (A, CNAME, etc) and one for reverse (PTR)

resource "aws_route53_zone" "primary" {
   name = "${var.aws_cluster_domain}"
   vpc_id = "${aws_vpc.vpc.id}"
}
resource "aws_route53_zone" "reverse" {
   name = "${var.aws_cluster_domain_reverse}"
   vpc_id = "${aws_vpc.vpc.id}"
}

This is for the same VPC,but it doesn't work.

In the UI, I have to create a zone, private, I can set a VPC
Then, I create another private zone, but not set the VPC, but after it's created, associate it.

I can't force the zone to be private unless I set a vpc_id, so I have a bit of a chicken-and-egg

How do I do this?

@ghost
Copy link

ghost commented Apr 21, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.