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.55k stars 3.87k forks source link

(aws_cloudtrail >> Trail): (Invalid request provided: Incorrect S3 bucket policy is detected for bucket) #31400

Open ayrawat17 opened 2 weeks ago

ayrawat17 commented 2 weeks ago

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

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

khushail commented 1 week ago

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.