awslabs / awslabs/aws-solutions-constructs

WafwebaclToApiGateway merges DefaultAction to Allow and Block

Open
#830 1 comment 0 reactions 0 assignees View on GitHub
bug needs-triage
Dominant language
TypeScript
Stars
1.4k
Forks
268
Avg merge
5h 18m
Merged PRs (30d)
5

Description

Hi,
I am trying to use WafwebaclToApiGateway construct and pass webaclProps with `defaultAction: {block: {}}`.
The synthesized CloudFormation template results in
```
"DefaultAction": {
"Allow": {},
"Block": {}
},
```
and deployment of that fails of course:
```
"Error reason: You have used none or multiple values for a field that requires exactly one value., field: DEFAULT_ACTION, parameter: DefaultAction
```

### Reproduction Steps

```
const api = new RestApi(this, 'Api', {
restApiName: 'Api',
deployOptions: {
stageName: 'prod',
accessLogDestination: new LogGroupLogDestination(prodLogGroup),
accessLogFormat: AccessLogFormat.jsonWithStandardFields()
},
});

new WafwebaclToApiGateway(this, 'WafWebACL', {
existingApiGatewayInterface: api,
webaclProps: {
defaultAction: {block: {}},
scope: 'REGIONAL',
visibilityConfig: {
cloudWatchMetricsEnabled: true,
metricName: "WebAclMetrics",
sampledRequestsEnabled: true,
},
}
});
```

### Error Log

```
Stack Deployments Failed: Error: The stack named AccountAssessment failed to deploy: UPDATE_ROLLBACK_COMPLETE: Resource handler returned message: "Error reason: You have used none or multiple values for a field that requires exactly one value., field: DEFAULT_ACTION, parameter: DefaultAction (Service: Wafv2, Status Code: 400, Request ID: a3235953-337b-44dd-96ea-45c8b0c02e0d, Extended Request ID: null)"
```

### Environment

- **CDK CLI Version :** 2.46.0
- **CDK Framework Version:** "aws-cdk-lib": "^2.31.2",
- **AWS Solutions Constructs Version :** "@aws-solutions-constructs/aws-wafwebacl-apigateway": "^2.26.0",
- **OS :** MacOS Monterey 12.6
- **Language :** TypeScript

### Other

From looking at the construct code, I learned that it's trying to merge my custom webaclProps with the default props.
I do not claim I understood in detail wich path through the code this merge takes, but I think it relies on usually correct merge logic for javascript objects and merges
```
defaultAction: {
allow: {}
},
```
and
```
defaultAction: {
block: {}
},
```
into
```
defaultAction: {
allow: {}
block: {}
},
```
This causes a problem, because in this case allow and block are modeled as different properties; the merge would work correctly if it were one property with two possible values instead.
It seems to me that the fix would require to handle allow/block as a special case, because the desired outcome in this case contradicts regular merge logic for javascript objects.

---

This is :bug: Bug Report

Contributor guide

Open the contributing guide

Research direction

Start at the WafwebaclToApiGateway construct and inspect how the supplied webaclProps are merged with the default web ACL properties. Reproduce the issue with defaultAction set to block and inspect the synthesized CloudFormation template; done means DefaultAction contains exactly the requested action and deployment no longer reports multiple values.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
api, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.