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): lookup security group by name #17246

Merged
merged 8 commits into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 5 additions & 1 deletion allowed-breaking-changes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,8 @@ strengthened:@aws-cdk/aws-lambda-event-sources.ManagedKafkaEventSourceProps

# Remove IO2 from autoscaling EbsDeviceVolumeType. This value is not supported
# at the moment and was not supported in the past.
removed:@aws-cdk/aws-autoscaling.EbsDeviceVolumeType.IO2
removed:@aws-cdk/aws-autoscaling.EbsDeviceVolumeType.IO2

# Changed property securityGroupId to optional because either securityGroupId or
# securityGroupName is required. Therefore securityGroupId is no longer mandatory.
weakened:@aws-cdk/cloud-assembly-schema.SecurityGroupContextQuery
27 changes: 27 additions & 0 deletions packages/@aws-cdk/aws-ec2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,33 @@ const mySecurityGroupWithoutInlineRules = new ec2.SecurityGroup(this, 'SecurityG
mySecurityGroupWithoutInlineRules.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(22), 'allow ssh access from the world');
```

### Importing an existing security group

If you know the ID and the configuration of the security group to import, you can use `SecurityGroup.fromSecurityGroupId`:

```ts
const sg = ec2.SecurityGroup.fromSecurityGroupId(this, 'SecurityGroupImport', 'security-group-import', {
jumic marked this conversation as resolved.
Show resolved Hide resolved
allowAllOutbound: true,
});
```

Alternatively, use method `SecurityGroup.fromLookupAttributes` to import security groups if you do not know the ID or the configuration details. You can lookup a security group by `securityGroupId` or by `securityGroupName`.

The result of the `SecurityGroup.fromLookupAttributes` operation will be written to a file called `cdk.context.json`. You must commit this file to source control so that the lookup values are available in non-privileged environments such as CI build steps, and to ensure your template builds are repeatable.

```ts fixture=with-vpc
const sg = ec2.SecurityGroup.fromLookupAttributes(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about dedicated fromLookupById and fromLookupByName methods? They could both still use a (private) fromLookupAttributes method, but it may make the API intent a bit more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to create an API similar to VpcLookupOptions. Now I compared it to fromLookupById and fromLookupByName again. For me, this seems to be a better option because the necessary parameters are defined by the API. I've implemented the following changes:

  • Deprecated method fromLookup, will be replaced by fromLookupById (same parameters).
  • New method fromLookupById with parameter securityGroupId. Parameter vpc not defined because the security group id is sufficient to clearly identify the security group.
  • New method fromLookupByName with parameters securityGroupName and vpc. vpc is defined as required parameter because it avoids problems if the same security group name is used multiple times per account and region. A security group is used in context of a VPC. Therefore, the VPC should be available in the CDK code. If it should be defined as optional later, we could changed it easily (no breaking change).

this, 'SecurityGroupLookup', {
// This imports a security group by name but you can also
// specify a 'securityGroupId.
securityGroupName: 'security-group-lookup',

// optional, lookup in specified VPC
vpc,
}
);
```

## Machine Images (AMIs)

AMIs control the OS that gets launched when you start your EC2 instance. The EC2
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-ec2/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export * from './network-acl';
export * from './network-acl-types';
export * from './port';
export * from './security-group';
export * from './security-group-lookup';
export * from './subnet';
export * from './peer';
export * from './volume';
Expand Down
35 changes: 35 additions & 0 deletions packages/@aws-cdk/aws-ec2/lib/security-group-lookup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { IVpc } from '.';
jumic marked this conversation as resolved.
Show resolved Hide resolved

/**
* Properties for looking up an existing SecurityGroup.
*
* Either `securityGroupName` or `securityGroupId` hast to be specified, otherwise an error is raised.
*/
export interface SecurityGroupLookupOptions {
jumic marked this conversation as resolved.
Show resolved Hide resolved
/**
* The name of the security group
*
* If given, will import the SecurityGroup with this name.
*
* @default Don't filter on securityGroupName
*/
readonly securityGroupName?: string;

/**
* The ID of the security group
*
* If given, will import the SecurityGroup with this ID.
*
* @default Don't filter on securityGroupId
*/
readonly securityGroupId?: string;

/**
* The VPC of the security group
*
* If given, will filter the SecurityGroup based on the VPC.
*
* @default Don't filter on VPC
*/
readonly vpc?: IVpc,
}
24 changes: 22 additions & 2 deletions packages/@aws-cdk/aws-ec2/lib/security-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Connections } from './connections';
import { CfnSecurityGroup, CfnSecurityGroupEgress, CfnSecurityGroupIngress } from './ec2.generated';
import { IPeer, Peer } from './peer';
import { Port } from './port';
import { SecurityGroupLookupOptions } from './security-group-lookup';
import { IVpc } from './vpc';

const SECURITY_GROUP_SYMBOL = Symbol.for('@aws-cdk/iam.SecurityGroup');
Expand Down Expand Up @@ -318,13 +319,32 @@ export class SecurityGroup extends SecurityGroupBase {
* Look up a security group by id.
*/
public static fromLookup(scope: Construct, id: string, securityGroupId: string) {
if (Token.isUnresolved(securityGroupId)) {
return this.fromLookupAttributes(scope, id, { securityGroupId });
}

/**
* Look up a security group.
*/
public static fromLookupAttributes(scope: Construct, id: string, options: SecurityGroupLookupOptions) {
if (Token.isUnresolved(options.securityGroupId) || Token.isUnresolved(options.securityGroupName) || Token.isUnresolved(options.vpc?.vpcId)) {
throw new Error('All arguments to look up a security group must be concrete (no Tokens)');
}

if (options.securityGroupId && options.securityGroupName) {
jumic marked this conversation as resolved.
Show resolved Hide resolved
throw new Error('\'securityGroupId\' and \'securityGroupName\' can not be specified both when looking up a security group');
}

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

const attributes: cxapi.SecurityGroupContextResponse = ContextProvider.getValue(scope, {
provider: cxschema.ContextProvider.SECURITY_GROUP_PROVIDER,
props: { securityGroupId },
props: {
securityGroupId: options.securityGroupId,
securityGroupName: options.securityGroupName,
vpcId: options.vpc?.vpcId,
},
dummyValue: {
securityGroupId: 'sg-12345',
allowAllOutbound: true,
Expand Down
169 changes: 169 additions & 0 deletions packages/@aws-cdk/aws-ec2/test/security-group.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,177 @@ describe('security group', () => {
expect(securityGroup.securityGroupId).toEqual('sg-12345');
expect(securityGroup.allowAllOutbound).toEqual(true);

});

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

// WHEN
const securityGroup = SecurityGroup.fromLookupAttributes(stack, 'stack', {
securityGroupId: 'sg-12345',
});

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

});

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

// WHEN
const securityGroup = SecurityGroup.fromLookupAttributes(stack, 'stack', {
securityGroupName: 'my-security-group',
});

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

});

test('can look up a security group by name and vpc', () => {
// 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
const securityGroup = SecurityGroup.fromLookupAttributes(stack, 'stack', {
securityGroupName: 'my-security-group',
vpc,
});

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

});

test('throws when securityGroupId and securityGroupName are specified both', () => {
// GIVEN
const app = new App();
const stack = new Stack(app, 'stack', {
env: {
account: '1234',
region: 'us-east-1',
},
});

// WHEN
expect(() => {
SecurityGroup.fromLookupAttributes(stack, 'stack', {
securityGroupId: 'sg-12345',
securityGroupName: 'my-security-group',
});
}).toThrow(/\'securityGroupId\' and \'securityGroupName\' can not be specified both when looking up a security group/);

});

test('throws when neither securityGroupId nor securityGroupName are specified', () => {
// GIVEN
const app = new App();
const stack = new Stack(app, 'stack', {
env: {
account: '1234',
region: 'us-east-1',
},
});

// WHEN
expect(() => {
SecurityGroup.fromLookupAttributes(stack, 'stack', {});
}).toThrow(/\'securityGroupId\' or \'securityGroupName\' must be specified to look up a security group/);

});

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

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

});

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

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

});

test('throws if vpc id 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: Lazy.string({ produce: () => 'vpc-1234' }),
availabilityZones: ['dummy1a', 'dummy1b', 'dummy1c'],
});

// WHEN
expect(() => {
SecurityGroup.fromLookupAttributes(stack, 'stack', {
securityGroupName: 'my-security-group',
vpc,
});
}).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 @@ -416,8 +416,24 @@ export interface SecurityGroupContextQuery {

/**
* Security group id
*
* @default - None
*/
readonly securityGroupId?: string;

/**
* Security group name
*
* @default - None
*/
readonly securityGroupId: string;
readonly securityGroupName?: string;

/**
* VPC ID
*
* @default - None
*/
readonly vpcId?: string;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -761,14 +761,21 @@
"type": "string"
},
"securityGroupId": {
"description": "Security group id",
"description": "Security group id (Default - None)",
"type": "string"
},
"securityGroupName": {
"description": "Security group name (Default - None)",
"type": "string"
},
"vpcId": {
"description": "VPC ID (Default - None)",
"type": "string"
}
},
"required": [
"account",
"region",
"securityGroupId"
"region"
]
},
"KeyContextQuery": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"14.0.0"}
{"version":"15.0.0"}
Loading