Aws-cdk: ARecord is created even with publicLoadBalancer is set to false in ApplicationLoadBalancedFargateService

Created on 12 Mar 2020  路  9Comments  路  Source: aws/aws-cdk

Even when the 'publicLoadBalancer' is set to false, ApplicationLoadBalancedFargateService still creates an ARecord with loadbalancer as target.

I don't see any reason to have an ARecord when the loadbalancer is private.

Reproduction Steps

    // CloudFormation Resources
    this.service = new ecsPatterns.ApplicationLoadBalancedFargateService(this, 'Service', {
        .......
        .......
        .......
        publicLoadBalancer: false,
        protocol: elbv2.ApplicationProtocol.HTTPS,
        domainName: `abc.com`,
        domainZone: serviceHostedZone,
        .......
        .......
        .......

    });

Error Log

Environment

  • CLI Version :
  • Framework Version:
  • OS :
  • Language :

Other

It doesn't make sense to create an ARecord which never gets resolved when the 'publicLoadBalancer' is set false. Ideally record creation can be optional or atleast do it only when publicLoadBalancer is set true.


This is :bug: Bug Report

@aws-cdaws-ecs-patterns bug p2

Most helpful comment

If it is public or not depends on the type of hosted zone you are using. But even if it is public this is not a real issue. I don't think hiding IPs is a security thing.

The DNS name the LB gets is an AWS one, but most of the time I want to use a vanity name to reach this for example via VPN/DirectConnect from on-prem. I don't see a reason why internal LBs should not have DNS records in my hosted zones.

Additionally, I think the ECS patterns package is marked stable so breaking changes, and this is one, are not allowed imho.

All 9 comments

Marked this as p2 bug because it indeed seems unnecessary but would't impact the functionalities.

I do not really understand why an A record needs to be a public endpoint? What about internal LBs with internal hostnames? Or am I missing something obvious?

Thanks @hoegertn for bringing it up. I pondered on this issue for quite a while as well due to the possible use case you mentioned.

Please correct me if I'm wrong. If an A record is created via Cloudformation with a private ip (i.e. private LBs), then it would be a public DNS record with a non-resolvable ip unless you are inside the VPC. Also, after some preliminary experiments, I found that a private LB automatically gets a DNS name without any A record.

After some discussions within the team, we deemed that to be an uncommon use case. But please let us know if my understanding of the issue is seriously wrong :)

If it is public or not depends on the type of hosted zone you are using. But even if it is public this is not a real issue. I don't think hiding IPs is a security thing.

The DNS name the LB gets is an AWS one, but most of the time I want to use a vanity name to reach this for example via VPN/DirectConnect from on-prem. I don't see a reason why internal LBs should not have DNS records in my hosted zones.

Additionally, I think the ECS patterns package is marked stable so breaking changes, and this is one, are not allowed imho.

I see. Let me quickly bring this back to the team and revert the commit if necessary. Thanks for bringing this up!

Reverting: https://github.com/aws/aws-cdk/pull/6987

Thanks again @hoegertn for bringing this up!

@phaniram would you be interested to see #7206 implemented? Will the proposed flag satisfy your use cases?

@hencrice Yes. If possible eligibility to override too would work.

I don't think we should be making everything configurable in the patterns (the patterns are designed to be well constructed services with opinionated defaults). As this doesn't appear to be a common usecase, and the benefits don't outweigh the complexity, I'm of the opinion that we shouldn't add this flag as it would add unnecessary complexity to the constructs themselves. The patterns will start to become unmaintainable/unusable if everything is added as a configuration.

A workaround is to implement the pattern using L2s and remove the logic that creates the ARecord. Feel free to reopen and let me know if you have any further questions around how to do this with L2s.

Was this page helpful?
0 / 5 - 0 ratings