Repository: sid88in/serverless-appsync-plugin
Pull request #582 (2023-03-20 05:14:05)

fix : support overrideAction for WAF
Fix for https://github.com/sid88in/serverless-appsync-plugin/issues/578.

When v1 to v2 migration, overrideAction in WAF code was missed to migrate.

override action is typically `None: {}`, or `Count : {}`
https://docs.aws.amazon.com/waf/latest/APIReference/API_OverrideAction.html

I referred around this code in v1 branch.
If `overrideAction` is set, use `overrideAction`. Otherwise, `Action` is used same as before.
https://github.com/sid88in/serverless-appsync-plugin/blob/v1.14.0/src/index.js#L1612

tested with AWS
- cloud formation was generated same as v1
- `sls deploy` suceeded

```
      "Name": "AWSManagedRulesCommonRuleSet",
      "Priority": 20,
      "Statement": {
        "ManagedRuleGroupStatement": {
          "VendorName": "AWS",
          "Name": "AWSManagedRulesCommonRuleSet"
        }
      },
      "VisibilityConfig": {
        "CloudWatchMetricsEnabled": true,
        "MetricName": "AWSManagedRulesCommonRuleSet",
        "SampledRequestsEnabled": true
      },
      "OverrideAction": {
        "None": {}
      }
    },
```

yasuto-nishii: @bboure 
Hi, to address WAF CFn issue, I created PR based on v1 implementation.
I had tested on my actual AWS env, also I verified jest result is expected one.
 
Hope this code helps to fix the bug.

https://github.com/sid88in/serverless-appsync-plugin/issues/578
https://github.com/sid88in/serverless-appsync-plugin/blob/v1.14.0/src/index.js#L1612
bboure: Sorry for taking so long to review this, and thanks for this!

I haven't used WAF for a long time, but the changes and explanations look good to me. 

You will need to rebase/resolve conflicts though.

yasuto-nishii: @bboure 
Thanks for reply.  I had rebase and resolved the conflict.

Patch:
diff --git a/doc/WAF.md b/doc/WAF.md
index 40d636f..13916f4 100644
--- a/doc/WAF.md
+++ b/doc/WAF.md
@@ -119,6 +119,22 @@ waf:
             - US
 ```
 
+```yml
+waf:
+  enabled: true
+  defaultAction: Block
+  rules:
+    # using ManagedRuleGroup
+    - name: "AWSManagedRulesCommonRuleSet"
+      priority: 20
+      overrideAction:
+        None: {}
+      statement:
+        ManagedRuleGroupStatement:
+          VendorName: "AWS"
+          Name: "AWSManagedRulesCommonRuleSet"
+```
+
 ### Per API Key rules
 
 In some cases, you might want to enable a rule for a given API key only. You can specify `wafRules` under the `appSync.apiKeys` attribute. The rules will apply only to that API key.
diff --git a/doc/general-config.md b/doc/general-config.md
index eb9400a..8cb9196 100644
--- a/doc/general-config.md
+++ b/doc/general-config.md
@@ -44,12 +44,16 @@ appSync:
 - `dataSources`: See [DataSources](dataSources.md)
 - `resolvers`: See [Resolvers](resolvers.md)
 - `pipelineFunctions`: See [Pipeline functions](pipeline-functions.md)
-- `substitutions`: See [Substitutions](substitutions.md)
+- `substitutions`: See [Substitutions](substitutions.md). Deprecated: Use environment variables.
+- `environment`: A list of environment variables for the API. See [Official Documentation](https://docs.aws.amazon.com/appsync/latest/devguide/environmental-variables.html)
 - `caching`: See [Cacing](caching.md)
 - `waf`: See [Web Application Firefall](WAF.md)
 - `logging`: See [Logging](#Logging)
 - `xrayEnabled`: Boolean. Enable or disable X-Ray tracing.
 - `visibility`: Optional. `GLOBAL` or `PRIVATE`. **Changing this value requires the replacement of the API.**
+- `introspection`: Boolean. Whether to enable introspection or not. Defaults to `true`.
+- `queryDepthLimit`: Optional. The maximum amount of nested level allowed per query. Must be between 1 and 75. If not specified: unlimited.
+- `resolverCountLimit`: Optional. The maximum number of resolvers a query can process. Must be between 1 and 1000. If not specified: unlimited.
 - `tags`: A key-value pair for tagging this AppSync API
 - `esbuild`: Custom esbuild options, or `false` See [Esbuild](#Esbuild)
 
diff --git a/doc/resolvers.md b/doc/resolvers.md
index 7e3d714..48de245 100644
--- a/doc/resolvers.md
+++ b/doc/resolvers.md
@@ -29,7 +29,7 @@ appSync:
 - `code`: The path of the JavaScript resolver handler file, relative to `serverless.yml`. If not specified, a [minimalistic default](#javascript-vs-vtl) is used.
 - `request`: The path to the VTL request mapping template file, relative to `serverless.yml`.
 - `response`: The path to the VTL response mapping template file, relative to `serverless.yml`.
-- `substitutions`: See [Variable Substitutions](substitutions.md)
+- `substitutions`: See [Variable Substitutions](substitutions.md). Deprecated: Use [environment variables](./general-config.md) instead.
 - `caching`: [See below](#Caching)
 - `sync`: [See SyncConfig](syncConfig.md)
 
diff --git a/doc/substitutions.md b/doc/substitutions.md
index 794953b..f43bc53 100644
--- a/doc/substitutions.md
+++ b/doc/substitutions.md
@@ -1,3 +1,5 @@
+> ⚠️ Substitutions are deprecated. Use [Environment Variables](./general-config.md) instead.
+
 # Substitutions
 
 `Substitutions` is a feature that allows you to replace some variables in your VTL mapping templates or JS resolvers with dynamic values.
@@ -30,27 +32,29 @@ appSync:
 <details open>
   <summary>VTL mapping template</summary>
 
-  ```vtl
-  {
-      "version" : "2018-05-29",
-      "operation" : "BatchPutItem",
-      "tables" : {
-          "${postsTable}": [...]
-      }
-  }
-  ```
+```vtl
+{
+    "version" : "2018-05-29",
+    "operation" : "BatchPutItem",
+    "tables" : {
+        "${postsTable}": [...]
+    }
+}
+```
+
 </details>
 
 <details open>
   <summary>JS Resolvers</summary>
-  
-  ```js
-  const tableName = '#postsTable#';
-  return {
-    operation: "BatchGetItem",
-    tables: {
-      [tableName]: { keys },
-    },
-  };
-  ```
+
+```js
+const tableName = '#postsTable#';
+return {
+  operation: 'BatchGetItem',
+  tables: {
+    [tableName]: { keys },
+  },
+};
+```
+
 </details>
diff --git a/src/resources/Api.ts b/src/resources/Api.ts
index 95e4192..92a9663 100644
--- a/src/resources/Api.ts
+++ b/src/resources/Api.ts
@@ -76,6 +76,7 @@ export class Api {
         Name: this.config.name,
         XrayEnabled: this.config.xrayEnabled || false,
         Tags: this.getTagsConfig(),
+        EnvironmentVariables: this.config.environment,
       },
     };
 
@@ -111,6 +112,24 @@ export class Api {
       });
     }
 
+    if (this.config.introspection !== undefined) {
+      merge(endpointResource.Properties, {
+        IntrospectionConfig: this.config.introspection ? 'ENABLED' : 'DISABLED',
+      });
+    }
+
+    if (this.config.queryDepthLimit !== undefined) {
+      merge(endpointResource.Properties, {
+        QueryDepthLimit: this.config.queryDepthLimit,
+      });
+    }
+
+    if (this.config.resolverCountLimit !== undefined) {
+      merge(endpointResource.Properties, {
+        ResolverCountLimit: this.config.resolverCountLimit,
+      });
+    }
+
     const resources = {
       [logicalId]: endpointResource,
     };
diff --git a/src/resources/Waf.ts b/src/resources/Waf.ts
index b9a6dfa..2d2e0bc 100644
--- a/src/resources/Waf.ts
+++ b/src/resources/Waf.ts
@@ -14,6 +14,7 @@ import {
   WafThrottleConfig,
 } from '../types/plugin';
 import { Api } from './Api';
+import { toCfnKeys } from '../utils';
 
 export class Waf {
   constructor(private api: Api, private config: WafConfig) {}
@@ -106,10 +107,10 @@ export class Waf {
     }
 
     const action: WafRuleAction = rule.action || 'Allow';
+    const overrideAction = rule.overrideAction;
 
     const result: CfnWafRule = {
       Name: rule.name,
-      Action: { [action]: {} },
       Priority: rule.priority,
       Statement: rule.statement,
       VisibilityConfig: this.getWafVisibilityConfig(
@@ -118,6 +119,12 @@ export class Waf {
       ),
     };
 
+    if (overrideAction) {
+      result.OverrideAction = toCfnKeys(overrideAction);
+    } else {
+      result.Action = { [action]: {} };
+    }
+
     return result;
   }
 
diff --git a/src/types/common.ts b/src/types/common.ts
index b281563..cc8157b 100644
--- a/src/types/common.ts
+++ b/src/types/common.ts
@@ -128,6 +128,7 @@ export type SyncConfig = {
 } & LambdaConfig;
 
 export type Substitutions = Record<string, string | IntrinsicFunction>;
+export type EnvironmentVariables = Record<string, string | IntrinsicFunction>;
 
 export type DsDynamoDBConfig = {
   type: 'AMAZON_DYNAMODB';
diff --git a/src/types/index.ts b/src/types/index.ts
index c8cfae9..e52f838 100644
--- a/src/types/index.ts
+++ b/src/types/index.ts
@@ -15,6 +15,7 @@ import {
   DsOpenSearchConfig,
   DsRelationalDbConfig,
   SyncConfig,
+  EnvironmentVariables,
 } from './common';
 export * from './common';
 
@@ -33,6 +34,7 @@ export type AppSyncConfig = {
     | Record<string, DataSourceConfig>[]
     | Record<string, DataSourceConfig>;
   substitutions?: Substitutions;
+  environment?: EnvironmentVariables;
   xrayEnabled?: boolean;
   logging?: LoggingConfig;
   caching?: CachingConfig;
@@ -40,6 +42,9 @@ export type AppSyncConfig = {
   tags?: Record<string, string>;
   visibility?: 'GLOBAL' | 'PRIVATE';
   esbuild?: BuildOptions | false;
+  introspection?: boolean;
+  queryDepthLimit?: number;
+  resolverCountLimit?: number;
 };
 
 export type BaseResolverConfig = {
diff --git a/src/types/plugin.ts b/src/types/plugin.ts
index f7e245d..0b8bf8f 100644
--- a/src/types/plugin.ts
+++ b/src/types/plugin.ts
@@ -15,6 +15,7 @@ import {
   DsEventBridgeConfig,
   DsNone,
   Substitutions,
+  EnvironmentVariables,
 } from './common';
 export * from './common';
 
@@ -29,6 +30,7 @@ export type AppSyncConfig = {
   resolvers: Record<string, ResolverConfig>;
   pipelineFunctions: Record<string, PipelineFunctionConfig>;
   substitutions?: Substitutions;
+  environment?: EnvironmentVariables;
   xrayEnabled?: boolean;
   logging?: LoggingConfig;
   caching?: CachingConfig;
@@ -36,6 +38,9 @@ export type AppSyncConfig = {
   tags?: Record<string, string>;
   visibility?: 'GLOBAL' | 'PRIVATE';
   esbuild?: BuildOptions | false;
+  introspection?: boolean;
+  queryDepthLimit?: number;
+  resolverCountLimit?: number;
 };
 
 export type BaseResolverConfig = {
diff --git a/src/validation.ts b/src/validation.ts
index bc2887e..4cc292e 100644
--- a/src/validation.ts
+++ b/src/validation.ts
@@ -255,6 +255,14 @@ export const appSyncSchema = {
       required: [],
       errorMessage: 'must be a valid substitutions definition',
     },
+    environment: {
+      type: 'object',
+      additionalProperties: {
+        $ref: '#/definitions/stringOrIntrinsicFunction',
+      },
+      required: [],
+      errorMessage: 'must be a valid environment definition',
+    },
     dataSource: {
       if: { type: 'object' },
       then: { $ref: '#/definitions/dataSourceConfig' },
@@ -683,7 +691,11 @@ export const appSyncSchema = {
       enum: ['GLOBAL', 'PRIVATE'],
       errorMessage: 'must be "GLOBAL" or "PRIVATE"',
     },
+    introspection: { type: 'boolean' },
+    queryDepthLimit: { type: 'integer', minimum: 1, maximum: 75 },
+    resolverCountLimit: { type: 'integer', minimum: 1, maximum: 1000 },
     substitutions: { $ref: '#/definitions/substitutions' },
+    environment: { $ref: '#/definitions/environment' },
     waf: {
       type: 'object',
       properties: {


Test patch:
diff --git a/src/__tests__/__snapshots__/api.test.ts.snap b/src/__tests__/__snapshots__/api.test.ts.snap
index 37d4314..000e5ce 100644
--- a/src/__tests__/__snapshots__/api.test.ts.snap
+++ b/src/__tests__/__snapshots__/api.test.ts.snap
@@ -20,6 +20,7 @@ Object {
         },
       ],
       "AuthenticationType": "API_KEY",
+      "EnvironmentVariables": undefined,
       "Name": "MyApi",
       "Tags": Array [
         Object {
@@ -47,6 +48,7 @@ Object {
   "GraphQlApi": Object {
     "Properties": Object {
       "AuthenticationType": "AWS_LAMBDA",
+      "EnvironmentVariables": undefined,
       "LambdaAuthorizerConfig": Object {
         "AuthorizerResultTtlInSeconds": undefined,
         "AuthorizerUri": Object {
diff --git a/src/__tests__/__snapshots__/waf.test.ts.snap b/src/__tests__/__snapshots__/waf.test.ts.snap
index 4101842..5d48bbc 100644
--- a/src/__tests__/__snapshots__/waf.test.ts.snap
+++ b/src/__tests__/__snapshots__/waf.test.ts.snap
@@ -411,6 +411,27 @@ Object {
 }
 `;
 
+exports[`Waf Custom rules should generate a custom rule with ManagedRuleGroup 1`] = `
+Object {
+  "Name": "MyRule1",
+  "OverrideAction": Object {
+    "None": Object {},
+  },
+  "Priority": 200,
+  "Statement": Object {
+    "ManagedRuleGroupStatement": Object {
+      "Name": "AWSManagedRulesCommonRuleSet",
+      "VendorName": "AWS",
+    },
+  },
+  "VisibilityConfig": Object {
+    "CloudWatchMetricsEnabled": true,
+    "MetricName": "MyRule1",
+    "SampledRequestsEnabled": true,
+  },
+}
+`;
+
 exports[`Waf Disable introspection should generate a preset rule 1`] = `
 Object {
   "Action": Object {
diff --git a/src/__tests__/api.test.ts b/src/__tests__/api.test.ts
index c8389f1..87405c9 100644
--- a/src/__tests__/api.test.ts
+++ b/src/__tests__/api.test.ts
@@ -15,6 +15,7 @@ describe('Api', () => {
           "GraphQlApi": Object {
             "Properties": Object {
               "AuthenticationType": "API_KEY",
+              "EnvironmentVariables": undefined,
               "Name": "MyApi",
               "Tags": Array [
                 Object {
@@ -42,6 +43,7 @@ describe('Api', () => {
           "GraphQlApi": Object {
             "Properties": Object {
               "AuthenticationType": "API_KEY",
+              "EnvironmentVariables": undefined,
               "Name": "MyApi",
               "Tags": Array [
                 Object {
@@ -58,6 +60,77 @@ describe('Api', () => {
       `);
     });
 
+    it('should compile the Api Resource with config', () => {
+      const api = new Api(
+        given.appSyncConfig({
+          introspection: false,
+          queryDepthLimit: 10,
+          resolverCountLimit: 20,
+        }),
+        plugin,
+      );
+      expect(api.compileEndpoint()).toMatchInlineSnapshot(`
+        Object {
+          "GraphQlApi": Object {
+            "Properties": Object {
+              "AuthenticationType": "API_KEY",
+              "EnvironmentVariables": undefined,
+              "IntrospectionConfig": "DISABLED",
+              "Name": "MyApi",
+              "QueryDepthLimit": 10,
+              "ResolverCountLimit": 20,
+              "Tags": Array [
+                Object {
+                  "Key": "stage",
+                  "Value": "Dev",
+                },
+              ],
+              "XrayEnabled": false,
+            },
+            "Type": "AWS::AppSync::GraphQLApi",
+          },
+        }
+      `);
+    });
+
+    it('should compile the Api Resource with Environments', () => {
+      const api = new Api(
+        given.appSyncConfig({
+          environment: {
+            TABLE_NAME: 'MyTable',
+            OTHER_TABLE: {
+              Ref: 'OtherTable',
+            },
+          },
+        }),
+        plugin,
+      );
+      expect(api.compileEndpoint()).toMatchInlineSnapshot(`
+        Object {
+          "GraphQlApi": Object {
+            "Properties": Object {
+              "AuthenticationType": "API_KEY",
+              "EnvironmentVariables": Object {
+                "OTHER_TABLE": Object {
+                  "Ref": "OtherTable",
+                },
+                "TABLE_NAME": "MyTable",
+              },
+              "Name": "MyApi",
+              "Tags": Array [
+                Object {
+                  "Key": "stage",
+                  "Value": "Dev",
+                },
+              ],
+              "XrayEnabled": false,
+            },
+            "Type": "AWS::AppSync::GraphQLApi",
+          },
+        }
+      `);
+    });
+
     it('should compile the Api Resource with logs enabled', () => {
       const api = new Api(
         given.appSyncConfig({
@@ -74,6 +147,7 @@ describe('Api', () => {
           "GraphQlApi": Object {
             "Properties": Object {
               "AuthenticationType": "API_KEY",
+              "EnvironmentVariables": undefined,
               "LogConfig": Object {
                 "CloudWatchLogsRoleArn": Object {
                   "Fn::GetAtt": Array [
@@ -183,6 +257,7 @@ describe('Api', () => {
                 },
               ],
               "AuthenticationType": "AMAZON_COGNITO_USER_POOLS",
+              "EnvironmentVariables": undefined,
               "Name": "MyApi",
               "Tags": Array [
                 Object {
diff --git a/src/__tests__/validation/__snapshots__/base.test.ts.snap b/src/__tests__/validation/__snapshots__/base.test.ts.snap
index 79de313..b2b9d27 100644
--- a/src/__tests__/validation/__snapshots__/base.test.ts.snap
+++ b/src/__tests__/validation/__snapshots__/base.test.ts.snap
@@ -57,5 +57,14 @@ exports[`Valdiation should validate  1`] = `
 /unknownPorp: invalid (unknown) property
 /xrayEnabled: must be boolean
 /visibility: must be \\"GLOBAL\\" or \\"PRIVATE\\"
+/introspection: must be boolean
+/queryDepthLimit: must be integer
+/resolverCountLimit: must be integer
+/environment: must be a valid environment definition
 /esbuild: must be an esbuild config object or false"
 `;
+
+exports[`Valdiation should validate  2`] = `
+"/queryDepthLimit: must be <= 75
+/resolverCountLimit: must be <= 1000"
+`;
diff --git a/src/__tests__/validation/base.test.ts b/src/__tests__/validation/base.test.ts
index 4f899d2..5824bc2 100644
--- a/src/__tests__/validation/base.test.ts
+++ b/src/__tests__/validation/base.test.ts
@@ -8,7 +8,14 @@ describe('Valdiation', () => {
       validateConfig({
         ...basicConfig,
         visibility: 'GLOBAL',
+        introspection: true,
+        queryDepthLimit: 10,
+        resolverCountLimit: 10,
         xrayEnabled: true,
+        environment: {
+          MY_TABLE: 'my-table',
+          MY_OTHER_TABLE: { Ref: 'MyOtherTable' },
+        },
         tags: {
           foo: 'bar',
         },
@@ -23,9 +30,21 @@ describe('Valdiation', () => {
     expect(function () {
       validateConfig({
         visibility: 'FOO',
+        introspection: 10,
+        queryDepthLimit: 'foo',
+        resolverCountLimit: 'bar',
         xrayEnabled: 'BAR',
         unknownPorp: 'foo',
         esbuild: 'bad',
+        environment: 'Bad',
+      });
+    }).toThrowErrorMatchingSnapshot();
+
+    expect(function () {
+      validateConfig({
+        ...basicConfig,
+        queryDepthLimit: 76,
+        resolverCountLimit: 1001,
       });
     }).toThrowErrorMatchingSnapshot();
   });
diff --git a/src/__tests__/waf.test.ts b/src/__tests__/waf.test.ts
index 9575f66..9cdc83c 100644
--- a/src/__tests__/waf.test.ts
+++ b/src/__tests__/waf.test.ts
@@ -168,6 +168,27 @@ describe('Waf', () => {
         ),
       ).toMatchSnapshot();
     });
+
+    it('should generate a custom rule with ManagedRuleGroup', () => {
+      expect(
+        waf.buildWafRule(
+          {
+            name: 'MyRule1',
+            priority: 200,
+            overrideAction: {
+              None: {},
+            },
+            statement: {
+              ManagedRuleGroupStatement: {
+                Name: 'AWSManagedRulesCommonRuleSet',
+                VendorName: 'AWS',
+              },
+            },
+          },
+          'Base',
+        ),
+      ).toMatchSnapshot();
+    });
   });
 
   describe('ApiKey rules', () => {