aws / aws/aws-cdk

SecurityGroup.fromLookupByName() doesnt have properties to set mutable to false

Open
#22,858 8 comments 1 reaction 0 assignees View on GitHub
@aws-cdk/aws-ec2 effort/medium feature-request p2
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
1d 19h
Merged PRs (30d)
74

Description

### Describe the bug

We have exiting Security groups which we want to lookup and use by name, but there is no option to set properties while using method SecurityGroup.fromLookupByName().
So have couple of questions:
- Is it possible to add SG properties to method SecurityGroup.fromLookupByName(). Because the whole purpose of doing a lookup is to use an existing SG.
- Why does cdk overwrites the Security Group and remove the rules defined in these SG's when lookedup from SG name.?
- Behaviour for different groups is different e.g somewhere it just added a new rule from ELB to SG's outbound rules, for other SG it completely removed the existing rules and added rule from ELB to SG. Is there any documentation which can explain the behaviour of mutability.
- Shouldnt mutability be set to false by default ?

For reference we are doing the following work around for now:

```
this.vpc = ec2.Vpc.fromLookup(this, 'Vpc', { isDefault: false });
let mutablePrivateSg = ec2.SecurityGroup.fromLookupByName(this, 'MutableInternalSg', 'SG1Name', this.vpc);
let mutablePublicSg = ec2.SecurityGroup.fromLookupByName(this, 'MutablePublicSg', 'SG2Name', this.vpc);

this.privateSg = ec2.SecurityGroup.fromSecurityGroupId(this, 'InternalSG', mutablePrivateSg.securityGroupId, {
mutable: false
});

this.publicSg = ec2.SecurityGroup.fromSecurityGroupId(this, 'PublicSG', mutablePublicSg.securityGroupId, {
mutable: false
});
```

### Expected Behavior

Either ec2.SecurityGroup.fromLookupByName() also has properties to be defined or mutable is set to default by false

### Current Behavior

Using ec2.SecurityGroup.fromLookupByName()doesnt allow to set properties like `{mutable: false}` which causes issues and overwrites our existing SG's rules.

### Reproduction Steps

1. Lookup an exiting SG by name, with existing rules
2. Use that SG in a newly created ASG.
3. Use this newly created ASG as a target to ELB

Outcome will be :
SG's will be overwritten as per below generated CFT (impact seems to be not consistent):

```
{
"Resources": {
"xxxxxxxxxxxxxxxxx": {
"Type": "AWS::EC2::SecurityGroupIngress",
"Properties": {
"IpProtocol": "tcp",
"CidrIp": "0.0.0.0/0",
"Description": "Allow from anyone on port 443",
"FromPort": 443,
"GroupId": "xxxxxxxxxxxxxxxxx",
"ToPort": 443
},
"Metadata": {
"aws:cdk:path": "xxxxxxxxxxxxxxxxx"
}
},
"xxxxxxxxxxxxxxxxx": {
"Type": "AWS::EC2::SecurityGroupIngress",
"Properties": {
"IpProtocol": "tcp",
"Description": "Load balancer to target",
"FromPort": 9000,
"GroupId": "xxxxxxxxxxxxxxxxx",
"SourceSecurityGroupId": "xxxxxxxxxxxxxxxxx",
"ToPort": 9000
},
"Metadata": {
"aws:cdk:path": "xxxxxxxxxxxxxxxxx"
}
},
"xxxxxxxxxxxxxxxxx": {
"Type": "AWS::EC2::SecurityGroupIngress",
"Properties": {
"IpProtocol": "tcp",
"Description": "Load balancer to target",
"FromPort": 9000,
"GroupId": "xxxxxxxxxxxxxxxxx",
"SourceSecurityGroupId": "xxxxxxxxxxxxxxxxx",
"ToPort": 9000
},
"Metadata": {
"aws:cdk:path": "xxxxxxxxxxxxxxxxx"
}
},
"xxxxxxxxxxxxxxxxx": {
"Type": "AWS::EC2::SecurityGroupEgress",
"Properties": {
"GroupId": "xxxxxxxxxxxxxxxxx",
"IpProtocol": "tcp",
"Description": "Load balancer to target",
"DestinationSecurityGroupId": "xxxxxxxxxxxxxxxxx",
"FromPort": 9000,
"ToPort": 9000
},
"Metadata": {
"aws:cdk:path": "xxxxxxxxxxxxxxxxx"
}
},
"xxxxxxxxxxxxxxxxx": {
"Type": "AWS::EC2::SecurityGroupIngress",
"Properties": {
"IpProtocol": "tcp",
"CidrIp": "0.0.0.0/0",
"Description": "Allow from anyone on port 443",
"FromPort": 443,
"GroupId": "xxxxxxxxxxxxxxxxx",
"ToPort": 443
},
"Metadata": {
"aws:cdk:path": "xxxxxxxxxxxxxxxxx"
}
},
"configPublicSgt xxxxxxxxxxxxxxxxx": {
"Type": "AWS::EC2::SecurityGroupEgress",
"Properties": {
"GroupId": "xxxxxxxxxxxxxxxxx",
"IpProtocol": "tcp",
"Description": "Load balancer to target",
"DestinationSecurityGroupId": "xxxxxxxxxxxxxxxxx",
"FromPort": 9000,
"ToPort": 9000
},
"Metadata": {
"aws:cdk:path": "xxxxxxxxxxxxxxxxx"
}
}
}
```

### Possible Solution

_No response_

### Additional Information/Context

_No response_

### CDK CLI Version

2.45.0

### Framework Version

_No response_

### Node.js Version

16

### OS

rhel 7

### Language

Typescript

### Language Version

TypeScript (4.8.4)

### Other information

_No response_

Contributor guide

Open the contributing guide

Research direction

Start at the SecurityGroup.fromLookupByName() entry point and inspect how the mutable option is handled for looked-up security groups. Reproduce the documented ASG and ELB sequence, compare the generated CloudFormation security-group rules, and review the existing discussion. Done means the mutability behavior is consistent and the requested option or default is covered by appropriate tests or documentation.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
cloud, infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.