Skip to content

Commit

Permalink
Merge pull request #7033 from serverless/improve-apigw-config-sdk-han…
Browse files Browse the repository at this point in the history 
…dling

Update API GW stage settings only when explicitly set
  • Loading branch information
@medikoo
medikoo committed Dec 3, 2019
2 parents 5e0c612 + 4efa19e commit 0b3f021
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 83 deletions.
108 changes: 58 additions & 50 deletions lib/plugins/aws/package/compile/events/apiGateway/lib/hack/updateStage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,54 +29,61 @@ module.exports = {
defaultApiGatewayLogLevel,
apiGatewayValidLogLevels,
updateStage() {
// this array is used to gather all the patch operations we need to
// perform on the stage
this.apiGatewayStagePatchOperations = [];
this.apiGatewayTagResourceParams = [];
this.apiGatewayUntagResourceParams = [];
this.apiGatewayStageState = {};
this.apiGatewayDeploymentId = null;
this.apiGatewayRestApiId = null;

return resolveAccountInfo
.call(this)
.then(resolveRestApiId.bind(this))
.then(() => {
if (!this.apiGatewayRestApiId) {
// Could not resolve REST API id automatically

const provider = this.state.service.provider;
const isTracingEnabled = provider.tracing && provider.tracing.apiGateway;
const areLogsEnabled = provider.logs && provider.logs.restApi;

if (!isTracingEnabled && !areLogsEnabled) {
// Do crash if there are no API Gateway customizations to apply
return null;
}
return BbPromise.try(() => {
const provider = this.state.service.provider;
this.hasTracingConfigured = provider.tracing && provider.tracing.apiGateway != null;
this.hasLogsConfigured = provider.logs && provider.logs.restApi != null;
this.hasTagsConfigured = provider.tags != null || provider.stackTags != null;

const errorMessage = [
'Rest API id could not be resolved.\n',
'This might be caused by a custom API Gateway configuration.\n\n',
'In given setup stage specific options such as ',
'`tracing`, `logs` and `tags` are not supported.\n\n',
'Please update your configuration (or open up an issue if you feel ',
"that there's a way to support your setup).",
].join('');
if (!this.hasTracingConfigured && !this.hasLogsConfigured && !this.hasTagsConfigured) {
return null;
}

throw new ServerlessError(errorMessage);
}
return resolveStage
.call(this)
.then(resolveDeploymentId.bind(this))
.then(ensureStage.bind(this))
.then(handleTracing.bind(this))
.then(handleLogs.bind(this))
.then(handleTags.bind(this))
.then(applyUpdates.bind(this))
.then(addTags.bind(this))
.then(removeTags.bind(this))
.then(removeAccessLoggingLogGroup.bind(this));
});
// this array is used to gather all the patch operations we need to
// perform on the stage
this.apiGatewayStagePatchOperations = [];
this.apiGatewayTagResourceParams = [];
this.apiGatewayUntagResourceParams = [];
this.apiGatewayStageState = {};
this.apiGatewayDeploymentId = null;
this.apiGatewayRestApiId = null;

return resolveAccountInfo
.call(this)
.then(resolveRestApiId.bind(this))
.then(() => {
if (!this.apiGatewayRestApiId) {
// Could not resolve REST API id automatically

if (!this.hasTracingConfigured && !this.hasLogsConfigured) {
// Do crash if there are no API Gateway customizations to apply
return null;
}

const errorMessage = [
'Rest API id could not be resolved.\n',
'This might be caused by a custom API Gateway configuration.\n\n',
'In given setup stage specific options such as ',
'`tracing`, `logs` and `tags` are not supported.\n\n',
'Please update your configuration (or open up an issue if you feel ',
"that there's a way to support your setup).",
].join('');

throw new ServerlessError(errorMessage);
}
return resolveStage
.call(this)
.then(resolveDeploymentId.bind(this))
.then(ensureStage.bind(this))
.then(handleTracing.bind(this))
.then(handleLogs.bind(this))
.then(handleTags.bind(this))
.then(applyUpdates.bind(this))
.then(addTags.bind(this))
.then(removeTags.bind(this))
.then(removeAccessLoggingLogGroup.bind(this));
});
});
},
};

Expand Down Expand Up @@ -182,8 +189,8 @@ function ensureStage() {
}

function handleTracing() {
const tracing = this.state.service.provider.tracing;
const tracingEnabled = tracing && tracing.apiGateway;
if (!this.hasTracingConfigured) return;
const tracingEnabled = this.state.service.provider.tracing.apiGateway;

let operation = { op: 'replace', path: '/tracingEnabled', value: 'false' };
if (tracingEnabled) {
Expand All @@ -193,8 +200,8 @@ function handleTracing() {
}

function handleLogs() {
const provider = this.state.service.provider;
const logs = provider.logs && provider.logs.restApi;
if (!this.hasLogsConfigured) return;
const logs = this.state.service.provider.logs.restApi;
const ops = this.apiGatewayStagePatchOperations;

let operations = [
Expand Down Expand Up @@ -270,6 +277,7 @@ function handleLogs() {
}

function handleTags() {
if (!this.hasTagsConfigured) return;
const provider = this.state.service.provider;
const tagsMerged = _.mapValues(Object.assign({}, provider.stackTags, provider.tags), v =>
String(v)
Expand Down
48 changes: 15 additions & 33 deletions lib/plugins/aws/package/compile/events/apiGateway/lib/hack/updateStage.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,19 +340,13 @@ describe('#updateStage()', () => {
});
});

it('should perform default actions if settings are not configure', () => {
it('should not perform any actions if settings are not configure', () => {
context.state.service.provider.tags = {
old: 'tag',
};
return updateStage.call(context).then(() => {
const patchOperations = [
{ op: 'replace', path: '/tracingEnabled', value: 'false' },
{ op: 'replace', path: '/*/*/logging/dataTrace', value: 'false' },
{ op: 'replace', path: '/*/*/logging/loglevel', value: 'OFF' },
];

expect(providerGetAccountInfoStub).to.be.calledOnce;
expect(providerRequestStub.args).to.have.length(4);
expect(providerRequestStub.args).to.have.length(3);
expect(providerRequestStub.args[0][0]).to.equal('APIGateway');
expect(providerRequestStub.args[0][1]).to.equal('getRestApis');
expect(providerRequestStub.args[0][2]).to.deep.equal({
Expand All @@ -365,22 +359,16 @@ describe('#updateStage()', () => {
restApiId: 'devRestApiId',
stageName: 'dev',
});
expect(providerRequestStub.args[2][0]).to.equal('APIGateway');
expect(providerRequestStub.args[2][1]).to.equal('updateStage');
expect(providerRequestStub.args[2][0]).to.equal('CloudWatchLogs');
expect(providerRequestStub.args[2][1]).to.equal('deleteLogGroup');
expect(providerRequestStub.args[2][2]).to.deep.equal({
restApiId: 'devRestApiId',
stageName: 'dev',
patchOperations,
});
expect(providerRequestStub.args[3][0]).to.equal('CloudWatchLogs');
expect(providerRequestStub.args[3][1]).to.equal('deleteLogGroup');
expect(providerRequestStub.args[3][2]).to.deep.equal({
logGroupName: '/aws/api-gateway/my-service-dev',
});
});
});

it('should create a new stage and proceed as usual if none can be found', () => {
context.state.service.provider.tracing = { apiGateway: false };
providerRequestStub
.withArgs('APIGateway', 'getStage', {
restApiId: 'devRestApiId',
Expand All @@ -406,11 +394,7 @@ describe('#updateStage()', () => {
.resolves();

return updateStage.call(context).then(() => {
const patchOperations = [
{ op: 'replace', path: '/tracingEnabled', value: 'false' },
{ op: 'replace', path: '/*/*/logging/dataTrace', value: 'false' },
{ op: 'replace', path: '/*/*/logging/loglevel', value: 'OFF' },
];
const patchOperations = [{ op: 'replace', path: '/tracingEnabled', value: 'false' }];

expect(providerGetAccountInfoStub).to.be.calledOnce;
expect(providerRequestStub.args).to.have.length(6);
Expand Down Expand Up @@ -455,6 +439,7 @@ describe('#updateStage()', () => {
});

it('should resolve custom restApiId', () => {
context.state.service.provider.tracing = { apiGateway: false };
providerRequestStub
.withArgs('APIGateway', 'getStage', {
restApiId: 'foobarfoo1',
Expand All @@ -472,6 +457,7 @@ describe('#updateStage()', () => {
});

it('should resolve custom APIGateway name', () => {
context.state.service.provider.tracing = { apiGateway: false };
providerRequestStub
.withArgs('APIGateway', 'getRestApis', {
limit: 500,
Expand Down Expand Up @@ -499,6 +485,7 @@ describe('#updateStage()', () => {
});

it('should resolve custom APIGateway resource', () => {
context.state.service.provider.tracing = { apiGateway: false };
const resources = context.serverless.service.provider.compiledCloudFormationTemplate.Resources;
resources.CustomApiGatewayRestApi = resources.ApiGatewayRestApi;
delete resources.ApiGatewayRestApi;
Expand All @@ -508,6 +495,7 @@ describe('#updateStage()', () => {
});

it('should resolve with a default api name if the AWS::ApiGateway::Resource is not present', () => {
context.state.service.provider.tracing = { apiGateway: false };
const resources = context.serverless.service.provider.compiledCloudFormationTemplate.Resources;
delete resources.ApiGatewayRestApi;
options.stage = 'prod';
Expand All @@ -517,6 +505,7 @@ describe('#updateStage()', () => {
});

it('should resolve expected restApiId when beyond 500 APIs are deployed', () => {
context.state.service.provider.tracing = { apiGateway: false };
providerRequestStub
.withArgs('APIGateway', 'getRestApis', {
limit: 500,
Expand Down Expand Up @@ -562,7 +551,6 @@ describe('#updateStage()', () => {

return updateStage.call(context).then(() => {
const patchOperations = [
{ op: 'replace', path: '/tracingEnabled', value: 'false' },
{
op: 'replace',
path: '/accessLogSettings/destinationArn',
Expand All @@ -578,7 +566,7 @@ describe('#updateStage()', () => {
];

expect(providerGetAccountInfoStub).to.be.calledOnce;
expect(providerRequestStub.args).to.have.length(4);
expect(providerRequestStub.args).to.have.length(3);
expect(providerRequestStub.args[0][0]).to.equal('APIGateway');
expect(providerRequestStub.args[0][1]).to.equal('getRestApis');
expect(providerRequestStub.args[0][2]).to.deep.equal({
Expand All @@ -598,12 +586,6 @@ describe('#updateStage()', () => {
stageName: 'dev',
patchOperations,
});
expect(providerRequestStub.args[3][0]).to.equal('APIGateway');
expect(providerRequestStub.args[3][1]).to.equal('untagResource');
expect(providerRequestStub.args[3][2]).to.deep.equal({
resourceArn: 'arn:aws:apigateway:us-east-1::/restapis/devRestApiId/stages/dev',
tagKeys: ['old'],
});
});
});

Expand Down Expand Up @@ -684,9 +666,9 @@ describe('#updateStage()', () => {
};

return updateStage.call(context).then(() => {
expect(providerRequestStub.args[4][0]).to.equal('CloudWatchLogs');
expect(providerRequestStub.args[4][1]).to.equal('deleteLogGroup');
expect(providerRequestStub.args[4][2]).to.deep.equal({
expect(providerRequestStub.args[3][0]).to.equal('CloudWatchLogs');
expect(providerRequestStub.args[3][1]).to.equal('deleteLogGroup');
expect(providerRequestStub.args[3][2]).to.deep.equal({
logGroupName: '/aws/api-gateway/my-service-dev',
});
});
Expand Down

0 comments on commit 0b3f021

Please sign in to comment.