Skip to content
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

fix(events-targets): cannot set retry policy to 0 retry attempts #21900

Merged
merged 15 commits into from
Sep 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-events-targets/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function bindBaseTargetConfig(props: TargetBaseProps) {

return {
deadLetterConfig: deadLetterQueue ? { arn: deadLetterQueue?.queueArn } : undefined,
retryPolicy: retryAttempts || maxEventAge
retryPolicy: (retryAttempts !== undefined && retryAttempts >= 0) || maxEventAge
? {
maximumRetryAttempts: retryAttempts,
maximumEventAgeInSeconds: maxEventAge?.toSeconds({ integral: true }),
Expand Down
52 changes: 52 additions & 0 deletions packages/@aws-cdk/aws-events-targets/test/batch/batch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,56 @@ describe('Batch job event target', () => {
],
});
});

test('specifying retry policy with 0 retryAttempts', () => {
// GIVEN
const rule = new events.Rule(stack, 'Rule', {
schedule: events.Schedule.expression('rate(1 hour)'),
});

// WHEN
const eventInput = {
buildspecOverride: 'buildspecs/hourly.yml',
};

rule.addTarget(new targets.BatchJob(
jobQueue.jobQueueArn,
jobQueue,
jobDefinition.jobDefinitionArn,
jobDefinition, {
event: events.RuleTargetInput.fromObject(eventInput),
retryAttempts: 0,
},
));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
ScheduleExpression: 'rate(1 hour)',
State: 'ENABLED',
Targets: [
{
Arn: {
Ref: 'MyQueueE6CA6235',
},
BatchParameters: {
JobDefinition: {
Ref: 'MyJob8719E923',
},
JobName: 'Rule',
},
Id: 'Target0',
Input: JSON.stringify(eventInput),
RetryPolicy: {
MaximumRetryAttempts: 0,
},
RoleArn: {
'Fn::GetAtt': [
'MyJobEventsRoleCF43C336',
'Arn',
],
},
},
],
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,52 @@ describe('CodeBuild event target', () => {
});
});

test('specifying retry policy with 0 retryAttempts', () => {
// GIVEN
const rule = new events.Rule(stack, 'Rule', {
schedule: events.Schedule.expression('rate(1 hour)'),
});

// WHEN
const eventInput = {
buildspecOverride: 'buildspecs/hourly.yml',
};

rule.addTarget(
new targets.CodeBuildProject(project, {
event: events.RuleTargetInput.fromObject(eventInput),
retryAttempts: 0,
}),
);

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
ScheduleExpression: 'rate(1 hour)',
State: 'ENABLED',
Targets: [
{
Arn: {
'Fn::GetAtt': [
'MyProject39F7B0AE',
'Arn',
],
},
Id: 'Target0',
Input: '{"buildspecOverride":"buildspecs/hourly.yml"}',
RetryPolicy: {
MaximumRetryAttempts: 0,
},
RoleArn: {
'Fn::GetAtt': [
'MyProjectEventsRole5B7D93F5',
'Arn',
],
},
},
],
});
});

test('use a Dead Letter Queue for the rule target', () => {
// GIVEN
const rule = new events.Rule(stack, 'Rule', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,56 @@ describe('CodePipeline event target', () => {
],
});
});

test('adds 0 retry attempts to the target configuration', () => {
// WHEN
rule.addTarget(new targets.CodePipeline(pipeline, {
retryAttempts: 0,
}));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
ScheduleExpression: 'rate(1 minute)',
State: 'ENABLED',
Targets: [
{
Arn: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':codepipeline:',
{
Ref: 'AWS::Region',
},
':',
{
Ref: 'AWS::AccountId',
},
':',
{
Ref: 'PipelineC660917D',
},
],
],
},
Id: 'Target0',
RetryPolicy: {
MaximumRetryAttempts: 0,
},
RoleArn: {
'Fn::GetAtt': [
'PipelineEventsRole46BEEA7C',
'Arn',
],
},
},
],
});
});
});

describe('with an explicit event role', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
"Id": "Target0",
"RetryPolicy": {
"MaximumEventAgeInSeconds": 7200,
"MaximumRetryAttempts": 2
"MaximumRetryAttempts": 0
}
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const queue = new sqs.Queue(stack, 'Queue');
timer3.addTarget(new targets.LambdaFunction(fn, {
deadLetterQueue: queue,
maxEventAge: cdk.Duration.hours(2),
retryAttempts: 2,
retryAttempts: 0,
}));

app.synth();
Expand Down
43 changes: 43 additions & 0 deletions packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,49 @@ test('specifying retry policy', () => {
});
});

test('specifying retry policy with 0 retryAttempts', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');

const fn = new lambda.Function(stack, 'MyLambda', {
code: new lambda.InlineCode('foo'),
handler: 'bar',
runtime: lambda.Runtime.PYTHON_3_9,
});

// WHEN
new events.Rule(stack, 'Rule', {
schedule: events.Schedule.rate(cdk.Duration.minutes(1)),
targets: [new targets.LambdaFunction(fn, {
retryAttempts: 0,
})],
});

// THEN
expect(() => app.synth()).not.toThrow();

// the Permission resource should be in the event stack
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
ScheduleExpression: 'rate(1 minute)',
State: 'ENABLED',
Targets: [
{
Arn: {
'Fn::GetAtt': [
'MyLambdaCCE802FB',
'Arn',
],
},
Id: 'Target0',
RetryPolicy: {
MaximumRetryAttempts: 0,
},
},
],
});
});

function newTestLambda(scope: constructs.Construct, suffix = '') {
return new lambda.Function(scope, `MyLambda${suffix}`, {
code: new lambda.InlineCode('foo'),
Expand Down
64 changes: 64 additions & 0 deletions packages/@aws-cdk/aws-events-targets/test/logs/log-group.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,3 +295,67 @@ testDeprecated('specifying retry policy and dead letter queue', () => {
],
});
});

testDeprecated('specifying retry policy with 0 retryAttempts', () => {
// GIVEN
const stack = new cdk.Stack();
const logGroup = new logs.LogGroup(stack, 'MyLogGroup', {
logGroupName: '/aws/events/MyLogGroup',
});
const rule1 = new events.Rule(stack, 'Rule', {
schedule: events.Schedule.rate(cdk.Duration.minutes(1)),
});

// WHEN
rule1.addTarget(new targets.CloudWatchLogGroup(logGroup, {
event: events.RuleTargetInput.fromObject({
timestamp: events.EventField.time,
message: events.EventField.fromPath('$'),
}),
retryAttempts: 0,
}));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
ScheduleExpression: 'rate(1 minute)',
State: 'ENABLED',
Targets: [
{
Arn: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':logs:',
{
Ref: 'AWS::Region',
},
':',
{
Ref: 'AWS::AccountId',
},
':log-group:',
{
Ref: 'MyLogGroup5C0DAD85',
},
],
],
},
Id: 'Target0',
InputTransformer: {
InputPathsMap: {
time: '$.time',
f2: '$',
},
InputTemplate: '{"timestamp":<time>,"message":<f2>}',
},
RetryPolicy: {
MaximumRetryAttempts: 0,
},
},
],
});
});
32 changes: 32 additions & 0 deletions packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,35 @@ test('specifying retry policy', () => {
],
});
});

test('specifying retry policy with 0 retryAttempts', () => {
const stack = new Stack();
const queue = new sqs.Queue(stack, 'MyQueue', { fifo: true });
const rule = new events.Rule(stack, 'MyRule', {
schedule: events.Schedule.rate(Duration.hours(1)),
});

// WHEN
rule.addTarget(new targets.SqsQueue(queue, {
retryAttempts: 0,
}));

Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
ScheduleExpression: 'rate(1 hour)',
State: 'ENABLED',
Targets: [
{
Arn: {
'Fn::GetAtt': [
'MyQueueE6CA6235',
'Arn',
],
},
Id: 'Target0',
RetryPolicy: {
MaximumRetryAttempts: 0,
},
},
],
});
});
Loading