Skip to content

Commit

Permalink
Add feedback from Luca
Browse files Browse the repository at this point in the history
  • Loading branch information
msambol committed Mar 23, 2024
1 parent e077b3d commit 2b6e7d8
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 19 deletions.
4 changes: 3 additions & 1 deletion packages/aws-cdk-lib/aws-sns/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ const topic = new sns.Topic(this, 'Topic', {
Add an SNS Topic to your stack with a specified signature version, which corresponds
to the hashing algorithm used while creating the signature of the notifications,
subscription confirmations, or unsubscribe confirmation messages sent by Amazon SNS.
The default is `1`.

The default signature version is `1` (`SHA1`).
SNS also supports signature version `2` (`SHA256`).

```ts
const topic = new sns.Topic(this, 'Topic', {
Expand Down
10 changes: 0 additions & 10 deletions packages/aws-cdk-lib/aws-sns/lib/topic-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,6 @@ export interface ITopic extends IResource, notifications.INotificationRuleTarget
*/
readonly fifo: boolean;

/**
* Corresponds to the hashing algorithm used while creating the signature of the notifications,
* subscription confirmations, or unsubscribe confirmation messages sent by Amazon SNS.
*
* @attribute
*/
readonly signatureVersion?: string;

/**
* Subscribe some endpoint to this topic
*/
Expand Down Expand Up @@ -79,8 +71,6 @@ export abstract class TopicBase extends Resource implements ITopic {

public abstract readonly contentBasedDeduplication: boolean;

public abstract readonly signatureVersion?: string;

/**
* Controls automatic creation of policy objects.
*
Expand Down
19 changes: 11 additions & 8 deletions packages/aws-cdk-lib/aws-sns/lib/topic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export interface TopicProps {
/**
* The list of delivery status logging configurations for the topic.
*
* For more information, see https://docs.aws.amazon.com/sns/latest/dg/sns-topic-attributes.html.
* @see https://docs.aws.amazon.com/sns/latest/dg/sns-topic-attributes.html.
*
* @default None
*/
Expand All @@ -71,7 +71,7 @@ export interface TopicProps {
/**
* Adds a statement to enforce encryption of data in transit when publishing to the topic.
*
* For more information, see https://docs.aws.amazon.com/sns/latest/dg/sns-security-best-practices.html#enforce-encryption-data-in-transit.
* @see https://docs.aws.amazon.com/sns/latest/dg/sns-security-best-practices.html#enforce-encryption-data-in-transit.
*
* @default false
*/
Expand All @@ -81,7 +81,7 @@ export interface TopicProps {
* The signature version corresponds to the hashing algorithm used while creating the signature of the notifications,
* subscription confirmations, or unsubscribe confirmation messages sent by Amazon SNS.
*
* For more information, see https://docs.aws.amazon.com/sns/latest/dg/sns-verify-signature-of-message.html.
* @see https://docs.aws.amazon.com/sns/latest/dg/sns-verify-signature-of-message.html.
*
* @default 1
*/
Expand All @@ -91,7 +91,7 @@ export interface TopicProps {
/**
* A logging configuration for delivery status of messages sent from SNS topic to subscribed endpoints.
*
* For more information, see https://docs.aws.amazon.com/sns/latest/dg/sns-topic-attributes.html.
* @see https://docs.aws.amazon.com/sns/latest/dg/sns-topic-attributes.html.
*/
export interface LoggingConfig {
/**
Expand Down Expand Up @@ -171,7 +171,6 @@ export class Topic extends TopicBase {
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 signatureVersion = '1';
protected autoCreatePolicy: boolean = false;
}

Expand All @@ -184,7 +183,6 @@ export class Topic extends TopicBase {
public readonly topicName: string;
public readonly contentBasedDeduplication: boolean;
public readonly fifo: boolean;
public readonly signatureVersion?: string;

protected readonly autoCreatePolicy: boolean = true;

Expand Down Expand Up @@ -219,7 +217,7 @@ export class Topic extends TopicBase {
if (props.fifo && props.topicName && !props.topicName.endsWith('.fifo')) {
cfnTopicName = this.physicalName + '.fifo';
} else if (props.fifo && !props.topicName) {
// Max lenght allowed by CloudFormation is 256, we subtract 5 to allow for ".fifo" suffix
// Max length allowed by CloudFormation is 256, we subtract 5 to allow for ".fifo" suffix
const prefixName = Names.uniqueResourceName(this, {
maxLength: 256 - 5,
separator: '-',
Expand All @@ -229,6 +227,12 @@ export class Topic extends TopicBase {
cfnTopicName = this.physicalName;
}

if (props.signatureVersion !== undefined) {
if (props.signatureVersion !== '1' && props.signatureVersion !== '2') {
throw new Error(`signatureVersion must be "1" or "2", received: "${props.signatureVersion}"`);
}
}

const resource = new CfnTopic(this, 'Resource', {
archivePolicy: props.messageRetentionPeriodInDays ? {
MessageRetentionPeriod: props.messageRetentionPeriodInDays,
Expand All @@ -249,7 +253,6 @@ export class Topic extends TopicBase {
this.topicName = this.getResourceNameAttribute(resource.attrTopicName);
this.fifo = props.fifo || false;
this.contentBasedDeduplication = props.contentBasedDeduplication || false;
this.signatureVersion = props.signatureVersion;
}

private renderLoggingConfigs(): CfnTopic.LoggingConfigProperty[] {
Expand Down
7 changes: 7 additions & 0 deletions packages/aws-cdk-lib/aws-sns/test/sns.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,14 @@ describe('Topic', () => {
Template.fromStack(stack).hasResourceProperties('AWS::SNS::Topic', {
'SignatureVersion': '2',
});
});

test('throw with incorrect signatureVersion', () => {
const stack = new cdk.Stack();

expect(() => new sns.Topic(stack, 'MyTopic', {
signatureVersion: '3',
})).toThrow(/signatureVersion must be "1" or "2", received: "3"/);
});
});

Expand Down

0 comments on commit 2b6e7d8

Please sign in to comment.