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

(lambda): EFS and Lambda create cyclic reference when deployed in separate stacks #18759

Closed
israelg99 opened this issue Feb 1, 2022 · 8 comments · Fixed by #28560
Closed
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@israelg99
Copy link

israelg99 commented Feb 1, 2022

What is the problem?

EFS (particularly AccessPoint) and Lambda create cyclic reference when deployed in separate stacks.

Very related: #5760, #10942, #11245.
All of which are P1. Check the reproducible code in those issues as well.

Reproduction Steps

Here is a working example:

export class TheEfsLambdaStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const vpc = new ec2.Vpc(this, 'Vpc', {
      maxAzs: 2, // Default is all AZs in the region
    });

    // Create a file system in EFS to store information
    const fs = new efs.FileSystem(this, 'FileSystem', {
      vpc,
      removalPolicy: cdk.RemovalPolicy.DESTROY
    });

    const accessPoint = fs.addAccessPoint('AccessPoint',{
      createAcl: {
        ownerGid: '1001',
        ownerUid: '1001',
        permissions: '750'
      },
      path:'/export/lambda',
      posixUser: {
        gid: '1001',
        uid: '1001'
      }
    });

    // This lambda function is given access to our EFS File System
    const efsLambda = new lambda.Function(this, 'efsLambdaFunction', {
      runtime: lambda.Runtime.PYTHON_3_8,
      code: lambda.Code.fromAsset('lambda-fns'), 
      handler: 'message_wall.lambda_handler',
      vpc: vpc,
      filesystem: lambda.FileSystem.fromEfsAccessPoint(accessPoint, '/mnt/msg')
    });

    // defines an API Gateway Http API resource backed by our "efsLambda" function.
    let api = new apigw.HttpApi(this, 'Endpoint', {
      defaultIntegration: new integrations.LambdaProxyIntegration({
        handler: efsLambda
      })
    });

   new cdk.CfnOutput(this, 'HTTP API Url', {
     value: api.url ?? 'Something went wrong with the deploy'
   });
  }
}

The code breaks if Function is deployed in separate stack and the AccessPoint is passed in props, with the following error:

/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/stack.ts:395
      throw new Error(`'${target.node.path}' depends on '${this.node.path}' (${cycle.join(', ')}). Adding this dependency (${reason}) would create a cyclic reference.`);
            ^
Error: 'Stack2' depends on 'Stack' ("Stack2/lambda/lambda/ServiceRole/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget1", "Stack2/lambda/lambda/ServiceRole/DefaultPolicy/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget1", "Stack2/lambda/lambda/SecurityGroup/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget1", "Stack2/lambda/lambda/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget1", "Stack2/lambda/lambda/ServiceRole/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget2", "Stack2/lambda/lambda/ServiceRole/DefaultPolicy/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget2", "Stack2/lambda/lambda/SecurityGroup/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget2", "Stack2/lambda/lambda/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget2", "Stack2/lambda/lambda/ServiceRole/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget3", "Stack2/lambda/lambda/ServiceRole/DefaultPolicy/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget3", "Stack2/lambda/lambda/SecurityGroup/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget3", "Stack2/lambda/lambda/Resource" depends on "Stack/efs/FileSystem/EfsMountTarget3", "Stack2/lambda/lambda/Resource" depends on "Stack/efs/FileSystem/EfsSecurityGroup/from StackefsFileSystemEfsSecurityGroup2F9415D6:ALL PORTS", "Stack2/lambda/lambda/Resource" depends on "Stack/efs/FileSystem/EfsSecurityGroup/from StackfunctionSecurityGroupD8C55079:2049", "Stack2/lambda/lambda/Resource" depends on "Stack/efs/FileSystem/EfsSecurityGroup/from Stack2clipmodeldownloadSecurityGroupB2C2BEB1:2049"). Adding this dependency (Stack -> Stack2/lambda/lambda/SecurityGroup/Resource.GroupId) would create a cyclic reference.
    at Stack._addAssemblyDependency (/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/stack.ts:395:13)
    at Object.addDependency (/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/deps.ts:52:20)
    at Stack.addDependency (/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/stack.ts:266:5)
    at resolveValue (/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/private/refs.ts:100:12)
    at Object.resolveReferences (/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/private/refs.ts:30:24)
    at Object.prepareApp (/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/private/prepare-app.ts:30:3)
    at Object.synthesize (/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/private/synthesis.ts:32:3)
    at App.synth (/Users/julian/work/project/node_modules/aws-cdk-lib/core/lib/stage.ts:90:23)
    at Object.<anonymous> (/Users/julian/work/project/bin/infra.ts:134:5)
    at Module._compile (internal/modules/cjs/loader.js:1072:14)

For more code insight check similar issues: #5760, #10942, #11245.

I can add simple self-contained reproducible code but it should be simple enough to infer.

What did you expect to happen?

Should work. We can pass Vpc in props across stacks, why EFS and others fail? The error is confusing as well.

What actually happened?

Fails as described above.

CDK CLI Version

2.10

Framework Version

No response

Node.js Version

14 LTS

OS

Mac

Language

Typescript

Language Version

TS 4.x

Other information

No response

@israelg99 israelg99 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 1, 2022
@github-actions github-actions bot added the @aws-cdk/aws-efs Related to Amazon Elastic File System label Feb 1, 2022
@israelg99
Copy link
Author

israelg99 commented Feb 1, 2022

Potential workaround is to not pass EFS as props, but get the EFS resource using FileSystem.fromFileSystemAttributes inside the lambda stack. Interestingly, passing VPC as props works well as usual. So this looks like an inconsistency in the interface.

@israelg99
Copy link
Author

@LukvonStrom Sounds good, I will write a unit test for it

@ryparker ryparker added the p1 label Feb 1, 2022
@corymhall
Copy link
Contributor

It looks like the issue is with this code. We already have a dependency Lambda -> AccessPoint and this is trying to create the security group rules in the AccessPoint stack (AccessPoint -> Lambda)

if (props.filesystem) {
if (props.filesystem.config.connections) {
props.filesystem.config.connections.allowDefaultPortFrom(this);
}
}

This should be updated to be something like this so that the security group rules are added in the lambda stack.

if (props.filesystem) {
  if (props.filesystem.config.connections) {
    this.connections.allowTo(props.filesystem.config.connections, props.filesystem.config.connections.defaultPort);
  }
}

@corymhall corymhall added the effort/small Small work item – less than a day of effort label Feb 4, 2022
@corymhall corymhall assigned kaizencc and unassigned corymhall Feb 4, 2022
@corymhall corymhall added @aws-cdk/aws-lambda Related to AWS Lambda and removed @aws-cdk/aws-efs Related to Amazon Elastic File System needs-triage This issue or PR still needs to be triaged. labels Feb 4, 2022
@kaizencc kaizencc changed the title EFS and Lambda create cyclic reference when deployed in separate stacks (lambda): EFS and Lambda create cyclic reference when deployed in separate stacks Feb 9, 2022
@kaizencc kaizencc removed their assignment Feb 9, 2022
@Annegies
Copy link

Annegies commented Apr 4, 2022

Hey, also running into this issue. Is there an update?

@ShivamJoker
Copy link

not sure even if this will be fixed in next year, it starts throwing error just passing env to the lambda

@fivepapertigers
Copy link

For those that are struggling for a workaround here, I hacked around this for my needs by using escape hatches (shoutout to @icj217 for the suggestion). Here's what my Python implementation looked like:

def repair_function_to_efs_security_groups(func: lambda_.Function, efs: efs.IFileSystem):
    """
    When an EFS filesystem is added to a Lambda Function (via the file_system= param)
    it automatically sets up networking access between the two by adding
    an ingress rule on the EFS security group. However, the ingress rule resource
    gets attached to whichever CDK Stack the EFS security group is defined on.
    If the Lambda Function is defined on a different stack, it then creates
    a circular dependency issue, where the EFS stack is dependent on the Lambda
    security group's ID and the Lambda stack is dependent on the EFS stack's file
    system object.

    To resolve this, we manually remove the ingress rule that gets automatically created
    and recreate it on the Lambda's stack instead.
    """

    # Collect IDs of all security groups attached to the function
    func_sec_groups = {
        func_sec_group.security_group_id
        for func_sec_group in func.connections.security_groups
    }
    # Iterate over the security groups attached to EFS
    for sec_group in efs.connections.security_groups:
        # Iterate over the security group's child nodes
        for child in sec_group.node.find_all():
            # If this is an ingress rule with a "source" equal to one of
            # the function's security groups
            if (
                isinstance(child, ec2.CfnSecurityGroupIngress) and
                child.source_security_group_id in func_sec_groups
            ):
                # Try to remove the node (raise an error if removal fails)
                node_id = child.node.id
                if not sec_group.node.try_remove_child(node_id):
                    raise RuntimeError(f'Could not remove child node: {node_id}')

    # Finally, configure the connection between the Function and the EFS drive
    # which will define the new ingress rule on the Lambda's stack instead.
    func.connections.allow_to_default_port(efs)

Adapt for your needs.

@moltar
Copy link
Contributor

moltar commented Nov 7, 2022

Thanks @fivepapertigers for this escape hatch!

Here's the same in TS:

    fileSystem.connections.securityGroups.forEach((fssg) => {
      fssg.node.findAll().forEach((child)=> {
        if (child instanceof CfnSecurityGroupIngress &&
          lambda.connections.securityGroups.some(({ securityGroupId }) => securityGroupId === child.sourceSecurityGroupId)) {
          fssg.node.tryRemoveChild(child.node.id);
        }
      });
    });

    lambda.connections.allowToDefaultPort(fileSystem);

@mergify mergify bot closed this as completed in #28560 Jan 13, 2024
mergify bot pushed a commit that referenced this issue Jan 13, 2024
…n separate stacks (#28560)

This PR fixed an error when deploying EFS and Lambda in separate stacks.

## Cause of the bug
Currently, when using EFS from Lambda, deploying EFS and Lambda in separate stacks creates incorrect resource dependencies and cannot be deployed correctly.
This error is caused by adding a security group setting in the Function construct to allow EFS and Lambda to communicate correctly.
By calling the `Connections.allowDefaultPortFrom` method of the Filesystem in the LambdaStack, IngressRule is created in the scope of the EfsStack.
Note that the `remoteRule` flag is false when calling `SecurityGroupBase.addIngressRule` at this time.
https://github.com/aws/aws-cdk/blob/dde59755cb71aee73a58f3b2c2068f2ae01e9b72/packages/aws-cdk-lib/aws-lambda/lib/function.ts#L1416
https://github.com/aws/aws-cdk/blob/dde59755cb71aee73a58f3b2c2068f2ae01e9b72/packages/aws-cdk-lib/aws-ec2/lib/connections.ts#L157
https://github.com/aws/aws-cdk/blob/dde59755cb71aee73a58f3b2c2068f2ae01e9b72/packages/aws-cdk-lib/aws-ec2/lib/security-group.ts#L84

Here is the minimal code to reproduce this error without EFS and Lambda.
```ts
#!/usr/bin/env node
import 'source-map-support/register';
import { App, Stack, StackProps } from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as ec2 from 'aws-cdk-lib/aws-ec2';

export class EfsStack extends Stack {
    public vpc: ec2.Vpc;
    public efsSg: ec2.SecurityGroup;

    constructor(scope: Construct, id: string, props?: StackProps) {
      super(scope, id, props);

      this.vpc = new ec2.Vpc(this, 'Vpc');

      this.efsSg = new ec2.SecurityGroup(this, 'SecurityGroup', {
        vpc: this.vpc,
        allowAllOutbound: true,
      });
    }
}

interface LambdaStackProps extends StackProps {
    vpc: ec2.Vpc;
    efsSg: ec2.SecurityGroup;
}

export class LambdaStack extends Stack {
    constructor(scope: Construct, id: string, props: LambdaStackProps) {
      super(scope, id, props);

      const lambdaSg = new ec2.SecurityGroup(this, 'SecurityGroup', {
        vpc: props.vpc,
        allowAllOutbound: true,
      });

      // Since `remoteRule` flag is set to false here, IngressRule is deployed in EfsStack scope.
      props.efsSg.addIngressRule(lambdaSg, ec2.Port.tcp(2049), '', false);
    }
}

const app = new App();
const efsStack = new EfsStack(app, 'EfsStack');
const lambdaStack = new LambdaStack(app, 'LambdaStack', {
    vpc: efsStack.vpc,
    efsSg: efsStack.efsSg,
});
```

By calling the `SecurityGroupBase.addIngressRule` method with the `remoteRule` flag true, the IngressRule will be deployed in the scope of the Lambda stack and the deployment will complete successfully.

## Changes
Fixed the SecurityGroup Rule configuration part in the Function construct to fix this error.
By changing the Function construct to call the `Connections.allowTo` method, the `remoteRule` flag is set to true when `allowTo` method calls `allowFrom` method and the EFS Security Group Ingress Rule will be correctly created in the scope of the Lambda stack.
https://github.com/aws/aws-cdk/blob/dde59755cb71aee73a58f3b2c2068f2ae01e9b72/packages/aws-cdk-lib/aws-ec2/lib/connections.ts#L139
https://github.com/aws/aws-cdk/blob/dde59755cb71aee73a58f3b2c2068f2ae01e9b72/packages/aws-cdk-lib/aws-ec2/lib/connections.ts#L141

Closes #18759

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
9 participants