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

default SecurityGroup on Fargate behind NLB should have allow all traffic ingress rule #3667

Closed
1 of 5 tasks
pahud opened this issue Aug 15, 2019 · 6 comments
Closed
1 of 5 tasks
Assignees
Labels
needs-triage This issue or PR still needs to be triaged.

Comments

@pahud
Copy link
Contributor

pahud commented Aug 15, 2019

Note: for support questions, please first reference our documentation, then use Stackoverflow. This repository's issues are intended for feature requests and bug reports.

  • I'm submitting a ...

    • 🪲 bug report
    • 🚀 feature request
    • 📚 construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    If the current behavior is a 🪲bug🪲: Please provide the steps to reproduce

Fargate TCP service running behind NLB will have a default security group with no Ingress rules, which leads to NLB health check failure and traffic can't ingress through the NLB.

  • What is the expected behavior (or behavior of feature suggested)?

My current dirty hack

圖片

we should enable the default SG to allow the Ingress from pubilc internet otherwise no traffic will be able to hit the Fargate task.

  • What is the motivation / use case for changing the behavior or adding this feature?

Fargate service running behind NLB.

  • Please tell us about your environment:

    • CDK CLI Version: 1.4.0 (build 175471f)
    • Module Version: 1.4.0
    • OS: OSX Mojave
    • Language: TypeScript
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)

@pahud pahud added the needs-triage This issue or PR still needs to be triaged. label Aug 15, 2019
@pahud pahud changed the title Ddefault SecurityGroup on Fargate behind NLB should have allow all traffic ingress rule default SecurityGroup on Fargate behind NLB should have allow all traffic ingress rule Aug 15, 2019
@pahud
Copy link
Contributor Author

pahud commented Aug 15, 2019

Correct me if I was wrong . As NLB has no security group, Fargate task running behind NLB for all public internet access should have an ingress rule wide open for all. This is different from ALB.

@karlpatr
Copy link

Making anything default to wide open for all only makes sense for projects that want the load balanced service to be accessible to the internet; I have some service-to-service NLBs for traffic that never leaves private subnets, and I wouldn't want a default setting opening those ports to the world. The compromise solution in my current project is to open the port to the CIDR of my VPN which makes it reachable by the NLB (and everything else running in the VPN) but not wider scopes.

I would actually prefer using either the private subnet CIDRs that my NLB is homed in or in the best case specifically the primary IPs for the ENIs that are bound to the load balancer itself so that the actual security rule fits the access need, but I'm pretty sure that both of the narrower scopes aren't available at template generation time. I do find it disappointing that there is no mechanism to refer to the health check IPs in the system.

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 17, 2019

Closing this as a duplicate of #3667

@davidsteed
Copy link

I want the NLB to be accessible from an api gateway vpc link eg.

new apigateway.VpcLink(this, 'MyVPCLink', {
targets: [myFargateService.loadBalancer],
});

and not accessible from the internet.

@karlpatr
Copy link

See #1490 for more discussion; the duplicate notice above accidentally referred to this issue :)

@whereisaaron
Copy link

$ cdk --version
2.34.2 (build 7abcbc6)

It took so many hours to work out how to route two different container ports (80 & 443) via two different Network Load Balancer Target Groups with CDKv2 that I seriously considered reverting to YAML where I could just change the 3 characters to fix the port number 😄 Surely routing TCP port 80 and port 433 to a container is not an uncommon use case? 🤔
Anyway, here it is for the benefit of other coming here, and thank you to @pahud for the workaround idea!

targetGroupPort80.AddTarget(new [] {
    service.LoadBalancerTarget(new LoadBalancerTargetOptions {
        ContainerName = myContainer.ContainerName,
        ContainerPort = 80,
        Protocol = Protocol.TCP
    }) 
});

service.Connections.SecurityGroups[0].AddIngressRule(
    Peer.Ipv4(myVpc.VpcCidrBlock),
    Port.Tcp(80),
    "Allow NLB traffic from VPC CIDR to port 80 Target Group"
);

targetGroupPort443.AddTarget(new [] {
    service.LoadBalancerTarget(new LoadBalancerTargetOptions {
        ContainerName = myContainer.ContainerName,
        ContainerPort = 443,
        Protocol = Protocol.TCP
    })
});

myService.Connections.SecurityGroups[0].AddIngressRule(
    Peer.Ipv4(myVpc.VpcCidrBlock),
    Port.Tcp(443),
    "Allow NLB traffic from VPC CIDR to port 443 Target Group"
);

ECS/Fargate services should automatically add the VPC or NLB subnet CIDR ingress rule to their Security Group when they made a target of an Network Load Balancer.

Also the default security group created by FargateService is missing a 'Name' tag with the stack path that other CDKv2-created objects get.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

6 participants