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

aws-events: Cannot grant putEvents to Service Principals #22080

Open
arcrank opened this issue Sep 16, 2022 · 6 comments
Open

aws-events: Cannot grant putEvents to Service Principals #22080

arcrank opened this issue Sep 16, 2022 · 6 comments
Labels
@aws-cdk/aws-events Related to CloudWatch Events bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@arcrank
Copy link
Contributor

arcrank commented Sep 16, 2022

Describe the bug

When trying to grantPutEventsTo an AWS SP, there is a no-op, and no warnings or errors. I would expect if we added a grant to a iam.ServicePrincipal that the underlying grant/policy would be created. We can add an SP to the event bus in the console. Tracing back code I myself didn't necessary find a place where this would have failed, or I would have expected if this was not possible to give a failure message.

Expected Behavior

I would expect the template to have grant policies attached. If for some reason you weren't allowed to add SPs, I would expect a failure message and error.

Current Behavior

Nothing is logged to the terminal when synthing the template snippet is

 "Resources": {
  "bus707364D1": {
   "Type": "AWS::Events::EventBus",
   "Properties": {
    "Name": "MyCustomEventBus"
   },
   "Metadata": {
    "aws:cdk:path": "xxx/bus/Resource"
   }
  },
  "busMyArchiveF1010141": {
   "Type": "AWS::Events::Archive",
   "Properties": {
    "SourceArn": {
     "Fn::GetAtt": [
      "bus707364D1",
      "Arn"
     ]
    },
    "ArchiveName": "MyCustomEventBusArchive",
    "Description": "MyCustomerEventBus Archive",
    "EventPattern": {
     "account": [
      "264988854622"
     ]
    },
    "RetentionDays": 365
   },

Reproduction Steps

    const bus = new events.EventBus(this, 'bus', {
      eventBusName: 'MyCustomEventBus'
    });
    
    bus.archive('MyArchive', {
      archiveName: 'MyCustomEventBusArchive',
      description: 'MyCustomerEventBus Archive',
      eventPattern: {
        account: [cdk.Stack.of(this).account],
      },
      retention: cdk.Duration.days(365),
    });

    bus.grantPutEventsTo(new iam.ServicePrincipal('lambda.amazonaws.com'));

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.23.0

Framework Version

No response

Node.js Version

14

OS

MacOs/Linux

Language

Typescript

Language Version

No response

Other information

No response

@arcrank arcrank added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 16, 2022
@github-actions github-actions bot added the @aws-cdk/aws-events Related to CloudWatch Events label Sep 16, 2022
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Sep 19, 2022
@rix0rrr rix0rrr removed their assignment Sep 19, 2022
@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 19, 2022

This should have worked the way you said. Investigation is required.

@pgarbe
Copy link
Contributor

pgarbe commented Nov 17, 2022

It also does not work for a cross-account scenario. The grantPutEventsTo function wants to add the permission to the grantee. It doesn't fail but also nothing will be synthesized. In that case it would need to add an EventBusPolicy to the event bus.

Example:

const eventBus = new cdk.aws_events.EventBus(this, 'Bus');
eventBus.grantPutEventsTo(new cdk.aws_iam.AccountPrincipal('123456789012'));

Workaround (from StackOverflow):

const eventBus = new cdk.aws_events.EventBus(this, 'Bus');
new cdk.aws_events.CfnEventBusPolicy(this, 'XAccountPolicy', {
    statementId: 'AllowXAccountPushEvents',
    action: 'events:PutEvents',
    eventBusName: eventBus.eventBusName,
    principal: '123456789012,
});

Any recommendation @rix0rrr how this should be implemented? Not sure if grantPutEventsTo should create that policy as it would break the pattern. But it should at least fail.

@Isaac-Kleinman
Copy link

It seems like the fix for this would be for grantPutEvents() to use iam.Grant.addToPrincipalOrResource() instead of iam.Grant.addToPrincipal().

@hassaku63
Copy link
Contributor

hassaku63 commented Nov 21, 2022

If what you want to do is that put EventBusPolicy to your custom EventBus, it seems currently L2 construct not support this use case. Needs to use CfnEventBusPolicy (same as this comments indicates).

(Below is my idea about how this use case could be implemented..)

According to other service that has resouce based policy such as SNS and SQS, service specific (resource based) Policy class is available and service class provides "addToResourcePolicy()" method.

e.x) addToResourcePolicy method in Topic class and TopicPolicy class

All AWS resources that support resource policies have a method called addToResourcePolicy(), which will automatically create a new resource policy if one doesn't exist yet, otherwise it will add to the existing policy.

Follow the above, like SNS and SQS, I think it would be a good idea to provide an addToResourcePolicy method for EventBus.

@trondhindenes
Copy link

Looks like this is a bug for several IGrantable types. I'm trying:

        self.ec2_events_receiver_bus = aws_events.EventBus(
            self,
            "receiverBus",
            event_bus_name="org-ec2-events",

        )
        self.ec2_events_receiver_bus.grant_all_put_events(
           aws_iam.OrganizationPrincipal(organization_id="myorg")
        )

But that also results in no changes. I'll use resource policy for now, but it would be awesome if this worked as intended.

@areller
Copy link

areller commented Mar 6, 2023

It also does not work for a cross-account scenario. The grantPutEventsTo function wants to add the permission to the grantee. It doesn't fail but also nothing will be synthesized. In that case it would need to add an EventBusPolicy to the event bus.

Example:

const eventBus = new cdk.aws_events.EventBus(this, 'Bus');
eventBus.grantPutEventsTo(new cdk.aws_iam.AccountPrincipal('123456789012'));

Workaround (from StackOverflow):

const eventBus = new cdk.aws_events.EventBus(this, 'Bus');
new cdk.aws_events.CfnEventBusPolicy(this, 'XAccountPolicy', {
    statementId: 'AllowXAccountPushEvents',
    action: 'events:PutEvents',
    eventBusName: eventBus.eventBusName,
    principal: '123456789012,
});

Any recommendation @rix0rrr how this should be implemented? Not sure if grantPutEventsTo should create that policy as it would break the pattern. But it should at least fail.

is there a workaround like this that works with service principals? @pgarbe

Edit:

This

const policyStatementId = `...`;
const policyStatement = new PolicyStatement({
  sid: policyStatementId,
  effect: Effect.ALLOW,
  actions: ['events:PutEvents'],
  resources: [eventsBus.eventBusArn],
  principals: [new ServicePrincipal(`...`)],
});

new CfnEventBusPolicy(this, policyStatementId, {
  statementId: policyStatementId,
  statement: policyStatement.toStatementJson(),
  eventBusName: `...`,
});

worked for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-events Related to CloudWatch Events bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

No branches or pull requests

7 participants