Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config option to avoid automatically deleting an APIGW domain when other base path mappings exist #389

Closed
wants to merge 5 commits into from
Closed

Add config option to avoid automatically deleting an APIGW domain when other base path mappings exist #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f513c34
feature: Add config option to preserve existing APIGW domains when ot…
Sep 24, 2020
3ccbcf0
chore: README update
Sep 24, 2020
7b9432b
fix: Lint
Sep 24, 2020
e333868
chore: Codacy issues
Sep 24, 2020
6778a66
chore: Codacy issues
Sep 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ securityPolicy | tls_1_2 | The security policy to apply to the custom domain nam
allowPathMatching | false | When updating an existing api mapping this will match on the basePath instead of the API ID to find existing mappings for an upsate. This should only be used when changing API types. For example, migrating a REST API to an HTTP API. See Changing API Types for more information. |
| autoDomain | `false` | Toggles whether or not the plugin will run `create_domain/delete_domain` as part of `sls deploy/remove` so that multiple commands are not required. |
| autoDomainWaitFor | `120` | How long to wait for create_domain to finish before starting deployment if domain does not exist immediately. |
| preserveExternalPathMappings | `false` | When `autoDomain` is set to true, and a deployment is removed, setting this wto `true` checks for additional API Gateway base path mappings before automatically deleting the domain, and avoids doing so if they exist. |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo wto should be to




Expand Down
2 changes: 2 additions & 0 deletions src/DomainConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class DomainConfig {
public securityPolicy: string | undefined;
public autoDomain: boolean | undefined;
public autoDomainWaitFor: string | undefined;
public preserveExternalPathMappings: boolean | undefined;

public domainInfo: DomainInfo | undefined;
public apiId: string | undefined;
Expand All @@ -44,6 +45,7 @@ class DomainConfig {
this.allowPathMatching = config.allowPathMatching;
this.autoDomain = config.autoDomain;
this.autoDomainWaitFor = config.autoDomainWaitFor;
this.preserveExternalPathMappings = config.preserveExternalPathMappings;

let basePath = config.basePath;
if (basePath == null || basePath.trim() === "") {
Expand Down
20 changes: 20 additions & 0 deletions src/aws/api-gateway-wrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,26 @@ class APIGatewayWrapper {
Globals.logInfo(`Unable to remove basepath mapping for ${domain.givenDomainName}`);
}
}

/**
* Checks for presence of external basepath mappings
*/
public async checkExternalPathMappings(domain: DomainConfig): Promise<boolean> {
Globals.logInfo(`Checking for additional base path mappings on ${domain.givenDomainName}...`);
const mappings = await getAWSPagedResults(
this.apiGatewayV2,
"getApiMappings",
"Items",
"NextToken",
"NextToken",
{DomainName: domain.givenDomainName},
);
return mappings.filter((mapping) =>
!(mapping.ApiId === domain.apiId ||
(mapping.ApiMappingKey === domain.basePath && domain.allowPathMatching)
),
).length > 0;
}
}

export = APIGatewayWrapper;
10 changes: 9 additions & 1 deletion src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,8 @@ class ServerlessCustomDomain {
*/
public async removeBasePathMappings(): Promise<void> {
await Promise.all(this.domains.map(async (domain) => {
const preserveExternalPathMappings = !!domain.preserveExternalPathMappings;
let noExternalPathMappingsOnDomain = !preserveExternalPathMappings;
try {
domain.apiId = await this.getApiId(domain);

Expand All @@ -315,7 +317,13 @@ class ServerlessCustomDomain {
} else {
domain.apiMapping = await this.apiGatewayWrapper.getBasePathMapping(domain);
await this.apiGatewayWrapper.deleteBasePathMapping(domain);
if (preserveExternalPathMappings) {
noExternalPathMappingsOnDomain = (
await this.apiGatewayWrapper.checkExternalPathMappings(domain) === false
);
}
}

} catch (err) {
if (err.message.indexOf("Failed to find CloudFormation") > -1) {
Globals.logInfo(`Unable to find Cloudformation Stack for ${domain.givenDomainName},
Expand All @@ -329,7 +337,7 @@ class ServerlessCustomDomain {
}

const autoDomain = domain.autoDomain;
if (autoDomain === true) {
if (autoDomain === true && noExternalPathMappingsOnDomain) {
Globals.logInfo("Deleting domain name after removing base path mapping.");
await this.deleteDomain(domain);
}
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface CustomDomain { // tslint:disable-line
autoDomain: boolean | undefined;
autoDomainWaitFor: string | undefined;
allowPathMatching: boolean | undefined;
preserveExternalPathMappings: boolean | undefined;
}

export interface ServerlessInstance { // tslint:disable-line
Expand Down
108 changes: 108 additions & 0 deletions test/unit-tests/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const constructPlugin = (customDomainOptions, multiple: boolean = false) => {
endpointType: customDomainOptions.endpointType,
hostedZoneId: customDomainOptions.hostedZoneId,
hostedZonePrivate: customDomainOptions.hostedZonePrivate,
preserveExternalPathMappings: customDomainOptions.preserveExternalPathMappings,
securityPolicy: customDomainOptions.securityPolicy,
stage: customDomainOptions.stage,
};
Expand Down Expand Up @@ -1840,7 +1841,114 @@ describe("Custom Domain Plugin", () => {
expect(spy).to.have.not.been.called();
});

it("removeBasePathMapping should not call deleteDomain when preserveExternalPathMappings is true and " +
"external mappings exist", async () => {
AWS.mock("CloudFormation", "describeStackResource", (params, callback) => {
callback(null, {
StackResourceDetail:
{
LogicalResourceId: "ApiGatewayRestApi",
PhysicalResourceId: "test_rest_api_id",
},
});
});
AWS.mock("ApiGatewayV2", "getApiMappings", (params, callback) => {
callback(null, {
Items: [
{ApiId: "test_rest_api_id", MappingKey: "test", ApiMappingId: "test_mapping_id", Stage: "test"},
{
ApiId: "test_rest_api_id_2",
ApiMappingId: "test_mapping_id",
MappingKey: "test",
Stage: "test",
},
],
});
});
AWS.mock("ApiGatewayV2", "deleteApiMapping", (params, callback) => {
callback(null, params);
});
AWS.mock("ApiGatewayV2", "deleteDomainName", (params, callback) => {
callback(null, params);
});
AWS.mock("ApiGatewayV2", "getDomainName", (params, callback) => {
callback(null, params);
});

const plugin = constructPlugin({
autoDomain: true,
basePath: "test_basepath",
createRoute53Record: false,
domainName: "test_domain",
preserveExternalPathMappings: true,
restApiId: "test_rest_api_id",
});
plugin.initializeVariables();
plugin.initAWSResources();

plugin.domains[0].apiMapping = {ApiMappingId: "test_mapping_id"};

const spy = chai.spy.on(plugin.apiGatewayWrapper.apiGatewayV2, "deleteDomainName");

await plugin.removeBasePathMappings();

expect(plugin.serverless.service.custom.customDomain.autoDomain).to.equal(true);
expect(plugin.serverless.service.custom.customDomain.preserveExternalPathMappings).to.equal(true);
expect(spy).to.have.not.been.called();
});

it("removeBasePathMapping should call deleteDomain when preserveExternalPathMappings is true and " +
"external mappings don't exist", async () => {
AWS.mock("CloudFormation", "describeStackResource", (params, callback) => {
callback(null, {
StackResourceDetail:
{
LogicalResourceId: "ApiGatewayRestApi",
PhysicalResourceId: "test_rest_api_id",
},
});
});
AWS.mock("ApiGatewayV2", "getApiMappings", (params, callback) => {
callback(null, {
Items: [
{ApiId: "test_rest_api_id", MappingKey: "test", ApiMappingId: "test_mapping_id", Stage: "test"},
],
});
});
AWS.mock("ApiGatewayV2", "deleteApiMapping", (params, callback) => {
callback(null, params);
});
AWS.mock("ApiGatewayV2", "deleteDomainName", (params, callback) => {
callback(null, params);
});
AWS.mock("ApiGatewayV2", "getDomainName", (params, callback) => {
callback(null, params);
});

const plugin = constructPlugin({
autoDomain: true,
basePath: "test_basepath",
createRoute53Record: false,
domainName: "test_domain",
preserveExternalPathMappings: true,
restApiId: "test_rest_api_id",
});
plugin.initializeVariables();
plugin.initAWSResources();

plugin.domains[0].apiMapping = {ApiMappingId: "test_mapping_id"};

const spy = chai.spy.on(plugin.apiGatewayWrapper.apiGatewayV2, "deleteDomainName");

await plugin.removeBasePathMappings();

expect(plugin.serverless.service.custom.customDomain.autoDomain).to.equal(true);
expect(plugin.serverless.service.custom.customDomain.preserveExternalPathMappings).to.equal(true);
expect(spy).to.have.been.called();
});

afterEach(() => {
AWS.restore();
consoleOutput = [];
});
});
Expand Down