-
Notifications
You must be signed in to change notification settings - Fork 4k
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
ECSPatterns: Support switch for http->https redirect #5583
Comments
@juhofriman Thank you so much for requesting this feature. I'm trying to repeat your stopgap example and running into trouble. Did you write this in javascript or a different language? I've created a typescript version here and can't figure how to add your code: https://gist.github.com/CaptainChemist/c126e8c9b8f497cecaa20c793c48312f I'm confused about whether you are creating a separate load balancer to handle the port 80 versus the one that is used for port 443 (that we add using the addListener method). I tried simply adding a listener on port 80 and have that code commented out in the gist, but as you mention the approach isn't a great one because it doesn't enforce SSL. |
I am new to AWS CDK. Could you please provide some sample for ApplicationLoadBalancedFargateService to add listener for https->https. when i give certificate it is throwing A domain name and zone is required when using the HTTPS protocol, i dont have Route53 hosted zone to specify. When i added manually listener to this service in console it is working, need sample for python CDK |
I was struggling with the same problem today and found a solution: You need to create a listener for your load balancer for HTTP on port 80.
And then you can redirect all incoming HTTP requests to HTTPS by adding a RedirectResponse like so:
I think this is a common problem and an example solution should be inserted to the cdk examples or documentation. |
@08en yeah I think we should add this to the elasticloadbalancingv2 docs if its not already there. |
Yes I would say so. I'd guess that the vast majority of public facing LBs created today would want to have this redirect. Should be easy enough to add in an existing example. |
@08en Your suggestion seems really promising, but since
I haven't found a way obtain a reference to the existing port 80 listener. (Something like Is there a solution to this? |
@merland the listener that is created by default is available as the |
Ah, I see. Thanks a lot!
…On Fri, 27 Mar 2020, 16:55 Adam Ruka, ***@***.***> wrote:
@merland <https://github.com/merland> the listener that is created by
default is available as the listener property on the
ApplicationLoadBalancedFargateService.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5583 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADOTZGDRH5EOBYRKOOPUALRJTEAJANCNFSM4KBMMOKQ>
.
|
It is still not clear to me how to get control of the listener (rules) in this context. It would be nice to be able to pass a list of listeners to the service constructor, it is not unusual to have more than one. I would love the suggestion above by @08en to work, but I can't see that it does. |
@merland Have you tried setting a specific listenerPort when creating the ApplicationLoadBalancedFargateService? It is one of the optional constructor properties. Having this set to 443 should allow you to add another http listener to your load balancer for port 80 after creation. Alternatively you can also pass the load balancer object as a whole directly to the ApplicationLoadBalancedFargateService by using the loadBalancer costructor property which is also optional. |
@08en Thanks! Changing the port got me one step forward. But then I got into trouble when trying to add a certificate... |
I wasn't able to use any of the workarounds in this thread out of the box, but they helped me find a solution in Typescript finally. To sum up, what I wanted to do was create an ApplicationLoadBalancedFargateService that has:
These are pretty standard requirements and I had expected this to be easy-peasy to do with CDK, but it was not! I will post my commented solution below, I am sure someone can point out things to simplify.
|
@merland I played with this for a bit, tell me if this doesn't fit your use-case. Seems to get around the need to use escape hatches.
|
@MrArnoldPalmer Your solution almost fits my use case. Except for:
Would be nice to lose the escape hatches, but It seems that ApplicationLoadBalancedFargateService only allows one certificate to be added to the default listener/port. |
Gotcha. We will have to explore API additions to support some more customization here. |
How could I achieve this without creating a |
@danilofuchs do you mean without creating a new hostedZone/domain? Or do you mean using an external DNS provider on the load balancer? The former you can achieve by using the The latter may require some investigation. The construct as it exists right now doesn't appear to be designed around that and you may have better luck dropping down to the |
My situation fits into the latter option. Our DNS is managed via CloudFlare. We want to be able to communicate our Fargate service with CloudFlare via HTTPS. It would be nice if the default DNS option available via Elastic Load Balancer ( The only other option I see is to expose our Fargate service via an internal domain name, managed by Route 53, and limit traffic to CloudFlare's IPs (which we already do) |
@danilofuchs I would need to investigate to be able to provide answers for this specific case. My current recommendation would be to create a stack overflow question about attaching DNS from cloudflare to an ALB/NLB using certs provided by ACM generally (not specific to CDK) and see how it is done. That may give you some hints as to what parameters need to be passed to the CFN resources being created and if the CDK constructs can be made to fit. Stack overflow may yield you better results since we have narrowed this particular issue down to "increased control over the ELB Listener created by constructs in @merland re-reading some docs, you can access the default listener created after initialization and add additional certs there. You can reference an existing Route53 hosted zone using const service = new ecsPatterns.ApplicationLoadBalancedFargateService( blah );
service.listener.addCertificates("MoreCerts", [ cert1, cert2 ]); There is also |
@MrArnoldPalmer Thanks for looking into this. |
At this point with the examples everyone has posted, I think we have demonstrated that this is supported. I would argue that customization beyond what Is in the examples is better suited to be implemented with lower level constructs in aws-ecs. If we uncover a specific use case that isn’t supported without escape hatches (ie passing multiple certs to the listener created by the construct) a separate issue can be created. |
Can you please also provide an example for ECS EC2 cluster? |
I am new to AWS CDK, facing issue with targets draining. Only one of the IP target is healthy and other IP targets(private network resource) of the same target group on port 80 are draining. I am getting billed for those targets since my NAT gateway flowlog shows bytes transferred to those deregistered IP's. Any help would be greatly appreciated. Am i missing something. I was able to access my app. The service is ApplicationLoadBalancedFargateService with https listerner on port 443 pointed to same target group of http listener on port 80. Please help |
I struggled days with that redirect issue but finally I found solution that works for me.
|
@MrArnoldPalmer For those that are not using Route53 hosted zones (but still using AWS certificates), trying to get an https endpoint working this seems like a large workaround. Especially for such a common use-case as redirecting http traffic to https. |
@iwarshak I agree. Patterns libraries in general being higher level they make more assumptions for the sake of ease/convenience. For more specific use cases using the L2 constructs is gonna make more sense. If we add more configurable properties to the patterns constructs their api surface area can become very similar to that of all of the underlying L2 constructs combined. That being said, what feature addition would the construct need to accommodate usage of third party DNS providers with certs from ACM to more easily setup the http->https redirect? |
@MrArnoldPalmer Well, I think it should AND I think there should be an option for I think it's a little more than the surface area overlapping the underlying API. In order to get it working now, I have to create my own load balancer, add an http listener with a redirect. I create my If this was something more specialized, I would agree with you. But this is http->https redirection, something that almost every single site does today. |
@iwarshak alright I’m with you there. If users provide a cert adding an option to automatically setup the redirect makes sense and maybe should even be the default since its something we probably actually want to encourage (though that would be a breaking change so maybe not default to start). I’ll make an issue for this. More investigation will have to be done in order to understand what we need to do to support external DNS providers and HTTPS more ergonomically within the patterns constructs. In general I would like to do this but just worry about further complicating a constructs API. |
Thanks @MrArnoldPalmer - I am not sure that anything related to external DNS providers has to be done? It's up to the end user to point DNS to the right place. In my mind it's
Much appreciated! |
I was able to solve my particular issue by adapting @LassiPes's solution to Typescript. Reading the CDK code that builds this pattern, I was able to pinpoint the main issue when trying to do this: the construct always creates a listener with type I suggest the pattern first checks to see if the load balancer already has listeners for the port. If so, assume the user configured it correctly. Although this can become weird as the targetGroup is not available before declaring the pattern. Another alternative is work on the That said, I think @iwarshak solution is much simpler and quite feasible. If I have a certificate, I should be able to link DNS with another service. Maybe adding a warning on synth so unsuspicious users configure Route53 if necessary? |
Some follow up: #8488 is the issue tracking the @iwarshak do you mind writing up an issue with an example of the conditions where you can't get your cert attached? Looking at the code here it "should" be honoring and attaching it regardless of a hosted zone being passed or not so want to make sure it gets described accurately. |
@MrArnoldPalmer The issue was not so much getting a certificate attached (from what I remember), it is that the default |
can someone show me how to retrieve the listener arn if it is attached to a existing load balancer? I'm using this to create the load balancer and the listener is automatic added to port 80.
|
For anyone coming in 2021- replace with |
@ScottHoward4 https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-ecs-patterns.ApplicationLoadBalancedFargateService.html see option redirect_http
|
It's common use case to redirect all http requests to https at the load balancer level. Use cases include UI:s that are served from containers and API implementations as well. Giving https upgrade automatically to your users (end users using application and developers testing your API's) is a usability issue. It's like saying: "here I am, but could you talk securely to me?".
Application Load Balancer has great support for this already, but unfortunately it seems like this use case is not supported in
ApplicationLoadBalancedFargateService
pattern. Same thing probably affects other ALB based high level patterns as well.Use Case
If you need to implement http to https redirect with ECS Patterns, you have to do some heavy lifting in order to be able to configure simple redirect.
ApplicationLoadBalancedFargateServiceProps
builder hasloadBalancer()
, but it seems like then you have to do the heavy VPC configuration and such.Workaround seems to be something like following.
This "decorates" used load balancer and merges configuration pretty intelligently and this is totally acceptable workaround for me. I personally feel that this is really common pattern in web applications to be able to listen both http and https traffic and upgrade all http requests to https, and it would help AWS and CDK newcomers to be able to define this really common use case in more easy way.
Proposed Solution
High level constructs could take additional "redirectHttpToHttps(boolean)", such as
If
redirectHttpToHttps
is called withtrue
, when certificate is not set, synth should fail, because then http listener which redirects traffic to target group is configured.More advanced option would be the ability to configure http traffic to target group, because there are applications in the wild that want (for some reason...) handle both http and https traffic. This could be done with something like:
httpPolicy(HttpPolicy.NO_LISTENER)
No listener for httphttpPolicy(HttpPolicy.REDIRECT_TO_HTTPS)
Do http->https redirecthttpPolicy(HttpPolicy.ROUTE_TO_TARGET_GROUP)
Route http traffic to target groupOther
This is just an idea I wanted to share. I don't mind rejection :) I would love to help out, and if this is regarded as a good idea, I just might take some time to investigate how this could be implemented.
This is a 🚀 Feature Request
The text was updated successfully, but these errors were encountered: