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

Option to create nice DNS record for ALB, created by this plugin #3

Closed
denis-ryzhkov opened this issue Oct 27, 2021 · 3 comments
Closed

Comments

@denis-ryzhkov
Copy link
Contributor

denis-ryzhkov commented Oct 27, 2021

Option to create nice DNS record for ALB, created by this plugin

This plugin creates ALB with unpredictable DNS name and no simple way to add a nice DNS record for it.

Please add an option like dnsRecord with domainName and hostedZoneId inside, and use it in this plugin code instead of the workaround below:

resources:
  Resources:

    AlbDnsRecord:
      Type: AWS::Route53::RecordSetGroup
      DependsOn:
      - ${self:custom.albResourceName}
      Properties:
        HostedZoneId: TODO_options_dnsRecord_hostedZoneId
        RecordSets:
        - Name: TODO_options_dnsRecord_domainName
          Type: A
          AliasTarget:
            DNSName:
              Fn::GetAtt:
              - ${self:custom.albResourceName}
              - DNSName
            HostedZoneId:
              Fn::GetAtt:
              - ${self:custom.albResourceName}
              - CanonicalHostedZoneID

custom:
    albResourceName: REDACTED_ServerlessService_Stage_ALB_Stage
    # ALB created by serverless-ecs-plugin
@gwdp
Copy link
Member

gwdp commented Oct 28, 2021

Hi @denis-ryzhkov,

That's a good idea and has been on the road map for a while, but we still have the problem that ACM-generated certificates involve some async handling that is currently not possible within cloud formation and having the Domain configured but leaving the SSL certificate management manually, not so useful.

Having support to Domains (route53) and SSL certs (ACM) would certainly go beyond the ECS functionality this plugin is proposed to do.
Nevertheless, you can always use another serverless plugin aside to set up your Domain and SSL certificate and make use of it on the ECS plugin, I believe leaving this extra work with other plugins have the benefit of the long term life of this plugin and code stability.

I have evaluated adding the plugin https://www.serverless.com/plugins/serverless-certificate-creator AND https://www.serverless.com/plugins/serverless-domain-manager into this plugin (internally) but still haven't gone too far.

For now, I recommend you use the following plugins if you are doing the domain configuration at the serverless layer.

@denis-ryzhkov
Copy link
Contributor Author

denis-ryzhkov commented Oct 29, 2021

Thank you, @gwdp!

I also tried to use https://www.serverless.com/plugins/serverless-domain-manager
but found it supports APIGateway case only, not the ECS+ALB case this serverless-ecs-plugin provides, right?

The simple dnsRecord option I propose to add to serverless-ecs-plugin would had saved me a lot of time! Instead, I had to invent that workaround above.

ACM cert is NOT needed for this dnsRecord option at all!
Please check the workaround code above - it never references any ACM cert.
The cert ARN is set in already-implemented ecs.services.listeners.certificateArns and it works OK.
User just tells which domain name in which Route53 hosted zone they want to create-or-update, and that's all.

Please add this dnsRecord option, or any other user who finds this workaround will have to find albResourceName: REDACTED_ServerlessService_Stage_ALB_Env value for their specific case, while the plugin already knows this value.

Look, this is ECS plugin, but it creates tons of non-ECS resources, it even creates ALB, the Service is almost ready to be used, but no reliable DNS name is provided... whooops.

Adding this dnsRecord option looks really simple (for your level of understanding the codebase) and will provide a really valuable feature to the plugin users. Please add it!! :)

@gwdp
Copy link
Member

gwdp commented Nov 1, 2021

Hello @denis-ryzhkov,

I see your point, however, this plugin does not propose to go through the DNS layer and deploy a complete SAAS solution for you; It's proposal to have a fully working ECS setup, which includes having the ALB to ECS can register the targets... Even more, when you think about security and complex architectures, DNS will not be a simple CNAME for you ECS ALB, and you should not have your route 53 zones at the same AWS account your service is deployed. So having this is a truly handy feature for small deployments but enforces a bad practice from a cloud engineer perspective and does not do any good for the plugin IMO.

Having the ability to create custom CloudFormation resources within your serverless definition, in your case, the DNS name for the load balancer is not a workaround but yes, the proposed solution if you want to do something else with the resources this plugin creates. https://www.serverless.com/framework/docs/providers/aws/guide/resources

I recognize the outputs of this plugin are roughly documented and need improvement (PR would be nice).

Another point would have this CNAME resource documented on the readme or somewhere else, so users that want to make it, although knowing that there are security concerns on doing so and the recommended approach is totally different, still want to make it (because it's simpler and fits their use case). (PR would be nice as well)

Regarding the ACM, if you are using HTTP over SSL, what you should be doing, ACM will be required, otherwise, ALB will refuse your request. So it's a required resource for sure, but missing its configuration will not throw an error on deployment but will cause issues along the way.

Having a partial solution that leads to a bad practice is not the goal of the plugin and having this implemented would lead more and more people doing so. Imagine configuring your service using HTTPS, having everything deployed, including the CNAME, but your SSL certificate, related to the CNAME this plugin created not working and not configured; Wouldn't that be more confusing?

Closing this for now as I don't see this Route53/CNAME support going forward, sorry :/

@gwdp gwdp closed this as completed Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants