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

cloudfront distribution should export route53 zone_id #6489

Closed
ryanking opened this issue May 4, 2016 · 7 comments
Closed

cloudfront distribution should export route53 zone_id #6489

ryanking opened this issue May 4, 2016 · 7 comments

Comments

@ryanking
Copy link
Contributor

ryanking commented May 4, 2016

Terraform Version

$ tf version
Terraform v0.6.15

Affected Resource(s)

Please list the resources as a list, for example:

  • cloudfront_distribution

Feature request

In order to create a route53 alias you need the zone id. So to create an alias to a cloudfront distribution, we need to have the cloudfront_distribution return its zone id.

@vancluever
Copy link
Contributor

Hey @ryanking, there was a conscious decision to leave this out of the resource, as it is a hardcoded value that is not exposed through the CloudFront API. See #5221 (comment) for the original discussion.

The zone ID is actually global for all CloudFront distributions - Z2FDTNDATAQYW2 - which is documented here.

I'm torn on this - I'm kind of weary of exposing arbitrary constants that are not exposed through the API, but also, at the same time, there seems to be a bit of an expectation that it will be there, from what I've seen from a couple of people here and on IRC, so maybe it's a good idea to have it in as a quality of life feature. I did something similar in cloudfront_origin_access_identity as well.

@radeksimko
Copy link
Member

To be fair we do this already for S3 buckets: https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_s3_bucket.go#L674

I think it's still better to have this hardcoded once in Terraform rather than sprinkled all over user's Terraform configs.

Also we should allow users to build the dependency chain where DNS records will only be created once the CF distribution is created - not simultaneously (which is the case when user hard-codes this IMO).

@apparentlymart
Copy link
Contributor

I would second @radeksimko's opinion here. Having this exposed is expected and intuitive in spite of the fact that we'd be working around a limitation in the native AWS API.

One of the big benefits Terraform provides, in my opinion, is making explicit what would normally be implicit: the relationships between different things are evident to those reading the Terraform configuration, even if they are not 100% familiar with the resource types being described... the reader doesn't have to chase down implicit relationships by noticing that an IP address over here matches with one over there, or whatever.

Just a few days ago one of my co-workers commented on a code review request I'd written asking why I'd hard-coded the zone_id one one alias (for a CloudFront distribution) and not another (for an ELB). He was pretty new to the idea of Route53 aliases anyway, and this was the first time he'd seen a Cloudfront distribution in a Terraform config. I'm pretty sure that the purpose would have been readily apparent and not worthy of question if this had been written in my config:

resource "aws_route53_record" "foo" {
    # ...

    alias {
        name = "${aws_cloudfront_distribution.foo.domain_name}"
        zone_id = "${aws_cloudfront_distribution.foo.zone_id}"
    }
}
resource "aws_route53_record" "origin" {
    # ...

    alias {
        name = "${aws_elb.foo.dns_name}"
        zone_id = "${aws_elb.foo.zone_id}"
    }
}

@radeksimko note that in this case the dependency was already created by referring to aws_cloudfront_distribution.foo.domain_name, so this extra edge is not technically necessary for Terraform but I'd argue that it's much more helpful to humans studying the config to see where this value is coming from and thus infer what it means. It's also easier to review: I can look at the above and believe that it's correct without having to look up an opaque value and confirm that it is what I think it is.

@radeksimko
Copy link
Member

so this extra edge is not technically necessary

True, I forgot about that dependency, you're right.

But it's still better to be explicit, as you mentioned.

@ryanking
Copy link
Contributor Author

ryanking commented May 5, 2016

Thanks @vancluever for the pointer. I was unable to find that.

At the very least this warrants a docs update. I know terraform pretty well and was unable to figure this out on my own.

On the other hand, exporting the zone, even if hardcoded would make life simpler.

@vancluever
Copy link
Contributor

Hey Guys,

Yeah, it definitely seems like there is a consensus here. The main thing that made me start thinking about this a bit more myself was when someone mentioned the dependency issue on IRC, ie: having the zone ID as an attribute allows a better natural dependency chain between a CloudFront distribution and its Route 53 records.

I can put in a PR soon, if there isn't one already.

vancluever pushed a commit to paybyphone/terraform that referenced this issue May 7, 2016
Added the hosted_zone_id attribute, which aliases to the Route 53
zone ID that can be used to route Alias Resource Record Sets to.

This fixes hashicorp#6489.
@stack72 stack72 closed this as completed in 84cd31c May 8, 2016
cristicalin pushed a commit to cristicalin/terraform that referenced this issue May 24, 2016
Added the hosted_zone_id attribute, which aliases to the Route 53
zone ID that can be used to route Alias Resource Record Sets to.

This fixes hashicorp#6489.
grubernaut pushed a commit to hashicorp/terraform-provider-aws that referenced this issue Jun 9, 2017
Added the hosted_zone_id attribute, which aliases to the Route 53
zone ID that can be used to route Alias Resource Record Sets to.

This fixes hashicorp/terraform#6489.
@ghost
Copy link

ghost commented Apr 26, 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 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants