aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

(cloudtrail): Setting managementEvents to ReadWriteType.NONE always leads to build failure #14076

Closed mariusingjer closed 2 years ago

mariusingjer commented 3 years ago

This works:

const s3ShimTrail = new cloudtrail.Trail(sourceStack, 'trail-for-bucket-to-event-bridge', {
});
s3ShimTrail.addS3EventSelector([{
  bucket: thebucket,
}], {
  includeManagementEvents: false
});

This does not:

const s3ShimTrail = new cloudtrail.Trail(sourceStack, 'trail-for-bucket-to-event-bridge', {
  managementEvents: cloudtrail.ReadWriteType.NONE,
});
s3ShimTrail.addS3EventSelector([{
  bucket: thebucket,  
}]);

I don't know if I am reading it wrong, but it surprised me and caused some debugging

Use Case

Ease of use, avoid debugging errors (CF will throw up with 400 in the second example)

Proposed Solution

Throw an error if you try using opt 2 or at least document the difference, I don't understand what the differences between the two configurations are

peterwoodworth commented 3 years ago

Thank you for submitting this issue @mariusingjer, and sorry for the long wait here.

This issue seems like a bug to me.

Below is what the CDK does when you set the managementEvents prop. All it's doing if you set the managementType prop to NONE is creating a new EventSelector for your trail which only sets the includeManagementEvents prop to false

https://github.com/aws/aws-cdk/blob/556ca93e2f66ea44b1d6f1649a11d5cc5168b3ef/packages/%40aws-cdk/aws-cloudtrail/lib/cloudtrail.ts#L255-L268

CloudFormation requires that if your EventSelector has this prop set to false the DataResource prop must be filled in, but the CDK doesn't set this property (understandably) which leads to a build failure. I don't think I can see a way where you can set managementEvents to NONE and successfully deploy.

According to the EventSelector docs I linked up above:

By default, trails created without specific event selectors will be configured to log all read and write management events, and no data events

However neither CDK nor CFN docs clearly state how to disable management events for your entire Trail, but I've found through some testing that you have to set includeManagementEvents to false on every Event Selector you add to your trail.

I agree with you here that the docs are pretty confusing, but that might be due to the code not functioning as intended. I think the issue would be fixed if when you set managementEvents to NONE that it automatically sets includeManagementEvents to false for every Event Selector added.

github-actions[bot] commented 2 years ago

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.