feat(kms): add multiRegion property to a Key
#31125
Conversation
github-actions
bot
added
the
beginning-contributor
kawabata-mcl
changed the title
kawabata-mcl
changed the title
kawabata-mcl
changed the title
multiRegion property to a Key
kawabata-mcl
changed the title
multiRegion property to a KeymultiRegion property to a Key
aws-cdk-automation
added
the
pr/needs-community-review
| * regardless of the value of the UpdateReplacePolicy attribute. | ||
| * This prevents you from accidentally deleting a KMS key by changing an immutable property value. | ||
| * | ||
| * @default - false |
There was a problem hiding this comment.
If the default value is false, it would be good to write it like this.
| * @default - false | |
| * @default false |
| * This prevents you from accidentally deleting a KMS key by changing an immutable property value. | ||
| * | ||
| * @default - false | ||
| */ |
There was a problem hiding this comment.
There's a document that's helpful, and it would be good to put that here.
| */ | |
| * @see https://docs.aws.amazon.com/kms/latest/developerguide/multi-region-keys-overview.html | |
| */ |
| @@ -41,6 +41,15 @@ const key = new kms.Key(this, 'MyKey', { | |||
| }); | |||
| ``` | |||
|
|
|||
|
|
|||
| Creates a multi-Region primary key: | |||
There was a problem hiding this comment.
nit. (Matched elsewhere.)
| Creates a multi-Region primary key: | |
| Create a multi-Region primary key: |
|
|
||
| class TestStack extends Stack { | ||
| constructor(scope: App) { | ||
| super(scope, 'TestStack'); |
There was a problem hiding this comment.
How about making the stack name specific to this integ test? Because there are often other integ tests with stacks of the same name, which will fail if we try to run them in parallel.
There was a problem hiding this comment.
I wasn't considering about the conflict with other stacks. Thanks for telling me.
| * For a multi-Region key, set to this property to true. | ||
| * For a single-Region key, omit this property or set it to false. The default value is false. | ||
| * |
There was a problem hiding this comment.
This is obvious and it is good to remove and simplify it.
| * For a multi-Region key, set to this property to true. | |
| * For a single-Region key, omit this property or set it to false. The default value is false. | |
| * |
| * Creates a multi-Region primary key that you can replicate in other AWS Regions. | ||
| * You can't change the MultiRegion value after the KMS key is created. |
There was a problem hiding this comment.
| * Creates a multi-Region primary key that you can replicate in other AWS Regions. | |
| * You can't change the MultiRegion value after the KMS key is created. | |
| * Creates a multi-Region primary key that you can replicate in other AWS Regions. | |
| * | |
| * You can't change the `multiRegion` value after the KMS key is created. |
| * For a multi-Region key, set to this property to true. | ||
| * For a single-Region key, omit this property or set it to false. The default value is false. | ||
| * | ||
| * IMPORTANT: If you change the value of the MultiRegion property on an existing KMS key, the update request fails, |
There was a problem hiding this comment.
| * IMPORTANT: If you change the value of the MultiRegion property on an existing KMS key, the update request fails, | |
| * IMPORTANT: If you change the value of the `multiRegion` property on an existing KMS key, the update request fails, |
| @@ -646,6 +646,17 @@ test('fails if key policy has no IAM principals', () => { | |||
| expect(() => app.synth()).toThrow(/A PolicyStatement used in a resource-based policy must specify at least one IAM principal/); | |||
| }); | |||
|
|
|||
| test('creates a multi-Region primary key', () => { | |||
There was a problem hiding this comment.
It's good to keep it simple.
| test('creates a multi-Region primary key', () => { | |
| test('multi-region primary key', () => { |
aws-cdk-automation
removed
the
pr/needs-community-review
|
@kawabata-mcl CI is failing, but this is caused by the previous snapshot file. Please try deleting the snapshot directory ( |
|
@go-to-k Thanks for replying. I will run the integ test again and resubmit. |
|
@go-to-k I pulled the latest version this morning and there were lots of diffs in packages/cdk-assets.
|
|
Such things do happen from time to time, but they are irrelevant differences to this PR and can be discarded. (Also, I just made another comment, which is my mistake. I have already removed that.) |
|
@go-to-k |
aws-cdk-automation
added
the
pr/needs-maintainer-review
|
Love it, thanks @kawabata-mcl for the changes and thanks @go-to-k for the thorough reviews! |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
aws-cdk-automation
removed
the
pr/needs-maintainer-review
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
None
Reason for this change
We can create a multi-Region primary key for a KMS key from cloudformation, but this was not supported in the AWS CDK L2 construct.
Description of changes
Add multiRegion property to KeyProps and set it in the CfnKey constructor.
Description of how you validated changes
Added both unit and integration tests.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license