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] Cannot avoid circular dependency when lambda and cloudwatch-events resources are in different stacks #10942

Closed
tmokmss opened this issue Oct 19, 2020 · 3 comments · Fixed by #11217
Assignees
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

@tmokmss
Copy link
Contributor

tmokmss commented Oct 19, 2020

From v1.68.0, it became impossible to avoid circular dependencies in certain cases.

The cases are for example:

  • when lambda function and cloudwatch events which invoke the lambda function are defined in different stacks.

I assume this PR #10622 introduced this behavior, but not sure.

Reproduction Steps

Define two stacks as below.
One is a stack which defines a lambda function.
Another is a stack which defines CloudWatch event to invoke the lambda repeatedly.

import cdk = require('@aws-cdk/core');
import lambda = require('@aws-cdk/aws-lambda');
import events = require('@aws-cdk/aws-events');
import eventsTarget = require('@aws-cdk/aws-events-targets');

class LambdaStack extends cdk.Stack {
    readonly handler: lambda.IFunction;

    constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
        super(scope, id, props);
        const handler = new lambda.Function(this, 'handler', {
            runtime: lambda.Runtime.NODEJS_12_X,
            handler: 'main.handler',
            code: lambda.Code.fromInline('hoge'),
            
        });

        this.handler = handler;
        // Uncomment this line to avoid circular dependency. It worked before v1.68.0
        // this.handler = lambda.Function.fromFunctionArn(this, `handler-ref`, handler.functionArn);
    }
}

class CwStack extends cdk.Stack {
    constructor(scope: cdk.App, id: string, handler: lambda.IFunction, props?: cdk.StackProps) {
        super(scope, id, props);
        const target = new eventsTarget.LambdaFunction(handler);

        new events.Rule(this, `rule`, {
            schedule: events.Schedule.rate(cdk.Duration.minutes(10)),
            targets: [target],
        });
    }
}

const lambdaStack = new LambdaStack(app, 'LambdaStack');
new CwStack(app, 'CwStack', lambdaStack.handler);

And run cdk synth

What did you expect to happen?

Before v1.68.0, the error below happens.

Error: 'LambdaStack' depends on 'CwStack' (LambdaStack -> CwStack/rule/Resource.Arn). 
Adding this dependency (CwStack -> LambdaStack/handler/Resource.Arn) would create a cyclic reference.

To avoid this error, as it was an only work-around I know, you can pass the reference of the lambda function retrieved by fromFunctionArn.
So uncommenting the line in the above code resolved this problem and cdk synth passes successfully.

What actually happened?

From v1.68.0, unfortunately, the reference of the lambda also creates circular dependencies.
So whether we use the actual lambda function or the reference of it, the same error below happens.

Error: 'LambdaStack' depends on 'CwStack' (LambdaStack -> CwStack/rule/Resource.Arn). 
Adding this dependency (CwStack -> LambdaStack/handler/Resource.Arn) would create a cyclic reference.

There's no possible work-around I found so far.

Environment

  • CLI Version : v1.68.0
  • Framework Version: v1.68.0
  • Node.js Version: v14.13.0
  • OS : macOS
  • Language (Version): TypeScript

Other

This composition of stacks are commonly used in our production environment, because there're many cases that several stacks share the same lambda function.
Rather than defining a lambda function in several stacks duplicately, we'd like to define it in a single stack and referring to it from other stacks.

It'll be great if you provide us with a option which enables the previous behavior of lambda reference. (maybe specifying canCreatePermissions from outside?)

public addPermission(id: string, permission: Permission) {
if (!this.canCreatePermissions) {
// FIXME: @deprecated(v2) - throw an error if calling `addPermission` on a resource that doesn't support it.
return;
}


This is 🐛 Bug Report

@tmokmss tmokmss added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 19, 2020
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Oct 19, 2020
nija-at pushed a commit that referenced this issue Oct 30, 2020
…a different stack

The Lambda Permission resource causes a cyclic dependency when the rule
is in a seprate stack from the lambda target for the rule.
(a picture is worth a thousand words)

```
     +-------------------+        +---------------+
     |Lamda Stack        |        |Event Stack    |
     |                   |        |               |
     |   +----------+    |        |    +------+   |
     |   |          |    |        |    |      |   |
     |   | Function |<-----------------+ Rule |   |
     |   |          |    |        |    |      |   |
     |   +----------+    |        |    +------+   |
     |        ^          |        |       ^       |
     |        |          |        |       |       |
     |  +-----+------+   |        |       |       |
     |  |            |   |        |       |       |
     |  | Permission +--------------------+       |
     |  |            |   |        |               |
     |  +------------+   |        |               |
     |                   |        |               |
     +-------------------+        +---------------+
```

The fix is to move the Permission resource into the event stack instead.

fixes #10942
@nija-at
Copy link
Contributor

nija-at commented Oct 30, 2020

Thanks for filing this issue. I've posted a fix here - #11217

@nija-at nija-at added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Oct 30, 2020
@mergify mergify bot closed this as completed in #11217 Oct 30, 2020
mergify bot pushed a commit that referenced this issue Oct 30, 2020
…a different stack (#11217)

The Lambda Permission resource causes a cyclic dependency when the rule
is in a seprate stack from the lambda target for the rule.
(a picture is worth a thousand words)

```
     +-------------------+        +---------------+
     |Lamda Stack        |        |Event Stack    |
     |                   |        |               |
     |   +----------+    |        |    +------+   |
     |   |          |    |        |    |      |   |
     |   | Function |<-----------------+ Rule |   |
     |   |          |    |        |    |      |   |
     |   +----------+    |        |    +------+   |
     |        ^          |        |       ^       |
     |        |          |        |       |       |
     |  +-----+------+   |        |       |       |
     |  |            |   |        |       |       |
     |  | Permission +--------------------+       |
     |  |            |   |        |               |
     |  +------------+   |        |               |
     |                   |        |               |
     +-------------------+        +---------------+
```

The fix is to move the Permission resource into the event stack instead.

fixes #10942


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
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.

@john-tipper
Copy link

This change actually breaks cross-account deployments, specifically when the Rule is in one account and the lambda is in another, because then the Event stack contains resources in 2 different accounts. See #13568. I think we need to come up with a different solution to the circular-dependency problem reported by the OP.

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
Development

Successfully merging a pull request may close this issue.

3 participants