From 4a9e68311018a42bc5961646dda4be6861f916a5 Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Fri, 5 Apr 2024 21:36:54 -0600 Subject: [PATCH] fix(sns): contentBasedDeduplication is always false for imported topic (#29542) Closes #29532. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../SNSInteg.assets.json | 4 +- .../SNSInteg.template.json | 53 ++++++++ .../test/integ.sns.js.snapshot/manifest.json | 20 ++- .../test/integ.sns.js.snapshot/tree.json | 127 +++++++++++++++++- .../test/aws-sns/test/integ.sns.ts | 14 ++ packages/aws-cdk-lib/aws-sns/lib/topic.ts | 46 ++++++- packages/aws-cdk-lib/aws-sns/test/sns.test.ts | 42 ++++++ 7 files changed, 296 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/SNSInteg.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/SNSInteg.assets.json index e35ec54617f5d..63d6be8c5fd2c 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/SNSInteg.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/SNSInteg.assets.json @@ -1,7 +1,7 @@ { "version": "36.0.0", "files": { - "d8c06dbfa1cbebf5e980040ba6d3b54dfab8d5cff21d8291d207717c4868bf4c": { + "d4f97c865d996fdcf59e005e5e95d931ee98935ca1098865663866fd1ee7b71d": { "source": { "path": "SNSInteg.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "d8c06dbfa1cbebf5e980040ba6d3b54dfab8d5cff21d8291d207717c4868bf4c.json", + "objectKey": "d4f97c865d996fdcf59e005e5e95d931ee98935ca1098865663866fd1ee7b71d.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/SNSInteg.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/SNSInteg.template.json index e7f2e9e204bb4..ef9be119f84b1 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/SNSInteg.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/SNSInteg.template.json @@ -119,6 +119,59 @@ "SignatureVersion": "2", "TopicName": "fooTopicSignatureVersion" } + }, + "MyTopic288CE2107": { + "Type": "AWS::SNS::Topic", + "Properties": { + "DisplayName": "fooDisplayName2", + "KmsMasterKeyId": { + "Fn::GetAtt": [ + "CustomKey1E6D0D07", + "Arn" + ] + }, + "TopicName": "fooTopic2" + } + }, + "PublishRoleF42F66B6": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "s3.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "PublishRoleDefaultPolicy9257B12D": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "sns:Publish", + "Effect": "Allow", + "Resource": { + "Ref": "MyTopic288CE2107" + } + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "PublishRoleDefaultPolicy9257B12D", + "Roles": [ + { + "Ref": "PublishRoleF42F66B6" + } + ] + } } }, "Parameters": { diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/manifest.json index 77c68a723d220..1a78781843870 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/manifest.json @@ -18,7 +18,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/d8c06dbfa1cbebf5e980040ba6d3b54dfab8d5cff21d8291d207717c4868bf4c.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/d4f97c865d996fdcf59e005e5e95d931ee98935ca1098865663866fd1ee7b71d.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ @@ -64,6 +64,24 @@ "data": "MyTopicSignatureVersionEDDB6A3B" } ], + "/SNSInteg/MyTopic2/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "MyTopic288CE2107" + } + ], + "/SNSInteg/PublishRole/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "PublishRoleF42F66B6" + } + ], + "/SNSInteg/PublishRole/DefaultPolicy/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "PublishRoleDefaultPolicy9257B12D" + } + ], "/SNSInteg/BootstrapVersion": [ { "type": "aws:cdk:logicalId", diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/tree.json index b5fe325f55885..9bc2212757d6c 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.js.snapshot/tree.json @@ -105,7 +105,7 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_sns.Topic", + "fqn": "aws-cdk-lib.aws_sns.TopicBase", "version": "0.0.0" } }, @@ -228,7 +228,130 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_sns.Topic", + "fqn": "aws-cdk-lib.aws_sns.TopicBase", + "version": "0.0.0" + } + }, + "MyTopic2": { + "id": "MyTopic2", + "path": "SNSInteg/MyTopic2", + "children": { + "Resource": { + "id": "Resource", + "path": "SNSInteg/MyTopic2/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::SNS::Topic", + "aws:cdk:cloudformation:props": { + "displayName": "fooDisplayName2", + "kmsMasterKeyId": { + "Fn::GetAtt": [ + "CustomKey1E6D0D07", + "Arn" + ] + }, + "topicName": "fooTopic2" + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.aws_sns.CfnTopic", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.aws_sns.TopicBase", + "version": "0.0.0" + } + }, + "ImportedTopic": { + "id": "ImportedTopic", + "path": "SNSInteg/ImportedTopic", + "constructInfo": { + "fqn": "aws-cdk-lib.aws_sns.TopicBase", + "version": "0.0.0" + } + }, + "PublishRole": { + "id": "PublishRole", + "path": "SNSInteg/PublishRole", + "children": { + "ImportPublishRole": { + "id": "ImportPublishRole", + "path": "SNSInteg/PublishRole/ImportPublishRole", + "constructInfo": { + "fqn": "aws-cdk-lib.Resource", + "version": "0.0.0" + } + }, + "Resource": { + "id": "Resource", + "path": "SNSInteg/PublishRole/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::IAM::Role", + "aws:cdk:cloudformation:props": { + "assumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "s3.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + } + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.aws_iam.CfnRole", + "version": "0.0.0" + } + }, + "DefaultPolicy": { + "id": "DefaultPolicy", + "path": "SNSInteg/PublishRole/DefaultPolicy", + "children": { + "Resource": { + "id": "Resource", + "path": "SNSInteg/PublishRole/DefaultPolicy/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::IAM::Policy", + "aws:cdk:cloudformation:props": { + "policyDocument": { + "Statement": [ + { + "Action": "sns:Publish", + "Effect": "Allow", + "Resource": { + "Ref": "MyTopic288CE2107" + } + } + ], + "Version": "2012-10-17" + }, + "policyName": "PublishRoleDefaultPolicy9257B12D", + "roles": [ + { + "Ref": "PublishRoleF42F66B6" + } + ] + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.aws_iam.CfnPolicy", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.aws_iam.Policy", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "aws-cdk-lib.aws_iam.Role", "version": "0.0.0" } }, diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.ts index 4cde4e63523cb..53a10f66df669 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns.ts @@ -44,11 +44,25 @@ class SNSInteg extends Stack { successFeedbackSampleRate: 50, }); + // Topic with signatureVersion new Topic(this, 'MyTopicSignatureVersion', { topicName: 'fooTopicSignatureVersion', displayName: 'fooDisplayNameSignatureVersion', signatureVersion: '2', }); + + // Can import topic + const topic2 = new Topic(this, 'MyTopic2', { + topicName: 'fooTopic2', + displayName: 'fooDisplayName2', + masterKey: key, + }); + const importedTopic = Topic.fromTopicArn(this, 'ImportedTopic', topic2.topicArn); + + const publishRole = new Role(this, 'PublishRole', { + assumedBy: new ServicePrincipal('s3.amazonaws.com'), + }); + importedTopic.grantPublish(publishRole); } } diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic.ts b/packages/aws-cdk-lib/aws-sns/lib/topic.ts index 31cd3b3defd7a..d52babdd5d30e 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/topic.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/topic.ts @@ -153,6 +153,24 @@ export enum LoggingProtocol { APPLICATION = 'application', } +/** + * Represents an SNS topic defined outside of this stack. + */ +export interface TopicAttributes { + /** + * The ARN of the SNS topic. + */ + readonly topicArn: string; + + /** + * Whether content-based deduplication is enabled. + * Only applicable for FIFO topics. + * + * @default false + */ + readonly contentBasedDeduplication?: boolean; +} + /** * A new SNS topic */ @@ -166,16 +184,34 @@ export class Topic extends TopicBase { * @param topicArn topic ARN (i.e. arn:aws:sns:us-east-2:444455556666:MyTopic) */ public static fromTopicArn(scope: Construct, id: string, topicArn: string): ITopic { + return Topic.fromTopicAttributes(scope, id, { topicArn }); + }; + + /** + * Import an existing SNS topic provided a topic attributes + * + * @param scope The parent creating construct + * @param id The construct's name + * @param attrs the attributes of the topic to import + */ + public static fromTopicAttributes(scope: Construct, id: string, attrs: TopicAttributes): ITopic { + const topicName = Stack.of(scope).splitArn(attrs.topicArn, ArnFormat.NO_RESOURCE_NAME).resource; + const fifo = topicName.endsWith('.fifo'); + + if (attrs.contentBasedDeduplication && !fifo) { + throw new Error('Cannot import topic; contentBasedDeduplication is only available for FIFO SNS topics.'); + } + class Import extends TopicBase { - public readonly topicArn = topicArn; - public readonly topicName = Stack.of(scope).splitArn(topicArn, ArnFormat.NO_RESOURCE_NAME).resource; - public readonly fifo = this.topicName.endsWith('.fifo'); - public readonly contentBasedDeduplication = false; + public readonly topicArn = attrs.topicArn; + public readonly topicName = topicName; + public readonly fifo = fifo; + public readonly contentBasedDeduplication = attrs.contentBasedDeduplication || false; protected autoCreatePolicy: boolean = false; } return new Import(scope, id, { - environmentFromArn: topicArn, + environmentFromArn: attrs.topicArn, }); } diff --git a/packages/aws-cdk-lib/aws-sns/test/sns.test.ts b/packages/aws-cdk-lib/aws-sns/test/sns.test.ts index 6bbb9bf0b63b3..ac3fc5f369084 100644 --- a/packages/aws-cdk-lib/aws-sns/test/sns.test.ts +++ b/packages/aws-cdk-lib/aws-sns/test/sns.test.ts @@ -455,6 +455,48 @@ describe('Topic', () => { expect(imported.fifo).toEqual(true); }); + test('fromTopicAttributes contentBasedDeduplication false', () => { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + const imported = sns.Topic.fromTopicAttributes(stack, 'Imported', { + topicArn: 'arn:aws:sns:*:123456789012:mytopic', + }); + + // THEN + expect(imported.topicName).toEqual('mytopic'); + expect(imported.topicArn).toEqual('arn:aws:sns:*:123456789012:mytopic'); + expect(imported.contentBasedDeduplication).toEqual(false); + }); + + test('fromTopicAttributes contentBasedDeduplication true', () => { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + const imported = sns.Topic.fromTopicAttributes(stack, 'Imported', { + topicArn: 'arn:aws:sns:*:123456789012:mytopic.fifo', + contentBasedDeduplication: true, + }); + + // THEN + expect(imported.topicName).toEqual('mytopic.fifo'); + expect(imported.topicArn).toEqual('arn:aws:sns:*:123456789012:mytopic.fifo'); + expect(imported.contentBasedDeduplication).toEqual(true); + }); + + test('fromTopicAttributes throws with contentBasedDeduplication on non-fifo topic', () => { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + expect(() => sns.Topic.fromTopicAttributes(stack, 'Imported', { + topicArn: 'arn:aws:sns:*:123456789012:mytopic', + contentBasedDeduplication: true, + })).toThrow(/Cannot import topic; contentBasedDeduplication is only available for FIFO SNS topics./); + }); + test('sets account for imported topic env', () => { // GIVEN const stack = new cdk.Stack();