feat(WAF): support associating WAF without creating #504
Conversation
| @@ -34,6 +34,7 @@ export type IamStatement = { | |||
|
|
|||
| export type WafConfig = { | |||
| enabled?: boolean; | |||
| arn?: string; | |||
There was a problem hiding this comment.
please add AJV validation for this.
|
Thank you. LGTM One last thing. Would you mind also updating the doc? (on both PRs) Thank you |
|
Absolutely! Added :) |
doc/WAF.md
Outdated
| - `visibilityConfig`: Optional. A [visibility config](https://docs.aws.amazon.com/waf/latest/APIReference/API_VisibilityConfig.html) for this WAF | ||
| - `name`: Metric name | ||
| - `cloudWatchMetricsEnabled`: A boolean indicating whether the associated resource sends metrics to Amazon CloudWatch | ||
| - `sampledRequestsEnabled`: A boolean indicating whether AWS WAF should store a sampling of the web requests that match the rule | ||
| - `rules`: An array of [rules](#rules). | ||
| - `rules`: Optional. An array of [rules](#rules). |
There was a problem hiding this comment.
Rules is required as per validation
I guess it makes sense to make it optional now.
But it would be great to make it either rules or arn required.
In fact, it would be allowing arn OR the other options (other options except enabled don't make sense if arn is used)
Let me think a bit about this and come with some suggestions.
Feel free to propose some too.
There was a problem hiding this comment.
Hey @bboure, not sure that we can express this through AJV Validation.
One option is to make arn, rules and other config Optional in the schema and express these rules through code?
There was a problem hiding this comment.
I have found some tricks that should make this possible.
I will have a go at it when I can.
There was a problem hiding this comment.
@rikhilrai Done.
I made some improvements on the validation principally.
I'm also considering renaming arn to something more explicit like webAclArn
WDYT?
|
hey @bboure, thanks for your changes. PR LGTM! |
|
Sorry, I lost track of this PR and now I'm not sure what it does anymore. Thank you |
| - `visibilityConfig`: Optional. A [visibility config](https://docs.aws.amazon.com/waf/latest/APIReference/API_VisibilityConfig.html) for this WAF | ||
| - `name`: Metric name | ||
| - `cloudWatchMetricsEnabled`: A boolean indicating whether the associated resource sends metrics to Amazon CloudWatch | ||
| - `sampledRequestsEnabled`: A boolean indicating whether AWS WAF should store a sampling of the web requests that match the rule | ||
| - `rules`: An array of [rules](#rules). | ||
| - `rules`: Required. An array of [rules](#rules). Optional when `arn` is present |
There was a problem hiding this comment.
This is a bit confusing.
It might let user think that you can pass new rules even if you pass a waWAF arn, which is not from what I can see in the code (the WAF is only associated).
(same for the other attributes).
This is probably fine for now. I'd like to reorganize the whole documentation anyway.
|
Thanks! I'll merge this now. About doc, I have some new plans to improve it. |
bboure
changed the title
|
🎉 This PR is included in version 2.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Co-authored-by: bboure <[email protected]>
closes #500
same as #501 but into v2 alpha base branch