CloudFront distribution and origin access identity support #5221

vancluever · 2016-02-19T22:25:16Z

Reference: #3330

I'm opening this new PR just to try and move this along. This branch is pretty much good to merge.

As mentioned in #3330, this is feature-complete CloudFront distribution resource, actually slightly more feature complete than the equivalent in CloudFormation (it doesn't seem like GZIP compression is supported yet, for example). Supports multiple origins, cache behaviours, and has a config layout like you would expect to have when sending the distribution configuration to API or CloudFormation.

I have also added aws_cloudfront_origin_access_identity, which allows origin access identities to be generated from Terraform rather than the console, definitely pushing this feature set past what CloudFormation has.

One other thing I'm waiting on is #5218, which will allow me to enforce a max instance of 1 on some of the complex types that need it (example: default_cache_behavior, viewer_certificate, etc). But if this is merged before that is I will just add these at a later time.

Again, if anyone wants to give it ago in a custom build, if you find anything that's an issue, let me know, otherwise the acceptance tests give a good spread of use cases.

vancluever · 2016-02-19T22:50:22Z

PS: This is failing right now because CloudFront is not yet vendored - if this is necessary, let me know.

whiteadam · 2016-02-22T23:15:23Z

+1

vancluever · 2016-02-25T19:01:45Z

Small update to aws_cloudfront_origin_access_identity:

Ignore originAccessIdentityConfig.Comment on update if value is not set (pointer is nil when comment is blank)

vancluever · 2016-03-02T00:22:37Z

Update! I've added the MaxItems constraint to the appropriate items into the schema now that #5218 has been merged. Should ensure a safer configuration workflow.

shanhchen · 2016-03-04T00:45:32Z

Thanks for this, wondering whether it is possible to add back the zone_id on the output? To make it easier to use CloudFront w/ Route 53 ALIAS records in the DSL:
http://docs.aws.amazon.com/Route53/latest/APIReference/CreateAliasRRSAPI.html
#1775

vancluever · 2016-03-04T01:34:30Z

@shanhchen sorry, but I don't think that's a good idea.

I was kind of curious as to why it was absent when I was doing my refactor, incidentally, and it turns out that it is a hardcoded value (if you look in the original commit history you can see the static d.Set("zone_id") value. It is not supplied by the CloudFront SDK, nor the API from what I would imagine.

I would say that if you want to reference the CloudFront zone ID, just reference the static value as instructed in the docs.

shanhchen · 2016-03-04T17:23:09Z

Thanks a lot @vancluever I will try that way.

catsby · 2016-03-08T16:01:23Z

Apologies for the silence here. Should I be looking here, or #3330 ? I think this is a replacement for that one, right?

catsby · 2016-03-08T16:05:45Z

Also, as mentioned above, this needs the CloudFront dependency vendored.. See here for details

https://github.com/hashicorp/terraform#adding-a-dependency

Thanks!

vancluever · 2016-03-08T18:03:30Z

Hey @catsby, great to hear this is getting attention now. This is indeed the replacement, so this is the one that should be reviewed. I encountered a bug that needs fixing the last couple of days, I'll add that, in addition to the vendoring of the CloudFront SDK bits today.

vancluever · 2016-03-09T00:23:56Z

OK, bug fixes are in and CloudFront deps vendored. Not too sure what happened to the travis job, was hoping to see if it passed tests. But this should be good to go regardless.

AlexanderEkdahl · 2016-03-10T08:20:34Z

Impressive work!

joshuaspence · 2016-03-15T01:46:57Z

Looking forward to it

nickschuch · 2016-03-22T04:42:20Z

Anything I can do to help? Super keen to get this over the line!

Custom build of the CloudFront support pull request (hashicorp/terraform#5221) rebased onto v0.6.14 release.

tmatilai · 2016-03-22T20:29:34Z

website/source/docs/providers/aws/r/cloudfront_origin_access_identity.html.markdown

+The following example below creates a CloudFront origin access identity.
+
+```
+resource "aws_origin_access_identity" "origin_access_identity" {


Should be aws_cloudfront_origin_access_identity. Same in the second example.

tmatilai · 2016-03-22T20:39:48Z

I rebased this PR on top of the recent 0.6.14 release and put the OS X package available here if someone wants to test it.

There is also a fancy Terraform version switcher: Yleisradio/homebrew-terraforms
With it you can just say chtf 0.6.14+cf.

blakesmith · 2016-04-02T14:52:47Z

@vancluever Was able to use your latest branch to setup a S3 static site fronted with CloudFront. Everything worked perfectly. Thanks for the patches!

blakesmith · 2016-04-04T12:12:56Z

If anyone else is interested in trying out this PR, I wrote up a blog post on using this to setup an S3 static site fronted with CloudFront: http://blakesmith.me/2016/04/02/terraform-aws-static-site-with-cloudfront.html

gpetras · 2016-04-06T15:52:05Z

I've tested this out and so far it works great. One question I have - how do I setup the default cache behavior to forward all headers? In the UI there's a dropdown to select 'All' but I can't figure out how to get the same effect in my default_cache_behavior. The headers parameter accepts a list. It'd be great if it accepted a keyword, ie headers = "all", or something to that effect. Thanks for all your work @vancluever!

vancluever · 2016-04-06T17:42:46Z

Hey @gpetras you are welcome! Also, I got this from the SDK docs:

If you want CloudFront to forward all headers to the origin and vary on all of them, specify 1 for Quantity and * for Name.

So you want to specify * for all headers.

PS: I've updated the docs to be specific on this.

* Includes a complete re-write of the old aws_cloudfront_web_distribution resource to bring it to feature parity with API and CloudFormation. * Also includes the aws_cloudfront_origin_access_identity resource to generate origin access identities for use with S3.

gpetras · 2016-04-07T22:16:22Z

Thanks @vancluever - I did that and it worked great - much appreciated!

liviudm · 2016-04-08T12:35:48Z

Hi,

When I'm trying to specify an ACM ARN I get the following error:

Errors:

  * aws_cloudfront_distribution.liviu_io_distribution: cannot parse '' as bool: strconv.ParseBool: parsing "arn:aws:acm:us-east-1:XXXXXX:certificate/ee12ab44-25bd-4bd4-b00f-d9f0a3a8ad68": invalid syntax

Here's the relevant part from my configuration:

viewer_certificate {
        acm_certificate_arn = "arn:aws:acm:us-east-1:XXXXXX:certificate/ee12ab44-25bd-4bd4-b00f-d9f0a3a8ad68"
        minimum_protocol_version = "TLSv1"
        ssl_support_method = "sni-only"
}

However, there are no errors if I set acm_certificate_arn to true. Here's the relevant output from terraform plan:

    viewer_certificate.#:                                                                                 "0" => "1"
    viewer_certificate.103543815.acm_certificate_arn:                                                     "" => "1"
    viewer_certificate.103543815.cloudfront_default_certificate:                                          "" => ""
    viewer_certificate.103543815.iam_certificate_id:                                                      "" => ""
    viewer_certificate.103543815.minimum_protocol_version:                                                "" => "TLSv1"
    viewer_certificate.103543815.ssl_support_method:                                                      "" => "sni-only"

Also, when trying to apply the configuration:

Error applying plan:

1 error(s) occurred:

* aws_cloudfront_distribution.liviu_io_distribution: unexpected EOF

Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.

I guess this isn't the intended behavior and I should specify a valid ACM ARN as the value for acm_certificate_arn, not true or false.

tmatilai · 2016-04-08T15:44:35Z

@liviudm I think you have an old version of the pull request. That should be fixed.
Also note that the fixes are squashed (and rebased), so you're gonna need to hard reset your working tree after fetching.

vancluever · 2016-04-08T19:52:29Z

@liviudm as @tmatilai mentioned, this was caused by a bug in the first push for the ACM feature (if you see the outdated commit comments you can see Teemu's comments in there). Updating your tree as per his instructions should work and I verified the present commit to make sure there wasn't a regression.

bradleyayers · 2016-04-10T23:38:44Z

website/source/docs/providers/aws/r/cloudfront_distribution.html.markdown

+		prefix = "myprefix"
+	}
+	aliases = [ "mysite.example.com", "yoursite.example.com" ]
+	default_cache_behavior {


~~In the documentation below it says that path_pattern is required, yet in this example it is absent. One or the other should be updated.~~ Edit: Oops nevermind, I missed the default_cache_behavior section that clearly states path_pattern isn't required.

bradleyayers · 2016-04-11T10:00:23Z

@vancluever I'm having an issue with migrating from

  viewer_certificate {
    cloudfront_default_certificate = true
  }

to

  viewer_certificate {
    acm_certificate_arn = "…"
  }

When I run terraform plan there are no changes.

catsby · 2016-04-14T19:03:06Z

builtin/providers/aws/cloudfront_distribution_configuration_structure.go

+//
+// Used by the aws_cloudfront_distribution Read function.
+func flattenDistributionConfig(d *schema.ResourceData, distributionConfig *cloudfront.DistributionConfig) {
+	d.Set("origins", flattenOrigins(distributionConfig.Origins))


for anything that is not a simple value like int or string, we need to check the error from d.Set to ensure it was set correctly. This applies to all the d.Set's here where a complex structure is being saved

catsby · 2016-04-14T19:54:39Z

Going to go ahead and merge this as is. My feedback was pretty minimal and can be picked up as we go.

Thanks so much for the work here @vancluever and @AlexanderEkdahl !

feanil · 2016-04-14T19:57:45Z

@catsby how long before this is available in a new version of terraform?

vancluever · 2016-04-14T21:20:08Z

Amazing! Thanks @catsby!

@bradleyayers - will look into your and @catsby's feedback soon, including attempting to reproduce the diff issue (gotta set up a few things so I can do ACM easily enough).

vancluever · 2016-04-15T16:43:42Z

@bradleyayers just a note - I'm working on the issue you reported and it looks like it mainly comes up if you only specify acm_certificate_arn and no other parameters - it doesn't look like the hash is being calculated properly on that set item. Not too sure why it's not hashing, but in the meantime, if you want to move forward, just add ssl_support_method, which needs to be there anyway for ACM and IAM certificates.

…#5221) * CloudFront implementation v3 * Update tests * Refactor - new resource: aws_cloudfront_distribution * Includes a complete re-write of the old aws_cloudfront_web_distribution resource to bring it to feature parity with API and CloudFormation. * Also includes the aws_cloudfront_origin_access_identity resource to generate origin access identities for use with S3.

ghost · 2020-04-26T02:19:51Z

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.

vancluever mentioned this pull request Feb 19, 2016

[WIP] provider/aws: CloudFront #3330

Closed

vancluever force-pushed the cloudfront_v3 branch 2 times, most recently from 89643ad to 8afddbf Compare February 20, 2016 01:34

radeksimko added enhancement provider/aws new-resource labels Feb 25, 2016

vancluever force-pushed the cloudfront_v3 branch from 8afddbf to 1a45497 Compare February 25, 2016 18:59

vancluever force-pushed the cloudfront_v3 branch from 1a45497 to 29c32a1 Compare March 2, 2016 00:17

vancluever mentioned this pull request Mar 2, 2016

Add MaxItems attribute to Schema #5218

Merged

catsby added the waiting-response An issue/pull request is waiting for a response from the community label Mar 8, 2016

vancluever force-pushed the cloudfront_v3 branch from 29c32a1 to 1428430 Compare March 9, 2016 00:19

catsby removed the waiting-response An issue/pull request is waiting for a response from the community label Mar 14, 2016

tmatilai pushed a commit to Yleisradio/homebrew-terraforms that referenced this pull request Mar 22, 2016

Add Terraform 0.6.14+cf

b568ed0

Custom build of the CloudFront support pull request (hashicorp/terraform#5221) rebased onto v0.6.14 release.

tmatilai reviewed Mar 22, 2016
View reviewed changes

vancluever force-pushed the cloudfront_v3 branch from 1428430 to cc43b4a Compare March 22, 2016 22:19

vancluever force-pushed the cloudfront_v3 branch from 58d75b9 to a16c528 Compare April 6, 2016 17:58

AlexanderEkdahl and others added 3 commits April 7, 2016 13:10

CloudFront implementation v3

a51e536

Update tests

16e7a07

vancluever force-pushed the cloudfront_v3 branch from a16c528 to 68b45f5 Compare April 7, 2016 20:12

bradleyayers reviewed Apr 10, 2016
View reviewed changes

catsby reviewed Apr 14, 2016
View reviewed changes

catsby merged commit a38ccbe into hashicorp:master Apr 14, 2016

vancluever mentioned this pull request Apr 15, 2016

provider/aws: CloudFront post-merge review updates #6196

Merged

vancluever deleted the cloudfront_v3 branch April 20, 2016 17:44

vancluever mentioned this pull request May 4, 2016

cloudfront distribution should export route53 zone_id #6489

Closed

ghost locked and limited conversation to collaborators Apr 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CloudFront distribution and origin access identity support #5221

CloudFront distribution and origin access identity support #5221

vancluever commented Feb 19, 2016

vancluever commented Feb 19, 2016

whiteadam commented Feb 22, 2016

vancluever commented Feb 25, 2016

vancluever commented Mar 2, 2016

shanhchen commented Mar 4, 2016

vancluever commented Mar 4, 2016

shanhchen commented Mar 4, 2016

catsby commented Mar 8, 2016

catsby commented Mar 8, 2016

vancluever commented Mar 8, 2016

vancluever commented Mar 9, 2016

AlexanderEkdahl commented Mar 10, 2016

joshuaspence commented Mar 15, 2016

nickschuch commented Mar 22, 2016

tmatilai Mar 22, 2016

tmatilai commented Mar 22, 2016

blakesmith commented Apr 2, 2016

blakesmith commented Apr 4, 2016

gpetras commented Apr 6, 2016

vancluever commented Apr 6, 2016 •

edited

Loading

gpetras commented Apr 7, 2016

liviudm commented Apr 8, 2016

tmatilai commented Apr 8, 2016

vancluever commented Apr 8, 2016

bradleyayers Apr 10, 2016

bradleyayers commented Apr 11, 2016

catsby Apr 14, 2016 •

edited

Loading

catsby commented Apr 14, 2016

feanil commented Apr 14, 2016

vancluever commented Apr 14, 2016

vancluever commented Apr 15, 2016

ghost commented Apr 26, 2020

CloudFront distribution and origin access identity support #5221

CloudFront distribution and origin access identity support #5221

Conversation

vancluever commented Feb 19, 2016

vancluever commented Feb 19, 2016

whiteadam commented Feb 22, 2016

vancluever commented Feb 25, 2016

vancluever commented Mar 2, 2016

shanhchen commented Mar 4, 2016

vancluever commented Mar 4, 2016

shanhchen commented Mar 4, 2016

catsby commented Mar 8, 2016

catsby commented Mar 8, 2016

vancluever commented Mar 8, 2016

vancluever commented Mar 9, 2016

AlexanderEkdahl commented Mar 10, 2016

joshuaspence commented Mar 15, 2016

nickschuch commented Mar 22, 2016

tmatilai Mar 22, 2016

Choose a reason for hiding this comment

tmatilai commented Mar 22, 2016

blakesmith commented Apr 2, 2016

blakesmith commented Apr 4, 2016

gpetras commented Apr 6, 2016

vancluever commented Apr 6, 2016 • edited Loading

gpetras commented Apr 7, 2016

liviudm commented Apr 8, 2016

tmatilai commented Apr 8, 2016

vancluever commented Apr 8, 2016

bradleyayers Apr 10, 2016

Choose a reason for hiding this comment

bradleyayers commented Apr 11, 2016

catsby Apr 14, 2016 • edited Loading

Choose a reason for hiding this comment

catsby commented Apr 14, 2016

feanil commented Apr 14, 2016

vancluever commented Apr 14, 2016

vancluever commented Apr 15, 2016

ghost commented Apr 26, 2020

vancluever commented Apr 6, 2016 •

edited

Loading

catsby Apr 14, 2016 •

edited

Loading