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): validate localMountPath format and length #31019

Merged
merged 12 commits into from
Aug 17, 2024
6 changes: 6 additions & 0 deletions packages/aws-cdk-lib/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,12 @@ export class Function extends FunctionBase {
// add additional managed policies when necessary
if (props.filesystem) {
const config = props.filesystem.config;
if (!/^\/mnt\/[a-zA-Z0-9-_.]+$/.test(config.localMountPath)) {
throw new Error(`Local mount path should match with ^/mnt/[a-zA-Z0-9-_.]+$ but given ${config.localMountPath}.`);
}
if (config.localMountPath.length > 160) {
throw new Error(`Local mount path can not be longer than 160 characters but has ${config.localMountPath.length} characters.`);
}
moelasmar marked this conversation as resolved.
Show resolved Hide resolved
if (config.policies) {
config.policies.forEach(p => {
this.role?.addToPrincipalPolicy(p);
Expand Down
60 changes: 60 additions & 0 deletions packages/aws-cdk-lib/aws-lambda/test/function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2923,6 +2923,66 @@ describe('function', () => {
});
});

test('validate localMountPath format when mounting efs', () => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Vpc', {
maxAzs: 3,
natGateways: 1,
});
const securityGroup = new ec2.SecurityGroup(stack, 'LambdaSG', {
vpc,
allowAllOutbound: false,
});

const fs = new efs.FileSystem(stack, 'Efs', {
vpc,
});
const accessPoint = fs.addAccessPoint('AccessPoint');

// THEN
expect(() => {
new lambda.Function(stack, 'MyFunction', {
vpc,
handler: 'foo',
securityGroups: [securityGroup],
runtime: lambda.Runtime.NODEJS_LATEST,
code: lambda.Code.fromAsset(path.join(__dirname, 'handler.zip')),
filesystem: lambda.FileSystem.fromEfsAccessPoint(accessPoint, '/not-mnt/foo-bar'),
});
}).toThrow();
Copy link
Contributor

Choose a reason for hiding this comment

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

bit of a nick pick, but do you think it would be a good idea to add the actual error message? We could check that the correct error is being thrown out, rather than just an error being thrown?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0e9cb46

});

test('validate localMountPath length when mounting efs', () => {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Vpc', {
maxAzs: 3,
natGateways: 1,
});
const securityGroup = new ec2.SecurityGroup(stack, 'LambdaSG', {
vpc,
allowAllOutbound: false,
});

const fs = new efs.FileSystem(stack, 'Efs', {
vpc,
});
const accessPoint = fs.addAccessPoint('AccessPoint');

// THEN
expect(() => {
new lambda.Function(stack, 'MyFunction', {
vpc,
handler: 'foo',
securityGroups: [securityGroup],
runtime: lambda.Runtime.NODEJS_LATEST,
code: lambda.Code.fromAsset(path.join(__dirname, 'handler.zip')),
filesystem: lambda.FileSystem.fromEfsAccessPoint(accessPoint, `/mnt/${'a'.repeat(160)}`),
});
}).toThrow();
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, would be a good idea to test against the actual error message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0e9cb46

});

test('correct security group is created when deployed in separate stacks', () => {
const app = new cdk.App();

Expand Down
Loading