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

elbv2: Investigate NetworkLoadBalancer #4268

Closed
jd-carroll opened this issue Sep 27, 2019 · 5 comments
Closed

elbv2: Investigate NetworkLoadBalancer #4268

jd-carroll opened this issue Sep 27, 2019 · 5 comments
Assignees
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@jd-carroll
Copy link
Contributor

jd-carroll commented Sep 27, 2019

Quick Update:

This is partly user error 🙏 (60%)

One of the core things I missed to begin with is that a NetworkLoadBalancer does not support Security Groups. Therefore they are fundamentally different from ApplicationLoadBalancer s.

However, there is still likely room to make the NetworkLoadBalancer a little more user friendly, specifically relating to the Security Group assigned to the instance.

Recap:

We are dealing with a NetworkLoadBalancer, therefore all of the security policies are defined at the instance level (not the load balancer level) (subnets aside). To change the ingress policy for the instance we either need access to the SecurityGroup of the AutoScalingGroup or the AutoScalingGroup itself (or add a new AutoScalingGroup). With either of these we are able to make the necessary changes (either edit the existing or add a new SecurityGroup) to enable access to the instances.

Problem:

The problem is that when you get the AutoScalingGroup from the cluster (cluster.autoscalingGroup), it returns a type of IAutoScalingGroup and not the AutoScalingGroup type. The IAutoScalingGroup does not have any reference to the underlying security groups and you are not able to add a new SecurityGroup.

All of these facts taken together make it very difficult to work with ECS & NetworkLoadBalancers from CDK.

Original Post:

It looks like there are a number of missing features from the NetworkLoadBalancer. Specifically, there are no security groups created by default when using the NetworkLoadBalancedEc2Service from the ecs-patterns package.

Reproduction Steps

First deploy the stack:

const app = new cdk.App();
const stack = new cdk.Stack(app, 'aws-ecs-network-lb', { 
  tags: { id: 'network-lb' }
});

// Create a cluster
const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 2 });

const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
cluster.addCapacity('DefaultAutoScalingGroup', {
  instanceType: ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.MICRO)
});

// create a task definition with CloudWatch Logs
const logging = new ecs.AwsLogDriver({ streamPrefix: "task" })

// Create Task Definition
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');
const container = taskDefinition.addContainer('web', {
  image: ecs.ContainerImage.fromRegistry('nginx:latest'),
  memoryLimitMiB: 256,
  logging
});

container.addPortMappings({
  containerPort: 80,
  hostPort: 80,
  protocol: ecs.Protocol.TCP
});

// Create Service
const service = new ecs.Ec2Service(stack, "Service", {
  cluster,
  taskDefinition
});

// Create ALB
const lb = new elbv2.NetworkLoadBalancer(stack, 'LB', {
  vpc,
  internetFacing: true
});
const listener = lb.addListener('PublicListener', { port: 80 });

// Attach ALB to ECS Service
listener.addTargets('ECS', {
  port: 80,
  targets: [service],
  // include health check (default is none)
  healthCheck: {
    interval: cdk.Duration.seconds(30)
  }
});

new cdk.CfnOutput(stack, 'LoadBalancerDNS', { value: lb.loadBalancerDnsName, });

app.synth();

Then look at the security groups associated with the EC2 instances. There are no ingress groups to allow traffic from the load balancer to the instance.

Error Log

N.A.

Environment

  • CLI Version : 1.9.0
  • Framework Version: 1.9.0
  • OS : macOS 10.14.6
  • Language : all

Other

Also given the issue described in #4267, I would expect to have this stack fail as well.


This is 🐛 Bug Report

@jd-carroll jd-carroll added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 27, 2019
@SomayaB SomayaB added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Sep 27, 2019
@jd-carroll jd-carroll changed the title NetworkLoadBalancer Missing Security Groups elbv2: NetworkLoadBalancer Unusable Sep 28, 2019
@jd-carroll
Copy link
Contributor Author

jd-carroll commented Sep 28, 2019

[Please note updated title and update at top]

This is partly user error & partly the differences identified below are legitimate differences. However... I still think that there are some missing features so I will leave the list below to further the conversation. The fix for most of these features will be pulling the existing implementation out of the corresponding ApplicationLoadBalancer resource and into the shared BaseLoadBalancer resource.

NetworkLoadBalancer

  • No connection support
  • No security group support
  • Cannot configure access logs

NetworkListener

  • No connection support
  • Missing TLS support

NetworkTargetGroup

  • Missing support for metrics

Again, I think the fix is going to pretty straight forward. In most cases it will be taking the current feature from the ApplicationLoadBalancer and pulling it into the BaseLoadBalancer. But before I start down that road, I'd like to confirm that this would be the supported direction.

@jd-carroll jd-carroll changed the title elbv2: NetworkLoadBalancer Unusable elbv2: Investigate NetworkLoadBalancer Unusable Sep 28, 2019
@jd-carroll
Copy link
Contributor Author

jd-carroll commented Sep 28, 2019

Update 3:

This is partly user error 🙏 (60%)

I've updated all of the existing comments with my latest knowledge, please take a gander at both and we can continue the conversation from there.

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 30, 2019

Closing as a duplicate of #1490, please reopen if you think this has been closed in error.

@rix0rrr rix0rrr closed this as completed Sep 30, 2019
@jd-carroll
Copy link
Contributor Author

jd-carroll commented Oct 1, 2019

Removing comment in favor of #4319

@jd-carroll jd-carroll changed the title elbv2: Investigate NetworkLoadBalancer Unusable elbv2: Investigate NetworkLoadBalancer Oct 1, 2019
@NGL321 NGL321 reopened this Oct 1, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 2, 2019

We have the new issue, so closing this one again.

@rix0rrr rix0rrr closed this as completed Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

4 participants