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

(module name): (short issue description)aws-cdk-lib/aws-wafv2: CfnWebACL and Cloudformation properties not matching part 2: Java #26127

Closed
TimVanBeek opened this issue Jun 27, 2023 · 4 comments
Labels
@aws-cdk/aws-wafregional Related to AWS WAF Regional response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@TimVanBeek
Copy link

Describe the bug

Trying to configure WAF logging to redact header values does not work if you try it this way, using Java CDK v2.85:

var header = CfnLoggingConfiguration.SingleHeaderProperty.builder() .name("header") .build();
var headerMatcher = CfnLoggingConfiguration.FieldToMatchProperty.builder() .singleHeader(header) .build();
CfnLoggingConfiguration loggingConfig = CfnLoggingConfiguration.Builder ... .redactedFields(List.of(headerMatcher)) ...

because this runs into this error message on a cdk deployment:

#/RedactedFields/0/SingleHeader: required key [Name] not found #/RedactedFields/0/SingleHeader: extraneous key [name] is not permitted

It would seem that the class SingleHeaderProperty has the property "name", but CloudFormation expects "Name".

This issue has been reported before, see #23679, however there seems to be no fix or workaround for Java. The solution to directly specify the SingleHeader object as JSON does only work in JavaScript, but not in Java.

Expected Behavior

Successful deployment with header field redacted in the log destination for the WAF.

Current Behavior

CDK deployment fails with the error message

#/RedactedFields/0/SingleHeader: required key [Name] not found #/RedactedFields/0/SingleHeader: extraneous key [name] is not permitted

Reproduction Steps

Create a CDK stack, create a CfnWebACL instance "yourWafAcl", a LogGroup with ARN "logGroupARN" and configure logging via:

var header = CfnLoggingConfiguration.SingleHeaderProperty .builder() .name("header") .build();
var headerMatcher = CfnLoggingConfiguration.FieldToMatchProperty.builder() .singleHeader(header) .build();

CfnLoggingConfiguration loggingConfig = CfnLoggingConfiguration.Builder
.create(this, "loggingConfig")
.resourceArn(yourWafAcl.getAttrArn())
.logDestinationConfigs(List.of(logGroupARN))
.redactedFields(List.of(headerMatcher))
.build();

and execute cdk deploy for your stack.

Possible Solution

Change "name" to "Name" in the class CfnLoggingConfiguration.SingleHeaderProperty? And all similar issues?

Additional Information/Context

Have a look at: #23679

This has already been reported and worked on, it just seems that there is no solution or workaround for any CDK language except JavaScript.

CDK CLI Version

2.85.0

Framework Version

2.85.0

Node.js Version

18.x.x

OS

Unix

Language

Java

Language Version

11

Other information

No response

@TimVanBeek TimVanBeek added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 27, 2023
@github-actions github-actions bot added the @aws-cdk/aws-wafregional Related to AWS WAF Regional label Jun 27, 2023
@peterwoodworth
Copy link
Contributor

CfnLoggingConfiguration.FieldToMatchProperty.builder().singleHeader() wants an arbitrarily defined object, not a CfnLoggingConfiguration.SingleHeaderProperty. If it takes in the latter, it won't know how to handle it properly (i.e. properly capitalize it) since it's expecting an object that it will directly copy into the template. See Generic Structures in our docs for how to supply a generic object.

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 27, 2023
@TimVanBeek
Copy link
Author

Thanks, that worked. I'd still like to point out that this could be a little bit clearer, both regarding the documentation and the API design itself ;-)

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

@peterwoodworth
Copy link
Contributor

Yeah thanks for the feedback @TimVanBeek, we are working on making sure this works either way it's defined (instead of having it be this way if the spec ever changes the type).

I'm not super sure what we can do in terms of documentation right now, but I have been thinking about that too. If I think of anything I'll be sure to get something up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-wafregional Related to AWS WAF Regional response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants