refactor(lambda): missing Lambda VPC and security validation (#26528)

Specifying the `securityGroups` property requires a `vpc`. This fix adds validation for the case when a `vpc` is not specified, but `securityGroups` is. It also adds validation for the case when both `securityGroups` and `allowAllOutbound` are specified (`allowAllOutbound` should be configured in the SGs). **Question for the reviewers** How should we handle the case of an empty list in `securityGroups`? (eg `securityGroups: []`) Closes #26508. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Aug 22, 2023 · 6087424 · 6087424
1 parent 47df704
commit 6087424
Show file tree

Hide file tree

Showing 3 changed files with 144 additions and 8 deletions.
diff --git a/packages/aws-cdk-lib/aws-eks/lib/kubectl-provider.ts b/packages/aws-cdk-lib/aws-eks/lib/kubectl-provider.ts
@@ -145,7 +145,7 @@ export class KubectlProvider extends NestedStack implements IKubectlProvider {
 
       // defined only when using private access
       vpc: cluster.kubectlPrivateSubnets ? cluster.vpc : undefined,
-      securityGroups: cluster.kubectlSecurityGroup ? [cluster.kubectlSecurityGroup] : undefined,
+      securityGroups: cluster.kubectlPrivateSubnets && cluster.kubectlSecurityGroup ? [cluster.kubectlSecurityGroup] : undefined,
       vpcSubnets: cluster.kubectlPrivateSubnets ? { subnets: cluster.kubectlPrivateSubnets } : undefined,
     });
 
@@ -194,7 +194,7 @@ export class KubectlProvider extends NestedStack implements IKubectlProvider {
       onEventHandler: handler,
       vpc: cluster.kubectlPrivateSubnets ? cluster.vpc : undefined,
       vpcSubnets: cluster.kubectlPrivateSubnets ? { subnets: cluster.kubectlPrivateSubnets } : undefined,
-      securityGroups: cluster.kubectlSecurityGroup ? [cluster.kubectlSecurityGroup] : undefined,
+      securityGroups: cluster.kubectlPrivateSubnets && cluster.kubectlSecurityGroup ? [cluster.kubectlSecurityGroup] : undefined,
     });
 
     this.serviceToken = provider.serviceToken;

diff --git a/packages/aws-cdk-lib/aws-lambda/lib/function.ts b/packages/aws-cdk-lib/aws-lambda/lib/function.ts
@@ -1222,11 +1222,20 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett
    * Lambda creation properties.
    */
   private configureVpc(props: FunctionProps): CfnFunction.VpcConfigProperty | undefined {
-    if ((props.securityGroup || props.allowAllOutbound !== undefined) && !props.vpc) {
-      throw new Error('Cannot configure \'securityGroup\' or \'allowAllOutbound\' without configuring a VPC');
+    if (props.securityGroup && props.securityGroups) {
+      throw new Error('Only one of the function props, securityGroup or securityGroups, is allowed');
     }
 
     if (!props.vpc) {
+      if (props.allowAllOutbound !== undefined) {
+        throw new Error('Cannot configure \'allowAllOutbound\' without configuring a VPC');
+      }
+      if (props.securityGroup) {
+        throw new Error('Cannot configure \'securityGroup\' without configuring a VPC');
+      }
+      if (props.securityGroups) {
+        throw new Error('Cannot configure \'securityGroups\' without configuring a VPC');
+      }
       if (props.vpcSubnets) {
         throw new Error('Cannot configure \'vpcSubnets\' without configuring a VPC');
       }
@@ -1237,12 +1246,12 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett
       throw new Error('Configure \'allowAllOutbound\' directly on the supplied SecurityGroup.');
     }
 
-    let securityGroups: ec2.ISecurityGroup[];
-
-    if (props.securityGroup && props.securityGroups) {
-      throw new Error('Only one of the function props, securityGroup or securityGroups, is allowed');
+    if (props.securityGroups && props.allowAllOutbound !== undefined) {
+      throw new Error('Configure \'allowAllOutbound\' directly on the supplied SecurityGroups.');
     }
 
+    let securityGroups: ec2.ISecurityGroup[];
+
     if (props.securityGroups) {
       securityGroups = props.securityGroups;
     } else {

diff --git a/packages/aws-cdk-lib/aws-lambda/test/function.test.ts b/packages/aws-cdk-lib/aws-lambda/test/function.test.ts
@@ -3308,6 +3308,133 @@ test('test 2.87.0 version hash stability', () => {
   });
 });
 
+describe('VPC configuration', () => {
+  test('with both securityGroup and securityGroups', () => {
+    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,
+    });
+    expect(() => new lambda.Function(stack, 'MyLambda', {
+      code: new lambda.InlineCode('foo'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.PYTHON_3_9,
+      securityGroup,
+      securityGroups: [securityGroup],
+    })).toThrow(/Only one of the function props, securityGroup or securityGroups, is allowed/);
+  });
+
+  test('with allowAllOutbound and no VPC', () => {
+    const stack = new cdk.Stack();
+    expect(() => new lambda.Function(stack, 'MyLambda', {
+      code: new lambda.InlineCode('foo'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.PYTHON_3_9,
+      allowAllOutbound: true,
+    })).toThrow(/Cannot configure 'allowAllOutbound' without configuring a VPC/);
+  });
+
+  test('with allowAllOutbound and no VPC', () => {
+    const stack = new cdk.Stack();
+    expect(() => new lambda.Function(stack, 'MyLambda', {
+      code: new lambda.InlineCode('foo'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.PYTHON_3_9,
+      allowAllOutbound: true,
+    })).toThrow(/Cannot configure 'allowAllOutbound' without configuring a VPC/);
+  });
+
+  test('with securityGroup and no VPC', () => {
+    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,
+    });
+    expect(() => new lambda.Function(stack, 'MyLambda', {
+      code: new lambda.InlineCode('foo'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.PYTHON_3_9,
+      securityGroup,
+    })).toThrow(/Cannot configure 'securityGroup' without configuring a VPC/);
+  });
+
+  test('with securityGroups and no VPC', () => {
+    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,
+    });
+    expect(() => new lambda.Function(stack, 'MyLambda', {
+      code: new lambda.InlineCode('foo'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.PYTHON_3_9,
+      securityGroups: [securityGroup],
+    })).toThrow(/Cannot configure 'securityGroups' without configuring a VPC/);
+  });
+
+  test('with vpcSubnets and no VPC', () => {
+    const stack = new cdk.Stack();
+    expect(() => new lambda.Function(stack, 'MyLambda', {
+      code: new lambda.InlineCode('foo'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.PYTHON_3_9,
+      vpcSubnets: { subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS },
+    })).toThrow(/Cannot configure 'vpcSubnets' without configuring a VPC/);
+  });
+
+  test('with securityGroup and allowAllOutbound', () => {
+    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,
+    });
+    expect(() => new lambda.Function(stack, 'MyLambda', {
+      vpc,
+      code: new lambda.InlineCode('foo'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.PYTHON_3_9,
+      securityGroup,
+      allowAllOutbound: false,
+    })).toThrow(/Configure 'allowAllOutbound' directly on the supplied SecurityGroup./);
+  });
+
+  test('with securityGroups and allowAllOutbound', () => {
+    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,
+    });
+    expect(() => new lambda.Function(stack, 'MyLambda', {
+      vpc,
+      code: new lambda.InlineCode('foo'),
+      handler: 'index.handler',
+      runtime: lambda.Runtime.PYTHON_3_9,
+      securityGroups: [securityGroup],
+      allowAllOutbound: false,
+    })).toThrow(/Configure 'allowAllOutbound' directly on the supplied SecurityGroups./);
+  });
+});
+
 function newTestLambda(scope: constructs.Construct) {
   return new lambda.Function(scope, 'MyLambda', {
     code: new lambda.InlineCode('foo'),