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

Jump to bottom

Update API GW stage settings only when explicitly set #7033

Merged
merged 3 commits into from
Dec 3, 2019
Merged

Update API GW stage settings only when explicitly set #7033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c50f54d
Improve configuration extemption
medikoo Dec 2, 2019
77be626
Ensure to return promise
medikoo Dec 2, 2019
4efa19e
Update configuration only if explicitly set
medikoo Dec 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
108 changes: 58 additions & 50 deletions lib/plugins/aws/package/compile/events/apiGateway/lib/hack/updateStage.js
View file
Open in desktop
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
Open in desktop
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