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

(logs) DataProtectionPolicy not displaying properly in the console #26728

Open
peterwoodworth opened this issue Aug 11, 2023 Discussed in #26669 · 4 comments
Open

(logs) DataProtectionPolicy not displaying properly in the console #26728

peterwoodworth opened this issue Aug 11, 2023 Discussed in #26669 · 4 comments
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@peterwoodworth
Copy link
Contributor

Discussed in #26669

Originally posted by ericxinzhang August 8, 2023
I'd like to apply a data protection policy to a log group and exactly followed the document but it's not working.

I believe the reason is somehow in the generated CFN template, all the field names (e.g. statement) of the DataProtectionPolicy property for the log group are in lower case while it should be in uppercase as per the doc.

I tried cdk deploy and the template can be deployed successfully, but the policy is not taking effect.

Could someone please enlighten me what I did wrong here?

Please refer to the following console log for details.

➜  data-protection git:(main) ✗ cdk init --language typescript app                                                                                                                                                                                 
Applying project template app for typescript
# Welcome to your CDK TypeScript project

This is a blank project for CDK development with TypeScript.

The `cdk.json` file tells the CDK Toolkit how to execute your app.

## Useful commands

* `npm run build`   compile typescript to js
* `npm run watch`   watch for changes and compile
* `npm run test`    perform the jest unit tests
* `cdk deploy`      deploy this stack to your default AWS account/region
* `cdk diff`        compare deployed stack with current state
* `cdk synth`       emits the synthesized CloudFormation template

Executing npm install...
✅ All done!

➜  data-protection git:(main) ✗ cdk --version                                                                                                                                                                                                      
2.90.0 (build 8c535e4)

➜  data-protection git:(main) ✗ cat bin/data-protection.ts        
#!/usr/bin/env node
import 'source-map-support/register';
import * as cdk from 'aws-cdk-lib';
import { DataProtectionStack } from '../lib/data-protection-stack';

const app = new cdk.App();
new DataProtectionStack(app, 'DataProtectionStack', {
});
➜  data-protection git:(main) ✗ cat lib/data-protection-stack.ts    
import * as cdk from "aws-cdk-lib";
import { Construct } from "constructs";

export class DataProtectionStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const dataProtectionPolicy = new cdk.aws_logs.DataProtectionPolicy({
      name: "EmailAndLatLngProrectionPolicy",
      identifiers: [
        cdk.aws_logs.DataIdentifier.EMAILADDRESS,
        cdk.aws_logs.DataIdentifier.LATLONG,
      ],
    });

    new cdk.aws_logs.LogGroup(this, "TestLogGroup", {
      logGroupName: "TestLogGroup",
      dataProtectionPolicy,
    });
  }
}

  data-protection git:(main)  cdk synth 
Resources:
  TestLogGroup4EEF7AD4:
    Type: AWS::Logs::LogGroup
    Properties:
      DataProtectionPolicy:
        name: EmailAndLatLngProrectionPolicy
        description: cdk generated data protection policy
        version: "2021-06-01"
        statement:
          - sid: audit-statement-cdk
            dataIdentifier:
              - Fn::Join:
                  - ""
                  - - "arn:"
                    - Ref: AWS::Partition
                    - :dataprotection::aws:data-identifier/EmailAddress
              - Fn::Join:
                  - ""
                  - - "arn:"
                    - Ref: AWS::Partition
                    - :dataprotection::aws:data-identifier/LatLong
            operation:
              audit:
                findingsDestination: {}
          - sid: redact-statement-cdk
            dataIdentifier:
              - Fn::Join:
                  - ""
                  - - "arn:"
                    - Ref: AWS::Partition
                    - :dataprotection::aws:data-identifier/EmailAddress
              - Fn::Join:
                  - ""
                  - - "arn:"
                    - Ref: AWS::Partition
                    - :dataprotection::aws:data-identifier/LatLong
            operation:
              deidentify:
                maskConfig: {}
      RetentionInDays: 731
    UpdateReplacePolicy: Retain
    DeletionPolicy: Retain
    Metadata:
      aws:cdk:path: DataProtectionStack/TestLogGroup/Resource
      ... (omitted)
```</div>
@peterwoodworth
Copy link
Contributor Author

peterwoodworth commented Aug 11, 2023

Please see the discussion here to see what the current behavior is like. The properties should probably be uppercase in the template. it's not clear to me whether this causes any bugs in actual behavior, or just the console.

@peterwoodworth peterwoodworth added bug This issue is a bug. p2 effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md labels Aug 11, 2023
@kirkwat
Copy link
Contributor

kirkwat commented Aug 12, 2023

after looking into this issue, I believe the problem is that in the logs.generated.ts file, the CloudFormation DataProtectionPolicy is being passed the entire object (as shown in the image) rather than setting each attribute with the uppercase letter.
image

My pull request previously changed the object variables to uppercase letters, but that did not follow the enforced naming convention.

@SimardeepSingh-zsh
Copy link

import * as cdk from "aws-cdk-lib";
import { Construct } from "constructs";

export class DataProtectionStack extends cdk.Stack {
constructor(scope: Construct, id: string, props?: cdk.StackProps) {
super(scope, id, props);

const dataProtectionPolicy = new cdk.aws_logs.DataProtectionPolicy({
  Name: "EmailAndLatLngProrectionPolicy", // Use uppercase 'Name' instead of 'name'
  Description: "cdk generated data protection policy", // Use uppercase 'Description'
  PolicyDocument: {
    Version: "2021-06-01",
    Statement: [
      {
        Sid: "audit-statement-cdk",
        DataIdentifier: [
          {
            Arn: cdk.Fn.join("", [
              "arn:",
              cdk.Aws.PARTITION,
              ":dataprotection::aws:data-identifier/EmailAddress",
            ]),
          },
          {
            Arn: cdk.Fn.join("", [
              "arn:",
              cdk.Aws.PARTITION,
              ":dataprotection::aws:data-identifier/LatLong",
            ]),
          },
        ],
        Operation: {
          Audit: {
            FindingsDestination: {},
          },
        },
      },
      {
        Sid: "redact-statement-cdk",
        DataIdentifier: [
          {
            Arn: cdk.Fn.join("", [
              "arn:",
              cdk.Aws.PARTITION,
              ":dataprotection::aws:data-identifier/EmailAddress",
            ]),
          },
          {
            Arn: cdk.Fn.join("", [
              "arn:",
              cdk.Aws.PARTITION,
              ":dataprotection::aws:data-identifier/LatLong",
            ]),
          },
        ],
        Operation: {
          Deidentify: {
            MaskConfig: {},
          },
        },
      },
    ],
  },
});

new cdk.aws_logs.LogGroup(this, "TestLogGroup", {
  logGroupName: "TestLogGroup",
  dataProtectionPolicy,
});

}
}

@aaythapa aaythapa assigned aaythapa and unassigned aaythapa Jan 11, 2024
@moelasmar moelasmar added the @aws-cdk/aws-logs Related to Amazon CloudWatch Logs label Aug 28, 2024
@cam8001
Copy link

cam8001 commented Oct 11, 2024

Just confirming my understanding:

  • CloudFormation property names should be PascalCased
  • the DataProtectionPolicy construct in aws-logs generates properties with camelCased property names
    • for example, dataIdentifier instead of DataIdentifier
  • Properties with camelCased property names are not recognised by CloudFormation, and so the configuration is not applied.

I have tried to sort this by explicitly specifying the keys in an object in the generation of the CfnLogGroup in log-group.ts.

I'm not sure if I need to go one level deeper to do the same for Statement, but the docs suggest so. It feels a little hacky to do ever deeper explicit specification of keys - if anyone else has any suggestions for a better approach, I'm very open to it!

PR incoming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
6 participants