Merge pull request #620 from amplify-education/hotfix/api-mappings-fi…

…ltering Filter by stage for the removing API mapping
amplify-education · Mar 6, 2024 · 5bd5af8 · 5bd5af8
2 parents 9675a20 + 8e7f8d9
commit 5bd5af8
Show file tree

Hide file tree

Showing 10 changed files with 4,393 additions and 785 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [7.3.7] - 2023-03-06
+
+### Fixed
+- Added filtering by stage for removing API mappings. That filter is skipped in case `allowPathMatching` is enabled.
+- Added throwing an error for a non-existing custom domain during base path mappings setup.
+
 ## [7.3.6] - 2023-02-13
 
 ### Changed

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-domain-manager",
-  "version": "7.3.6",
+  "version": "7.3.7",
   "engines": {
     "node": ">=14"
   },
@@ -50,8 +50,8 @@
   },
   "devDependencies": {
     "@types/mocha": "^10.0.6",
-    "@types/node": "^20.11.10",
-    "@types/randomstring": "^1.1.11",
+    "@types/node": "^20.11.24",
+    "@types/randomstring": "^1.1.12",
     "@types/shelljs": "^0.8.15",
     "aws-sdk-client-mock": "^3.0.1",
     "chai": "^4.4.1",
@@ -63,7 +63,7 @@
     "eslint-plugin-promise": "^5.2.0",
     "@typescript-eslint/parser": "^5.62.0",
     "@typescript-eslint/eslint-plugin": "^5.62.0",
-    "mocha": "^10.2.0",
+    "mocha": "^10.3.0",
     "mocha-param": "^2.0.1",
     "nyc": "^15.1.0",
     "randomstring": "^1.3.0",
@@ -74,20 +74,20 @@
     "typescript": "^5.1.6 && <5.2"
   },
   "dependencies": {
-    "@aws-sdk/client-acm": "^3.460.0",
-    "@aws-sdk/client-api-gateway": "^3.460.0",
-    "@aws-sdk/client-apigatewayv2": "^3.460.0",
-    "@aws-sdk/client-cloudformation": "^3.460.0",
-    "@aws-sdk/client-route-53": "^3.460.0",
-    "@aws-sdk/client-s3": "^3.460.0",
-    "@aws-sdk/credential-providers": "^3.460.0",
-    "@smithy/config-resolver": "^2.0.19",
-    "@smithy/node-config-provider": "^2.1.6",
-    "@smithy/node-http-handler": "^2.1.10",
-    "@smithy/smithy-client": "^2.1.16",
-    "@smithy/types": "^2.6.0",
-    "@smithy/util-retry": "^2.0.7",
-    "proxy-agent": "^6.3.1"
+    "@aws-sdk/client-acm": "^3.525.0",
+    "@aws-sdk/client-api-gateway": "^3.525.0",
+    "@aws-sdk/client-apigatewayv2": "^3.525.0",
+    "@aws-sdk/client-cloudformation": "^3.526.0",
+    "@aws-sdk/client-route-53": "^3.525.0",
+    "@aws-sdk/client-s3": "^3.525.0",
+    "@aws-sdk/credential-providers": "^3.525.0",
+    "@smithy/config-resolver": "^2.1.4",
+    "@smithy/node-config-provider": "^2.2.4",
+    "@smithy/node-http-handler": "^2.4.1",
+    "@smithy/smithy-client": "^2.4.2",
+    "@smithy/types": "^2.10.1",
+    "@smithy/util-retry": "^2.1.3",
+    "proxy-agent": "^6.4.0"
   },
   "peerDependencies": {
     "serverless": "^2.60 || ^3.0.0"

diff --git a/src/aws/api-gateway-v1-wrapper.ts b/src/aws/api-gateway-v1-wrapper.ts
@@ -81,7 +81,12 @@ class APIGatewayV1Wrapper extends APIGatewayBase {
     }
   }
 
-  public async getCustomDomain (domain: DomainConfig): Promise<DomainInfo> {
+  /**
+   * Get Custom Domain Info
+   * @param domain: DomainConfig
+   * @param silent: To issue an error or not. Not by default.
+   */
+  public async getCustomDomain (domain: DomainConfig, silent: boolean = true): Promise<DomainInfo> {
     // Make API call
     try {
       const domainInfo: GetDomainNameCommandOutput = await this.apiGateway.send(
@@ -91,7 +96,7 @@ class APIGatewayV1Wrapper extends APIGatewayBase {
       );
       return new DomainInfo(domainInfo);
     } catch (err) {
-      if (!err.$metadata || err.$metadata.httpStatusCode !== 404) {
+      if (!err.$metadata || err.$metadata.httpStatusCode !== 404 || !silent) {
         throw new Error(
           `V1 - Unable to fetch information about '${domain.givenDomainName}':\n${err.message}`
         );
@@ -122,8 +127,7 @@ class APIGatewayV1Wrapper extends APIGatewayBase {
       Logging.logInfo(`V1 - Created API mapping '${domain.basePath}' for '${domain.givenDomainName}'`);
     } catch (err) {
       throw new Error(
-        `V1 - Make sure the '${domain.givenDomainName}' exists.
-                 Unable to create base path mapping for '${domain.givenDomainName}':\n${err.message}`
+        `V1 - Unable to create base path mapping for '${domain.givenDomainName}':\n${err.message}`
       );
     }
   }

diff --git a/src/aws/api-gateway-v2-wrapper.ts b/src/aws/api-gateway-v2-wrapper.ts
@@ -83,8 +83,9 @@ class APIGatewayV2Wrapper extends APIGatewayBase {
   /**
    * Get Custom Domain Info
    * @param domain: DomainConfig
+   * @param silent: To issue an error or not. Not by default.
    */
-  public async getCustomDomain (domain: DomainConfig): Promise<DomainInfo> {
+  public async getCustomDomain (domain: DomainConfig, silent: boolean = true): Promise<DomainInfo> {
     // Make API call
     try {
       const domainInfo: GetDomainNameCommandOutput = await this.apiGateway.send(
@@ -94,7 +95,7 @@ class APIGatewayV2Wrapper extends APIGatewayBase {
       );
       return new DomainInfo(domainInfo);
     } catch (err) {
-      if (!err.$metadata || err.$metadata.httpStatusCode !== 404) {
+      if (!err.$metadata || err.$metadata.httpStatusCode !== 404 || !silent) {
         throw new Error(
           `V2 - Unable to fetch information about '${domain.givenDomainName}':\n${err.message}`
         );

diff --git a/src/index.ts b/src/index.ts
@@ -353,7 +353,7 @@ class ServerlessCustomDomain {
         return mapping.apiId === domain.apiId;
       });
       domain.apiMapping = filteredMappings ? filteredMappings[0] : null;
-      domain.domainInfo = await apiGateway.getCustomDomain(domain);
+      domain.domainInfo = await apiGateway.getCustomDomain(domain, false);
 
       if (!domain.apiMapping) {
         await apiGateway.createBasePathMapping(domain);
@@ -385,7 +385,7 @@ class ServerlessCustomDomain {
             if (domain.allowPathMatching) {
               return mapping.basePath === domain.basePath;
             }
-            return mapping.apiId === domain.apiId;
+            return mapping.apiId === domain.apiId && mapping.stage === domain.stage;
           });
           if (domain.preserveExternalPathMappings) {
             externalBasePathExists = mappings.length > filteredMappings.length;

diff --git a/src/models/apigateway-base.ts b/src/models/apigateway-base.ts
@@ -5,7 +5,7 @@ import DomainConfig = require("./domain-config");
 abstract class APIGatewayBase {
     abstract createCustomDomain(domain: DomainConfig): Promise<DomainInfo>;
 
-    abstract getCustomDomain(domain: DomainConfig): Promise<DomainInfo>;
+    abstract getCustomDomain(domain: DomainConfig, silent?: boolean): Promise<DomainInfo>;
 
     abstract deleteCustomDomain(domain: DomainConfig): Promise<void>;
 

diff --git a/test/unit-tests/aws/api-gateway-v1-wrapper.test.ts b/test/unit-tests/aws/api-gateway-v1-wrapper.test.ts
@@ -339,6 +339,10 @@ describe("API Gateway V1 wrapper checks", () => {
           restApiId: "test_rest_api_id",
           basePath: "test",
           stage: "test"
+        },{
+          restApiId: "test_rest_api_id2",
+          basePath: "test2",
+          stage: "dummy"
         }]
       });
 
@@ -349,7 +353,8 @@ describe("API Gateway V1 wrapper checks", () => {
 
       const actualResult = await apiGatewayV1Wrapper.getBasePathMappings(dc);
       const expectedResult = [
-        new ApiGatewayMap("test_rest_api_id", "test", "test", null)
+        new ApiGatewayMap("test_rest_api_id", "test", "test", null),
+        new ApiGatewayMap("test_rest_api_id2", "test2", "dummy", null)
       ];
 
       expect(actualResult).to.eql(expectedResult);

diff --git a/test/unit-tests/aws/api-gateway-v2-wrapper.test.ts b/test/unit-tests/aws/api-gateway-v2-wrapper.test.ts
@@ -372,6 +372,11 @@ describe("API Gateway V2 wrapper checks", () => {
           ApiMappingKey: "test",
           Stage: "test",
           ApiMappingId: "test_id"
+        },{
+          ApiId: "test_rest_api_id2",
+          ApiMappingKey: "test2",
+          Stage: "dummy",
+          ApiMappingId: "test_id2"
         }]
       });
 
@@ -382,7 +387,8 @@ describe("API Gateway V2 wrapper checks", () => {
 
       const actualResult = await apiGatewayV2Wrapper.getBasePathMappings(dc);
       const expectedResult = [
-        new ApiGatewayMap("test_rest_api_id", "test", "test", "test_id")
+        new ApiGatewayMap("test_rest_api_id", "test", "test", "test_id"),
+        new ApiGatewayMap("test_rest_api_id2", "test2", "dummy", "test_id2")
       ];
 
       expect(actualResult).to.eql(expectedResult);

diff --git a/test/unit-tests/index.test.ts b/test/unit-tests/index.test.ts
@@ -490,6 +490,10 @@ describe("Custom Domain Plugin", () => {
       });
       APIGatewayMock.on(GetBasePathMappingsCommand).resolves({
         items: [{
+          restApiId: "test_rest_api_id",
+          basePath: "test2",
+          stage: "dummy"
+        }, {
           restApiId: "test_rest_api_id",
           basePath: "test",
           stage: "test"
@@ -518,8 +522,6 @@ describe("Custom Domain Plugin", () => {
       const deleteDomainSpy = chaiSpy.on(plugin, "deleteDomain");
       await plugin.hooks["before:remove:remove"]();
 
-      const commandCalls = APIGatewayMock.commandCalls(DeleteBasePathMappingCommand);
-      expect(commandCalls.length).to.equal(1);
       expect(deleteDomainSpy).to.have.been.called();
     });