Skip to content

Commit

Permalink
Merge pull request #620 from amplify-education/hotfix/api-mappings-fi…
Browse files Browse the repository at this point in the history
…ltering

Filter by stage for the removing API mapping
  • Loading branch information
rddimon authored Mar 6, 2024
2 parents 9675a20 + 8e7f8d9 commit 5bd5af8
Show file tree
Hide file tree
Showing 10 changed files with 4,393 additions and 785 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5,092 changes: 4,338 additions & 754 deletions package-lock.json

Large diffs are not rendered by default.

36 changes: 18 additions & 18 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "serverless-domain-manager",
"version": "7.3.6",
"version": "7.3.7",
"engines": {
"node": ">=14"
},
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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"
Expand Down
12 changes: 8 additions & 4 deletions src/aws/api-gateway-v1-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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}`
);
Expand Down Expand Up @@ -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}`
);
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/aws/api-gateway-v2-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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}`
);
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/models/apigateway-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand Down
7 changes: 6 additions & 1 deletion test/unit-tests/aws/api-gateway-v1-wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}]
});

Expand All @@ -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);
Expand Down
8 changes: 7 additions & 1 deletion test/unit-tests/aws/api-gateway-v2-wrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}]
});

Expand All @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions test/unit-tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
});

Expand Down

0 comments on commit 5bd5af8

Please sign in to comment.