-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Hard to allow connections to/from a plain SecurityGroup object #579
Comments
The quick workaround is to make the following class: class ConnectableSecurityGroup implements IConnectable {
public readonly connections: Connections;
constructor(securityGroup: SecurityGroup) {
this.connections = new Connections(securityGroup);
}
} and use as follows: cluster.connections.allowConnectionsFrom(new ConnectableSecurityGroup(securityGroup), ...); (But the correct solution will look different) |
Just to make sure I understand: we just want to have SecurityGroup implement IConnectable, correct? |
I think so. Might require reordering some files. |
I think we might take this opportunity to simplify the class diagram of the whole SecurityGroup business somewhat. Requirements: machine.connections.allowTo(securityGroup);
machine.connections.allowTo(machine);
machine.connections.allowTo(new AnyIPv4());
machine.connections.allowTo(new DynamoDBInYourVPC()); And other securitygroups or things with securitygroups need to reciprocate connectability whenever that makes sense. Current stateRight now it looks like this:
SimpleThe simplest thing we could do this the following:
This has the following downsides:
machine1.securityGroup.allowTo(machine1.securityGroup);
// Instead of
machine1.connections.allowTo(machine1);
|
Okay, I think this is the simplification we're looking for:
|
Refactoring of the object model for connection/security groups so that a SecurityGroup object can be used as the target of an .allowTo() statement: cluster.connections.allowTo(securityGroup) Also add SecurityGroupRef.import() to allow importing a non-constructed SecurityGroup into the construct tree. As part of the refactoring: - Get rid of IDefaultConnectable, the functionality has been folded into IConnectable/Connections. - Get rid of ISecurityGroup. - Rename IConnectionPeer => ISecurityGroupRule. - Drastically simplify implementation, get rid of recursion and classes to enable the recursion to terminate. All complex logic is now nicely contained within Connections. This change is BREAKING to connections-enabled construct writers, but transparent to application builders. Fixes #579.
Refactoring of the object model for connection/security groups so that a SecurityGroup object can be used as the target of an .allowTo() statement: cluster.connections.allowTo(securityGroup) SecurityGroupRef is now abstract and needs to be constructed using SecurityGroupRef.import(). This is also the mechanism for importing a non-constructed SecurityGroup into the construct tree. As part of the refactoring: - Get rid of IDefaultConnectable, the functionality has been folded into IConnectable/Connections. - Get rid of ISecurityGroup. - Rename IConnectionPeer => ISecurityGroupRule. - Drastically simplify implementation, get rid of recursion and classes to enable the recursion to terminate. All complex logic is now nicely contained within Connections. This change is BREAKING to connections-enabled construct writers, but transparent to application builders. Fixes #579.
We currently only allow creating connections to and from an IConnectable, which SecurityGroup itself is not.
The text was updated successfully, but these errors were encountered: