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

feat(ec2): security group lookup by name, vpc, and owner #30334

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-ec2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ const sg = ec2.SecurityGroup.fromSecurityGroupId(this, 'SecurityGroupImport', 's
});
```

Alternatively, use lookup methods to import security groups if you do not know the ID or the configuration details. Method `SecurityGroup.fromLookupByName` looks up a security group if the security group ID is unknown.
Alternatively, use lookup methods to import security groups if you do not know the ID or the configuration details. Method `SecurityGroup.fromLookupByName` looks up a security group if the security group ID is unknown, and you can uniquely identify the security group by name or by name and owner.

```ts fixture=with-vpc
const sg = ec2.SecurityGroup.fromLookupByName(this, 'SecurityGroupLookup', 'security-group-name', vpc);
Expand Down
27 changes: 21 additions & 6 deletions packages/aws-cdk-lib/aws-ec2/lib/security-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ export class SecurityGroup extends SecurityGroupBase {
/**
* Look up a security group by name.
*/
public static fromLookupByName(scope: Construct, id: string, securityGroupName: string, vpc: IVpc) {
return this.fromLookupAttributes(scope, id, { securityGroupName, vpc });
public static fromLookupByName(scope: Construct, id: string, securityGroupName: string, vpc: IVpc, owner?: string) {
return this.fromLookupAttributes(scope, id, { securityGroupName, vpc, owner });
}

/**
Expand Down Expand Up @@ -434,7 +434,12 @@ export class SecurityGroup extends SecurityGroupBase {
* Look up a security group.
*/
private static fromLookupAttributes(scope: Construct, id: string, options: SecurityGroupLookupOptions) {
if (Token.isUnresolved(options.securityGroupId) || Token.isUnresolved(options.securityGroupName) || Token.isUnresolved(options.vpc?.vpcId)) {
if (
Token.isUnresolved(options.securityGroupId) ||
Token.isUnresolved(options.securityGroupName) ||
Token.isUnresolved(options.vpc?.vpcId) ||
Token.isUnresolved(options.owner)
) {
throw new Error('All arguments to look up a security group must be concrete (no Tokens)');
}

Expand All @@ -444,6 +449,7 @@ export class SecurityGroup extends SecurityGroupBase {
securityGroupId: options.securityGroupId,
securityGroupName: options.securityGroupName,
vpcId: options.vpc?.vpcId,
owner: options.owner,
},
dummyValue: {
securityGroupId: 'sg-12345678',
Expand Down Expand Up @@ -515,8 +521,8 @@ export class SecurityGroup extends SecurityGroupBase {
this.securityGroup = new CfnSecurityGroup(this, 'Resource', {
groupName: this.physicalName,
groupDescription,
securityGroupIngress: Lazy.any({ produce: () => this.directIngressRules }, { omitEmptyArray: true } ),
securityGroupEgress: Lazy.any({ produce: () => this.directEgressRules }, { omitEmptyArray: true } ),
securityGroupIngress: Lazy.any({ produce: () => this.directIngressRules }, { omitEmptyArray: true }),
securityGroupEgress: Lazy.any({ produce: () => this.directEgressRules }, { omitEmptyArray: true }),
vpcId: props.vpc.vpcId,
});

Expand Down Expand Up @@ -653,7 +659,7 @@ export class SecurityGroup extends SecurityGroupBase {
const description = this.allowAllOutbound ? ALLOW_ALL_RULE.description : MATCH_NO_TRAFFIC.description;
super.addEgressRule(peer, port, description, false);
} else {
const rule = this.allowAllOutbound? ALLOW_ALL_RULE : MATCH_NO_TRAFFIC;
const rule = this.allowAllOutbound ? ALLOW_ALL_RULE : MATCH_NO_TRAFFIC;
this.directEgressRules.push(rule);
}
}
Expand Down Expand Up @@ -843,4 +849,13 @@ interface SecurityGroupLookupOptions {
* @default Don't filter on VPC
*/
readonly vpc?: IVpc;

/**
* The Owner of the security group
*
* If given, will filter the SecurityGroup based on the Owner.
*
* @default Don't filter on Owner
*/
readonly owner?: string;
}
46 changes: 46 additions & 0 deletions packages/aws-cdk-lib/aws-ec2/test/security-group.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,31 @@ describe('security group lookup', () => {

});

test('can look up a security group by name, vpc, and owner', () => {
// GIVEN
const account = '1234';
const app = new App();
const stack = new Stack(app, 'stack', {
env: {
account: account,
region: 'us-east-1',
},
});

const vpc = Vpc.fromVpcAttributes(stack, 'VPC', {
vpcId: 'vpc-1234',
availabilityZones: ['dummy1a', 'dummy1b', 'dummy1c'],
});

// WHEN
const securityGroup = SecurityGroup.fromLookupByName(stack, 'SG1', 'my-security-group', vpc, account);

// THEN
expect(securityGroup.securityGroupId).toEqual('sg-12345678');
expect(securityGroup.allowAllOutbound).toEqual(true);

});

test('can look up a security group and use it as a peer', () => {
// GIVEN
const app = new App();
Expand Down Expand Up @@ -676,6 +701,27 @@ describe('security group lookup', () => {

});

test('throws if owner is tokenized', () => {
// GIVEN
const app = new App();
const stack = new Stack(app, 'stack', {
env: {
account: '1234',
region: 'us-east-1',
},
});

const vpc = Vpc.fromVpcAttributes(stack, 'VPC', {
vpcId: 'vpc-1234',
availabilityZones: ['dummy1a', 'dummy1b', 'dummy1c'],
});

// WHEN
expect(() => {
SecurityGroup.fromLookupByName(stack, 'stack', 'my-security-group', vpc, Lazy.string({ produce: () => '1234' }));
}).toThrow('All arguments to look up a security group must be concrete (no Tokens)');
});

});

function testRulesAreInlined(contextDisableInlineRules: boolean | undefined | null, optionsDisableInlineRules: boolean | undefined) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,13 @@ export interface SecurityGroupContextQuery {
* @default - None
*/
readonly vpcId?: string;

/**
* Owner
*
* @default - None
*/
readonly owner?: string;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,10 @@
"vpcId": {
"description": "VPC ID (Default - None)",
"type": "string"
},
"owner": {
"description": "Owner (Default - None)",
"type": "string"
}
},
"required": [
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"36.0.0"}
{"version":"37.0.0"}
8 changes: 7 additions & 1 deletion packages/aws-cdk/lib/context-providers/security-groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export class SecurityGroupContextProviderPlugin implements ContextProviderPlugin
throw new Error('\'securityGroupId\' and \'securityGroupName\' can not be specified both when looking up a security group');
}

if (!args.securityGroupId && !args.securityGroupName) {
if (!args.securityGroupId && !args.securityGroupName) {
throw new Error('\'securityGroupId\' or \'securityGroupName\' must be specified to look up a security group');
}

Expand All @@ -37,6 +37,12 @@ export class SecurityGroupContextProviderPlugin implements ContextProviderPlugin
Values: [args.securityGroupName],
});
}
if (args.owner) {
filters.push({
Name: 'owner-id',
Values: [args.owner],
});
}

const response = await ec2.describeSecurityGroups({
GroupIds: args.securityGroupId ? [args.securityGroupId] : undefined,
Expand Down
58 changes: 58 additions & 0 deletions packages/aws-cdk/test/context-providers/security-groups.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,64 @@ describe('security group context provider plugin', () => {
expect(res.allowAllOutbound).toEqual(true);
});

test('looks up by security group name, vpc id, and owner', async () => {
// GIVEN
const provider = new SecurityGroupContextProviderPlugin(mockSDK);

AWS.mock('EC2', 'describeSecurityGroups', (_params: aws.EC2.DescribeSecurityGroupsRequest, cb: AwsCallback<aws.EC2.DescribeSecurityGroupsResult>) => {
expect(_params).toEqual({
Filters: [
{
Name: 'vpc-id',
Values: ['vpc-1234567'],
},
{
Name: 'group-name',
Values: ['my-security-group'],
},
{
Name: 'owner-id',
Values: ['1234'],
},
],
});
cb(null, {
SecurityGroups: [
{
GroupId: 'sg-1234',
IpPermissionsEgress: [
{
IpProtocol: '-1',
IpRanges: [
{ CidrIp: '0.0.0.0/0' },
],
},
{
IpProtocol: '-1',
Ipv6Ranges: [
{ CidrIpv6: '::/0' },
],
},
],
},
],
});
});

// WHEN
const res = await provider.getValue({
account: '1234',
region: 'us-east-1',
securityGroupName: 'my-security-group',
vpcId: 'vpc-1234567',
owner: '1234',
});

// THEN
expect(res.securityGroupId).toEqual('sg-1234');
expect(res.allowAllOutbound).toEqual(true);
});

test('detects non all-outbound egress', async () => {
// GIVEN
const provider = new SecurityGroupContextProviderPlugin(mockSDK);
Expand Down
Loading