Skip to content

Commit

Permalink
fix: Fix handling of redrivePolicy in SNS events. (#7277)
Browse files Browse the repository at this point in the history
  • Loading branch information
tcastelli committed Feb 19, 2020
1 parent c09f718 commit 292b1ca
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 48 deletions.
33 changes: 31 additions & 2 deletions docs/providers/aws/events/sns.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ functions:

## Setting a redrive policy

This event definition creates an SNS topic that sends messages to a Dead Letter Queue (defined by its ARN) when the associated lambda is not available. In this example, messages that aren't delivered to the `dispatcher` Lambda (because the lambda service is down or irresponsive) will end in `myDLQ`
This event definition creates an SNS topic that sends messages to a Dead Letter Queue (defined by its ARN) when the associated lambda is not available. In this example, messages that aren't delivered to the `dispatcher` Lambda (because the lambda service is down or irresponsive) will end in `myDLQ`.

```yml
functions:
Expand All @@ -159,7 +159,20 @@ functions:
- sns:
topicName: dispatcher
redrivePolicy:
deadLetterTargetArn: !Ref myDLQ
deadLetterTargetArn: arn:aws:sqs:us-east-1:11111111111:myDLQ
```

To define the Dead Letter Queue, you can alternatively use the the resource name with `deadLetterTargetRef`

```yml
functions:
dispatcher:
handler: dispatcher.handler
events:
- sns:
topicName: dispatcher
redrivePolicy:
deadLetterTargetRef: myDLQ

resources:
Resources:
Expand All @@ -168,3 +181,19 @@ resources:
Properties:
QueueName: myDLQ
```

Or if you want to use values from other stacks, you can
also use `deadLetterTargetImport` to define the DLQ url and arn with exported values

```yml
functions:
dispatcher:
handler: dispatcher.handler
events:
- sns:
topicName: dispatcher
redrivePolicy:
deadLetterTargetImport:
arn: MyShared-DLQArn
url: MyShared-DLQUrl
```
9 changes: 8 additions & 1 deletion docs/providers/aws/guide/serverless.yml.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,14 @@ functions:
- dog
- cat
redrivePolicy:
deadLetterTargetArn: arn:aws:sqs:region:XXXXXX:myDLQ
# (1) ARN
deadLetterTargetArn: arn:aws:sqs:us-east-1:11111111111:myDLQ
# (2) Ref (resource defined in same CF stack)
deadLetterTargetRef: myDLQ
# (3) Import (resource defined in outer CF stack)
deadLetterTargetImport:
arn: MyShared-DLQArn
url: MyShared-DLQUrl
- sqs:
arn: arn:aws:sqs:region:XXXXXX:myQueue
batchSize: 10
Expand Down
5 changes: 5 additions & 0 deletions lib/plugins/aws/lib/naming.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,11 @@ module.exports = {
getTopicLogicalId(topicName) {
return `SNSTopic${this.normalizeTopicName(topicName)}`;
},
getTopicDLQPolicyLogicalId(functionName, topicName) {
return `${this.normalizeTopicName(topicName)}To${this.getNormalizedFunctionName(
functionName
)}DLQPolicy`;
},

// Schedule
getScheduleId(functionName) {
Expand Down
149 changes: 109 additions & 40 deletions lib/plugins/aws/package/compile/events/sns/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class AwsCompileSNSEvents {
let topicName;
let region;
let displayName = '';
let redrivePolicy;
if (typeof event.sns === 'object') {
if (event.sns.arn) {
topicArn = event.sns.arn;
Expand Down Expand Up @@ -122,6 +123,111 @@ class AwsCompileSNSEvents {
throw new this.serverless.classes.Error(errorMessage);
}

if (event.sns.redrivePolicy) {
const {
deadLetterTargetArn,
deadLetterTargetRef,
deadLetterTargetImport,
} = event.sns.redrivePolicy;
let targetArn;
let targetUrl;
if (!deadLetterTargetArn && !deadLetterTargetRef && !deadLetterTargetImport) {
throw new this.serverless.classes.Error(
'redrivePolicy must be specified with deadLetterTargetArn, deadLetterTargetRef or deadLetterTargetImport'
);
}

if (deadLetterTargetArn) {
if (
typeof deadLetterTargetArn !== 'string' ||
!deadLetterTargetArn.startsWith('arn:')
) {
throw new this.serverless.classes.Error(
'Invalid deadLetterTargetArn specified, it must be an string starting with arn:'
);
} else {
targetArn = deadLetterTargetArn;
// arn:aws:sqs:us-east-1:11111111111:myDLQ
const [deQueueName, deAccount, deRegion] = deadLetterTargetArn
.split(':')
.reverse();
targetUrl = {
'Fn::Join': [
'',
`https://sqs.${deRegion}.`,
{ Ref: 'AWS::URLSuffix' },
`/${deAccount}/${deQueueName}`,
],
};
}
} else if (deadLetterTargetRef) {
if (typeof deadLetterTargetRef !== 'string') {
throw new this.serverless.classes.Error(
'Invalid deadLetterTargetRef specified, it must be a string'
);
}
targetArn = {
'Fn::GetAtt': [deadLetterTargetRef, 'Arn'],
};
targetUrl = {
Ref: deadLetterTargetRef,
};
} else {
if (
typeof deadLetterTargetImport !== 'object' ||
!deadLetterTargetImport.arn ||
!deadLetterTargetImport.url
) {
throw new this.serverless.classes.Error(
'Invalid deadLetterTargetImport specified, it must be an object containing arn and url fields'
);
}
targetArn = {
'Fn::ImportValue': deadLetterTargetImport.arn,
};
targetUrl = {
'Fn::ImportValue': deadLetterTargetImport.url,
};
}

redrivePolicy = {
deadLetterTargetArn: targetArn,
};

const queuePolicyLogicalId = this.provider.naming.getTopicDLQPolicyLogicalId(
functionName,
topicName
);

Object.assign(template.Resources, {
[queuePolicyLogicalId]: {
Type: 'AWS::SQS::QueuePolicy',
Properties: {
PolicyDocument: {
Version: '2012-10-17',
Id: queuePolicyLogicalId,
Statement: [
{
Effect: 'Allow',
Principal: {
Service: 'sns.amazonaws.com',
},
Action: 'sqs:SendMessage',
Resource: targetArn,
Condition: {
ArnEquals: {
'aws:SourceArn': topicArn,
},
},
},
],
},
Queues: [targetUrl],
},
},
});
}

const lambdaLogicalId = this.provider.naming.getLambdaLogicalId(functionName);

const endpoint = {
Expand All @@ -142,7 +248,7 @@ class AwsCompileSNSEvents {
Protocol: 'lambda',
Endpoint: endpoint,
FilterPolicy: event.sns.filterPolicy,
RedrivePolicy: event.sns.redrivePolicy,
RedrivePolicy: redrivePolicy,
Region: region,
},
},
Expand Down Expand Up @@ -183,10 +289,7 @@ class AwsCompileSNSEvents {
});
}

if (
event.sns.filterPolicy ||
(event.sns.redrivePolicy && event.sns.redrivePolicy.deadLetterTargetArn)
) {
if (event.sns.filterPolicy || redrivePolicy) {
_.merge(template.Resources, {
[subscriptionLogicalId]: {
Type: 'AWS::SNS::Subscription',
Expand All @@ -195,7 +298,7 @@ class AwsCompileSNSEvents {
Ref: topicLogicalId,
},
FilterPolicy: event.sns.filterPolicy,
RedrivePolicy: event.sns.redrivePolicy,
RedrivePolicy: redrivePolicy,
}),
},
});
Expand Down Expand Up @@ -223,40 +326,6 @@ class AwsCompileSNSEvents {
},
},
});

if (event.sns.redrivePolicy && event.sns.redrivePolicy.deadLetterTargetArn) {
const queuePolicyLogicalId = this.provider.naming.getQueueLogicalId(
functionName,
`${topicName}DLQ`
);
Object.assign(template.Resources, {
[queuePolicyLogicalId]: {
Type: 'AWS::SQS::QueuePolicy',
Properties: {
PolicyDocument: {
Version: '2012-10-17',
Id: queuePolicyLogicalId,
Statement: [
{
Effect: 'Allow',
Principal: {
Service: 'sns.amazonaws.com',
},
Action: 'sqs:SendMessage',
Resource: event.sns.redrivePolicy.deadLetterTargetArn,
Condition: {
ArnEquals: {
'aws:SourceArn': topicArn,
},
},
},
],
},
Queues: [event.sns.redrivePolicy.deadLetterTargetArn],
},
},
});
}
}
});
}
Expand Down
93 changes: 88 additions & 5 deletions lib/plugins/aws/package/compile/events/sns/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ describe('AwsCompileSNSEvents', () => {
).to.equal('AWS::Lambda::Permission');
});

it('should link topic to corresponding dlq when redrivePolicy is defined', () => {
it('should link topic to corresponding dlq when redrivePolicy is defined by arn string', () => {
awsCompileSNSEvents.serverless.service.functions = {
first: {
events: [
Expand All @@ -606,9 +606,48 @@ describe('AwsCompileSNSEvents', () => {
topicName: 'Topic 1',
displayName: 'Display name for topic 1',
redrivePolicy: {
deadLetterTargetArn: {
'Fn::GetAtt': ['SNSDLQ', 'Arn'],
},
deadLetterTargetArn: 'arn:aws:sqs:us-east-1:11111111111:myDLQ',
},
},
},
],
},
};

awsCompileSNSEvents.compileSNSEvents();

expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.SNSTopicTopic1.Type
).to.equal('AWS::SNS::Topic');
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.FirstLambdaPermissionTopic1SNS.Type
).to.equal('AWS::Lambda::Permission');
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.FirstSnsSubscriptionTopic1.Type
).to.equal('AWS::SNS::Subscription');
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.FirstSnsSubscriptionTopic1.Properties.RedrivePolicy
).to.eql({ deadLetterTargetArn: 'arn:aws:sqs:us-east-1:11111111111:myDLQ' });
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.Topic1ToFirstDLQPolicy.Type
).to.equal('AWS::SQS::QueuePolicy');
});

it('should link topic to corresponding dlq when redrivePolicy is defined with resource ref', () => {
awsCompileSNSEvents.serverless.service.functions = {
first: {
events: [
{
sns: {
topicName: 'Topic 1',
displayName: 'Display name for topic 1',
redrivePolicy: {
deadLetterTargetRef: 'SNSDLQ',
},
},
},
Expand Down Expand Up @@ -648,7 +687,51 @@ describe('AwsCompileSNSEvents', () => {
).to.eql({ deadLetterTargetArn: { 'Fn::GetAtt': ['SNSDLQ', 'Arn'] } });
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.FirstEventSourceMappingSQSTopic1DLQ.Type
.Topic1ToFirstDLQPolicy.Type
).to.equal('AWS::SQS::QueuePolicy');
});

it('should link topic to corresponding dlq when redrivePolicy is defined with resource ref', () => {
awsCompileSNSEvents.serverless.service.functions = {
first: {
events: [
{
sns: {
topicName: 'Topic 1',
displayName: 'Display name for topic 1',
redrivePolicy: {
deadLetterTargetImport: {
arn: 'myDLQArn',
url: 'myDLQUrl',
},
},
},
},
],
},
};

awsCompileSNSEvents.compileSNSEvents();

expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.SNSTopicTopic1.Type
).to.equal('AWS::SNS::Topic');
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.FirstLambdaPermissionTopic1SNS.Type
).to.equal('AWS::Lambda::Permission');
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.FirstSnsSubscriptionTopic1.Type
).to.equal('AWS::SNS::Subscription');
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.FirstSnsSubscriptionTopic1.Properties.RedrivePolicy
).to.eql({ deadLetterTargetArn: { 'Fn::ImportValue': 'myDLQArn' } });
expect(
awsCompileSNSEvents.serverless.service.provider.compiledCloudFormationTemplate.Resources
.Topic1ToFirstDLQPolicy.Type
).to.equal('AWS::SQS::QueuePolicy');
});
});
Expand Down

0 comments on commit 292b1ca

Please sign in to comment.