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

aws-wafv2: LoggingFilterProperty generating incorrect CloudFormation Template #23709

Closed
automartin5000 opened this issue Jan 17, 2023 · 10 comments

Comments

@automartin5000
Copy link

automartin5000 commented Jan 17, 2023

Describe the bug

At some point since version 2.50 (I think), there seems to have been a breaking change to the CloudFormation resource being created by the aws-wafv2 LoggingFilterProperty resulting in the following error message:

#/LoggingFilter: required key [DefaultBehavior] not found
#/LoggingFilter: required key [Filters] not found
#/LoggingFilter: extraneous key [defaultBehavior] is not permitted
#/LoggingFilter: extraneous key [filters] is not permitted

This seems to be related to the wrong casing (see details below).

Expected Behavior

CloudFormation code that uses pascal casing as described in the API

Current Behavior

The following CloudFormation code is created with camel cased properties:

"LoggingFilter": {
     "defaultBehavior": "DROP",
     "filters": [
      {
       "requirement": "MEETS_ANY",
       "conditions": [
        {
         "actionCondition": {
          "action": "BLOCK"
         }
        },
        {
         "actionCondition": {
          "action": "COUNT"
         }
        }
       ],
       "behavior": "KEEP"
      }
     ]
    },

This results in the following error message when attempting to deploy:

#/LoggingFilter: required key [DefaultBehavior] not found
#/LoggingFilter: required key [Filters] not found
#/LoggingFilter: extraneous key [defaultBehavior] is not permitted
#/LoggingFilter: extraneous key [filters] is not permitted

Reproduction Steps

We're using the following code to build our logging configuration:

export abstract class LoggingFilterConfiguration {
  /**
   * Keep logs that don't match any of the specified filter conditions.
   * @param filters
   * @returns LoggingFilterConfiguration
   */
  public static defaultKeep(
    filters: any[],
  ): wafv2.CfnLoggingConfiguration.LoggingFilterProperty {
    return {
      defaultBehavior: LoggingFilterBehavior.KEEP,
      filters: filters,
    };
  }

  /**
   * Drop logs that don't match any of the specified filter conditions.
   * @param filters
   * @returns LoggingFilterConfiguration
   */
  public static defaultDrop(
    filters: any[],
  ): wafv2.CfnLoggingConfiguration.LoggingFilterProperty {
    return {
      defaultBehavior: LoggingFilterBehavior.DROP,
      filters: filters,
    };
  }
}

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.61.1

Framework Version

No response

Node.js Version

18

OS

Mac OS 13.1

Language

Typescript

Language Version

4.7.4

Other information

No response

@automartin5000 automartin5000 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 17, 2023
@eloymg
Copy link

eloymg commented Jan 17, 2023

I think that exist the same bug with:

  • wafv2.CfnWebACL.SingleHeaderProperty
  • wafv2.CfnLoggingConfiguration.SingleHeaderProperty
    Generate the property with "name" no "Name" that is correct.

@pahud
Copy link
Contributor

pahud commented Feb 16, 2023

According to this cfnspec, I think the loggingFilter property should be a LoggingFilter type rather than any(described in the API reference). I need to discuss this internally with the team.

@pahud pahud added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Feb 16, 2023
@pahud pahud self-assigned this Feb 16, 2023
@pahud
Copy link
Contributor

pahud commented Feb 16, 2023

This issue is related to #23679 (comment) as described by @peterwoodworth. At this moment, as loggingFilter property in L1 is not strongly typed, you will need to capitalize the property to synthesize correct CFN template.

@pahud pahud added p2 and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Feb 16, 2023
@pahud pahud removed their assignment Feb 17, 2023
@peterwoodworth
Copy link
Contributor

The way described above is the proper way to define this from now on. Sorry for any inconvenience, we're looking into making sure this doesn't happen again

@github-actions
Copy link

⚠️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.

@JohannesTimmreck-MHP
Copy link

Hi @peterwoodworth,

I believe that this problem was not resolved in the fix(WAFv2): add patch to revert struct names #24651 as it did not modify the LoggingFilterProperty.

@peterwoodworth
Copy link
Contributor

That's already been done before that PR here @JohannesTimmreck-MHP

@JohannesTimmreck-MHP
Copy link

JohannesTimmreck-MHP commented Apr 14, 2023

Hi @peterwoodworth,
as it seems the link you send corrected the LoggingFilterConfiguration, but it also does not mention the CfnLoggingConfiguration.LoggingFilterProperty, for which the error in regards to the mismatch of "DefaultBehavior"-"defaultBehavior" and "Filters"-"filters" properties still occurs.
I just tested the properties on the 2.74.0 version

@peterwoodworth
Copy link
Contributor

The type that this construct originally expected still gets generated - you just shouldn't use the type because the construct doesn't want to accept this type anymore. The patch here is changing the type that CfnLoggingConfiguration expects for its loggingConfiguration property.

@JohannesTimmreck-MHP
Copy link

Ahh alright, thank you for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants