aws
(aws_cloudtrail >> Trail): (Invalid request provided: Incorrect S3 bucket policy is detected for bucket)
Describe the bug
Unable to create Organization Trail in management account when using the i.e as per the doc : ++++++++++++++++++++++++++++++++++++++++ new cloudtrail.Trail(this, 'OrganizationTrail', { isOrganizationTrail: true, orgId: "o-xxxxxxxxx", }); ++++++++++++++++++++++++++++++++++++++++ This fail with the "Invalid request provided: Incorrect S3 bucket policy is detected for bucket" every-time.
Only when we add the trailName property explicitly to some string name it is then we are somehow able to mitigate the "Invalid request provided: Incorrect S3 bucket policy is detected for bucket" ++++++++++++++++++++++++++++++++++++++++ new cloudtrail.Trail(this, 'OrganizationTrail', { isOrganizationTrail: true, orgId: "o-xxxxxxxxx", trailName: "trailname123" }); ++++++++++++++++++++++++++++++++++++++++
Regression Issue
- [ ] Select this option if this issue appears to be a regression.
Last Known Working CDK Version
2.157.0
Expected Behavior
To work without explicitly passing the trailName: property .
Current Behavior
CreateTrial API fails with the "Invalid request provided: Incorrect S3 bucket policy is detected for bucket" when using the following code to create Organization Trail: ++++++++++++++++++++++++++++++++++++++++ new cloudtrail.Trail(this, 'OrganizationTrail', { isOrganizationTrail: true, orgId: "o-xxxxxxxxx", }); ++++++++++++++++++++++++++++++++++++++++
Reproduction Steps
Use : ++++++++++++++++++++++++++++++++++++++++ new cloudtrail.Trail(this, 'OrganizationTrail', { isOrganizationTrail: true, orgId: "o-xxxxxxxxx", }); ++++++++++++++++++++++++++++++++++++++++
Possible Solution
To add trailName property explicitly
Additional Information/Context
NA
CDK CLI Version
2.157.0
Framework Version
No response
Node.js Version
v16.14.2
OS
MAC
Language
TypeScript
Language Version
No response
Other information
No response
Hi @ayrawat17 , thanks for reaching out.
The issue is reproducible. Sharing the observation -
const cldtrail = new cloudtrail.Trail(this, 'cloudtrail001', {
trailName: 'cloudtrail001',
isOrganizationTrail: true,
});
this is the snippet of synthesized bucket policy PutObject created -
{
"Action": "s3:PutObject",
"Condition": {
"StringEquals": {
"s3:x-amz-acl": "bucket-owner-full-control",
"aws:SourceArn": {
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":cloudtrail:",
{
"Ref": "AWS::Region"
},
":",
{
"Ref": "AWS::AccountId"
},
":trail/undefined"
]
]
}
}
},
where ":trail/undefined" is worth concerning.
Ideally it should not have been undefined and expected to be like this. So when you add trailName , it gets updated to -
"Action": "s3:PutObject",
"Condition": {
"StringEquals": {
"s3:x-amz-acl": "bucket-owner-full-control",
"aws:SourceArn": {
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":cloudtrail:",
{
"Ref": "AWS::Region"
},
":",
{
"Ref": "AWS::AccountId"
},
":trail/cloudtrail001"
]
]
Now according to CDK, this trailName should be generated by AWS Cloudformation but is left blank and unchecked
https://github.com/aws/aws-cdk/blob/95c49abdfa4ad77a0c0fcb82a230778dcc2ea59a/packages/aws-cdk-lib/aws-cloudtrail/lib/cloudtrail.ts#L100C1-L105C31
/**
* The name of the trail. We recommend customers do not set an explicit name.
*
* @default - AWS CloudFormation generated name.
*/
readonly trailName?: string;
Referring further, it looks like the trailName is passed onto Bucket Policy which seems to be leading to error when left blank -
https://github.com/aws/aws-cdk/blob/95c49abdfa4ad77a0c0fcb82a230778dcc2ea59a/packages/aws-cdk-lib/aws-cloudtrail/lib/cloudtrail.ts#L278C1-L287C6
https://github.com/aws/aws-cdk/blob/95c49abdfa4ad77a0c0fcb82a230778dcc2ea59a/packages/aws-cdk-lib/aws-cloudtrail/lib/cloudtrail.ts#L283
So yes, this is pretty much an issue when Trailname is left blank.
Thanks for reporting this. I am marking this as P3 as it has a workaround. Marking it as such means it won't be immediately addressed by the team but its on team's radar. Anyone from the community or team is welcome to work on it.
It seems that simply using CfnTrail.attrArn would suffice, but since the CfnTrail itself already has a dependency on the S3 Bucket Policy, it results in a circular reference.
const trail = new CfnTrail(this, 'Resource', {...});
if (props.isOrganizationTrail) {
this.s3bucket.addToResourcePolicy(new iam.PolicyStatement({
resources: [this.s3bucket.arnForObjects(
`AWSLogs/${props.orgId}/*`,
)],
actions: ['s3:PutObject'],
principals: [cloudTrailPrincipal],
conditions: {
StringEquals: {
's3:x-amz-acl': 'bucket-owner-full-control',
'aws:SourceArn': trail.attrArn, // updated
},
},
}));
}
// This dependency has already existed.
if (this.s3bucket.policy) {
trail.node.addDependency(this.s3bucket.policy);
}
Result:
Template is undeployable, these resources have a dependency cycle: OrganizationTrailS3Policy140E0681 -> OrganizationTrail7F291DBA -> OrganizationTrailS3Policy140E0681:
{
"OrganizationTrailS3Policy140E0681": {
"Type": "AWS::S3::BucketPolicy",
"Properties": {
"Bucket": {
"Ref": "OrganizationTrailS39966E64E"
},
"PolicyDocument": {
"Statement": [
...
{
"Action": "s3:PutObject",
"Condition": {
"StringEquals": {
"s3:x-amz-acl": "bucket-owner-full-control",
"aws:SourceArn": {
"Fn::GetAtt": [
"OrganizationTrail7F291DBA",
"Arn"
]
}
}
},
"Effect": "Allow",
"Principal": {
"Service": "cloudtrail.amazonaws.com"
},
"Resource": {
"Fn::Join": [
"",
[
{
"Fn::GetAtt": [
"OrganizationTrailS39966E64E",
"Arn"
]
},
"/AWSLogs/undefined/*"
]
]
}
}
],
"Version": "2012-10-17"
}
}
},
"OrganizationTrail7F291DBA": {
"Type": "AWS::CloudTrail::Trail",
"Properties": {
"EnableLogFileValidation": true,
"EventSelectors": [],
"IncludeGlobalServiceEvents": true,
"IsLogging": true,
"IsMultiRegionTrail": true,
"IsOrganizationTrail": true,
"S3BucketName": {
"Ref": "OrganizationTrailS39966E64E"
}
},
"DependsOn": [
"OrganizationTrailS3Policy140E0681"
]
}
}
Therefore, it seems challenging to create a correct Source Arn that includes the TrailName when props.trailName is undefined.
Same problem here, failing to create the Org Cloudtrail. Any news on the fix?
