awslabs / aws-solutions-constructs

The AWS Solutions Constructs Library is an open-source extension of the AWS Cloud Development Kit (AWS CDK) that provides multi-service, well-architected patterns for quickly defining solutions
https://docs.aws.amazon.com/solutions/latest/constructs/
Apache License 2.0
1.24k stars 247 forks source link

Type of webaclProps in aws-wafwebacl-cloudfront could be a Partial #916

Closed jameshoward closed 1 year ago

jameshoward commented 1 year ago

buildWebacl from waf-helper uses consolidateProps to merge in the provided webaclProps with the defaults. However, if you want to override just one of the props when constructing a WafwebaclToCloudFront, it results in a type error:

new WafwebaclToCloudFront(this, 'waf', {
  existingCloudFrontWebDistribution: distribution,
  webaclProps: {
    rules: [
      wrapManagedRuleSet('AWSManagedRulesBotControlRuleSet', 'AWS', 0),
    ]
  }
});

Type '{ rules: RuleProperty[]; }' is missing the following properties from type 'CfnWebACLProps': defaultAction, scope, visibilityConfigts(2739)

The type only needs to be a partial, since it's merged with the defaults.

Use Case

It would make it easier to just override individual ACL props, such as the rules.

Proposed Solution

Change WafwebaclToCloudFrontProps so webaclProps becomes a Partial of waf.CfnWebACLProps:

export interface WafwebaclToCloudFrontProps {
  …
  readonly webaclProps?: Partial<waf.CfnWebACLProps>;
}

Other

As a side note, this doesn't actually seem to override the rules and triggers warnings about overriding properties of the existing rules. The docs suggest it should work:

To use a different collection of managed rule sets, specify a new rules property.

Workaround

At the moment I've used the code below as a workaround but it means a redundant call to DefaultWafwebaclProps, which will be called again in buildWebacl. The other option would be a @ts-ignore.

new WafwebaclToCloudFront(this, 'waf', {
  existingCloudFrontWebDistribution: distribution,
  webaclProps: {
    ...DefaultWafwebaclProps('CLOUDFRONT'),
    rules: [
      wrapManagedRuleSet('AWSManagedRulesBotControlRuleSet', 'AWS', 0),
    ]
  }
});

This is a :rocket: Feature Request

biffgaut commented 1 year ago

Thanks, we'll take a look. While Partial will not work because it cannot be translated to the Java and Python versions of the constructs, we have handled this with | any in other situations.

biffgaut commented 1 year ago

This has been completed (allowing | any since partial doesn't work with JSII). It will be in the next release, sometime this week.