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

fix(lambda): cannot create lambda in public subnets #9468

Merged
merged 6 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 0 additions & 10 deletions packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -824,17 +824,7 @@ export class Function extends FunctionBase {
}
}

// Pick subnets, make sure they're not Public. Routing through an IGW
// won't work because the ENIs don't get a Public IP.
// Why are we not simply forcing vpcSubnets? Because you might still be choosing
// Isolated networks or selecting among 2 sets of Private subnets by name.
const { subnetIds } = props.vpc.selectSubnets(props.vpcSubnets);
const publicSubnetIds = new Set(props.vpc.publicSubnets.map(s => s.subnetId));
for (const subnetId of subnetIds) {
if (publicSubnetIds.has(subnetId)) {
throw new Error('Not possible to place Lambda Functions in a Public subnet');
}
}

// List can't be empty here, if we got this far you intended to put your Lambda
// in subnets. We're going to guarantee that we get the nice error message by
Expand Down
42 changes: 38 additions & 4 deletions packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,29 @@ export = {
test.done();
},

'picking public subnets is not allowed'(test: Test) {
'picking any subnet type is allowed'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'VPC');
const vpc = new ec2.Vpc(stack, 'VPC', {
subnetConfiguration: [
{
name: 'Public',
subnetType: ec2.SubnetType.PUBLIC,
},
{
name: 'Private',
subnetType: ec2.SubnetType.PRIVATE,
},
{
name: 'Isolated',
subnetType: ec2.SubnetType.ISOLATED,
},
],
});

// WHEN
test.throws(() => {
new lambda.Function(stack, 'Lambda', {
test.doesNotThrow(() => {
new lambda.Function(stack, 'PublicLambda', {
code: new lambda.InlineCode('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_10_X,
Expand All @@ -228,6 +243,25 @@ export = {
});
});

test.doesNotThrow(() => {
new lambda.Function(stack, 'PrivateLambda', {
code: new lambda.InlineCode('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_10_X,
vpc,
vpcSubnets: { subnetType: ec2.SubnetType.PRIVATE },
});
});

test.doesNotThrow(() => {
new lambda.Function(stack, 'IsolatedLambda', {
code: new lambda.InlineCode('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NODEJS_10_X,
vpc,
vpcSubnets: { subnetType: ec2.SubnetType.ISOLATED },
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

These assertions should be replaced with assertions that the CloudFormation template is correctly configured.

This will probably need to be split into separate tests for each subnet type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These assertions should be replaced with assertions that the CloudFormation template is correctly configured.

You mean like a hasResourceLike assertion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

test.done();
},
};
Expand Down