(aws-dynamodb): Support customer managed keys for global tables

[weimizhu]

We are seeing an exception thrown when we build with CDK while using DynamoDB Global tables with a customer managed CMK.

Reproduction Steps

In app.ts of CDK package:

import { Table, AttributeType, TableEncryption } from 'monocdk/aws-dynamodb';
import { Key } from 'monocdk/aws-kms';

const kms_key = new Key(this, `risk-rank-key`);

tableProps = {
      tableName: 'RiskRankTable',
      partitionKey: { name: 'customerId', type: AttributeType.STRING },
      sortKey: { name: 'uid', type: AttributeType.STRING },
      encryption: TableEncryption.CUSTOMER_MANAGED,
      encryptionKey: kms_key
      replicationRegions: ['us-west-2'], // add more regions here
      pointInTimeRecovery: true,
}

const table = new Table(this, 'risk-rank-table', {
      ...tableProps,
});

What did you expect to happen?

We expected that the CDK build would succeed and synthesize the stacks to be deployed.

What actually happened?

An exception is thrown during the build that states Error: TableEncryption.CUSTOMER_MANAGED is not supported by DynamoDB Global Tables (where replicationRegions was set)

Environment

• CDK CLI Version : 1.115.0
• Framework Version: Not sure
• Node.js Version: 12.x
• OS : Amazon Linux 2 x86_64
• Language (Version): Typescript (4.2.3)

Other

Our team is trying to create a global DynamoDB table that uses a customer managed CMK for server side encryption. We are using the replicationRegions and encryption: TableEncryption.CUSTOMER_MANAGED options that are provided in the Table construct. When we run a build with the CDK, there is an error that states Error: TableEncryption.CUSTOMER_MANAGED is not supported by DynamoDB Global Tables (where replicationRegions was set), which seems to be caused by this code here: https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-dynamodb/lib/table.ts#L1620

Previously reached out to Open Source Developer Support and spoke with @peterwoodworth .
He clarified that DynamoDB supports customer managed CMKs and can also be deployed through Cloudformation, and also to open a bug and see if this is something we want to do.

---

This is 🐛 Bug Report

[peterwoodworth]

I've confirmed with @madeline-k that this is something we'd like to do. Would you like to submit a PR which removes the error?

[weimizhu]

Thank you for looking into this. I can definitely submit a PR to fix this issue, but I have one quick question.

Based on this article and my own testing, it looks like you need to provide an in-region KMS key for each region where a replica is created. Will the change proposed handle that scenario?

[peterwoodworth]

Thanks for bringing this up, I hadn't caught that. Can you elaborate on what you've done to test this?

If you're correct, then I suspect more changes will be necessary. I'll research this more tomorrow, and label it as a feature request assuming you're correct

[weimizhu]

Absolutely. The testing I did was through the AWS console, where I went to my deployed DynamoDB table (which was initially deployed with the TableEncryption.AWS_MANAGED setting) and manually changed the Encryption Type. If you select Customer Managed CMK, it notifies you that you have to select a key for each Global table replica, as shown in the attached screen shots. Based on this, I would assume that Cloudformation will most likely have the same requirement when configuring the KMS key for the global replicas.

[peterwoodworth]

It does look like this will take more effort than initially required. @madeline-k is the expert here and the owner of the construct, I think this will best be left to her 😄

[madeline-k]

From the launch announcement:

When you choose to use a customer managed customer master key (CMK) in KMS to protect your data in DynamoDB global tables, each regional replica of your global table requires an in-region customer managed key.

And from @weimizhu's testing with the console, it is obvious we will need a mechanism for specifying multiple customer-managed keys. So I agree with y'all that just removing the validation won't cut it here.

Digging a little deeper, the replicationRegions on the Table construct uses a custom resource to create the replica tables. I think this is because the AWS::DynamoDb::GlobalTable cloudformation resource was not released until May 2021.

But, now that there is a GlobalTable cfn resource, you can use the CfnGlobalTable construct to get customer managed keys for global tables! See

If you plan to use customer-managed KMS keys, you must provide a key for each replica using the ReplicaSpecification.ReplicaSSESpecification property.

in the docs here and see here for specifying the keys in the replica specification.

[madeline-k]

TLDR: Use CfnGlobalTable if you want to be able to use customer managed keys on global tables.

[madeline-k]

To add this feature to the L2 DynamoDB construct library, I see three options:

1. Use AWS::DynamoDb::GlobalTable instead of AWS::DynamoDb::Table as the underlying resource for Table.
   a. We can't actually do this, because that would be a breaking change for users with existing tables, and could cause table deletion.
2. Update the current Table implementation to specify a key for each replica table. The code here will need modification. I think the UpdateTable API should allow us to add a replica with it's own key:https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_CreateReplicationGroupMemberAction.html#DDB-Type-CreateReplicationGroupMemberAction-KMSMasterKeyId.
   a. This option will also require some changes to TableProps so that the user can specify a list of encryption keys. For this I would suggest adding a encryptedReplicationRegions property which would be an array of { string: region; key: kms.IKey }. However, this is kindof clunky.
3. Create a new construct GlobalTable and use the AWS::DynamoDb::GlobalTable cfn resource for that. And, deprecate the replicationRegions property in Table in favor of this new construct.

I'd recommend option 3 here. @weimizhu If you are able to take a stab at this and open a PR, please do! Also, as an FYI I am the temporary owner for the dynamodb module while @skinny85 is on vacation. For a new feature implementation like this, he will need to review the PR as well.

[weimizhu]

Thank you for looking into this matter in detail. I will definitely take a shot at this, and the details you have mentioned will be very helpful! Option 3 also sounds like the best method from my POV as well, so I will proceed with that approach.

[madeline-k]

Link: #16118

[skinny85]

To add this feature to the L2 DynamoDB construct library, I see three options:

1. Use AWS::DynamoDb::GlobalTable instead of AWS::DynamoDb::Table as the underlying resource for Table.
   a. We can't actually do this, because that would be a breaking change for users with existing tables, and could cause table deletion.
2. Update the current Table implementation to specify a key for each replica table. The code here will need modification. I think the UpdateTable API should allow us to add a replica with it's own key:https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_CreateReplicationGroupMemberAction.html#DDB-Type-CreateReplicationGroupMemberAction-KMSMasterKeyId.
   a. This option will also require some changes to TableProps so that the user can specify a list of encryption keys. For this I would suggest adding a encryptedReplicationRegions property which would be an array of { string: region; key: kms.IKey }. However, this is kindof clunky.
3. Create a new construct GlobalTable and use the AWS::DynamoDb::GlobalTable cfn resource for that. And, deprecate the replicationRegions property in Table in favor of this new construct.

I'll add my take here: I think we should do 1., just behind a feature flag 🙂.

EDIT: I changed my mind 😜. I think nr 2 is the way to go here.

[DevOps-RV]

+1

[aaa-khoa]

+1, I'd also like this feature :)

[nicolasfeoli]

+1

[peterwoodworth]

Please leave your +1s on the original issue (click the 👍🏻), and refrain from posting +1 comments. Comments can clog discussion, and aren't part of our direct process for deciding what to work on (while thumbs ups on the original post are!)

[zylkav]

I built an interim solution for this with L1 cdk constructs.
Please have a look:

Typescript:
https://github.com/zylkav/aws-cdk-examples/tree/feat/global-ddb-with-cmk/typescript/ddb/global-table-with-cmk

Python:
https://github.com/zylkav/aws-cdk-examples/tree/feat/global-ddb-with-cmk/python/ddb/global-table-with-cmk

[mspinelli23]

+1, do we have an ETA on this one?

[mlaota]

I'll add my take here: I think we should do 1., just behind a feature flag 🙂.
EDIT: I changed my mind 😜. I think nr 2 is the way to go here.

Why is (2) the preferred implementation?

[skinny85]

I'll add my take here: I think we should do 1., just behind a feature flag 🙂.
EDIT: I changed my mind 😜. I think nr 2 is the way to go here.

Why is (2) the preferred implementation?

I just like it more than the alternatives.

[MattJColes]

Looking for an ETA too, I'd rather avoid having to use L1 constructs like CfnGlobalTable.

[mrgrain]

We are going to implement a new L2 for AWS::DynamoDb::GlobalTable based on this RFC which should address the issue.
https://github.com/aws/aws-cdk-rfcs/blob/master/text/0510-dynamodb-global-table.md

[evgenyka]

Closing in favor of #16118, which will handle the issue.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.