From 7579b5c997c3c43b5b4e5b7b17f584b89ed0ca87 Mon Sep 17 00:00:00 2001 From: Nicholas Law Date: Mon, 9 Nov 2020 17:56:46 +1100 Subject: [PATCH 1/4] Add kmsKey property to LogGroup construct --- packages/@aws-cdk/aws-logs/lib/log-group.ts | 9 ++++++++ packages/@aws-cdk/aws-logs/package.json | 2 ++ .../@aws-cdk/aws-logs/test/test.loggroup.ts | 21 ++++++++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-logs/lib/log-group.ts b/packages/@aws-cdk/aws-logs/lib/log-group.ts index e041b16b29bbd..5a1fd8d5092a8 100644 --- a/packages/@aws-cdk/aws-logs/lib/log-group.ts +++ b/packages/@aws-cdk/aws-logs/lib/log-group.ts @@ -1,5 +1,6 @@ import * as cloudwatch from '@aws-cdk/aws-cloudwatch'; import * as iam from '@aws-cdk/aws-iam'; +import { IKey } from '@aws-cdk/aws-kms'; import { IResource, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { LogStream } from './log-stream'; @@ -273,6 +274,13 @@ export enum RetentionDays { * Properties for a LogGroup */ export interface LogGroupProps { + /** + * The KMS Key to encrypt the log group with. + * + * @default No KMS key + */ + readonly kmsKey?: IKey; + /** * Name of the log group. * @@ -363,6 +371,7 @@ export class LogGroup extends LogGroupBase { } const resource = new CfnLogGroup(this, 'Resource', { + kmsKeyId: props.kmsKey?.keyId, logGroupName: this.physicalName, retentionInDays, }); diff --git a/packages/@aws-cdk/aws-logs/package.json b/packages/@aws-cdk/aws-logs/package.json index 275a2cc729f49..f2e0056471229 100644 --- a/packages/@aws-cdk/aws-logs/package.json +++ b/packages/@aws-cdk/aws-logs/package.json @@ -86,6 +86,7 @@ "dependencies": { "@aws-cdk/aws-cloudwatch": "0.0.0", "@aws-cdk/aws-iam": "0.0.0", + "@aws-cdk/aws-kms": "0.0.0", "@aws-cdk/aws-s3-assets": "0.0.0", "@aws-cdk/core": "0.0.0", "constructs": "^3.2.0" @@ -94,6 +95,7 @@ "peerDependencies": { "@aws-cdk/aws-cloudwatch": "0.0.0", "@aws-cdk/aws-iam": "0.0.0", + "@aws-cdk/aws-kms": "0.0.0", "@aws-cdk/aws-s3-assets": "0.0.0", "@aws-cdk/core": "0.0.0", "constructs": "^3.2.0" diff --git a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts index 22330ae0e3b1f..d9bd45c01c579 100644 --- a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts +++ b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts @@ -1,10 +1,29 @@ import { expect, haveResource, matchTemplate } from '@aws-cdk/assert'; import * as iam from '@aws-cdk/aws-iam'; +import { Key } from '@aws-cdk/aws-kms'; import { CfnParameter, RemovalPolicy, Stack } from '@aws-cdk/core'; import { Test } from 'nodeunit'; import { LogGroup, RetentionDays } from '../lib'; export = { + 'set kms key when provided'(test: Test) { + // GIVEN + const stack = new Stack(); + const kmsKey = new Key(stack, 'Key'); + + // WHEN + new LogGroup(stack, 'LogGroup', { + kmsKey, + }); + + // THEN + expect(stack).to(haveResource('AWS::Logs::LogGroup', { + KmsKeyId: { Ref: 'Key961B73FD' }, + })); + + test.done(); + }, + 'fixed retention'(test: Test) { // GIVEN const stack = new Stack(); @@ -326,4 +345,4 @@ function dataDrivenTests(cases: any[][], body: (test: Test, ...args: any[]) => v }; } return ret; -} \ No newline at end of file +} From 4d83ce59f2b6ee903cca485e1bf666b2a0a1652c Mon Sep 17 00:00:00 2001 From: Nicholas Law Date: Mon, 9 Nov 2020 18:36:44 +1100 Subject: [PATCH 2/4] Add docs for encrypting Log Groups --- packages/@aws-cdk/aws-logs/README.md | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-logs/README.md b/packages/@aws-cdk/aws-logs/README.md index 038b6db28c3a2..df24dc549ea08 100644 --- a/packages/@aws-cdk/aws-logs/README.md +++ b/packages/@aws-cdk/aws-logs/README.md @@ -41,11 +41,32 @@ lambda](https://docs.aws.amazon.com/lambda/latest/dg/monitoring-cloudwatchlogs.h This is implemented using a [CloudFormation custom resource](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-cfn-customresource.html) which pre-creates the log group if it doesn't exist, and sets the specified log retention period (never expire, by default). - + By default, the log group will be created in the same region as the stack. The `logGroupRegion` property can be used to configure log groups in other regions. This is typically useful when controlling retention for log groups auto-created by global services that publish their log group to a specific region, such as AWS Chatbot creating a log group in `us-east-1`. - + +### Encrypting Log Groups + +By default, log group data is always encrypted in CloudWatch Logs. You have the +option to encrypt log group data using a AWS KMS customer master key (CMK) should +you not wish to use the default AWS encryption. Keep in mind that if you decide to +encrypt a log group, any service or IAM identity that needs to read the encrypted +log streams in the future will require the same CMK to decrypt the data. + +Here's a simple example of creating an encrypted Log Group using a KMS CMK. + +```ts +const kmsKey = new kms.Key(this, 'Key'); + +new LogGroup(this, 'LogGroup', { + kmsKey, +}); +``` + +See the AWS documentation for more detailed information about [encrypting CloudWatch +Logs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/encrypt-log-data-kms.html). + ### Subscriptions and Destinations Log events matching a particular filter can be sent to either a Lambda function @@ -100,7 +121,7 @@ the name `Namespace/MetricName`. #### Exposing Metric on a Metric Filter -You can expose a metric on a metric filter by calling the `MetricFilter.metric()` API. +You can expose a metric on a metric filter by calling the `MetricFilter.metric()` API. This has a default of `statistic = 'avg'` if the statistic is not set in the `props`. ```ts From 0a95722d88ed4152070e3715b31b12099d245958 Mon Sep 17 00:00:00 2001 From: Nicholas Law Date: Fri, 13 Nov 2020 16:11:50 +1100 Subject: [PATCH 3/4] Minor adjustments for review comments --- packages/@aws-cdk/aws-logs/README.md | 4 ++-- packages/@aws-cdk/aws-logs/lib/log-group.ts | 8 ++++---- packages/@aws-cdk/aws-logs/test/test.loggroup.ts | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/aws-logs/README.md b/packages/@aws-cdk/aws-logs/README.md index df24dc549ea08..880113852e355 100644 --- a/packages/@aws-cdk/aws-logs/README.md +++ b/packages/@aws-cdk/aws-logs/README.md @@ -57,10 +57,10 @@ log streams in the future will require the same CMK to decrypt the data. Here's a simple example of creating an encrypted Log Group using a KMS CMK. ```ts -const kmsKey = new kms.Key(this, 'Key'); +import * as kms from '@aws-cdk/aws-kms'; new LogGroup(this, 'LogGroup', { - kmsKey, + encryptionKey: new kms.Key(this, 'Key'), }); ``` diff --git a/packages/@aws-cdk/aws-logs/lib/log-group.ts b/packages/@aws-cdk/aws-logs/lib/log-group.ts index 5a1fd8d5092a8..c8c2c56f34294 100644 --- a/packages/@aws-cdk/aws-logs/lib/log-group.ts +++ b/packages/@aws-cdk/aws-logs/lib/log-group.ts @@ -1,6 +1,6 @@ import * as cloudwatch from '@aws-cdk/aws-cloudwatch'; import * as iam from '@aws-cdk/aws-iam'; -import { IKey } from '@aws-cdk/aws-kms'; +import * as kms from '@aws-cdk/aws-kms'; import { IResource, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { LogStream } from './log-stream'; @@ -277,9 +277,9 @@ export interface LogGroupProps { /** * The KMS Key to encrypt the log group with. * - * @default No KMS key + * @default - log group is encrypted with the default master key */ - readonly kmsKey?: IKey; + readonly encryptionKey?: kms.IKey; /** * Name of the log group. @@ -371,7 +371,7 @@ export class LogGroup extends LogGroupBase { } const resource = new CfnLogGroup(this, 'Resource', { - kmsKeyId: props.kmsKey?.keyId, + kmsKeyId: props.encryptionKey?.keyId, logGroupName: this.physicalName, retentionInDays, }); diff --git a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts index d9bd45c01c579..37971f523028a 100644 --- a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts +++ b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts @@ -1,6 +1,6 @@ import { expect, haveResource, matchTemplate } from '@aws-cdk/assert'; import * as iam from '@aws-cdk/aws-iam'; -import { Key } from '@aws-cdk/aws-kms'; +import * as kms from '@aws-cdk/aws-kms'; import { CfnParameter, RemovalPolicy, Stack } from '@aws-cdk/core'; import { Test } from 'nodeunit'; import { LogGroup, RetentionDays } from '../lib'; @@ -9,11 +9,11 @@ export = { 'set kms key when provided'(test: Test) { // GIVEN const stack = new Stack(); - const kmsKey = new Key(stack, 'Key'); + const encryptionKey = new kms.Key(stack, 'Key'); // WHEN new LogGroup(stack, 'LogGroup', { - kmsKey, + encryptionKey, }); // THEN From 62f2c87f9f5febaefde5378e76f85d3d6ffbd596 Mon Sep 17 00:00:00 2001 From: Nicholas Law Date: Fri, 13 Nov 2020 17:03:00 +1100 Subject: [PATCH 4/4] Use KMS Key ARN instead of Key ID --- packages/@aws-cdk/aws-logs/lib/log-group.ts | 2 +- packages/@aws-cdk/aws-logs/test/test.loggroup.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-logs/lib/log-group.ts b/packages/@aws-cdk/aws-logs/lib/log-group.ts index c8c2c56f34294..83c4a8e170b89 100644 --- a/packages/@aws-cdk/aws-logs/lib/log-group.ts +++ b/packages/@aws-cdk/aws-logs/lib/log-group.ts @@ -371,7 +371,7 @@ export class LogGroup extends LogGroupBase { } const resource = new CfnLogGroup(this, 'Resource', { - kmsKeyId: props.encryptionKey?.keyId, + kmsKeyId: props.encryptionKey?.keyArn, logGroupName: this.physicalName, retentionInDays, }); diff --git a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts index 37971f523028a..333ae71a976c3 100644 --- a/packages/@aws-cdk/aws-logs/test/test.loggroup.ts +++ b/packages/@aws-cdk/aws-logs/test/test.loggroup.ts @@ -18,7 +18,8 @@ export = { // THEN expect(stack).to(haveResource('AWS::Logs::LogGroup', { - KmsKeyId: { Ref: 'Key961B73FD' }, + KmsKeyId: { 'Fn::GetAtt': ['Key961B73FD', 'Arn'] }, + })); test.done();