From c17e2b5df535e38e7f1219f974649250500ecfe1 Mon Sep 17 00:00:00 2001
From: Eugene Cheung <81188333+echeung-amzn@users.noreply.github.com>
Date: Wed, 3 Jul 2024 09:49:43 -0400
Subject: [PATCH] feat(wafv2): better handle contextually required region prop
 (#535)

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_
---
 lib/facade/MonitoringAspect.ts                |  8 +++-
 .../aws-wafv2/WafV2MetricFactory.ts           |  4 ++
 .../MonitoringAspect.test.ts.snap             | 44 ++++++++++++++++---
 .../aws-wafv2/WafV2Monitoring.test.ts         | 20 +++++++++
 4 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/lib/facade/MonitoringAspect.ts b/lib/facade/MonitoringAspect.ts
index 2b35b0bf..e959ab09 100644
--- a/lib/facade/MonitoringAspect.ts
+++ b/lib/facade/MonitoringAspect.ts
@@ -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";
@@ -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,
       });
     }
diff --git a/lib/monitoring/aws-wafv2/WafV2MetricFactory.ts b/lib/monitoring/aws-wafv2/WafV2MetricFactory.ts
index 102dbac3..a7cc09c5 100644
--- a/lib/monitoring/aws-wafv2/WafV2MetricFactory.ts
+++ b/lib/monitoring/aws-wafv2/WafV2MetricFactory.ts
@@ -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,
diff --git a/test/facade/__snapshots__/MonitoringAspect.test.ts.snap b/test/facade/__snapshots__/MonitoringAspect.test.ts.snap
index 413db18b..646447c2 100644
--- a/test/facade/__snapshots__/MonitoringAspect.test.ts.snap
+++ b/test/facade/__snapshots__/MonitoringAspect.test.ts.snap
@@ -9212,15 +9212,31 @@ Object {
               Object {
                 "Ref": "AWS::Region",
               },
-              "\\",\\"metrics\\":[[\\"AWS/WAFV2\\",\\"AllowedRequests\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Allowed\\",\\"stat\\":\\"Sum\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Count\\",\\"showUnits\\":false}}}},{\\"type\\":\\"metric\\",\\"width\\":8,\\"height\\":5,\\"x\\":8,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"Blocked Requests\\",\\"region\\":\\"",
+              "\\",\\"metrics\\":[[\\"AWS/WAFV2\\",\\"AllowedRequests\\",\\"Region\\",\\"",
               Object {
                 "Ref": "AWS::Region",
               },
-              "\\",\\"metrics\\":[[\\"AWS/WAFV2\\",\\"BlockedRequests\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Blocked\\",\\"stat\\":\\"Sum\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Count\\",\\"showUnits\\":false}}}},{\\"type\\":\\"metric\\",\\"width\\":8,\\"height\\":5,\\"x\\":16,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"Blocked Requests (rate)\\",\\"region\\":\\"",
+              "\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Allowed\\",\\"stat\\":\\"Sum\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Count\\",\\"showUnits\\":false}}}},{\\"type\\":\\"metric\\",\\"width\\":8,\\"height\\":5,\\"x\\":8,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"Blocked Requests\\",\\"region\\":\\"",
               Object {
                 "Ref": "AWS::Region",
               },
-              "\\",\\"metrics\\":[[{\\"label\\":\\"Blocked (rate)\\",\\"expression\\":\\"100 * (blocked / (allowed + blocked))\\"}],[\\"AWS/WAFV2\\",\\"AllowedRequests\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Allowed\\",\\"stat\\":\\"Sum\\",\\"visible\\":false,\\"id\\":\\"allowed\\"}],[\\"AWS/WAFV2\\",\\"BlockedRequests\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Blocked\\",\\"stat\\":\\"Sum\\",\\"visible\\":false,\\"id\\":\\"blocked\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Rate\\",\\"showUnits\\":false}}}}]}",
+              "\\",\\"metrics\\":[[\\"AWS/WAFV2\\",\\"BlockedRequests\\",\\"Region\\",\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Blocked\\",\\"stat\\":\\"Sum\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Count\\",\\"showUnits\\":false}}}},{\\"type\\":\\"metric\\",\\"width\\":8,\\"height\\":5,\\"x\\":16,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"Blocked Requests (rate)\\",\\"region\\":\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"metrics\\":[[{\\"label\\":\\"Blocked (rate)\\",\\"expression\\":\\"100 * (blocked / (allowed + blocked))\\"}],[\\"AWS/WAFV2\\",\\"AllowedRequests\\",\\"Region\\",\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Allowed\\",\\"stat\\":\\"Sum\\",\\"visible\\":false,\\"id\\":\\"allowed\\"}],[\\"AWS/WAFV2\\",\\"BlockedRequests\\",\\"Region\\",\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Blocked\\",\\"stat\\":\\"Sum\\",\\"visible\\":false,\\"id\\":\\"blocked\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Rate\\",\\"showUnits\\":false}}}}]}",
             ],
           ],
         },
@@ -9238,15 +9254,31 @@ Object {
               Object {
                 "Ref": "AWS::Region",
               },
-              "\\",\\"metrics\\":[[\\"AWS/WAFV2\\",\\"AllowedRequests\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Allowed\\",\\"stat\\":\\"Sum\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Count\\",\\"showUnits\\":false}}}},{\\"type\\":\\"metric\\",\\"width\\":8,\\"height\\":6,\\"x\\":8,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"Blocked Requests\\",\\"region\\":\\"",
+              "\\",\\"metrics\\":[[\\"AWS/WAFV2\\",\\"AllowedRequests\\",\\"Region\\",\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Allowed\\",\\"stat\\":\\"Sum\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Count\\",\\"showUnits\\":false}}}},{\\"type\\":\\"metric\\",\\"width\\":8,\\"height\\":6,\\"x\\":8,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"Blocked Requests\\",\\"region\\":\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"metrics\\":[[\\"AWS/WAFV2\\",\\"BlockedRequests\\",\\"Region\\",\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Blocked\\",\\"stat\\":\\"Sum\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Count\\",\\"showUnits\\":false}}}},{\\"type\\":\\"metric\\",\\"width\\":8,\\"height\\":6,\\"x\\":16,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"Blocked Requests (rate)\\",\\"region\\":\\"",
+              Object {
+                "Ref": "AWS::Region",
+              },
+              "\\",\\"metrics\\":[[{\\"label\\":\\"Blocked (rate)\\",\\"expression\\":\\"100 * (blocked / (allowed + blocked))\\"}],[\\"AWS/WAFV2\\",\\"AllowedRequests\\",\\"Region\\",\\"",
               Object {
                 "Ref": "AWS::Region",
               },
-              "\\",\\"metrics\\":[[\\"AWS/WAFV2\\",\\"BlockedRequests\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Blocked\\",\\"stat\\":\\"Sum\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Count\\",\\"showUnits\\":false}}}},{\\"type\\":\\"metric\\",\\"width\\":8,\\"height\\":6,\\"x\\":16,\\"y\\":1,\\"properties\\":{\\"view\\":\\"timeSeries\\",\\"title\\":\\"Blocked Requests (rate)\\",\\"region\\":\\"",
+              "\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Allowed\\",\\"stat\\":\\"Sum\\",\\"visible\\":false,\\"id\\":\\"allowed\\"}],[\\"AWS/WAFV2\\",\\"BlockedRequests\\",\\"Region\\",\\"",
               Object {
                 "Ref": "AWS::Region",
               },
-              "\\",\\"metrics\\":[[{\\"label\\":\\"Blocked (rate)\\",\\"expression\\":\\"100 * (blocked / (allowed + blocked))\\"}],[\\"AWS/WAFV2\\",\\"AllowedRequests\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Allowed\\",\\"stat\\":\\"Sum\\",\\"visible\\":false,\\"id\\":\\"allowed\\"}],[\\"AWS/WAFV2\\",\\"BlockedRequests\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Blocked\\",\\"stat\\":\\"Sum\\",\\"visible\\":false,\\"id\\":\\"blocked\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Rate\\",\\"showUnits\\":false}}}}]}",
+              "\\",\\"Rule\\",\\"ALL\\",{\\"label\\":\\"Blocked\\",\\"stat\\":\\"Sum\\",\\"visible\\":false,\\"id\\":\\"blocked\\"}]],\\"yAxis\\":{\\"left\\":{\\"min\\":0,\\"label\\":\\"Rate\\",\\"showUnits\\":false}}}}]}",
             ],
           ],
         },
diff --git a/test/monitoring/aws-wafv2/WafV2Monitoring.test.ts b/test/monitoring/aws-wafv2/WafV2Monitoring.test.ts
index 0f950e41..17bc40c4 100644
--- a/test/monitoring/aws-wafv2/WafV2Monitoring.test.ts
+++ b/test/monitoring/aws-wafv2/WafV2Monitoring.test.ts
@@ -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", {