fix(events-targets): EventBus IAM statements are only added for the f…

…irst target (#20479) If the `EventBus` constructor is called with no arguments, then attaching more than a single target to its policy will silently fail to add them. This is because of a strange edge case in the implementation that was not accounted for previously; it is possible for `props.role` to be `undefined`, yet `singletonEventRole()` is still capable of finding the desired role. `singletonEventRole()` does not add the new statements to any IAM policies that it finds, so as a result adding multiple targets does not add any of them. Fixes #19407. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Jun 1, 2022 · 74318c7 · 74318c7
1 parent 264c02e
commit 74318c7
Show file tree

Hide file tree

Showing 12 changed files with 138 additions and 55 deletions.
diff --git a/packages/@aws-cdk/aws-events-targets/lib/api-destination.ts b/packages/@aws-cdk/aws-events-targets/lib/api-destination.ts
@@ -83,13 +83,16 @@ export class ApiDestination implements events.IRuleTarget {
       addToDeadLetterQueueResourcePolicy(_rule, this.props.deadLetterQueue);
     }
 
+    const role = this.props?.eventRole ?? singletonEventRole(this.apiDestination);
+    role.addToPrincipalPolicy(new iam.PolicyStatement({
+      resources: [this.apiDestination.apiDestinationArn],
+      actions: ['events:InvokeApiDestination'],
+    }));
+
     return {
       ...(this.props ? bindBaseTargetConfig(this.props) : {}),
       arn: this.apiDestination.apiDestinationArn,
-      role: this.props?.eventRole ?? singletonEventRole(this.apiDestination, [new iam.PolicyStatement({
-        resources: [this.apiDestination.apiDestinationArn],
-        actions: ['events:InvokeApiDestination'],
-      })]),
+      role,
       input: this.props.event,
       targetResource: this.apiDestination,
       httpParameters,

diff --git a/packages/@aws-cdk/aws-events-targets/lib/api-gateway.ts b/packages/@aws-cdk/aws-events-targets/lib/api-gateway.ts
@@ -98,16 +98,20 @@ export class ApiGateway implements events.IRuleTarget {
       this.props?.path || '/',
       this.props?.stage || this.restApi.deploymentStage.stageName,
     );
+
+    const role = this.props?.eventRole || singletonEventRole(this.restApi);
+    role.addToPrincipalPolicy(new iam.PolicyStatement({
+      resources: [restApiArn],
+      actions: [
+        'execute-api:Invoke',
+        'execute-api:ManageConnections',
+      ],
+    }));
+
     return {
       ...(this.props ? bindBaseTargetConfig(this.props) : {}),
       arn: restApiArn,
-      role: this.props?.eventRole || singletonEventRole(this.restApi, [new iam.PolicyStatement({
-        resources: [restApiArn],
-        actions: [
-          'execute-api:Invoke',
-          'execute-api:ManageConnections',
-        ],
-      })]),
+      role,
       deadLetterConfig: this.props?.deadLetterQueue && { arn: this.props.deadLetterQueue?.queueArn },
       input: this.props?.postBody,
       targetResource: this.restApi,

diff --git a/packages/@aws-cdk/aws-events-targets/lib/batch.ts b/packages/@aws-cdk/aws-events-targets/lib/batch.ts
@@ -87,20 +87,21 @@ export class BatchJob implements events.IRuleTarget {
       addToDeadLetterQueueResourcePolicy(rule, this.props.deadLetterQueue);
     }
 
+    // When scoping resource-level access for job submission, you must provide both job queue and job definition resource types.
+    // https://docs.aws.amazon.com/batch/latest/userguide/ExamplePolicies_BATCH.html#iam-example-restrict-job-def
+    const role = singletonEventRole(this.jobDefinitionScope);
+    role.addToPrincipalPolicy(new iam.PolicyStatement({
+      actions: ['batch:SubmitJob'],
+      resources: [
+        this.jobDefinitionArn,
+        this.jobQueueArn,
+      ],
+    }));
+
     return {
       ...bindBaseTargetConfig(this.props),
       arn: this.jobQueueArn,
-      // When scoping resource-level access for job submission, you must provide both job queue and job definition resource types.
-      // https://docs.aws.amazon.com/batch/latest/userguide/ExamplePolicies_BATCH.html#iam-example-restrict-job-def
-      role: singletonEventRole(this.jobDefinitionScope, [
-        new iam.PolicyStatement({
-          actions: ['batch:SubmitJob'],
-          resources: [
-            this.jobDefinitionArn,
-            this.jobQueueArn,
-          ],
-        }),
-      ]),
+      role,
       input: this.props.event,
       targetResource: this.jobQueueScope,
       batchParameters,

diff --git a/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts b/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts
@@ -44,15 +44,16 @@ export class CodeBuildProject implements events.IRuleTarget {
       addToDeadLetterQueueResourcePolicy(_rule, this.props.deadLetterQueue);
     }
 
+    const role = this.props.eventRole || singletonEventRole(this.project);
+    role.addToPrincipalPolicy(new iam.PolicyStatement({
+      actions: ['codebuild:StartBuild'],
+      resources: [this.project.projectArn],
+    }));
+
     return {
       ...bindBaseTargetConfig(this.props),
       arn: this.project.projectArn,
-      role: this.props.eventRole || singletonEventRole(this.project, [
-        new iam.PolicyStatement({
-          actions: ['codebuild:StartBuild'],
-          resources: [this.project.projectArn],
-        }),
-      ]),
+      role,
       input: this.props.event,
       targetResource: this.project,
     };

diff --git a/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts b/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts
@@ -26,14 +26,17 @@ export class CodePipeline implements events.IRuleTarget {
   }
 
   public bind(_rule: events.IRule, _id?: string): events.RuleTargetConfig {
+    const role = this.options.eventRole || singletonEventRole(this.pipeline);
+    role.addToPrincipalPolicy(new iam.PolicyStatement({
+      resources: [this.pipeline.pipelineArn],
+      actions: ['codepipeline:StartPipelineExecution'],
+    }));
+
     return {
       ...bindBaseTargetConfig(this.options),
       id: '',
       arn: this.pipeline.pipelineArn,
-      role: this.options.eventRole || singletonEventRole(this.pipeline, [new iam.PolicyStatement({
-        resources: [this.pipeline.pipelineArn],
-        actions: ['codepipeline:StartPipelineExecution'],
-      })]),
+      role,
       targetResource: this.pipeline,
     };
   }

diff --git a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts
@@ -118,12 +118,9 @@ export class EcsTask implements events.IRuleTarget {
     this.taskCount = props.taskCount ?? 1;
     this.platformVersion = props.platformVersion;
 
-    if (props.role) {
-      const role = props.role;
-      this.createEventRolePolicyStatements().forEach(role.addToPrincipalPolicy.bind(role));
-      this.role = role;
-    } else {
-      this.role = singletonEventRole(this.taskDefinition, this.createEventRolePolicyStatements());
+    this.role = props.role ?? singletonEventRole(this.taskDefinition);
+    for (const stmt of this.createEventRolePolicyStatements()) {
+      this.role.addToPrincipalPolicy(stmt);
     }
 
     // Security groups are only configurable with the "awsvpc" network mode.

diff --git a/packages/@aws-cdk/aws-events-targets/lib/event-bus.ts b/packages/@aws-cdk/aws-events-targets/lib/event-bus.ts
@@ -36,10 +36,8 @@ export class EventBus implements events.IRuleTarget {
   constructor(private readonly eventBus: events.IEventBus, private readonly props: EventBusProps = {}) { }
 
   bind(rule: events.IRule, _id?: string): events.RuleTargetConfig {
-    if (this.props.role) {
-      this.props.role.addToPrincipalPolicy(this.putEventStatement());
-    }
-    const role = this.props.role ?? singletonEventRole(rule, [this.putEventStatement()]);
+    const role = this.props.role ?? singletonEventRole(rule);
+    role.addToPrincipalPolicy(this.putEventStatement());
 
     if (this.props.deadLetterQueue) {
       addToDeadLetterQueueResourcePolicy(rule, this.props.deadLetterQueue);

diff --git a/packages/@aws-cdk/aws-events-targets/lib/kinesis-firehose-stream.ts b/packages/@aws-cdk/aws-events-targets/lib/kinesis-firehose-stream.ts
@@ -31,14 +31,16 @@ export class KinesisFirehoseStream implements events.IRuleTarget {
    * result from a Event Bridge event.
    */
   public bind(_rule: events.IRule, _id?: string): events.RuleTargetConfig {
-    const policyStatements = [new iam.PolicyStatement({
+    const role = singletonEventRole(this.stream);
+    role.addToPrincipalPolicy(new iam.PolicyStatement({
       actions: ['firehose:PutRecord', 'firehose:PutRecordBatch'],
       resources: [this.stream.attrArn],
-    })];
+    }));
+
 
     return {
       arn: this.stream.attrArn,
-      role: singletonEventRole(this.stream, policyStatements),
+      role,
       input: this.props.message,
       targetResource: this.stream,
     };

diff --git a/packages/@aws-cdk/aws-events-targets/lib/kinesis-stream.ts b/packages/@aws-cdk/aws-events-targets/lib/kinesis-stream.ts
@@ -45,14 +45,15 @@ export class KinesisStream implements events.IRuleTarget {
    * result from a CloudWatch event.
    */
   public bind(_rule: events.IRule, _id?: string): events.RuleTargetConfig {
-    const policyStatements = [new iam.PolicyStatement({
+    const role = singletonEventRole(this.stream);
+    role.addToPrincipalPolicy(new iam.PolicyStatement({
       actions: ['kinesis:PutRecord', 'kinesis:PutRecords'],
       resources: [this.stream.streamArn],
-    })];
+    }));
 
     return {
       arn: this.stream.streamArn,
-      role: singletonEventRole(this.stream, policyStatements),
+      role,
       input: this.props.message,
       targetResource: this.stream,
       kinesisParameters: this.props.partitionKeyPath ? { partitionKeyPath: this.props.partitionKeyPath } : undefined,

diff --git a/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts b/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts
@@ -29,11 +29,8 @@ export class SfnStateMachine implements events.IRuleTarget {
   private readonly role: iam.IRole;
 
   constructor(public readonly machine: sfn.IStateMachine, private readonly props: SfnStateMachineProps = {}) {
-    if (props.role) {
-      props.role.grant(new iam.ServicePrincipal('events.amazonaws.com'));
-    }
     // no statements are passed because we are configuring permissions by using grant* helper below
-    this.role = props.role ?? singletonEventRole(machine, []);
+    this.role = props.role ?? singletonEventRole(machine);
     machine.grantStartExecution(this.role);
   }
 

diff --git a/packages/@aws-cdk/aws-events-targets/lib/util.ts b/packages/@aws-cdk/aws-events-targets/lib/util.ts
@@ -71,7 +71,7 @@ export function bindBaseTargetConfig(props: TargetBaseProps) {
  * events have the same target, they will share a role.
  * @internal
  */
-export function singletonEventRole(scope: IConstruct, policyStatements: iam.PolicyStatement[]): iam.IRole {
+export function singletonEventRole(scope: IConstruct): iam.IRole {
   const id = 'EventsRole';
   const existing = scope.node.tryFindChild(id) as iam.IRole;
   if (existing) { return existing; }
@@ -81,8 +81,6 @@ export function singletonEventRole(scope: IConstruct, policyStatements: iam.Poli
     assumedBy: new iam.ServicePrincipal('events.amazonaws.com'),
   });
 
-  policyStatements.forEach(role.addToPolicy.bind(role));
-
   return role;
 }
 

diff --git a/packages/@aws-cdk/aws-events-targets/test/event-bus/event-rule-target.test.ts b/packages/@aws-cdk/aws-events-targets/test/event-bus/event-rule-target.test.ts
@@ -167,3 +167,81 @@ test('with a Dead Letter Queue specified', () => {
     ],
   });
 });
+
+test('event buses are correctly added to the rule\'s principal policy', () => {
+  const stack = new Stack();
+  const rule = new events.Rule(stack, 'Rule', {
+    schedule: events.Schedule.expression('rate(1 min)'),
+  });
+
+  const bus1 = new events.EventBus(stack, 'bus' + 1);
+  const bus2 = new events.EventBus(stack, 'bus' + 2);
+
+  rule.addTarget(new targets.EventBus(bus1));
+  rule.addTarget(new targets.EventBus(bus2));
+
+  Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
+    Targets: [
+      {
+        Arn: {
+          'Fn::GetAtt': [
+            'bus110C385DC',
+            'Arn',
+          ],
+        },
+        Id: 'Target0',
+        RoleArn: {
+          'Fn::GetAtt': [
+            'RuleEventsRoleC51A4248',
+            'Arn',
+          ],
+        },
+      },
+      {
+        Arn: {
+          'Fn::GetAtt': [
+            'bus22D01F126',
+            'Arn',
+          ],
+        },
+        Id: 'Target1',
+        RoleArn: {
+          'Fn::GetAtt': [
+            'RuleEventsRoleC51A4248',
+            'Arn',
+          ],
+        },
+      },
+    ],
+  });
+  Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
+    PolicyDocument: {
+      Statement: [
+        {
+          Effect: 'Allow',
+          Action: 'events:PutEvents',
+          Resource: {
+            'Fn::GetAtt': [
+              'bus110C385DC',
+              'Arn',
+            ],
+          },
+        },
+        {
+          Effect: 'Allow',
+          Action: 'events:PutEvents',
+          Resource: {
+            'Fn::GetAtt': [
+              'bus22D01F126',
+              'Arn',
+            ],
+          },
+        },
+      ],
+      Version: '2012-10-17',
+    },
+    Roles: [{
+      Ref: 'RuleEventsRoleC51A4248',
+    }],
+  });
+});