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

Allow full control of security groups for ApplicationListener #361

Open
clstokes opened this issue Jul 10, 2019 · 5 comments
Open

Allow full control of security groups for ApplicationListener #361

clstokes opened this issue Jul 10, 2019 · 5 comments
Labels
kind/enhancement Improvements or new features

Comments

@clstokes
Copy link
Contributor

ApplicationListener seems to have two modes when it comes to security groups:

  • If external == true, assume control of the associated load balancer's security groups and open up to the entire public internet. This also sets scheme to internet-facing when calling AWS.
  • If external == false, assume no control of the associated load balancer's security group and set scheme to internal.

Part of this logic is at

if (args.external !== false) {
const args = {
location: new x.ec2.AnyIPv4Location(),
ports: new x.ec2.TcpPorts(port),
description: pulumi.interpolate`Externally available at port ${port}`,
};
for (let i = 0, n = this.loadBalancer.securityGroups.length; i < n; i++) {
const securityGroup = this.loadBalancer.securityGroups[i];
securityGroup.createIngressRule(`${name}-external-${i}-ingress`, args, { parent: this });
securityGroup.createEgressRule(`${name}-external-${i}-egress`, args, { parent: this });
}
}

This does not allow for users to fully control the security groups of their load balancers and forces them to pick one of two options that might not fit their needs.

I suggest that if a security group is provided that it is used without modification. In this scenario, the user should be knowledgable enough to fully control the security group's rules.

Additionally the ApplicationListener modifying the associated load balancer's security groups provides a confusing "misdirection" when trying to understand this behavior. I'd like it if we could separate this somehow so that when a user is working with a ApplicationListener, the scope of what they're working on is clear and constrained.

@ypresto
Copy link

ypresto commented Feb 18, 2020

It is also problematic because the created security group assumes that a target's port is the same as a listener port, i.e. ingress lb:80 -> egress target:80.

@ypresto
Copy link

ypresto commented Feb 18, 2020

Additionally the ApplicationListener modifying the associated load balancer's security groups

I actually expect awsx package to hide SecurityGroup as much as possible, like AWS CDK's high-level constructs.
We can some options to disable it or #293 (comment) , or we can use aws package for handcrafting SecurityGroup and etc.

@devgnx
Copy link

devgnx commented Jun 3, 2020

I need this

@jeanlescure
Copy link

I actually expect awsx package to hide SecurityGroup as much as possible

The ethos is fine, but not allowing a way to override is naive and irresponsible, as it narrows the ability to use this library to "happy paths" only since there's no way for you to guess all scenarios your users will try to apply awsx to.

I'm really worried about having picked Pulumi for orchestration for the project my company is working on as we have already invested a lot of time migrating and from what I read there's no workaround to set custom SecurityGroups for our services, which is a must for us.

@PabloJomer
Copy link

I need this.

@lukehoban lukehoban added impact/breaking Fixing this issue will require a breaking change impact/performance Something is slower than expected kind/enhancement Improvements or new features and removed impact/breaking Fixing this issue will require a breaking change impact/performance Something is slower than expected labels Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

7 participants