Skip to content

Commit

Permalink
feat(ec2): security group lookup via filters (#30625)
Browse files Browse the repository at this point in the history
Closes #30331.

This will improve the security group lookup functionality for importing existing security groups into a CDK stack.

I added the ability to lookup existing security groups via more filters. Filters are supported by the [DescribeSecurityGroups API](https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_DescribeSecurityGroups.html), and using these filters can be immensely useful for looking up existing security groups, especially if your account or organization follows predictable rules regarding things like security group tags.

I added unit tests similar to the ones that test the normal lookup by ID or name.

- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
jdukewich authored and duranbe committed Aug 25, 2024
1 parent 1aa09bb commit eb7276b
Showing 1 changed file with 88 additions and 0 deletions.
88 changes: 88 additions & 0 deletions packages/aws-cdk/test/context-providers/security-groups.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import * as AWS from 'aws-sdk-mock';
import * as AWS from 'aws-sdk-mock';
/* eslint-disable import/order */
import * as aws from 'aws-sdk';

import { SecurityGroupContextProviderPlugin, hasAllTrafficEgress } from '../../lib/context-providers/security-groups';


import { SecurityGroupContextProviderPlugin, hasAllTrafficEgress } from '../../lib/context-providers/security-groups';

import { MockSdkProvider } from '../util/mock-sdk';
Expand Down Expand Up @@ -296,6 +300,74 @@ describe('security group context provider plugin', () => {
expect(res.allowAllOutbound).toEqual(true);
});

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

AWS.mock('EC2', 'describeSecurityGroups', (_params: aws.EC2.DescribeSecurityGroupsRequest, cb: AwsCallback<aws.EC2.DescribeSecurityGroupsResult>) => {
expect(_params).toEqual({
GroupIds: undefined,
Filters: [
{
Name: 'description',
Values: ['my description'],
},
{
Name: 'tag-key',
Values: ['tagA', 'tagB'],
},
{
Name: 'owner-id',
Values: ['012345678901'],
},
{
Name: 'tag:tagC',
Values: ['valueC', 'otherValueC'],
},
{
Name: 'tag:tagD',
Values: ['valueD'],
},
],
});
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',
ownerId: '012345678901',
description: 'my description',
tagKeys: ['tagA', 'tagB'],
tags: { tagC: ['valueC', 'otherValueC'], tagD: ['valueD'] },
});

// 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 Expand Up @@ -389,6 +461,22 @@ describe('security group context provider plugin', () => {
).rejects.toThrow(/\'securityGroupId\' and \'securityGroupName\' can not be specified both when looking up a security group/i);
});

<<<<<<< HEAD
test('errors when neither securityGroupId nor securityGroupName are specified', async () => {
// GIVEN
const provider = new SecurityGroupContextProviderPlugin(mockSDK);

// WHEN
await expect(
provider.getValue({
account: '123456789012',
region: 'us-east-1',
}),
).rejects.toThrow(/\'securityGroupId\' or \'securityGroupName\' must be specified to look up a security group/i);
});

=======
>>>>>>> abc78bfa61 (feat(ec2): security group lookup via filters (#30625))
test('identifies allTrafficEgress from SecurityGroup permissions', () => {
expect(
hasAllTrafficEgress({
Expand Down

0 comments on commit eb7276b

Please sign in to comment.