feat(logs): support DataProtectionPolicy in LogGroup construct #23402

kchg · 2022-12-20T01:01:37Z

Sensitive data protection for CloudWatch Logs was launched at re:Invent 2022. This feature will enable that property under DataProtectionPolicy as a JSON object in the LogGroup construct.

Use case: A data protection policy can help safeguard sensitive data that's ingested by the log group by auditing and masking the sensitive log data. When a user who does not have permission to view masked data views a log event that includes masked data, the sensitive data is replaced by asterisks.

closes #23399

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Construct Runtime Dependencies:

This PR adds new construct runtime dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2022-12-20T01:01:55Z

peterwoodworth

Thanks for the PR @kchg!

As it is now, this PR doesn't introduce any functionality that isn't already easily achievable through escape hatches. So, instead of asking our users to fully understand how to build a data protection policy and supply that directly as an object, it would be great to abstract this such that the dataProtectionPolicy prop takes in a class with a name like DataProtectionPolicy that can contain one or more configurable DataPolicyStatements. See our Policy and PolicyStatement classes in our IAM module for reference on what this might look like 🙂 These classes contain methods to modify existing statements, and ways to add statements to an existing policy. I think these methods would be useful in this case as well. Open to thoughts/suggestions!

kchg · 2023-01-05T18:42:22Z

Thanks for the feedback @peterwoodworth! I'll make some changes here to abstract away the details of creating a data protection policy, so that users can just provide a list of data identifiers and any audit destinations.

aws-cdk-automation · 2023-01-11T00:09:07Z

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

Pull request has been modified.

kchg · 2023-01-13T05:32:30Z

Hi @peterwoodworth, this PR should allow users to add a data protection policy to a log group without knowing the actual structure of the policy. Could you please review this? Thanks.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

aws-cdk-automation · 2023-01-22T00:09:35Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2023-01-23T00:08:24Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

…ctionPolicy

kchg · 2023-05-02T01:58:32Z

Hi @corymhall, thank you for the review, I've made changes or addressed each comment from your review, please let me know what you think.

packages/aws-cdk-lib/aws-logs/lib/data-protection-policy.ts

Pull request has been modified.

corymhall

Looking good! Just a couple more suggestions.

corymhall · 2023-05-04T14:16:17Z

packages/aws-cdk-lib/aws-logs/lib/data-protection-policy.ts

+/**
+ * Represents a data protection policy in a log group.
+ */
+export class DataProtectionPolicy {


Yeah on second look I was thinking about extending construct because the constructor took a scope argument. I think we should modify this API a bit to be something like.

class DataProtectionPolicy { constructor(props: DataProtectionPolicyProps) { ... } public bind(scope: Construct): DataProtectionPolicyConfig { ... } }

and then

new CfnLogGroup(this, 'Resource', { ..., dataProtectionPolicy: props.dataProtectionPolicy.bind(this),

corymhall · 2023-05-04T14:16:50Z

packages/aws-cdk-lib/aws-logs/lib/log-group.ts

+  /**
+   * Data Protection Policy for this log group.
+   *
+   * @default null


Suggested change

* @default null

* @default - no data protection policy

- Add bind method to remove need for scope from constructor - Made DataIdentifier required and add check for length

Pull request has been modified.

corymhall

Looking good! Just a couple of minor things

packages/aws-cdk-lib/aws-logs/lib/data-protection-policy.ts

corymhall · 2023-05-05T14:13:50Z

packages/aws-cdk-lib/aws-logs/lib/data-protection-policy.ts

+   * @param scope the Construct scope
+   * @returns the data protection policy
+   */
+  public bind(scope: Construct): DataProtectionPolicyConfig {


Since this is a public API, we should allow for non-breaking changes that we might need to
make in the future. We can either make this an internal API by adding
@internal and changing the name to _bind() or we can add a second optional
props argument that is empty for now. public bind(scope: Construct, props: DataProtectionPolicyBindProps = {})

If you don't think there is any use case for using this outside of the internal
library usage then I would recommend going with @internal.

Updated to _bind()

corymhall · 2023-05-05T14:15:09Z

packages/@aws-cdk-testing/framework-integ/test/aws-logs/test/integ.log-group.js

@@ -0,0 +1,28 @@
+"use strict";


This file should not have been git ignored. Can you check?

Looks like the .gitignore was updated since the commit adding these files, I've removed this file from the PR.

corymhall · 2023-05-05T14:15:22Z

packages/@aws-cdk-testing/framework-integ/test/aws-logs/test/integ.log-group.d.ts

@@ -0,0 +1 @@
+export {};


This file should have been git ignored, can you check?

Removed this file.

- Made the bind() method internal with _bind() - Removed gitignored files

Pull request has been modified.

mergify · 2023-05-08T10:16:06Z

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).

Pull request has been modified.

aws-cdk-automation · 2023-05-08T10:57:36Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: a4be031
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2023-05-08T10:57:56Z

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).

renatoargh · 2023-05-16T05:06:18Z

Hey, thanks for the great PR!

A question: this PR introduces the ability to associate a DataProtectionPolicy to a LogGroup when you create it via the new LogGroup({ ... }) constructor. However, I would like to associate a DataProtectionPolicy with an existing LogGroup to use the data protection feature with an existing lambda. I know I can retrieve an existing LogGroup like this:

const existingLogGroup = LogGroup.fromLogGroupName(
    scope,
    'ExistingLambdaLogGroup',
    '/aws/lambda/my-existing-function-name'
);

But then I am unable to do something like existingLogGroup.addDataProtectionPolicy(new DataProtectionPolicy({ ... })).

Are there any plans to support it or Is there any way to make it work that I cannot see?

I tried creating a logGroup from scratch and associating when creating the lambda but it doesn't seem possible to make lambda use a different log group than the default /aws/lambda/${functionName}.

Again, thanks a lot!

kevincifuentes-eb · 2023-06-27T10:45:42Z

@renatoargh Did you find any workaround for that use case? Thanks!

kevincifuentes-eb · 2023-06-29T07:36:45Z

Any idea if this is possible somehow? I'm also using CDK for our infrastructure and don't see a way to set a custom log group or simply modify the existing one on the lambda to add that DataProtectionPolicy.
Thanks for your help!

kevincifuentes-eb · 2023-07-05T14:15:04Z

If just someone gets to the same PR here, I finally was able to implement it using AWSCustomResource executing a AWSSDKCall when the CloudFormation is created (onCreate)

github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Dec 20, 2022

aws-cdk-automation requested a review from a team December 20, 2022 01:01

github-actions bot added feature-request A feature should be added or improved. p2 labels Dec 20, 2022

peterwoodworth previously requested changes Dec 20, 2022

View reviewed changes

Kevin Chiang added 2 commits January 11, 2023 21:19

feat(aws-logs): support DataProtectionPolicy in LogGroup construct

ee5d62c

Implement data-protection-policy for LogGroup

232799b

kchg force-pushed the LogGroup-DataProtectionPolicy branch from d820ef6 to 232799b Compare January 11, 2023 21:19

Update log-group readme for data-protection-policy

9e531b9

kchg force-pushed the LogGroup-DataProtectionPolicy branch from 9edc82e to 9e531b9 Compare January 11, 2023 22:23

github-actions bot added the effort/medium Medium work item – several days of effort label Jan 11, 2023

kchg requested a review from peterwoodworth January 18, 2023 00:11

kchg added 2 commits January 17, 2023 19:11

Merge branch 'main' into LogGroup-DataProtectionPolicy

bed1c26

Delete package-lock.json

a70916a

aws-cdk-automation previously requested changes Jan 18, 2023

View reviewed changes

kchg changed the title ~~feat(aws-logs): support DataProtectionPolicy in LogGroup construct~~ feat(logs): support DataProtectionPolicy in LogGroup construct Jan 18, 2023

kchg requested review from aws-cdk-automation and peterwoodworth and removed request for peterwoodworth and aws-cdk-automation January 18, 2023 00:15

Merge branch 'main' of github.com:aws/aws-cdk into LogGroup-DataProte…

856c95a

…ctionPolicy

kchg removed the request for review from peterwoodworth January 23, 2023 17:19

Update data protection documentation

134b3cb

kchg requested a review from corymhall May 2, 2023 01:58

corymhall previously requested changes May 2, 2023

View reviewed changes

packages/aws-cdk-lib/aws-logs/lib/data-protection-policy.ts Outdated Show resolved Hide resolved

packages/aws-cdk-lib/aws-logs/lib/data-protection-policy.ts Outdated Show resolved Hide resolved

Updates based on PR review

1512b72

kchg requested a review from corymhall May 2, 2023 15:41

corymhall previously requested changes May 4, 2023

View reviewed changes

Updates based on PR review

0a8edc6

- Add bind method to remove need for scope from constructor - Made DataIdentifier required and add check for length

kchg requested a review from corymhall May 4, 2023 17:40

corymhall previously requested changes May 5, 2023

View reviewed changes

Updates based on PR review

4354dc0

- Made the bind() method internal with _bind() - Removed gitignored files

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 5, 2023

kchg requested a review from corymhall May 5, 2023 22:00

Update packages/aws-cdk-lib/aws-logs/lib/data-protection-policy.ts

13d6110

corymhall previously approved these changes May 8, 2023

View reviewed changes

Merge branch 'main' into LogGroup-DataProtectionPolicy

a4be031

corymhall approved these changes May 8, 2023

View reviewed changes

mergify bot merged commit ed3962a into aws:main May 8, 2023
7 checks passed

filletofish mentioned this pull request Jul 2, 2023

feat(scheduler): ScheduleGroup #26196

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(logs): support DataProtectionPolicy in LogGroup construct #23402

feat(logs): support DataProtectionPolicy in LogGroup construct #23402

kchg commented Dec 20, 2022 •

edited by mrgrain

gitpod-io bot commented Dec 20, 2022

peterwoodworth left a comment

kchg commented Jan 5, 2023

aws-cdk-automation commented Jan 11, 2023

kchg commented Jan 13, 2023

aws-cdk-automation left a comment

aws-cdk-automation commented Jan 22, 2023

aws-cdk-automation commented Jan 23, 2023

kchg commented May 2, 2023

corymhall left a comment

corymhall May 4, 2023

corymhall May 4, 2023

kchg May 4, 2023

corymhall left a comment

corymhall May 5, 2023

kchg May 5, 2023

corymhall May 5, 2023

kchg May 5, 2023

corymhall May 5, 2023

kchg May 5, 2023

mergify bot commented May 8, 2023

aws-cdk-automation commented May 8, 2023

mergify bot commented May 8, 2023

renatoargh commented May 16, 2023

kevincifuentes-eb commented Jun 27, 2023

kevincifuentes-eb commented Jun 29, 2023

kevincifuentes-eb commented Jul 5, 2023

feat(logs): support DataProtectionPolicy in LogGroup construct #23402

feat(logs): support DataProtectionPolicy in LogGroup construct #23402

Conversation

kchg commented Dec 20, 2022 • edited by mrgrain

All Submissions:

Adding new Construct Runtime Dependencies:

New Features

gitpod-io bot commented Dec 20, 2022

peterwoodworth left a comment

Choose a reason for hiding this comment

kchg commented Jan 5, 2023

aws-cdk-automation commented Jan 11, 2023

kchg commented Jan 13, 2023

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jan 22, 2023

aws-cdk-automation commented Jan 23, 2023

kchg commented May 2, 2023

corymhall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corymhall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented May 8, 2023

aws-cdk-automation commented May 8, 2023

AWS CodeBuild CI Report

mergify bot commented May 8, 2023

renatoargh commented May 16, 2023

kevincifuentes-eb commented Jun 27, 2023

kevincifuentes-eb commented Jun 29, 2023

kevincifuentes-eb commented Jul 5, 2023

kchg commented Dec 20, 2022 •

edited by mrgrain