Skip to content

Commit

Permalink
feat(wafv2): better handle contextually required region prop
Browse files Browse the repository at this point in the history
  • Loading branch information
echeung-amzn committed Jul 2, 2024
1 parent cf26388 commit be81173
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 be81173

Please sign in to comment.