From 10a633c8cda9f21b85c82f911d88641f3a362c4d Mon Sep 17 00:00:00 2001
From: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
Date: Tue, 11 May 2021 04:09:19 -0500
Subject: [PATCH] fix(lambda): custom resource fails to connect to efs
 filesystem (#14431)

Fixes: https://github.com/aws/aws-cdk/issues/14430

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 packages/@aws-cdk/aws-lambda/lib/function.ts  | 20 ++++++
 .../@aws-cdk/aws-lambda/test/function.test.ts | 64 +++++++++++++++++++
 .../integ.lambda.filesystem.expected.json     |  4 +-
 3 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts
index 4a0fc2d916ec0..0565ba2a98b1e 100644
--- a/packages/@aws-cdk/aws-lambda/lib/function.ts
+++ b/packages/@aws-cdk/aws-lambda/lib/function.ts
@@ -701,10 +701,30 @@ export class Function extends FunctionBase {
     this.currentVersionOptions = props.currentVersionOptions;
 
     if (props.filesystem) {
+      if (!props.vpc) {
+        throw new Error('Cannot configure \'filesystem\' without configuring a VPC.');
+      }
       const config = props.filesystem.config;
       if (config.dependency) {
         this.node.addDependency(...config.dependency);
       }
+      // There could be a race if the Lambda is used in a CustomResource. It is possible for the Lambda to
+      // fail to attach to a given FileSystem if we do not have a dependency on the SecurityGroup ingress/egress
+      // rules that were created between this Lambda's SG & the Filesystem SG.
+      this.connections.securityGroups.forEach(sg => {
+        sg.node.findAll().forEach(child => {
+          if (child instanceof CfnResource && child.cfnResourceType === 'AWS::EC2::SecurityGroupEgress') {
+            resource.node.addDependency(child);
+          }
+        });
+      });
+      config.connections?.securityGroups.forEach(sg => {
+        sg.node.findAll().forEach(child => {
+          if (child instanceof CfnResource && child.cfnResourceType === 'AWS::EC2::SecurityGroupIngress') {
+            resource.node.addDependency(child);
+          }
+        });
+      });
     }
   }
 
diff --git a/packages/@aws-cdk/aws-lambda/test/function.test.ts b/packages/@aws-cdk/aws-lambda/test/function.test.ts
index e4ac71c25c8d1..26a27ca76aad2 100644
--- a/packages/@aws-cdk/aws-lambda/test/function.test.ts
+++ b/packages/@aws-cdk/aws-lambda/test/function.test.ts
@@ -1841,6 +1841,7 @@ describe('function', () => {
       const accessPoint = fs.addAccessPoint('AccessPoint');
       // WHEN
       new lambda.Function(stack, 'MyFunction', {
+        vpc,
         handler: 'foo',
         runtime: lambda.Runtime.NODEJS_12_X,
         code: lambda.Code.fromAsset(path.join(__dirname, 'handler.zip')),
@@ -1879,6 +1880,69 @@ describe('function', () => {
         ],
       });
     });
+
+    test('throw error mounting efs with no vpc', () => {
+      // GIVEN
+      const stack = new cdk.Stack();
+      const vpc = new ec2.Vpc(stack, 'Vpc', {
+        maxAzs: 3,
+        natGateways: 1,
+      });
+
+      const fs = new efs.FileSystem(stack, 'Efs', {
+        vpc,
+      });
+      const accessPoint = fs.addAccessPoint('AccessPoint');
+
+      // THEN
+      expect(() => {
+        new lambda.Function(stack, 'MyFunction', {
+          handler: 'foo',
+          runtime: lambda.Runtime.NODEJS_12_X,
+          code: lambda.Code.fromAsset(path.join(__dirname, 'handler.zip')),
+          filesystem: lambda.FileSystem.fromEfsAccessPoint(accessPoint, '/mnt/msg'),
+        });
+      }).toThrow();
+    });
+
+    test('verify deps 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');
+      // WHEN
+      new lambda.Function(stack, 'MyFunction', {
+        vpc,
+        handler: 'foo',
+        securityGroups: [securityGroup],
+        runtime: lambda.Runtime.NODEJS_12_X,
+        code: lambda.Code.fromAsset(path.join(__dirname, 'handler.zip')),
+        filesystem: lambda.FileSystem.fromEfsAccessPoint(accessPoint, '/mnt/msg'),
+      });
+
+      // THEN
+      expect(stack).toHaveResource('AWS::Lambda::Function', {
+        DependsOn: [
+          'EfsEfsMountTarget195B2DD2E',
+          'EfsEfsMountTarget2315C927F',
+          'EfsEfsSecurityGroupfromLambdaSG20491B2F751D',
+          'LambdaSGtoEfsEfsSecurityGroupFCE2954020499719694A',
+          'MyFunctionServiceRoleDefaultPolicyB705ABD4',
+          'MyFunctionServiceRole3C357FF2',
+        ],
+      }, ResourcePart.CompleteDefinition);
+    });
   });
 
   describe('code config', () => {
diff --git a/packages/@aws-cdk/aws-lambda/test/integ.lambda.filesystem.expected.json b/packages/@aws-cdk/aws-lambda/test/integ.lambda.filesystem.expected.json
index 6d9334ab8999f..abb79b5123377 100644
--- a/packages/@aws-cdk/aws-lambda/test/integ.lambda.filesystem.expected.json
+++ b/packages/@aws-cdk/aws-lambda/test/integ.lambda.filesystem.expected.json
@@ -801,6 +801,7 @@
         "EfsEfsMountTarget195B2DD2E",
         "EfsEfsMountTarget2315C927F",
         "EfsEfsMountTarget36646B9A0",
+        "EfsEfsSecurityGroupfromawscdklambda1MyLambdaSecurityGroup86B085EE20490D9864A8",
         "MyLambdaServiceRoleDefaultPolicy5BBC6F68",
         "MyLambdaServiceRole4539ECB6"
       ]
@@ -1048,7 +1049,8 @@
         "MyLambdaServiceRoleDefaultPolicy5BBC6F68",
         "MyLambdaServiceRole4539ECB6",
         "MyLambda2ServiceRoleDefaultPolicy2BECE79D",
-        "MyLambda2ServiceRoleD09B370C"
+        "MyLambda2ServiceRoleD09B370C",
+        "securityGroupfromawscdklambda1MyLambda2SecurityGroup7492F70D20498301D9D2"
       ]
     }
   }