Skip to content

Commit 5549f16

Browse files
VarunWachaspatiVarunWachaspati
authored
fix(events-targets): cannot set retry policy to 0 retry attempts (#21900)
Currently, it is not possible to set 0 retry attempts for any of the EventBridge targets that support a retry policy as 0 is falsy value and hence the following conditional doesn't evaluate to true - https://github.com/aws/aws-cdk/blob/c607ca51be1042f091b4e4419f20bec75863055c/packages/%40aws-cdk/aws-events-targets/lib/util.ts#L54-L59 Changed the conditional logic to allow 0 retry attempts for all supported targets along with unit tests. fixes [#21864 ](#21864) ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent f3f4814 commit 5549f16

File tree

10 files changed

+336
-3
lines changed

10 files changed

+336
-3
lines changed

‎packages/@aws-cdk/aws-events-targets/lib/util.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export function bindBaseTargetConfig(props: TargetBaseProps) {
5151

5252
return {
5353
deadLetterConfig: deadLetterQueue ? { arn: deadLetterQueue?.queueArn } : undefined,
54-
retryPolicy: retryAttempts || maxEventAge
54+
retryPolicy: (retryAttempts !== undefined && retryAttempts >= 0) || maxEventAge
5555
? {
5656
maximumRetryAttempts: retryAttempts,
5757
maximumEventAgeInSeconds: maxEventAge?.toSeconds({ integral: true }),

‎packages/@aws-cdk/aws-events-targets/test/batch/batch.test.ts

+52
Original file line numberDiff line numberDiff line change
@@ -237,4 +237,56 @@ describe('Batch job event target', () => {
237237
],
238238
});
239239
});
240+
241+
test('specifying retry policy with 0 retryAttempts', () => {
242+
// GIVEN
243+
const rule = new events.Rule(stack, 'Rule', {
244+
schedule: events.Schedule.expression('rate(1 hour)'),
245+
});
246+
247+
// WHEN
248+
const eventInput = {
249+
buildspecOverride: 'buildspecs/hourly.yml',
250+
};
251+
252+
rule.addTarget(new targets.BatchJob(
253+
jobQueue.jobQueueArn,
254+
jobQueue,
255+
jobDefinition.jobDefinitionArn,
256+
jobDefinition, {
257+
event: events.RuleTargetInput.fromObject(eventInput),
258+
retryAttempts: 0,
259+
},
260+
));
261+
262+
// THEN
263+
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
264+
ScheduleExpression: 'rate(1 hour)',
265+
State: 'ENABLED',
266+
Targets: [
267+
{
268+
Arn: {
269+
Ref: 'MyQueueE6CA6235',
270+
},
271+
BatchParameters: {
272+
JobDefinition: {
273+
Ref: 'MyJob8719E923',
274+
},
275+
JobName: 'Rule',
276+
},
277+
Id: 'Target0',
278+
Input: JSON.stringify(eventInput),
279+
RetryPolicy: {
280+
MaximumRetryAttempts: 0,
281+
},
282+
RoleArn: {
283+
'Fn::GetAtt': [
284+
'MyJobEventsRoleCF43C336',
285+
'Arn',
286+
],
287+
},
288+
},
289+
],
290+
});
291+
});
240292
});

‎packages/@aws-cdk/aws-events-targets/test/codebuild/codebuild.test.ts

+46
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,52 @@ describe('CodeBuild event target', () => {
170170
});
171171
});
172172

173+
test('specifying retry policy with 0 retryAttempts', () => {
174+
// GIVEN
175+
const rule = new events.Rule(stack, 'Rule', {
176+
schedule: events.Schedule.expression('rate(1 hour)'),
177+
});
178+
179+
// WHEN
180+
const eventInput = {
181+
buildspecOverride: 'buildspecs/hourly.yml',
182+
};
183+
184+
rule.addTarget(
185+
new targets.CodeBuildProject(project, {
186+
event: events.RuleTargetInput.fromObject(eventInput),
187+
retryAttempts: 0,
188+
}),
189+
);
190+
191+
// THEN
192+
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
193+
ScheduleExpression: 'rate(1 hour)',
194+
State: 'ENABLED',
195+
Targets: [
196+
{
197+
Arn: {
198+
'Fn::GetAtt': [
199+
'MyProject39F7B0AE',
200+
'Arn',
201+
],
202+
},
203+
Id: 'Target0',
204+
Input: '{"buildspecOverride":"buildspecs/hourly.yml"}',
205+
RetryPolicy: {
206+
MaximumRetryAttempts: 0,
207+
},
208+
RoleArn: {
209+
'Fn::GetAtt': [
210+
'MyProjectEventsRole5B7D93F5',
211+
'Arn',
212+
],
213+
},
214+
},
215+
],
216+
});
217+
});
218+
173219
test('use a Dead Letter Queue for the rule target', () => {
174220
// GIVEN
175221
const rule = new events.Rule(stack, 'Rule', {

‎packages/@aws-cdk/aws-events-targets/test/codepipeline/pipeline.test.ts

+50
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,56 @@ describe('CodePipeline event target', () => {
157157
],
158158
});
159159
});
160+
161+
test('adds 0 retry attempts to the target configuration', () => {
162+
// WHEN
163+
rule.addTarget(new targets.CodePipeline(pipeline, {
164+
retryAttempts: 0,
165+
}));
166+
167+
// THEN
168+
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
169+
ScheduleExpression: 'rate(1 minute)',
170+
State: 'ENABLED',
171+
Targets: [
172+
{
173+
Arn: {
174+
'Fn::Join': [
175+
'',
176+
[
177+
'arn:',
178+
{
179+
Ref: 'AWS::Partition',
180+
},
181+
':codepipeline:',
182+
{
183+
Ref: 'AWS::Region',
184+
},
185+
':',
186+
{
187+
Ref: 'AWS::AccountId',
188+
},
189+
':',
190+
{
191+
Ref: 'PipelineC660917D',
192+
},
193+
],
194+
],
195+
},
196+
Id: 'Target0',
197+
RetryPolicy: {
198+
MaximumRetryAttempts: 0,
199+
},
200+
RoleArn: {
201+
'Fn::GetAtt': [
202+
'PipelineEventsRole46BEEA7C',
203+
'Arn',
204+
],
205+
},
206+
},
207+
],
208+
});
209+
});
160210
});
161211

162212
describe('with an explicit event role', () => {

‎packages/@aws-cdk/aws-events-targets/test/lambda/events.integ.snapshot/lambda-events.template.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@
148148
"Id": "Target0",
149149
"RetryPolicy": {
150150
"MaximumEventAgeInSeconds": 7200,
151-
"MaximumRetryAttempts": 2
151+
"MaximumRetryAttempts": 0
152152
}
153153
}
154154
]

‎packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const queue = new sqs.Queue(stack, 'Queue');
3434
timer3.addTarget(new targets.LambdaFunction(fn, {
3535
deadLetterQueue: queue,
3636
maxEventAge: cdk.Duration.hours(2),
37-
retryAttempts: 2,
37+
retryAttempts: 0,
3838
}));
3939

4040
app.synth();

‎packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts

+43
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,49 @@ test('specifying retry policy', () => {
376376
});
377377
});
378378

379+
test('specifying retry policy with 0 retryAttempts', () => {
380+
// GIVEN
381+
const app = new cdk.App();
382+
const stack = new cdk.Stack(app, 'Stack');
383+
384+
const fn = new lambda.Function(stack, 'MyLambda', {
385+
code: new lambda.InlineCode('foo'),
386+
handler: 'bar',
387+
runtime: lambda.Runtime.PYTHON_3_9,
388+
});
389+
390+
// WHEN
391+
new events.Rule(stack, 'Rule', {
392+
schedule: events.Schedule.rate(cdk.Duration.minutes(1)),
393+
targets: [new targets.LambdaFunction(fn, {
394+
retryAttempts: 0,
395+
})],
396+
});
397+
398+
// THEN
399+
expect(() => app.synth()).not.toThrow();
400+
401+
// the Permission resource should be in the event stack
402+
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
403+
ScheduleExpression: 'rate(1 minute)',
404+
State: 'ENABLED',
405+
Targets: [
406+
{
407+
Arn: {
408+
'Fn::GetAtt': [
409+
'MyLambdaCCE802FB',
410+
'Arn',
411+
],
412+
},
413+
Id: 'Target0',
414+
RetryPolicy: {
415+
MaximumRetryAttempts: 0,
416+
},
417+
},
418+
],
419+
});
420+
});
421+
379422
function newTestLambda(scope: constructs.Construct, suffix = '') {
380423
return new lambda.Function(scope, `MyLambda${suffix}`, {
381424
code: new lambda.InlineCode('foo'),

‎packages/@aws-cdk/aws-events-targets/test/logs/log-group.test.ts

+64
Original file line numberDiff line numberDiff line change
@@ -295,3 +295,67 @@ testDeprecated('specifying retry policy and dead letter queue', () => {
295295
],
296296
});
297297
});
298+
299+
testDeprecated('specifying retry policy with 0 retryAttempts', () => {
300+
// GIVEN
301+
const stack = new cdk.Stack();
302+
const logGroup = new logs.LogGroup(stack, 'MyLogGroup', {
303+
logGroupName: '/aws/events/MyLogGroup',
304+
});
305+
const rule1 = new events.Rule(stack, 'Rule', {
306+
schedule: events.Schedule.rate(cdk.Duration.minutes(1)),
307+
});
308+
309+
// WHEN
310+
rule1.addTarget(new targets.CloudWatchLogGroup(logGroup, {
311+
event: events.RuleTargetInput.fromObject({
312+
timestamp: events.EventField.time,
313+
message: events.EventField.fromPath('$'),
314+
}),
315+
retryAttempts: 0,
316+
}));
317+
318+
// THEN
319+
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
320+
ScheduleExpression: 'rate(1 minute)',
321+
State: 'ENABLED',
322+
Targets: [
323+
{
324+
Arn: {
325+
'Fn::Join': [
326+
'',
327+
[
328+
'arn:',
329+
{
330+
Ref: 'AWS::Partition',
331+
},
332+
':logs:',
333+
{
334+
Ref: 'AWS::Region',
335+
},
336+
':',
337+
{
338+
Ref: 'AWS::AccountId',
339+
},
340+
':log-group:',
341+
{
342+
Ref: 'MyLogGroup5C0DAD85',
343+
},
344+
],
345+
],
346+
},
347+
Id: 'Target0',
348+
InputTransformer: {
349+
InputPathsMap: {
350+
time: '$.time',
351+
f2: '$',
352+
},
353+
InputTemplate: '{"timestamp":<time>,"message":<f2>}',
354+
},
355+
RetryPolicy: {
356+
MaximumRetryAttempts: 0,
357+
},
358+
},
359+
],
360+
});
361+
});

‎packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts

+32
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,35 @@ test('specifying retry policy', () => {
252252
],
253253
});
254254
});
255+
256+
test('specifying retry policy with 0 retryAttempts', () => {
257+
const stack = new Stack();
258+
const queue = new sqs.Queue(stack, 'MyQueue', { fifo: true });
259+
const rule = new events.Rule(stack, 'MyRule', {
260+
schedule: events.Schedule.rate(Duration.hours(1)),
261+
});
262+
263+
// WHEN
264+
rule.addTarget(new targets.SqsQueue(queue, {
265+
retryAttempts: 0,
266+
}));
267+
268+
Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
269+
ScheduleExpression: 'rate(1 hour)',
270+
State: 'ENABLED',
271+
Targets: [
272+
{
273+
Arn: {
274+
'Fn::GetAtt': [
275+
'MyQueueE6CA6235',
276+
'Arn',
277+
],
278+
},
279+
Id: 'Target0',
280+
RetryPolicy: {
281+
MaximumRetryAttempts: 0,
282+
},
283+
},
284+
],
285+
});
286+
});

0 commit comments

Comments
 (0)