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.41k stars 3.8k forks source link

aws-dynamodb: addToResourcePolicy has no effect #30793

Open greg5123334 opened 2 weeks ago

greg5123334 commented 2 weeks ago

Describe the bug

DynamoDB's TableV2 addToResourcePolicy method is not taking effect.

Expected Behavior

statements should be added to existing policy document. and in the absence of an existing policy document, one should be created on first call of addToResourcePolicy as documented:

A resource policy will be automatically created upon the first call to addToResourcePolicy.

Current Behavior

addToResourcePolicy has no effect on changesets.

Reproduction Steps

1. Initial deploy WITHOUT policy nor statement

    // table with resource policy
    const table = new dynamodb.TableV2(this, "TableTestV2-1", {
      partitionKey: {
        name: "id",
        type: dynamodb.AttributeType.STRING,
      },
      removalPolicy: RemovalPolicy.DESTROY,
      deletionProtection: false,
    });

2. Include addToResourcePolicy WITHOUT policy

Diff

    const statement_one = new iam.PolicyStatement({
      actions: ["dynamodb:GetItem"],
      principals: [new iam.AccountRootPrincipal()],
      resources: ["*"],
    });

    // table with resource policy
    const table = new dynamodb.TableV2(this, "TableTestV2-1", {
      partitionKey: {
        name: "id",
        type: dynamodb.AttributeType.STRING,
      },
      removalPolicy: RemovalPolicy.DESTROY,
      deletionProtection: false,
    });
    table.addToResourcePolicy(statement_one);

cdk diff

Stack SandboxStack
Hold on while we create a read-only change set to get a diff with accurate replacement information (use --no-change-set to use a less accurate but faster template-only diff)
There were no differences

✨  Number of stacks with differences: 0

Deploy first addToResourcePolicy

aws dynamodb get-resource-policy --resource-arn arn:aws:dynamodb:eu-west-1:000000000000:table/SandboxStack-TableTestV215EEA02B7-FUG73UWOHO6C
An error occurred (PolicyNotFoundException) when calling the GetResourcePolicy operation: Resource-based policy not found for the provided ResourceArn: arn:aws:dynamodb:eu-west-1:000000000000:table/SandboxStack-TableTestV215EEA02B7-FUG73UWOHO6C

NO POLICY ADDED!!

3. Add policy via resourcePolicy prop

    const statement_one = new iam.PolicyStatement({
      actions: ["dynamodb:GetItem"],
      principals: [new iam.AccountRootPrincipal()],
      resources: ["*"],
    });

    // resource policy document
    const policy = new iam.PolicyDocument({
      statements: [statement_one],
    });

    // table with resource policy
    const table = new dynamodb.TableV2(this, "TableTestV2-1", {
      partitionKey: {
        name: "id",
        type: dynamodb.AttributeType.STRING,
      },
      removalPolicy: RemovalPolicy.DESTROY,
      deletionProtection: false,
      resourcePolicy: policy,
    });

deploy

aws dynamodb get-resource-policy --resource-arn arn:aws:dynamodb:eu-west-1:000000000000:table/SandboxStack-TableTestV215EEA02B7-38MHFK131KH9
{
    "Policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"arn:aws:iam::000000000000:root\"},\"Action\":\"dynamodb:GetItem\",\"Resource\":\"*\"}]}",
    "RevisionId": "1720511820026"
}

4. add second statement via addToResourcePolicy method

    const statement_one = new iam.PolicyStatement({
      actions: ["dynamodb:GetItem"],
      principals: [new iam.AccountRootPrincipal()],
      resources: ["*"],
    });

    // resource policy document
    const policy = new iam.PolicyDocument({
      statements: [statement_one],
    });

    // table with resource policy
    const table = new dynamodb.TableV2(this, "TableTestV2-1", {
      partitionKey: {
        name: "id",
        type: dynamodb.AttributeType.STRING,
      },
      removalPolicy: RemovalPolicy.DESTROY,
      deletionProtection: false,
      resourcePolicy: policy,
    });

    const statement_two = new iam.PolicyStatement({
      actions: ["dynamodb:GetItem", "dynamodb:PutItem"],
      principals: [new iam.AccountRootPrincipal()],
      resources: ["*"],
    });
    table.addToResourcePolicy(statement_two);

diff

cdk diff

Stack SandboxStack
Hold on while we create a read-only change set to get a diff with accurate replacement information (use --no-change-set to use a less accurate but faster template-only diff)
There were no differences

✨  Number of stacks with differences: 0

deploy

aws dynamodb get-resource-policy --resource-arn arn:aws:dynamodb:eu-west-1:000000000000:table/SandboxStack-TableTestV215EEA02B7-38MHFK131KH9
{
    "Policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"arn:aws:iam::000000000000:root\"},\"Action\":\"dynamodb:GetItem\",\"Resource\":\"*\"}]}",
    "RevisionId": "1720511820026"
}

second statement NOT included in diff nor in deployment

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.148.0 (build e5740c0)

Framework Version

No response

Node.js Version

v20.12.2

OS

Ubuntu 22.04.4 LTS

Language

TypeScript

Language Version

5.4.5

Other information

No response

khushail commented 2 weeks ago

Hey @greg5123334 , thanks for sharing the detailed repro code. I can confirm that the mentioned method addToResourcePolicy() does not add the required policy.

https://github.com/aws/aws-cdk/blob/7cd0f65a669abf5144a705fec0652f3b550a9340/packages/aws-cdk-lib/aws-dynamodb/lib/table-v2-base.ts#L474C1-L483C2

https://github.com/aws/aws-cdk/blob/7cd0f65a669abf5144a705fec0652f3b550a9340/packages/aws-cdk-lib/aws-dynamodb/lib/table-v2-base.ts#L474

On checking this method, it looks like the method should return the object updated with policy statements but its returning assertions. Also the inherent object being created is PolicyDocument which adds statement. Here is the PR link that introduced this change. Marking it as P3 since it has a workaround.

jaimm commented 1 week ago

I'm encountering issues with using grantRead against a dynamodb table (v1) construct where no resource policy is being added. Looking at the addToResourcePolicy for table.ts, it seems the logic is identical to the table-v2-base.ts, so I believe this affects both table constructs.