From 50b7aecf1d497a17bbe93e2ad8eefacecbd9eb2d Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Tue, 19 Mar 2024 15:01:51 -0600 Subject: [PATCH 01/11] fix(sns): contentBasedDeduplication is always false for imported topic --- packages/aws-cdk-lib/aws-sns/lib/topic.ts | 5 +-- packages/aws-cdk-lib/aws-sns/test/sns.test.ts | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic.ts b/packages/aws-cdk-lib/aws-sns/lib/topic.ts index 31cd3b3defd7a..157518847bdff 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/topic.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/topic.ts @@ -164,13 +164,14 @@ export class Topic extends TopicBase { * @param scope The parent creating construct * @param id The construct's name * @param topicArn topic ARN (i.e. arn:aws:sns:us-east-2:444455556666:MyTopic) + * @param contentBasedDeduplication If content-based deduplication is enabled */ - public static fromTopicArn(scope: Construct, id: string, topicArn: string): ITopic { + public static fromTopicArn(scope: Construct, id: string, topicArn: string, contentBasedDeduplication?: boolean): ITopic { 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 contentBasedDeduplication = contentBasedDeduplication ?? false; protected autoCreatePolicy: boolean = false; } 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..45fb1804fbc0d 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,37 @@ describe('Topic', () => { expect(imported.fifo).toEqual(true); }); + test('fromTopicArn contentBasedDeduplication true', () => { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + const topic = new sns.Topic(stack, 'MyTopic', { + topicName: 'MyTopic', + fifo: true, + contentBasedDeduplication: true, + }); + const imported = sns.Topic.fromTopicArn(stack, 'Imported', topic.topicArn, true); + + // THEN + expect(imported.contentBasedDeduplication).toEqual(true); + }); + + test('fromTopicArn contentBasedDeduplication not provided (false)', () => { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + const topic = new sns.Topic(stack, 'MyTopic', { + topicName: 'MyTopic', + fifo: true, + }); + const imported = sns.Topic.fromTopicArn(stack, 'Imported', topic.topicArn); + + // THEN + expect(imported.contentBasedDeduplication).toEqual(false); + }); + test('sets account for imported topic env', () => { // GIVEN const stack = new cdk.Stack(); From 3fa52dc9011d37294e63e5c4e50dbb2a42f0b75d Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Tue, 19 Mar 2024 15:18:28 -0600 Subject: [PATCH 02/11] fix linter --- packages/aws-cdk-lib/awslint.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/aws-cdk-lib/awslint.json b/packages/aws-cdk-lib/awslint.json index 3e0a4fe6f0cb7..df3f41af27c9b 100644 --- a/packages/aws-cdk-lib/awslint.json +++ b/packages/aws-cdk-lib/awslint.json @@ -192,6 +192,7 @@ "from-signature:aws-cdk-lib.aws_iam.Role.fromRoleName", "from-method:aws-cdk-lib.aws_lambda.FunctionUrl", "from-signature:aws-cdk-lib.aws_secretsmanager.Secret.fromSecretNameV2.params[2]", + "from-signature:aws-cdk-lib.aws_sns.Topic.fromTopicArn", "attribute-tag:aws-cdk-lib.aws_apigateway.RequestAuthorizer.authorizerArn", "attribute-tag:aws-cdk-lib.aws_apigateway.TokenAuthorizer.authorizerArn", "attribute-tag:aws-cdk-lib.aws_codecommit.Repository.repositoryCloneUrlGrc", From b4516aac69edeed35b3977e3f77a0cba29a98e57 Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Sun, 24 Mar 2024 19:43:49 -0600 Subject: [PATCH 03/11] Add feedback from Luca --- packages/aws-cdk-lib/aws-sns/lib/topic.ts | 59 +++++++++++++++++-- packages/aws-cdk-lib/aws-sns/test/sns.test.ts | 26 ++++---- packages/aws-cdk-lib/awslint.json | 1 - 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic.ts b/packages/aws-cdk-lib/aws-sns/lib/topic.ts index 157518847bdff..7c88797cbb545 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/topic.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/topic.ts @@ -3,7 +3,7 @@ import { CfnTopic } from './sns.generated'; import { ITopic, TopicBase } from './topic-base'; import { IRole } from '../../aws-iam'; import { IKey } from '../../aws-kms'; -import { ArnFormat, Lazy, Names, Stack, Token } from '../../core'; +import { ArnFormat, Fn, Lazy, Names, Stack, Token } from '../../core'; /** * Properties for a new SNS topic @@ -153,6 +153,22 @@ 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. + */ + readonly contentBasedDeduplication?: boolean; +} + /** * A new SNS topic */ @@ -164,14 +180,13 @@ export class Topic extends TopicBase { * @param scope The parent creating construct * @param id The construct's name * @param topicArn topic ARN (i.e. arn:aws:sns:us-east-2:444455556666:MyTopic) - * @param contentBasedDeduplication If content-based deduplication is enabled */ - public static fromTopicArn(scope: Construct, id: string, topicArn: string, contentBasedDeduplication?: boolean): ITopic { + public static fromTopicArn(scope: Construct, id: string, topicArn: string): ITopic { 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 = contentBasedDeduplication ?? false; + public readonly contentBasedDeduplication = false; protected autoCreatePolicy: boolean = false; } @@ -180,6 +195,25 @@ export class Topic extends TopicBase { }); } + /** + * 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 { + class Import extends TopicBase { + public readonly topicArn = attrs.topicArn; + public readonly topicName = extractNameFromArn(attrs.topicArn); + public readonly fifo = this.topicName.endsWith('.fifo'); + public readonly contentBasedDeduplication = attrs.contentBasedDeduplication || false; + protected autoCreatePolicy: boolean = false; + } + + return new Import(scope, id); + } + public readonly topicArn: string; public readonly topicName: string; public readonly contentBasedDeduplication: boolean; @@ -285,3 +319,20 @@ export class Topic extends TopicBase { this.loggingConfigs.push(config); } } + +/** + * Given an opaque (token) ARN, returns a CloudFormation expression that extracts the topic + * name from the ARN. + * + * Function ARNs look like this: + * + * arn:aws:sns:region:account-id:topic-name + * + * ..which means that in order to extract the `topic-name` component from the ARN, we can + * split the ARN using ":" and select the component in index 5. + * + * @returns `FnSelect(5, FnSplit(':', arn))` + */ +function extractNameFromArn(arn: string) { + return Fn.select(5, Fn.split(':', arn)); +} 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 45fb1804fbc0d..a7d3f4092efd4 100644 --- a/packages/aws-cdk-lib/aws-sns/test/sns.test.ts +++ b/packages/aws-cdk-lib/aws-sns/test/sns.test.ts @@ -455,35 +455,35 @@ describe('Topic', () => { expect(imported.fifo).toEqual(true); }); - test('fromTopicArn contentBasedDeduplication true', () => { + test('fromTopicAttributes contentBasedDeduplication false', () => { // GIVEN const stack = new cdk.Stack(); // WHEN - const topic = new sns.Topic(stack, 'MyTopic', { - topicName: 'MyTopic', - fifo: true, - contentBasedDeduplication: true, + const imported = sns.Topic.fromTopicAttributes(stack, 'Imported', { + topicArn: 'arn:aws:sns:*:123456789012:mytopic', }); - const imported = sns.Topic.fromTopicArn(stack, 'Imported', topic.topicArn, true); // THEN - expect(imported.contentBasedDeduplication).toEqual(true); + expect(imported.topicName).toEqual('mytopic'); + expect(imported.topicArn).toEqual('arn:aws:sns:*:123456789012:mytopic'); + expect(imported.contentBasedDeduplication).toEqual(false); }); - test('fromTopicArn contentBasedDeduplication not provided (false)', () => { + test('fromTopicAttributes contentBasedDeduplication true', () => { // GIVEN const stack = new cdk.Stack(); // WHEN - const topic = new sns.Topic(stack, 'MyTopic', { - topicName: 'MyTopic', - fifo: true, + const imported = sns.Topic.fromTopicAttributes(stack, 'Imported', { + topicArn: 'arn:aws:sns:*:123456789012:mytopic.fifo', + contentBasedDeduplication: true, }); - const imported = sns.Topic.fromTopicArn(stack, 'Imported', topic.topicArn); // THEN - expect(imported.contentBasedDeduplication).toEqual(false); + expect(imported.topicName).toEqual('mytopic.fifo'); + expect(imported.topicArn).toEqual('arn:aws:sns:*:123456789012:mytopic.fifo'); + expect(imported.contentBasedDeduplication).toEqual(true); }); test('sets account for imported topic env', () => { diff --git a/packages/aws-cdk-lib/awslint.json b/packages/aws-cdk-lib/awslint.json index df3f41af27c9b..3e0a4fe6f0cb7 100644 --- a/packages/aws-cdk-lib/awslint.json +++ b/packages/aws-cdk-lib/awslint.json @@ -192,7 +192,6 @@ "from-signature:aws-cdk-lib.aws_iam.Role.fromRoleName", "from-method:aws-cdk-lib.aws_lambda.FunctionUrl", "from-signature:aws-cdk-lib.aws_secretsmanager.Secret.fromSecretNameV2.params[2]", - "from-signature:aws-cdk-lib.aws_sns.Topic.fromTopicArn", "attribute-tag:aws-cdk-lib.aws_apigateway.RequestAuthorizer.authorizerArn", "attribute-tag:aws-cdk-lib.aws_apigateway.TokenAuthorizer.authorizerArn", "attribute-tag:aws-cdk-lib.aws_codecommit.Repository.repositoryCloneUrlGrc", From 53396fe81fbf7a5004fd44ae7c3a5025f7334922 Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Sun, 24 Mar 2024 20:33:38 -0600 Subject: [PATCH 04/11] trigger build From fe50635cbb2796075c9f6e8c9957ca1be061463e Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Sun, 24 Mar 2024 20:52:11 -0600 Subject: [PATCH 05/11] add default --- packages/aws-cdk-lib/aws-sns/lib/topic.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic.ts b/packages/aws-cdk-lib/aws-sns/lib/topic.ts index 7c88797cbb545..9697151a8131f 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/topic.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/topic.ts @@ -165,6 +165,8 @@ export interface TopicAttributes { /** * Whether content-based deduplication is enabled. * Only applicable for FIFO topics. + * + * @default false */ readonly contentBasedDeduplication?: boolean; } From 4e798d4ff9d29d8dc3ddf542f1a2134693edee47 Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Tue, 26 Mar 2024 14:40:25 -0700 Subject: [PATCH 06/11] add feedback from Luca --- packages/aws-cdk-lib/aws-sns/lib/topic.ts | 39 ++++------------------- 1 file changed, 7 insertions(+), 32 deletions(-) diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic.ts b/packages/aws-cdk-lib/aws-sns/lib/topic.ts index 9697151a8131f..f3e9cf4277dd3 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/topic.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/topic.ts @@ -3,7 +3,7 @@ import { CfnTopic } from './sns.generated'; import { ITopic, TopicBase } from './topic-base'; import { IRole } from '../../aws-iam'; import { IKey } from '../../aws-kms'; -import { ArnFormat, Fn, Lazy, Names, Stack, Token } from '../../core'; +import { ArnFormat, Lazy, Names, Stack, Token } from '../../core'; /** * Properties for a new SNS topic @@ -184,18 +184,8 @@ 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 { - 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; - protected autoCreatePolicy: boolean = false; - } - - return new Import(scope, id, { - environmentFromArn: topicArn, - }); - } + return Topic.fromTopicAttributes(scope, id, { topicArn }); + }; /** * Import an existing SNS topic provided a topic attributes @@ -207,13 +197,15 @@ export class Topic extends TopicBase { public static fromTopicAttributes(scope: Construct, id: string, attrs: TopicAttributes): ITopic { class Import extends TopicBase { public readonly topicArn = attrs.topicArn; - public readonly topicName = extractNameFromArn(attrs.topicArn); + public readonly topicName = Stack.of(scope).splitArn(attrs.topicArn, ArnFormat.NO_RESOURCE_NAME).resource; public readonly fifo = this.topicName.endsWith('.fifo'); public readonly contentBasedDeduplication = attrs.contentBasedDeduplication || false; protected autoCreatePolicy: boolean = false; } - return new Import(scope, id); + return new Import(scope, id, { + environmentFromArn: attrs.topicArn, + }); } public readonly topicArn: string; @@ -321,20 +313,3 @@ export class Topic extends TopicBase { this.loggingConfigs.push(config); } } - -/** - * Given an opaque (token) ARN, returns a CloudFormation expression that extracts the topic - * name from the ARN. - * - * Function ARNs look like this: - * - * arn:aws:sns:region:account-id:topic-name - * - * ..which means that in order to extract the `topic-name` component from the ARN, we can - * split the ARN using ":" and select the component in index 5. - * - * @returns `FnSelect(5, FnSplit(':', arn))` - */ -function extractNameFromArn(arn: string) { - return Fn.select(5, Fn.split(':', arn)); -} From dbba9ec6bae1c24aabc4fe134ad4fd96f0cf344c Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Tue, 26 Mar 2024 14:46:51 -0700 Subject: [PATCH 07/11] update integration tests --- .../test/aws-sns/test/integ.sns-fifo.ts | 8 +++++++- .../framework-integ/test/aws-sns/test/integ.sns.ts | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts index a4dffe0ed2b35..45838e639de3b 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts @@ -5,12 +5,18 @@ class SNSFifoInteg extends Stack { constructor(scope: App, id: string, props?: StackProps) { super(scope, id, props); - new Topic(this, 'MyTopic', { + const topic = new Topic(this, 'MyTopic', { topicName: 'fooTopic', displayName: 'fooDisplayName', contentBasedDeduplication: true, fifo: true, }); + + // Can import topic from attributes + Topic.fromTopicAttributes(this, 'ImportedTopic', { + topicArn: topic.topicArn, + contentBasedDeduplication: true, + }) } } 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); } } From 9525f084c05e65af2fa7ea0f77b46377c88ec95a Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Tue, 26 Mar 2024 20:38:28 -0600 Subject: [PATCH 08/11] fix semicolon --- .../framework-integ/test/aws-sns/test/integ.sns-fifo.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts index 45838e639de3b..8eb455b497e6f 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts @@ -16,7 +16,7 @@ class SNSFifoInteg extends Stack { Topic.fromTopicAttributes(this, 'ImportedTopic', { topicArn: topic.topicArn, contentBasedDeduplication: true, - }) + }); } } From bd6d663d252a613d161b6b1df10c717b61cfd5ba Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Wed, 27 Mar 2024 08:25:57 -0600 Subject: [PATCH 09/11] int test --- .../framework-integ/test/aws-sns/test/integ.sns-fifo.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts index 8eb455b497e6f..a4dffe0ed2b35 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns/test/integ.sns-fifo.ts @@ -5,18 +5,12 @@ class SNSFifoInteg extends Stack { constructor(scope: App, id: string, props?: StackProps) { super(scope, id, props); - const topic = new Topic(this, 'MyTopic', { + new Topic(this, 'MyTopic', { topicName: 'fooTopic', displayName: 'fooDisplayName', contentBasedDeduplication: true, fifo: true, }); - - // Can import topic from attributes - Topic.fromTopicAttributes(this, 'ImportedTopic', { - topicArn: topic.topicArn, - contentBasedDeduplication: true, - }); } } From c466407ae5ddcdddd83384b66f7b70b9f22e73d4 Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Wed, 27 Mar 2024 09:19:53 -0600 Subject: [PATCH 10/11] fix int test --- .../test/integ.sns.js.snapshot/tree.json | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) 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..248295eafb745 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 @@ -232,6 +232,129 @@ "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": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "ImportedTopic": { + "id": "ImportedTopic", + "path": "SNSInteg/ImportedTopic", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "PublishRole": { + "id": "PublishRole", + "path": "SNSInteg/PublishRole", + "children": { + "ImportPublishRole": { + "id": "ImportPublishRole", + "path": "SNSInteg/PublishRole/ImportPublishRole", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.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": "constructs.Construct", + "version": "10.3.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": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, "BootstrapVersion": { "id": "BootstrapVersion", "path": "SNSInteg/BootstrapVersion", From 1c533f0f31af5bb082b40b7639c8ca0a285fad07 Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Fri, 5 Apr 2024 17:41:05 -0600 Subject: [PATCH 11/11] contentBasedDeduplication only on fifo topics --- .../SNSInteg.assets.json | 4 +- .../SNSInteg.template.json | 53 +++++++++++++++++++ .../test/integ.sns.js.snapshot/manifest.json | 20 ++++++- .../test/integ.sns.js.snapshot/tree.json | 36 ++++++------- packages/aws-cdk-lib/aws-sns/lib/topic.ts | 11 +++- packages/aws-cdk-lib/aws-sns/test/sns.test.ts | 11 ++++ 6 files changed, 112 insertions(+), 23 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 248295eafb745..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,7 @@ } }, "constructInfo": { - "fqn": "aws-cdk-lib.aws_sns.Topic", + "fqn": "aws-cdk-lib.aws_sns.TopicBase", "version": "0.0.0" } }, @@ -253,22 +253,22 @@ } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.3.0" + "fqn": "aws-cdk-lib.aws_sns.CfnTopic", + "version": "0.0.0" } } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.3.0" + "fqn": "aws-cdk-lib.aws_sns.TopicBase", + "version": "0.0.0" } }, "ImportedTopic": { "id": "ImportedTopic", "path": "SNSInteg/ImportedTopic", "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.3.0" + "fqn": "aws-cdk-lib.aws_sns.TopicBase", + "version": "0.0.0" } }, "PublishRole": { @@ -279,8 +279,8 @@ "id": "ImportPublishRole", "path": "SNSInteg/PublishRole/ImportPublishRole", "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.3.0" + "fqn": "aws-cdk-lib.Resource", + "version": "0.0.0" } }, "Resource": { @@ -304,8 +304,8 @@ } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.3.0" + "fqn": "aws-cdk-lib.aws_iam.CfnRole", + "version": "0.0.0" } }, "DefaultPolicy": { @@ -339,20 +339,20 @@ } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.3.0" + "fqn": "aws-cdk-lib.aws_iam.CfnPolicy", + "version": "0.0.0" } } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.3.0" + "fqn": "aws-cdk-lib.aws_iam.Policy", + "version": "0.0.0" } } }, "constructInfo": { - "fqn": "constructs.Construct", - "version": "10.3.0" + "fqn": "aws-cdk-lib.aws_iam.Role", + "version": "0.0.0" } }, "BootstrapVersion": { diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic.ts b/packages/aws-cdk-lib/aws-sns/lib/topic.ts index f3e9cf4277dd3..d52babdd5d30e 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/topic.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/topic.ts @@ -195,10 +195,17 @@ export class Topic extends TopicBase { * @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 = attrs.topicArn; - public readonly topicName = Stack.of(scope).splitArn(attrs.topicArn, ArnFormat.NO_RESOURCE_NAME).resource; - public readonly fifo = this.topicName.endsWith('.fifo'); + public readonly topicName = topicName; + public readonly fifo = fifo; public readonly contentBasedDeduplication = attrs.contentBasedDeduplication || false; protected autoCreatePolicy: boolean = false; } 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 a7d3f4092efd4..ac3fc5f369084 100644 --- a/packages/aws-cdk-lib/aws-sns/test/sns.test.ts +++ b/packages/aws-cdk-lib/aws-sns/test/sns.test.ts @@ -486,6 +486,17 @@ describe('Topic', () => { 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();