Skip to content

Commit

Permalink
feat(wafv2): better handle contextually required region prop (#535)
Browse files Browse the repository at this point in the history
1. Throw an error at build time if we're missing the prop when it's
needed.
2. Automatically get the region when needed when using `monitorScope`.

---

_By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license_
  • Loading branch information
echeung-amzn authored Jul 3, 2024
1 parent cf26388 commit c17e2b5
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 7 deletions.
8 changes: 7 additions & 1 deletion lib/facade/MonitoringAspect.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IAspect } from "aws-cdk-lib";
import { IAspect, Stack } from "aws-cdk-lib";
import * as apigw from "aws-cdk-lib/aws-apigateway";
import * as apigwv2 from "aws-cdk-lib/aws-apigatewayv2";
import * as appsync from "aws-cdk-lib/aws-appsync";
Expand Down Expand Up @@ -430,8 +430,14 @@ export class MonitoringAspect implements IAspect {
this.props.webApplicationFirewallAclV2,
);
if (isEnabled && node instanceof wafv2.CfnWebACL) {
const regionProps: Record<string, string> = {};
if (node.scope === "REGIONAL") {
regionProps.region = Stack.of(node).region;
}

this.monitoringFacade.monitorWebApplicationFirewallAclV2({
acl: node,
...regionProps,
...props,
});
}
Expand Down
4 changes: 4 additions & 0 deletions lib/monitoring/aws-wafv2/WafV2MetricFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ export class WafV2MetricFactory extends BaseMetricFactory<WafV2MetricFactoryProp
constructor(metricFactory: MetricFactory, props: WafV2MetricFactoryProps) {
super(metricFactory, props);

if (props.acl.scope === "REGIONAL" && !props.region) {
throw new Error(`region is required if CfnWebACL has "REGIONAL" scope`);
}

this.dimensions = {
Rule: AllRulesDimensionValue,
WebACL: props.acl.name,
Expand Down
44 changes: 38 additions & 6 deletions test/facade/__snapshots__/MonitoringAspect.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions test/monitoring/aws-wafv2/WafV2Monitoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,26 @@ test("snapshot test: no alarms", () => {
expect(Template.fromStack(stack)).toMatchSnapshot();
});

test("with REGIONAL ACL but no region prop, throws error", () => {
const stack = new Stack();
const acl = new CfnWebACL(stack, "DummyAcl", {
name: "DummyAclName",
defaultAction: { allow: {} },
scope: "REGIONAL",
visibilityConfig: {
sampledRequestsEnabled: true,
cloudWatchMetricsEnabled: true,
metricName: "DummyMetricName",
},
});

const scope = new TestMonitoringScope(stack, "Scope");

expect(() => new WafV2Monitoring(scope, { acl })).toThrow(
`region is required if CfnWebACL has "REGIONAL" scope`,
);
});

test("snapshot test: all alarms", () => {
const stack = new Stack();
const acl = new CfnWebACL(stack, "DummyAcl", {
Expand Down

0 comments on commit c17e2b5

Please sign in to comment.