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

Unexpected stack circular dependency when subscribing a lambda in one stack to a SNS topic in another stack #1643

Closed
nathanpeck opened this issue Jan 30, 2019 · 7 comments · Fixed by #1645
Labels
@aws-cdk/aws-sns Related to Amazon Simple Notification Service @aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort

Comments

@nathanpeck
Copy link
Member

nathanpeck commented Jan 30, 2019

The following code is attempting to make an SNS topic in one stack, and a Lambda in another stack

const cdk = require('@aws-cdk/cdk');
const sns = require('@aws-cdk/aws-sns');
const lambda = require('@aws-cdk/aws-lambda');
const lambdaEvents = require('@aws-cdk/aws-lambda-event-sources');

class SharedResources extends cdk.Stack {
  constructor(parent, id, props) {
    super(parent, id, props);

    this.toCrawlTopic = new sns.Topic(this, 'test-topic', {
      displayName: 'Topic'
    });
  }
}

class SubscribedLambda extends cdk.Stack {
  constructor(parent, id, props) {
    super(parent, id, props);

    const subscribedLambda = new lambda.Function(this, 'crawl-lambda', {
      runtime: lambda.Runtime.NodeJS810,
      handler: 'crawl.handle',
      code: lambda.Code.asset('./app'),
    });

    subscribedLambda.addEventSource(new lambdaEvents.SnsEventSource(props.toCrawlTopic));
  }
}

class App extends cdk.App {
  constructor(argv) {
    super(argv);

    const sharedResources = new SharedResources(this, 'shared-resources');

    new SubscribedLambda(this, 'npm-follower', {
      toCrawlTopic: sharedResources.toCrawlTopic
    });
  }
}

new App().run();

However when I deploy I get this error:

Error: Stack 'shared-resources' already depends on stack 'npm-follower'. Adding this dependency would create a cyclic reference.
    at SharedResources.addDependency (/Users/peckn/Documents/Code/test-case/node_modules/@aws-cdk/cdk/lib/cloudformation/stack.js:185:19)
    at CfnReference.consumeFromStack (/Users/peckn/Documents/Code/test-case/node_modules/@aws-cdk/cdk/lib/cloudformation/cfn-tokens.js:53:28)
    at SharedResources.prepare (/Users/peckn/Documents/Code/test-case/node_modules/@aws-cdk/cdk/lib/cloudformation/stack.js:334:21)
    at ConstructNode.prepareTree (/Users/peckn/Documents/Code/test-case/node_modules/@aws-cdk/cdk/lib/core/construct.js:248:27)
    at App.synthesizeStack (/Users/peckn/Documents/Code/test-case/node_modules/@aws-cdk/cdk/lib/app.js:55:19)
    at App.synthesizeStacks (/Users/peckn/Documents/Code/test-case/node_modules/@aws-cdk/cdk/lib/app.js:85:27)
    at App.run (/Users/peckn/Documents/Code/test-case/node_modules/@aws-cdk/cdk/lib/app.js:43:26)
    at Object.<anonymous> (/Users/peckn/Documents/Code/test-case/minimal.js:42:68)
    at Module._compile (module.js:649:30)
    at Object.Module._extensions..js (module.js:660:10)

My expectation is that the npm-follower stack would get a dependency on the shared-resources stack but not the other way around. Somehow it becomes a circular dependency though

@sam-goodwin
Copy link
Contributor

sam-goodwin commented Jan 30, 2019

Looks like the subscription is created in the stack that owns the Topic and references the Function. A permission is then added to the Function referencing the topic, completing the circular dependency.

https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-sns/lib/topic-ref.ts#L219

I think the solution is to always created the subscription in the function's stack.

@sam-goodwin
Copy link
Contributor

sam-goodwin commented Jan 30, 2019

Try working around by explicitly exporting the topic and importing into the lambda stack:

subscribedLambda.addEventSource(new lambdaEvents.SnsEventSource(
  sns.Topic.import(this, 'Topic', props.toCrawlTopic.export())));

I think this is a subtle edge-case of the cross-stack automatic import/export feature. Before, manually exporting and importing the topic would have placed the topic in the subscribers stack, so calling subscribeLambda would place the Subscription in that same stack. Now that you're actually referencing a Topic in the shared stack, the subscription is created in that stack, causing the circular dependency.

@sam-goodwin sam-goodwin added bug This issue is a bug. @aws-cdk/core Related to core CDK functionality @aws-cdk/aws-sns Related to Amazon Simple Notification Service labels Jan 30, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 31, 2019

I think this is a duplicate. Let me check. But you're right, we need to create the sub under the queue

@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 31, 2019

#1534

@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 31, 2019

Oh wait, it's a different occurrence of the same pattern.

rix0rrr pushed a commit that referenced this issue Jan 31, 2019
Create the SNS Subscription object under the subscriber
instead of the subscribee, avoiding cyclic stack dependencies
if those objects would be in different stacks.

Fixes #1643, fixes #1534.
@nathanpeck
Copy link
Member Author

Awesome thanks @rix0rrr @sam-goodwin!

@fulghum fulghum added the effort/small Small work item – less than a day of effort label Feb 4, 2019
rix0rrr added a commit that referenced this issue Feb 6, 2019
Create the SNS Subscription object under the subscriber
instead of the subscribee, avoiding cyclic stack dependencies
if those objects would be in different stacks.

Fixes #1643, fixes #1534.

DEPLOYMENT IMPACT: this will recreate any subscription objects under new
names. It should not have visible impact on your deployments.
@wywarren
Copy link

wywarren commented May 7, 2020

Is there a way to do the export import method under python? I've been running into a similar issue with api gateway and lambda being declared and linked in two separate stacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-sns Related to Amazon Simple Notification Service @aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants