provider/aws: API Gateway custom domains and base path mappings #8353

apparentlymart · 2016-08-21T16:51:28Z

This is a continuation of what @jarrodjackson started in #7708. It begins with his commit from that PR to add aws_api_gateway_domain_name, and then builds on it with the aws_api_gateway_base_path_mapping implementation from #6175 and documentation for both.

This pair of resources allow an API to be presented at a given path prefix on a custom hostname. The separation between domain name and base path mapping means that multiple APIs or API stages from a single AWS account can be presented on a single domain name.

aws_api_gateway_domain_name resource and acceptance tests (from [WIP] provider/aws: API Gateway Domain Name. #7708)
- Documentation
aws_api_gateway_base_path_mapping resource and acceptance tests (extracted from provider/aws: API Gateway additional resources #6175)
- Documentation

Once complete, this should fix #7421.

radeksimko · 2016-08-21T17:51:04Z

FYI: #6175 contains both resources. That said nobody had time to review and/or merge that PR yet and it would need to be rebased 1st either way.

The domain resource in this PR looks like it's ready for review, so to prevent another stale WIP PR, would you mind keeping this PR focused on aws_api_gateway_domain_name only and then open another one for aws_api_gateway_base_path_mapping?

apparentlymart · 2016-08-21T20:48:42Z

@radeksimko The domain name resource is pointless without the base path mapping, so my intent was to finish them both up here today (based on the work done by Jarrod and Ryan) and hopefully get these things finally merged.

I've just finished rebasing the base path mapping part of Ryan's mega-PR (and adapting it a little so it can work without the other resources he implemented) and I'm just wrapping up the docs now, so I should be able to get this out of WIP today.

apparentlymart · 2016-08-21T21:30:58Z

@radeksimko I think this is ready for a review pass now. 😀

radeksimko · 2016-08-22T10:17:02Z

builtin/providers/aws/resource_aws_api_gateway_base_path_mapping.go

+	if err != nil {
+		if err, ok := err.(awserr.Error); ok && err.Code() == "NotFoundException" {
+			d.SetId("")
+			return nil


It would be IMO helpful to have a WARN log message here (like we have in some other resources), so that if we ever end up debugging missing resources we'd be able to track it down.

radeksimko · 2016-08-22T11:02:17Z

@apparentlymart I left you some comments there, but vast majority of them are nitpicks. The only real biggie is one failing acceptance test due to uniqueness of domain names within AWS.

Otherwise this is looking pretty good.

radeksimko · 2016-08-22T11:03:24Z

builtin/providers/aws/resource_aws_api_gateway_domain_name_test.go

+  certificate_chain = "%v"
+  certificate_name = "tf-acc-apigateway-domain-name"
+  certificate_private_key = "%v"
+  domain_name = "test-api-gateway-domain-cert0.com"


And this should be randomised as well.

apparentlymart · 2016-08-22T15:31:41Z

Thanks for the feedback, @radeksimko. Agreed that randomizing that domain name seems unavoidable since it needs to be unique across the whole of AWS... will need to do some more work here to generate certs on the fly, and hope that API Gateway isn't also checking that the certificate chain leads back to some "real" CA that it trusts.... 😟

apparentlymart · 2016-08-22T16:16:04Z

Chatted a bit with @radeksimko elsewhere. He suggested that we should just replace the static, self-signed cert here with a wildcard one and then randomly generate a subdomain, which makes a lot of sense and should be easier than what I suggested.

@jarrodjackson pointed out that he made these certs using a Terraform config, which should be easy enough to adapt to make a wildcard cert: https://github.com/jarrodjackson/cert-generator

apparentlymart · 2016-08-22T16:56:42Z

Using a wildcard cert seems to have worked to address the hostname conflict problem. I'll fix up the rest of this in a little while.

API Gateway allows users to "claim" a domain name for use as a custom hostname for deployed API endpoints. The domain name resource just claims the domain name; a user would then use a "base path mapping" resource (to be implemented in a later commit) to map a particular API to a particular path prefix on that domain. The acceptance tests contain some TLS certificates that expire in 2026; we'll need to generate some more certificates before we get there.

apparentlymart · 2016-08-22T18:56:27Z

@radeksimko I've addressed the remaining comments now, except the one about doing updates on the base path mapping resource. I agree we should do this, but I also agree that it'd be good to address that in a separate PR since I think this approach will work for 90% of situations and make API Gateway setup with Terraform more complete.

(The main situation I see where it could potentially cause problems is if I want to change the stage of a pre-existing API. As implemented here Terraform will delete the old mapping and then create a replacement, which is likely to cause a brief outage where no mapping is present. Doing an update in-place would presumably avoid that. But I don't expect people will be frequently changing the stage of a deployed API.)

radeksimko · 2016-08-22T22:07:57Z

builtin/providers/aws/resource_aws_api_gateway_base_path_mapping.go

+	})
+
+	id := fmt.Sprintf("%s/%s", d.Get("domain_name").(string), d.Get("base_path").(string))
+	d.SetId(id)


I think the ID should be set only if there are no errors - i.e. below the condition, not above. Theoretically Read() will wipe the resource during next plan, but it would be cleaner if it didn't ever end up in the state.

Oh yes, of course. 🌴

radeksimko · 2016-08-22T22:08:26Z

Besides one last nitpick this LGTM.

API Gateway allows users to "claim" a domain name for use as a custom hostname for deployed API endpoints, and then use this base path mapping resource to expose a particular API deployment at a path on such a domain. The acceptance tests use certificates from the aws_api_gateway_domain_name tests which expire in 2026; we'll need to generate some more certificates before we get there.

apparentlymart · 2016-08-22T22:38:55Z

Thanks, @radeksimko. I addressed that last comment so I'm going to merge once the tests have passed.

apparentlymart · 2016-08-22T22:48:18Z

Thanks to @jarrodjackson and @ryansroberts for doing all the work here! 😀

OliverCole · 2016-08-24T19:10:58Z

While I wait for this to land in a relase - one question for @apparentlymart - you were talking about generating a certificate on the fly? What if we have a fully valid one? Will we be able to use that?

apparentlymart · 2016-08-24T21:27:35Z

@OliverCole that issue with certs was specific to Terraform's acceptance tests. For real-world use, you can use any certificate you have as long as Terraform has access the PEM-encoded cert, cert chain and private key.

API Gateway is unfortunately divergent from the "usual" AWS approach: normally we can register a certificate separately in IAM/ACM and then reference it from other services, but for API Gateway it's necessary to upload the cert to the API Gateway API and then it will create a cert in IAM on your behalf. We can't do anything about this different workflow, but the Terraform resources will give you the same capabilities as you get when registering domains and path mappings via the UI.

OliverCole · 2016-08-24T21:37:31Z

@apparentlymart Great, thanks for clarifying, and thanks for working on it!

ghost · 2020-04-22T02:39:07Z

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.

apparentlymart added enhancement provider/aws new-resource labels Aug 21, 2016

apparentlymart force-pushed the f-aws-api-gateway-custom-domain branch from abcd233 to 091da7f Compare August 21, 2016 20:45

apparentlymart force-pushed the f-aws-api-gateway-custom-domain branch 2 times, most recently from e9d31dd to 332bfaf Compare August 21, 2016 21:19

apparentlymart changed the title ~~[WIP] provider/aws: API Gateway custom domains and base path mappings~~ provider/aws: API Gateway custom domains and base path mappings Aug 21, 2016

radeksimko reviewed Aug 22, 2016
View reviewed changes

radeksimko added the waiting-response An issue/pull request is waiting for a response from the community label Aug 22, 2016

radeksimko removed the waiting-response An issue/pull request is waiting for a response from the community label Aug 22, 2016

apparentlymart force-pushed the f-aws-api-gateway-custom-domain branch from 332bfaf to 6aacd66 Compare August 22, 2016 16:54

apparentlymart force-pushed the f-aws-api-gateway-custom-domain branch from 6aacd66 to 72dde55 Compare August 22, 2016 18:45

radeksimko reviewed Aug 22, 2016
View reviewed changes

ryansroberts and others added 2 commits August 22, 2016 15:36

website: Docs for AWS API Gateway domain name and base path mapping

c4255f1

apparentlymart force-pushed the f-aws-api-gateway-custom-domain branch from 72dde55 to c4255f1 Compare August 22, 2016 22:37

apparentlymart merged commit c4255f1 into master Aug 22, 2016

apparentlymart added a commit that referenced this pull request Aug 22, 2016

Merge #8353: New resources for custom domains on AWS API Gateway

38289dd

This was referenced Aug 22, 2016

[WIP] provider/aws: API Gateway Domain Name. #7708

Closed

provider/aws: API Gateway additional resources #6175

Closed

radeksimko deleted the f-aws-api-gateway-custom-domain branch August 23, 2016 06:04

ghost locked and limited conversation to collaborators Apr 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/aws: API Gateway custom domains and base path mappings #8353

provider/aws: API Gateway custom domains and base path mappings #8353

apparentlymart commented Aug 21, 2016 •

edited

Loading

radeksimko commented Aug 21, 2016

apparentlymart commented Aug 21, 2016

apparentlymart commented Aug 21, 2016

radeksimko Aug 22, 2016

radeksimko commented Aug 22, 2016

radeksimko Aug 22, 2016

apparentlymart commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

radeksimko Aug 22, 2016

apparentlymart Aug 22, 2016

radeksimko commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

OliverCole commented Aug 24, 2016

apparentlymart commented Aug 24, 2016

OliverCole commented Aug 24, 2016

ghost commented Apr 22, 2020

provider/aws: API Gateway custom domains and base path mappings #8353

provider/aws: API Gateway custom domains and base path mappings #8353

Conversation

apparentlymart commented Aug 21, 2016 • edited Loading

radeksimko commented Aug 21, 2016

apparentlymart commented Aug 21, 2016

apparentlymart commented Aug 21, 2016

radeksimko Aug 22, 2016

Choose a reason for hiding this comment

radeksimko commented Aug 22, 2016

radeksimko Aug 22, 2016

Choose a reason for hiding this comment

apparentlymart commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

radeksimko Aug 22, 2016

Choose a reason for hiding this comment

apparentlymart Aug 22, 2016

Choose a reason for hiding this comment

radeksimko commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

apparentlymart commented Aug 22, 2016

OliverCole commented Aug 24, 2016

apparentlymart commented Aug 24, 2016

OliverCole commented Aug 24, 2016

ghost commented Apr 22, 2020

apparentlymart commented Aug 21, 2016 •

edited

Loading