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

CloudTrail addS3EventSelector wrongly documented and not working #1841

Closed
MichaelHindley opened this issue Feb 22, 2019 · 2 comments · Fixed by #1854
Closed

CloudTrail addS3EventSelector wrongly documented and not working #1841

MichaelHindley opened this issue Feb 22, 2019 · 2 comments · Fixed by #1854
Labels
@aws-cdk/aws-events Related to CloudWatch Events @aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug.

Comments

@MichaelHindley
Copy link

MichaelHindley commented Feb 22, 2019

https://github.com/awslabs/aws-cdk/blob/v0.24.0/packages/@aws-cdk/aws-cloudtrail/lib/index.ts#L202 is wrongly documented on https://awslabs.github.io/aws-cdk/refs/_aws-cdk_aws-cloudtrail.html?highlight=adds3eventselector, they have different signatures.

The second argument is an Enum, not an Object.

The first argument is not able to work with s3 buckets.

Trying to add an array of strings, for example:

trail.addS3EventSelector([bucketArn], cloudtrail.ReadWriteType.WriteOnly)

results in the error:

Value bucketArn for DataResources.Values is invalid. (Service: AWSCloudTrail; Status Code: 400; Error Code: InvalidEventSelectorsException;

If instead of bucketArn you supply arn:aws:s3::: it works, but that makes it so that you can only have a trail for all s3 buckets or for none of them.

If I go to the AWS Console and try to add the bucket manually, it works and seems to be the equivalent of checking the "Add all buckets in your account" checkbox.

This case is also not handled in the test https://github.com/awslabs/aws-cdk/blob/d7371f0f836ce8b0e08df1673672f904bde1cfe1/packages/%40aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts.

If I can do anything else to help I am able to but I dont even know where to start debugging this internally!

@RomainMuller RomainMuller added bug This issue is a bug. @aws-cdk/aws-s3 Related to Amazon S3 @aws-cdk/aws-events Related to CloudWatch Events labels Feb 25, 2019
@RomainMuller
Copy link
Contributor

Hey @MichaelHindley - the issue (I mean beyond the fact the documentation and code disagree on the signature) is you cannot pass bucketArn as a prefix... You have to pass an object ARN prefix...

You'd get the correct behavior (I believe) by writing:

trail.addS3EvenetSelector([bucket.arnForObjects('')]);

It is impossible for the CDK to determine whether the value you pass in a prefix is valid or not, particularly in the case the value is not available at synthesis time (as in the example above - assuming bucket is defined in the CDK app - the object ARNs are the result of Fn::Joins).

RomainMuller added a commit that referenced this issue Feb 25, 2019
The documentation suggested one could pass an object as the second
argument to `CloudTrail.addS3EventSelector`, however the real argument
was `ReadWriteType`.

Additionally the documentation suggested incorrect uses of the API that
would lead to invalid templates being generated.

BREAKING CHANGE: The `CloudTrail.addS3EventSelector` accepts an options
object instead of only a `ReadWriteType` value.

Fixes #1841
RomainMuller added a commit that referenced this issue Feb 28, 2019
The documentation suggested one could pass an object as the second
argument to `CloudTrail.addS3EventSelector`, however the real argument
was `ReadWriteType`.

Additionally the documentation suggested incorrect uses of the API that
would lead to invalid templates being generated.

BREAKING CHANGE: The `CloudTrail.addS3EventSelector` accepts an options
object instead of only a `ReadWriteType` value.

Fixes #1841
@MichaelHindley
Copy link
Author

Thank you very much!

bucket.arnForObjects('') did indeed result in the correct behaviour in this case :)

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 @aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants