-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
S3 SqsDestination and SnsDestination creates a cyclic reference #11245
Comments
For reference #5760, the fix may be similar |
Apparently there is some kind of what-was-first-chicken-or-egg situation and topic's stack can't leave without the bucket's Custom::S3BucketNotifications resource for some reason. |
+1 |
1 similar comment
+1 |
+1 |
1 similar comment
+1 |
I'm adding my issue to this to bump awareness. I create all of my buckets in a single class, and made each of the buckets an attribute of the class. I imported the MyBuckets class into my main stack, which passed the necessary buckets to the sub stacks. In one of the stacks, I need to add an S3 event trigger onto a Lambda, and that's where I'm getting the cyclic reference. Is there a workaround? I also get the cyclic reference error if I attempt to add the S3 bucket as an event source to the lambda function. What's odd is I followed the same process for an SQS queue and did not have an issue (create the queue in its own class, pass the object to the constructs/stacks that need it, use it as event source on a Lambda) ` MyBuckets Classself.bucket1 = s3.Bucket(self, "Bucket1") Main Stackimport MyBuckets buckets = MyBuckets(...) subStack = Stack(self, "SubStack", a_bucket=buckets.bucket1) Sub Stackdef init(..., a_bucket: Bucket) myLambda = lambda.Function(....) |
I'm also hitting this when using SNS as an event destination, any workaround or movement on this issue? |
So thought I’d share the workaround I’ve put in place as it may help someone else who comes across this thread. In my situation I was configuring a basic fan-out pattern across stacks as shown below but essentially to fix it I had to create the resources that resulted in a cyclic reference within the same stack. Before (broken):
After (working):
This works but clouds the separations of concerns for each of the stacks in question in my case. @SomayaB Do you think this issue is worthy of bumping to a P1 given it’s Lambda counterpart was also a P1? |
+1 |
Same issue, a fix would be great! |
Low effort fix: rather than accept the |
This issue is due to the fact that // SQS stack
const sqsQueue = new sqs.Queue(this, 'Queue', {
retentionPeriod: Duration.days(14),
receiveMessageWaitTime: Duration.seconds(20),
visibilityTimeout: Duration.hours(1),
})
sqsQueue.addToResourcePolicy(new iam.PolicyStatement({
actions: ['sqs:SendMessage'],
effect: Effect.ALLOW,
principals: [new ServicePrincipal('s3.amazonaws.com')],
resources: [sqsQueue.queueArn],
conditions: {
StringEquals: {
"aws:SourceAccount": this.account,
},
ArnLike: {
// Allows all buckets to send notifications since we haven't created the bucket yet.
"aws:SourceArn": "arn:aws:s3:*:*:*"
}
}
}))
sqsQueue.addToResourcePolicy(new iam.PolicyStatement({
actions: ['sqs:SendMessage'],
effect: Effect.ALLOW,
principals: [new ServicePrincipal('sns.amazonaws.com')],
resources: [sqsQueue.queueArn],
conditions: {
ArnLike: {
// Allows all sns topics to send notifications since we haven't created the topic yet.
"aws:SourceArn": `arn:aws:sns:*:${this.account}:*`
}
}
})) // S3 stack
const bucket = new s3.Bucket(this, 'Bucket', {
versioned: true,
})
// Importing the queue tricks the dependency validator and causes it to not try and modify the policy.
const queue = sqs.Queue.fromQueueArn(this, `Queue`, props.beanstalkPrepStack.sqsQueue.queueArn)
bucket.addObjectCreatedNotification(new s3n.SqsDestination(queue)) This should provide adequate security for most use cases since it is scoped to the account level but it's not perfect. |
Separated out the services
Other possible workaround is to use AwsCustomResource. You basically specify an AWS SDK call to be executed onCreate, onUpdate or onDelete - internally this creates a Lambda that will do the real work: Note: This replaces the existing notification configuration with the configuration you include in the parameter. Check: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketNotificationConfiguration.html // bucket created in stack1
// queue and notification in stack2
queue.addPermission(`AllowS3Invocation`, {
action: 'sqs:SendMessage',
principal: new ServicePrincipal('s3.amazonaws.com'),
sourceArn: bucket.bucketArn
})
const notificationResource = new AwsCustomResource(this, `NotificationCustomResource`, {
logRetention: RetentionDays.THREE_DAYS,
policy: AwsCustomResourcePolicy.fromStatements([
new PolicyStatement({
effect: Effect.ALLOW,
actions: ['s3:PutBucketNotification'],
resources: [bucket.bucketArn, `${ bucket.bucketArn }/*`],
})
]),
onCreate: {
service: 'S3',
action: 'putBucketNotificationConfiguration',
parameters: {
Bucket: bucket.bucketName,
NotificationConfiguration: {
QueueConfigurations: [
{
Events:['s3:ObjectCreated:*'],
QueueArn: queue.queueArn,
}
]
}
},
physicalResourceId: PhysicalResourceId.of(`${ id + Date.now().toString() }`),
},
})
notificationResource.node.addDependency(queue.permissionsNode.findChild('AllowS3Invocation')) |
+1 to this, as issue 5760 has been erroneously closed. |
+1 |
Any update on this? It's still an issue in the latest CDK version. |
This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue. |
The usual way to deal with the cross Stack cyclic references issue is to use the "export attribute and import resources" mechanism (see CloudFormation import * as s3 from 'aws-cdk-lib/aws-s3';
import * as sns from 'aws-cdk-lib/aws-sns';
import * as cdk from 'aws-cdk-lib';
import * as s3n from 'aws-cdk-lib/aws-s3-notifications';
import { Construct } from 'constructs';
class BucketStack extends cdk.Stack {
private readonly bucket: s3.Bucket;
constructor(scope: Construct, id: string, props?: cdk.StackProps) {
super(scope, id, props);
this.bucket = new s3.Bucket(this, 'my-bucket');
}
/**
*
* @param scope Construct which will interact with the bucket. If the scope's Stack is not the same as the BucketStack,
* a cross stack reference will be created using the `Stack.exportValue()` method.
*
* @returns the bucket that was created with this stack.
*/
public getBucket(scope?: Construct): s3.IBucket {
// return this.bucket; // --> does not support cross stacks scenario
if (scope === undefined || cdk.Stack.of(scope) === this) {
return this.bucket;
} else {
const exportedBucketName = this.exportValue(this.bucket.bucketName);
return s3.Bucket.fromBucketName(scope, 'my-imported-bucket', exportedBucketName);
}
}
}
class TopicStack extends cdk.Stack {
private readonly topic: sns.Topic;
constructor(scope: Construct, id: string, props?: cdk.StackProps) {
super(scope, id, props);
this.topic = new sns.Topic(this, 'my-topic');
}
/**
*
* @param scope Construct which will interact with the topic. If the scope's Stack is not the same as the TopicStack,
* a cross stack reference will be created using the `Stack.exportValue()` method.
*
* @returns the topic that was created with this stack.
*/
public getTopic(scope?: Construct): sns.ITopic {
// return this.topic; // --> does not support cross stack scenario
if (scope === undefined || cdk.Stack.of(scope) === this) {
return this.topic;
} else {
const exportedTopicArn = this.exportValue(this.topic.topicArn);
return sns.Topic.fromTopicArn(scope, 'my-imported-topic', exportedTopicArn);
}
}
}
export class S3EventNotifications {
private readonly bucket: s3.IBucket;
private readonly topic: sns.ITopic;
constructor(app: cdk.App) {
const bucketStack = new BucketStack(app, 'MyBucketStack');
const topicStack = new TopicStack(app, 'MyTopicStack');
this.bucket = bucketStack.getBucket(topicStack);
this.topic = topicStack.getTopic();
this.bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SnsDestination(this.topic));
}
} In this example the responsibility of creating an export/import resource is handled by the stack which own the resource. Another option would be to move this logic into the Here is the result of the 2 stacks synthesize: cdk synth MyBucketStack Resources:
mybucket15E130AF:
Type: AWS::S3::Bucket
UpdateReplacePolicy: Retain
DeletionPolicy: Retain
Outputs:
ExportsOutputRefmybucket15E130AFA0000000:
Value:
Ref: mybucket15E130AF
Export:
Name: MyBucketStack:ExportsOutputRefmybucket15E130AFA0000000 cdk synth MyTopicStack Resources:
mytopicA51900AA:
Type: AWS::SNS::Topic
mytopicPolicy0AEB5F49:
Type: AWS::SNS::TopicPolicy
Properties:
PolicyDocument:
Statement:
- Action: sns:Publish
Condition:
ArnLike:
aws:SourceArn:
Fn::Join:
- ""
- - "arn:"
- Ref: AWS::Partition
- ":s3:::"
- Fn::ImportValue: MyBucketStack:ExportsOutputRefmybucket15E130AFA0000000
Effect: Allow
Principal:
Service: s3.amazonaws.com
Resource:
Ref: mytopicA51900AA
Sid: "0"
Version: "2012-10-17"
Topics:
- Ref: mytopicA51900AA
myimportedbucketNotificationsAC5303A0:
Type: Custom::S3BucketNotifications
Properties:
ServiceToken:
Fn::GetAtt:
- BucketNotificationsHandler000a0087b7544547bf325f094a3db8347ECC3691
- Arn
BucketName:
Fn::ImportValue: MyBucketStack:ExportsOutputRefmybucket15E130AFA0000000
NotificationConfiguration:
TopicConfigurations:
- Events:
- s3:ObjectCreated:Put
TopicArn:
Ref: mytopicA51900AA
Managed: false
DependsOn:
- mytopicPolicy0AEB5F49
- mytopicA51900AA
BucketNotificationsHandler000a0087b7544547bf325f094a3db834RoleB6FB88EC:
Type: AWS::IAM::Role
Properties:
AssumeRolePolicyDocument:
Statement:
- Action: sts:AssumeRole
Effect: Allow
Principal:
Service: lambda.amazonaws.com
Version: "2012-10-17"
ManagedPolicyArns:
- Fn::Join:
- ""
- - "arn:"
- Ref: AWS::Partition
- :iam::aws:policy/service-role/AWSLambdaBasicExecutionRole
BucketNotificationsHandler000a0087b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36:
Type: AWS::IAM::Policy
Properties:
PolicyDocument:
Statement:
- Action:
- s3:GetBucketNotification
- s3:PutBucketNotification
Effect: Allow
Resource: "*"
Version: "2012-10-17"
PolicyName: BucketNotificationsHandler000a0087b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36
Roles:
- Ref: BucketNotificationsHandler000a0087b7544547bf325f094a3db834RoleB6FB88EC
BucketNotificationsHandler000a0087b7544547bf325f094a3db8347ECC3691:
Type: AWS::Lambda::Function
Properties:
Description: AWS CloudFormation handler for "Custom::S3BucketNotifications" resources (@aws-cdk/aws-s3)
Code: ... Tested with This being said, I am interested in cases where this kind of workaround is not possible. Please, feel free to share some examples. |
Comments on closed issues and PRs are hard for our team to see. |
Reproduction Steps
What actually happened?
The text was updated successfully, but these errors were encountered: