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

DynamoDB Global Table's grant*() gives permission only to regional resources #7362

Closed
SachinShekhar opened this issue Apr 15, 2020 · 15 comments · Fixed by #7453 or bifravst/cloudformation-cleaner#13
Assignees
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB bug This issue is a bug. in-progress This issue is being actively worked on. p1

Comments

@SachinShekhar
Copy link
Contributor

new Table(...).grant(grantee, ...actions), new Table(...).grantFullAccess(grantee), new Table(...).grantReadData(grantee) and new Table(...).grantReadWriteData(grantee) grant access to the table and its indexes only in the region in which the table is created, even when replicationRegions is not undefined.

Reproduction Steps

  1. Create a global table in a stack with us-east-1 region:
table1 = new dynamodb.Table(this, 'Table1', {
...
...
tableName: 'Table1', // You need to share this table across environments
replicationRegions: ['us-west-1']
});
  1. Share the above table to a different stack with us-west-1 region.

  2. In the us-west-1 stack, create a lambda function and grant it the read permission of the global table.

myLambda = new lambda.Function(...);

table1.grantWriteAccess(myLambda);

Unfortunately, the myLambda won't be able to read from the same region (us-west-1) table1 replica because of wrong iam permissions. If you look at the service role for myLambda, you'll find this policy statement:

{
            "Action": [
                "dynamodb:Query",
               ...
               ...
            ],
            "Resource": [
                "arn:aws:dynamodb:us-east-1:<account no.>:table/Table1",
               "arn:aws:dynamodb:us-east-1:<account no.>:table/Table1/index/*"
            ],
            "Effect": "Allow"
        }

After finding replicationRegions, CDK should either replace us-east-1 with * or add more regional arns to Resource.

I also can't modify the table1 arn to create my own iam policy statement.

table1.tableArn.replace('us-east-1', '*');

This doesn't work because tableArn is not a string at runtime even if I give physical name to the table.

I had to manually edit policy statements in production which has created a drift in CloudFormation Stack.

Error Log

No error log. Human introspection is needed.

Environment

  • CLI Version : 1.32.0
  • Framework Version: 1.32.0
  • OS : Windows 10 Pro
  • Language : Typescript

This is 🐛 Bug Report

@SachinShekhar SachinShekhar added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 15, 2020
@jogold
Copy link
Contributor

jogold commented Apr 15, 2020

This is tricky... and I'm not sure that the normal grantXxx() should grant in all regions, maybe another method for this grantRegionsXxx()? @eladb @RomainMuller opinion on this?

In the meantime you should be able to do something like this I think:

// stack in us-west-1
// tableInUsEast1 is the table exposed in the stack in us-east-1
const table1 = dynamodb.Table.fromTableName(this, 'Table1', tableInUsEast1.tableName);
table1.grantReadData(myLambda)

@jogold
Copy link
Contributor

jogold commented Apr 15, 2020

  • Share the above table to a different stack with us-west-1 region.

How are you doing this? Values cannot be shared across regions

@SachinShekhar
Copy link
Contributor Author

SachinShekhar commented Apr 15, 2020

@jogold

How are you doing this? Values cannot be shared across regions

If table1 variable is public, you can read it outside stack and then push it into another stack through props.

stack1 = new Stack1(...);
new Stack2(this, 'Stack2', {
...
...
table: stack1.table1
});

table property needs to be present in the props of Stack2. I transfer lots of things from one stack to another this way which works. e.g. putting table1.tableName into a lambda env variable.

@jogold
Copy link
Contributor

jogold commented Apr 15, 2020

table property needs to be present in the props of Stack2. I transfer lots of things from one stack to another this way.

Yes, of course, but not across regions

@SachinShekhar
Copy link
Contributor Author

SachinShekhar commented Apr 15, 2020

@jogold

Yes, of course, but not across regions

It works for me. In one of my apps, I have one GlobalStack with env region us-east-1 and I loop over a region array to create multiple RegionalStack with env region set to current array value. And, I share global resources from GlobalStack to RegionalStacks.

It works.

@SachinShekhar
Copy link
Contributor Author

@jogold

I'm not sure that the normal grantXxx() should grant in all regions, maybe another method for this grantRegionsXxx()?

How about introducing a new optional parameter for grantXxx() which can take regions array and/or regionIntersection boolean flag (if set to true, current stack region if it is present in replicationRegions array)?

@jogold
Copy link
Contributor

jogold commented Apr 15, 2020

@jogold

Yes, of course, but not across regions

It works for me. In one of my apps, I have one GlobalStack with env region us-east-1 and I loop over a region array to create multiple RegionalStack with env region set to current array value. And, I share global resources from GlobalStack to RegionalStacks.

It works.

This is only possible for concrete values, not for deploy time values (tokens, like a dynamodb table name).

@SachinShekhar
Copy link
Contributor Author

SachinShekhar commented Apr 15, 2020

@jogold

This is only possible for concrete values, not for deploy time values (tokens, like a dynamodb table name).

Yes, that's why, I provided tableName in the original example. And, sharing this is not pointless because I create tableName programmatically using stage and other variables present in my StackSet and AppSet constructs.

@jogold
Copy link
Contributor

jogold commented Apr 15, 2020

So if you're sharing a concrete string across regions, the solution is to use dynamodb.Table.fromTableName in the regional stack and then the grantXxx() will work.

@skinny85
Copy link
Contributor

A little besides the issue, but cross-region/account tokens are definitely possible :). Through something called physical names (set here in DynamoDB, and later used here).

@jogold
Copy link
Contributor

jogold commented Apr 15, 2020

A little besides the issue, but cross-region/account tokens are definitely possible :). Through something called physical names (set here in DynamoDB, and later used here).

Thanks for the clarification @skinny85. You confirm that this only work for resources with concrete (well defined) names not for deploy time given names?

@skinny85
Copy link
Contributor

A little besides the issue, but cross-region/account tokens are definitely possible :). Through something called physical names (set here in DynamoDB, and later used here).

Thanks for the clarification @skinny85. You confirm that this only work for resources with concrete (well defined) names not for deploy time given names?

Yes, I can confirm that. If a resource doesn't have a physical name defined (for DynamoDB, that's the tableName property), doing a cross-account/region reference will result in a (synthesis-time) error.

@SachinShekhar
Copy link
Contributor Author

SachinShekhar commented Apr 15, 2020

There's also core.PhysicalName.GENERATE_IF_NEEDED (it's designed especially for cross-region/account resource sharing purpose), but it requires stack's env variable to be completely populated (see this: #7297). I want to use it, but never tried it because of this limitation.

@SachinShekhar
Copy link
Contributor Author

@jogold

So if you're sharing a concrete string across regions, the solution is to use dynamodb.Table.fromTableName in the regional stack and then the grantXxx() will work.

In theory, this should work. But, the imported table object doesn't seem fully equivalent of original table. When I use importedTable.grantReadAccess(), it's not granting read access to indexes of the table as before.

Yes, with imported table, I can extract arn to manually create iam policy.

@jogold
Copy link
Contributor

jogold commented Apr 15, 2020

it's not granting read access to indexes of the table as before.

Good point.

@SomayaB SomayaB added the @aws-cdk/aws-dynamodb Related to Amazon DynamoDB label Apr 16, 2020
@RomainMuller RomainMuller added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 20, 2020
jogold added a commit to jogold/aws-cdk that referenced this issue Apr 20, 2020
Add all replication regions when granting permissions.

Closes aws#7362
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Apr 20, 2020
@mergify mergify bot closed this as completed in #7453 Apr 23, 2020
mergify bot pushed a commit that referenced this issue Apr 23, 2020
Add all replication regions when granting permissions.

Closes #7362
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-dynamodb Related to Amazon DynamoDB bug This issue is a bug. in-progress This issue is being actively worked on. p1
Projects
None yet
5 participants