fix(events-targets): cannot set retry policy to 0 retry attempts (aws…

…#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 [aws#21864 ](aws#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*
hacker65536 · Sep 30, 2022 · fb171c8 · fb171c8
1 parent 4445ddd
commit fb171c8
Show file tree

Hide file tree

Showing 10 changed files with 336 additions and 3 deletions.
diff --git a/packages/@aws-cdk/aws-events-targets/lib/util.ts b/packages/@aws-cdk/aws-events-targets/lib/util.ts
@@ -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 }),

diff --git a/packages/@aws-cdk/aws-events-targets/test/batch/batch.test.ts b/packages/@aws-cdk/aws-events-targets/test/batch/batch.test.ts
@@ -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',
+            ],
+          },
+        },
+      ],
+    });
+  });
 });
diff --git a/packages/@aws-cdk/aws-events-targets/test/codebuild/codebuild.test.ts b/packages/@aws-cdk/aws-events-targets/test/codebuild/codebuild.test.ts
@@ -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', {

diff --git a/packages/@aws-cdk/aws-events-targets/test/codepipeline/pipeline.test.ts b/packages/@aws-cdk/aws-events-targets/test/codepipeline/pipeline.test.ts
@@ -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', () => {

diff --git a/...@aws-cdk/aws-events-targets/test/lambda/events.integ.snapshot/lambda-events.template.json b/...@aws-cdk/aws-events-targets/test/lambda/events.integ.snapshot/lambda-events.template.json
@@ -148,7 +148,7 @@
       "Id": "Target0",
       "RetryPolicy": {
        "MaximumEventAgeInSeconds": 7200,
-       "MaximumRetryAttempts": 2
+       "MaximumRetryAttempts": 0
       }
      }
     ]

diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.ts
@@ -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();

diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts
@@ -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'),

diff --git a/packages/@aws-cdk/aws-events-targets/test/logs/log-group.test.ts b/packages/@aws-cdk/aws-events-targets/test/logs/log-group.test.ts
@@ -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,
+        },
+      },
+    ],
+  });
+});
diff --git a/packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts b/packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts
@@ -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,
+        },
+      },
+    ],
+  });
+});