From f2fdc0e10ba26f283f098283512f22c0f2c57887 Mon Sep 17 00:00:00 2001 From: exoego Date: Tue, 19 Feb 2019 22:35:08 +0900 Subject: [PATCH 01/12] Add AWS APIGateway Stage resource to configure stage. --- lib/plugins/aws/lib/naming.js | 3 + .../compile/events/apiGateway/index.js | 3 + .../compile/events/apiGateway/index.test.js | 6 +- .../compile/events/apiGateway/lib/apiKeys.js | 2 +- .../events/apiGateway/lib/apiKeys.test.js | 4 +- .../events/apiGateway/lib/deployment.js | 4 -- .../events/apiGateway/lib/deployment.test.js | 1 - .../compile/events/apiGateway/lib/stage.js | 37 +++++++++++ .../events/apiGateway/lib/stage.test.js | 65 +++++++++++++++++++ 9 files changed, 116 insertions(+), 9 deletions(-) create mode 100644 lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js create mode 100644 lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js diff --git a/lib/plugins/aws/lib/naming.js b/lib/plugins/aws/lib/naming.js index 21631aa63..1b747305e 100644 --- a/lib/plugins/aws/lib/naming.js +++ b/lib/plugins/aws/lib/naming.js @@ -245,6 +245,9 @@ module.exports = { getUsagePlanKeyLogicalId(usagePlanKeyNumber) { return `ApiGatewayUsagePlanKey${usagePlanKeyNumber}`; }, + getStageLogicalId() { + return 'ApiGatewayStage'; + }, // S3 getDeploymentBucketLogicalId() { diff --git a/lib/plugins/aws/package/compile/events/apiGateway/index.js b/lib/plugins/aws/package/compile/events/apiGateway/index.js index 12dd5a692..197065c6b 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/index.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/index.js @@ -13,6 +13,7 @@ const compileMethods = require('./lib/method/index'); const compileAuthorizers = require('./lib/authorizers'); const compileDeployment = require('./lib/deployment'); const compilePermissions = require('./lib/permissions'); +const compileStage = require('./lib/stage'); const getMethodAuthorization = require('./lib/method/authorization'); const getMethodIntegration = require('./lib/method/integration'); const getMethodResponses = require('./lib/method/responses'); @@ -37,6 +38,7 @@ class AwsCompileApigEvents { compileCors, compileMethods, compileAuthorizers, + compileStage, compileDeployment, compilePermissions, getMethodAuthorization, @@ -58,6 +60,7 @@ class AwsCompileApigEvents { .then(this.compileCors) .then(this.compileMethods) .then(this.compileAuthorizers) + .then(this.compileStage) .then(this.compileDeployment) .then(this.compileApiKeys) .then(this.compileUsagePlan) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/index.test.js b/lib/plugins/aws/package/compile/events/apiGateway/index.test.js index ee08997f4..bf47ea7d0 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/index.test.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/index.test.js @@ -41,6 +41,7 @@ describe('AwsCompileApigEvents', () => { let compileDeploymentStub; let compileUsagePlanStub; let compilePermissionsStub; + let compileStageStub; let disassociateUsagePlanStub; beforeEach(() => { @@ -56,6 +57,8 @@ describe('AwsCompileApigEvents', () => { .stub(awsCompileApigEvents, 'compileUsagePlan').resolves(); compilePermissionsStub = sinon .stub(awsCompileApigEvents, 'compilePermissions').resolves(); + compileStageStub = sinon + .stub(awsCompileApigEvents, 'compileStage').resolves(); disassociateUsagePlanStub = sinon .stub(disassociateUsagePlan, 'disassociateUsagePlan').resolves(); }); @@ -97,7 +100,8 @@ describe('AwsCompileApigEvents', () => { expect(compileRestApiStub.calledAfter(validateStub)).to.be.equal(true); expect(compileResourcesStub.calledAfter(compileRestApiStub)).to.be.equal(true); expect(compileMethodsStub.calledAfter(compileResourcesStub)).to.be.equal(true); - expect(compileDeploymentStub.calledAfter(compileMethodsStub)).to.be.equal(true); + expect(compileStageStub.calledAfter(compileMethodsStub)).to.be.equal(true); + expect(compileDeploymentStub.calledAfter(compileStageStub)).to.be.equal(true); expect(compileUsagePlanStub.calledAfter(compileDeploymentStub)).to.be.equal(true); expect(compilePermissionsStub.calledAfter(compileUsagePlanStub)).to.be.equal(true); diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/apiKeys.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/apiKeys.js index 069455a3e..42530ec81 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/apiKeys.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/apiKeys.js @@ -31,7 +31,7 @@ module.exports = { StageName: this.provider.getStage(), }], }, - DependsOn: this.apiGatewayDeploymentLogicalId, + DependsOn: this.apiGatewayStageLogicalId, }, }); }); diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/apiKeys.test.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/apiKeys.test.js index 188eb1743..f36ff0e6a 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/apiKeys.test.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/apiKeys.test.js @@ -27,7 +27,7 @@ describe('#compileApiKeys()', () => { }; awsCompileApigEvents = new AwsCompileApigEvents(serverless, options); awsCompileApigEvents.apiGatewayRestApiLogicalId = 'ApiGatewayRestApi'; - awsCompileApigEvents.apiGatewayDeploymentLogicalId = 'ApiGatewayDeploymentTest'; + awsCompileApigEvents.apiGatewayStageLogicalId = 'ApiGatewayStageTest'; }); it('should compile api key resource', () => @@ -72,7 +72,7 @@ describe('#compileApiKeys()', () => { .Resources[ awsCompileApigEvents.provider.naming.getApiKeyLogicalId(1) ].DependsOn - ).to.equal('ApiGatewayDeploymentTest'); + ).to.equal('ApiGatewayStageTest'); }) ); diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/deployment.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/deployment.js index 9eb923ba5..d88a18088 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/deployment.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/deployment.js @@ -5,15 +5,11 @@ const BbPromise = require('bluebird'); module.exports = { compileDeployment() { - this.apiGatewayDeploymentLogicalId = this.provider.naming - .generateApiGatewayDeploymentLogicalId(); - _.merge(this.serverless.service.provider.compiledCloudFormationTemplate.Resources, { [this.apiGatewayDeploymentLogicalId]: { Type: 'AWS::ApiGateway::Deployment', Properties: { RestApiId: this.provider.getApiGatewayRestApiId(), - StageName: this.provider.getStage(), }, DependsOn: this.apiGatewayMethodLogicalIds, }, diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/deployment.test.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/deployment.test.js index 98be9ee27..ed9f40acc 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/deployment.test.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/deployment.test.js @@ -44,7 +44,6 @@ describe('#compileDeployment()', () => { RestApiId: { Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, }, - StageName: 'dev', }, }); }) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js new file mode 100644 index 000000000..31c544600 --- /dev/null +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js @@ -0,0 +1,37 @@ +'use strict'; + +const _ = require('lodash'); +const BbPromise = require('bluebird'); + +module.exports = { + compileStage() { + this.apiGatewayDeploymentLogicalId = this.provider.naming + .generateApiGatewayDeploymentLogicalId(); + this.apiGatewayStageLogicalId = this.provider.naming.getStageLogicalId(); + + const tagsDict = [this.provider.stackTags, this.provider.tags].reduce((lastTags, newTags) => { + if (_.isPlainObject(newTags)) { + return _.extend(lastTags, newTags); + } + return lastTags; + }, { STAGE: this.provider.getStage() }); + const Tags = _.entriesIn(tagsDict).map(pair => ({ + Key: pair[0], + Value: pair[1], + })); + + _.merge(this.serverless.service.provider.compiledCloudFormationTemplate.Resources, { + [this.apiGatewayStageLogicalId]: { + Type: 'AWS::ApiGateway::Stage', + Properties: { + DeploymentId: { Ref: this.apiGatewayDeploymentLogicalId }, + RestApiId: this.provider.getApiGatewayRestApiId(), + StageName: this.provider.getStage(), + Tags, + }, + }, + }); + + return BbPromise.resolve(); + }, +}; diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js new file mode 100644 index 000000000..b63ad7c5a --- /dev/null +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js @@ -0,0 +1,65 @@ +'use strict'; + +const expect = require('chai').expect; +const AwsCompileApigEvents = require('../index'); +const Serverless = require('../../../../../../../Serverless'); +const AwsProvider = require('../../../../../provider/awsProvider'); + +describe('#compileStage()', () => { + let serverless; + let provider; + let awsCompileApigEvents; + + beforeEach(() => { + const options = { + stage: 'dev', + region: 'us-east-1', + }; + serverless = new Serverless(); + provider = new AwsProvider(serverless, options); + serverless.setProvider('aws', provider); + serverless.service.provider.compiledCloudFormationTemplate = { + Resources: {}, + Outputs: {}, + }; + awsCompileApigEvents = new AwsCompileApigEvents(serverless, options); + awsCompileApigEvents.apiGatewayDeploymentLogicalId = 'DeploymentId'; + awsCompileApigEvents.apiGatewayRestApiLogicalId = 'ApiGatewayRestApi'; + awsCompileApigEvents.provider = provider; + }); + + it('should add tag from provider.stackTags and provider.tags', () => { + provider.stackTags = { + foo: 'bar', + // override stage + STAGE: 'middle-priority', + }; + provider.tags = { + // override stackTags + foo: 'high-priority', + }; + + awsCompileApigEvents.compileStage().then(() => { + const template = awsCompileApigEvents.serverless.service.provider + .compiledCloudFormationTemplate; + const stageLogicalId = Object.keys(template.Resources)[0]; + + expect(template.Resources[stageLogicalId]).to.deep.equal({ + Type: 'AWS::ApiGateway::Stage', + Properties: { + DeploymentId: { + Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, + }, + RestApiId: { + Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, + }, + StageName: 'dev', + Tags: [ + { Key: 'STAGE', Value: 'middle-priority' }, + { Key: 'foo', Value: 'high-priority' }, + ], + }, + }); + }); + }); +}); From 61175554dfae3f3aff1606ceec170733fb865585 Mon Sep 17 00:00:00 2001 From: exoego Date: Thu, 21 Mar 2019 16:11:34 +0900 Subject: [PATCH 02/12] Stage name should be unique between deployments. --- lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js index 31c544600..c8bfb4dfa 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js @@ -26,7 +26,7 @@ module.exports = { Properties: { DeploymentId: { Ref: this.apiGatewayDeploymentLogicalId }, RestApiId: this.provider.getApiGatewayRestApiId(), - StageName: this.provider.getStage(), + StageName: `${this.provider.getStage()}${(new Date()).getTime()}`, Tags, }, }, From af7e961e142aba3229285288671cbd3ba29408e8 Mon Sep 17 00:00:00 2001 From: exoego Date: Thu, 21 Mar 2019 16:23:01 +0900 Subject: [PATCH 03/12] compileStage should use apiGatewayDeploymentLogicalId generated in compileDeployment. --- lib/plugins/aws/package/compile/events/apiGateway/index.js | 2 +- lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/index.js b/lib/plugins/aws/package/compile/events/apiGateway/index.js index 197065c6b..d20f39702 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/index.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/index.js @@ -60,8 +60,8 @@ class AwsCompileApigEvents { .then(this.compileCors) .then(this.compileMethods) .then(this.compileAuthorizers) - .then(this.compileStage) .then(this.compileDeployment) + .then(this.compileStage) .then(this.compileApiKeys) .then(this.compileUsagePlan) .then(this.compileUsagePlanKeys) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js index c8bfb4dfa..0994b9832 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js @@ -5,8 +5,6 @@ const BbPromise = require('bluebird'); module.exports = { compileStage() { - this.apiGatewayDeploymentLogicalId = this.provider.naming - .generateApiGatewayDeploymentLogicalId(); this.apiGatewayStageLogicalId = this.provider.naming.getStageLogicalId(); const tagsDict = [this.provider.stackTags, this.provider.tags].reduce((lastTags, newTags) => { From 1476330092234888d3386b4b20726b8d026ff362 Mon Sep 17 00:00:00 2001 From: exoego Date: Thu, 21 Mar 2019 16:48:40 +0900 Subject: [PATCH 04/12] Fix test. --- .../aws/package/compile/events/apiGateway/index.test.js | 6 +++--- .../aws/package/compile/events/apiGateway/lib/stage.test.js | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/index.test.js b/lib/plugins/aws/package/compile/events/apiGateway/index.test.js index bf47ea7d0..c4ef0e033 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/index.test.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/index.test.js @@ -100,9 +100,9 @@ describe('AwsCompileApigEvents', () => { expect(compileRestApiStub.calledAfter(validateStub)).to.be.equal(true); expect(compileResourcesStub.calledAfter(compileRestApiStub)).to.be.equal(true); expect(compileMethodsStub.calledAfter(compileResourcesStub)).to.be.equal(true); - expect(compileStageStub.calledAfter(compileMethodsStub)).to.be.equal(true); - expect(compileDeploymentStub.calledAfter(compileStageStub)).to.be.equal(true); - expect(compileUsagePlanStub.calledAfter(compileDeploymentStub)).to.be.equal(true); + expect(compileDeploymentStub.calledAfter(compileMethodsStub)).to.be.equal(true); + expect(compileStageStub.calledAfter(compileDeploymentStub)).to.be.equal(true); + expect(compileUsagePlanStub.calledAfter(compileStageStub)).to.be.equal(true); expect(compilePermissionsStub.calledAfter(compileUsagePlanStub)).to.be.equal(true); awsCompileApigEvents.validate.restore(); diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js index b63ad7c5a..08fa2753b 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js @@ -44,7 +44,8 @@ describe('#compileStage()', () => { .compiledCloudFormationTemplate; const stageLogicalId = Object.keys(template.Resources)[0]; - expect(template.Resources[stageLogicalId]).to.deep.equal({ + const actual = template.Resources[stageLogicalId]; + expect(actual).to.deep.equal({ Type: 'AWS::ApiGateway::Stage', Properties: { DeploymentId: { @@ -53,7 +54,8 @@ describe('#compileStage()', () => { RestApiId: { Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, }, - StageName: 'dev', + // ignore StageName since it is random + StageName: actual.Properties.StageName, Tags: [ { Key: 'STAGE', Value: 'middle-priority' }, { Key: 'foo', Value: 'high-priority' }, From 7383281a6bd970bc1e66bc1128802b90b00adb03 Mon Sep 17 00:00:00 2001 From: exoego Date: Wed, 17 Apr 2019 11:34:29 +0900 Subject: [PATCH 05/12] Do not add Tags from stage implicitly to avoid breaking change. --- .../compile/events/apiGateway/lib/stage.js | 2 +- .../events/apiGateway/lib/stage.test.js | 145 +++++++++--------- 2 files changed, 75 insertions(+), 72 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js index 1c851d0ea..21f0f3b48 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js @@ -16,7 +16,7 @@ module.exports = { return _.extend(lastTags, newTags); } return lastTags; - }, { STAGE: this.provider.getStage() }); + }); const Tags = _.entriesIn(tagsDict).map(pair => ({ Key: pair[0], Value: pair[1], diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js index aa74b469e..2909eaf14 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js @@ -31,10 +31,6 @@ describe('#compileStage()', () => { stage = awsCompileApigEvents.provider.getStage(); stageLogicalId = awsCompileApigEvents.provider.naming .getStageLogicalId(); - // setting up AWS X-Ray tracing - awsCompileApigEvents.serverless.service.provider.tracing = { - apiGateway: true, - }; // mocking the result of a Deployment resource since we remove the stage name // when using the Stage resource awsCompileApigEvents.serverless.service.provider.compiledCloudFormationTemplate @@ -45,82 +41,89 @@ describe('#compileStage()', () => { }; }); - it('should create a dedicated stage resource if tracing is configured', () => awsCompileApigEvents - .compileStage().then(() => { - const resources = awsCompileApigEvents.serverless.service.provider - .compiledCloudFormationTemplate.Resources; + describe('tracing', () => { + beforeEach(() => { + // setting up AWS X-Ray tracing + awsCompileApigEvents.serverless.service.provider.tracing = { + apiGateway: true, + }; + }); - expect(resources[stageLogicalId]).to.deep.equal({ - Type: 'AWS::ApiGateway::Stage', - Properties: { - RestApiId: { - Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, + it('should create a dedicated stage resource if tracing is configured', () => + awsCompileApigEvents.compileStage().then(() => { + const resources = awsCompileApigEvents.serverless.service.provider + .compiledCloudFormationTemplate.Resources; + + expect(resources[stageLogicalId]).to.deep.equal({ + Type: 'AWS::ApiGateway::Stage', + Properties: { + RestApiId: { + Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, + }, + DeploymentId: { + Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, + }, + StageName: 'dev', + Tags: [], + TracingEnabled: true, }, - DeploymentId: { - Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, + }); + + expect(resources[awsCompileApigEvents.apiGatewayDeploymentLogicalId]).to.deep.equal({ + Properties: {}, + }); + }) + ); + + it('should NOT create a dedicated stage resource if tracing is not enabled', () => { + awsCompileApigEvents.serverless.service.provider.tracing = {}; + + return awsCompileApigEvents.compileStage().then(() => { + const resources = awsCompileApigEvents.serverless.service.provider + .compiledCloudFormationTemplate.Resources; + + // eslint-disable-next-line + expect(resources[stageLogicalId]).not.to.exist; + + expect(resources[awsCompileApigEvents.apiGatewayDeploymentLogicalId]).to.deep.equal({ + Properties: { + StageName: stage, }, - StageName: 'dev', - TracingEnabled: true, - }, - }); - - expect(resources[awsCompileApigEvents.apiGatewayDeploymentLogicalId]).to.deep.equal({ - Properties: {}, - }); - }) - ); - - it('should NOT create a dedicated stage resource if tracing is not enabled', () => { - awsCompileApigEvents.serverless.service.provider.tracing = {}; - - return awsCompileApigEvents.compileStage().then(() => { - const resources = awsCompileApigEvents.serverless.service.provider - .compiledCloudFormationTemplate.Resources; - - // eslint-disable-next-line - expect(resources[stageLogicalId]).not.to.exist; - - expect(resources[awsCompileApigEvents.apiGatewayDeploymentLogicalId]).to.deep.equal({ - Properties: { - StageName: stage, - }, + }); }); }); }); - it('should add tag from provider.stackTags and provider.tags', () => { - provider.stackTags = { - foo: 'bar', - // override stage - STAGE: 'middle-priority', - }; - provider.tags = { - // override stackTags - foo: 'high-priority', - }; + describe('tags', () => { + it('should add tag from provider.stackTags and provider.tags', () => { + provider.stackTags = { + foo: 'bar', + }; + provider.tags = { + // override stackTags + foo: 'high-priority', + }; - awsCompileApigEvents.compileStage().then(() => { - const template = awsCompileApigEvents.serverless.service.provider - .compiledCloudFormationTemplate; - const stageLogicalId = Object.keys(template.Resources)[0]; - - const actual = template.Resources[stageLogicalId]; - expect(actual).to.deep.equal({ - Type: 'AWS::ApiGateway::Stage', - Properties: { - DeploymentId: { - Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, + awsCompileApigEvents.compileStage().then(() => { + const template = awsCompileApigEvents.serverless.service.provider + .compiledCloudFormationTemplate; + const actual = template.Resources[stageLogicalId]; + expect(actual).to.deep.equal({ + Type: 'AWS::ApiGateway::Stage', + Properties: { + DeploymentId: { + Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, + }, + RestApiId: { + Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, + }, + StageName: stage, + TracingEnabled: false, + Tags: [ + { Key: 'foo', Value: 'high-priority' }, + ], }, - RestApiId: { - Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, - }, - // ignore StageName since it is random - StageName: actual.Properties.StageName, - Tags: [ - { Key: 'STAGE', Value: 'middle-priority' }, - { Key: 'foo', Value: 'high-priority' }, - ], - }, + }); }); }); }); From dd8b60675c7a2fcccdce6f32e43d68561df2dafa Mon Sep 17 00:00:00 2001 From: exoego Date: Wed, 17 Apr 2019 11:55:39 +0900 Subject: [PATCH 06/12] Split test into granular cases. --- .../events/apiGateway/lib/stage.test.js | 93 ++++++++++++++++--- 1 file changed, 79 insertions(+), 14 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js index 2909eaf14..124f8a8d0 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js @@ -95,32 +95,97 @@ describe('#compileStage()', () => { }); describe('tags', () => { - it('should add tag from provider.stackTags and provider.tags', () => { + it('should create a dedicated stage resource if provider.stackTags is configured', () => { provider.stackTags = { - foo: 'bar', - }; - provider.tags = { - // override stackTags - foo: 'high-priority', + foo: '1', }; awsCompileApigEvents.compileStage().then(() => { - const template = awsCompileApigEvents.serverless.service.provider - .compiledCloudFormationTemplate; - const actual = template.Resources[stageLogicalId]; - expect(actual).to.deep.equal({ + const resources = awsCompileApigEvents.serverless.service.provider + .compiledCloudFormationTemplate.Resources; + expect(resources[awsCompileApigEvents.apiGatewayDeploymentLogicalId]).to.deep.equal({ + Properties: {}, + }); + + expect(resources[stageLogicalId]).to.deep.equal({ Type: 'AWS::ApiGateway::Stage', Properties: { - DeploymentId: { - Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, - }, RestApiId: { Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, }, + DeploymentId: { + Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, + }, StageName: stage, TracingEnabled: false, Tags: [ - { Key: 'foo', Value: 'high-priority' }, + { Key: 'foo', Value: '1' }, + ], + }, + }); + }); + }); + + it('should create a dedicated stage resource if provider.tags is configured', () => { + provider.tags = { + foo: '1', + }; + + awsCompileApigEvents.compileStage().then(() => { + const resources = awsCompileApigEvents.serverless.service.provider + .compiledCloudFormationTemplate.Resources; + expect(resources[awsCompileApigEvents.apiGatewayDeploymentLogicalId]).to.deep.equal({ + Properties: {}, + }); + + expect(resources[stageLogicalId]).to.deep.equal({ + Type: 'AWS::ApiGateway::Stage', + Properties: { + RestApiId: { + Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, + }, + DeploymentId: { + Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, + }, + StageName: stage, + TracingEnabled: false, + Tags: [ + { Key: 'foo', Value: '1' }, + ], + }, + }); + }); + }); + + it('should override provider.stackTags by provider.tags', () => { + provider.stackTags = { + foo: 'from-stackTags', + bar: 'from-stackTags', + }; + provider.tags = { + foo: 'from-tags', + buz: 'from-tags', + }; + + awsCompileApigEvents.compileStage().then(() => { + const resources = awsCompileApigEvents.serverless.service.provider + .compiledCloudFormationTemplate.Resources; + + expect(resources[stageLogicalId]).to.deep.equal({ + Type: 'AWS::ApiGateway::Stage', + Properties: { + RestApiId: { + Ref: awsCompileApigEvents.apiGatewayRestApiLogicalId, + }, + DeploymentId: { + Ref: awsCompileApigEvents.apiGatewayDeploymentLogicalId, + }, + StageName: stage, + TracingEnabled: false, + Tags: [ + { Key: 'foo', Value: 'from-tags' }, + { Key: 'bar', Value: 'from-stackTags' }, + { Key: 'buz', Value: 'from-tags' }, ], }, }); From dd112c6a1bfd4aa51983985888876953c39b6423 Mon Sep 17 00:00:00 2001 From: exoego Date: Wed, 17 Apr 2019 11:56:53 +0900 Subject: [PATCH 07/12] Update comments. --- .../aws/package/compile/events/apiGateway/lib/stage.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js index 21f0f3b48..9e6ac2801 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js @@ -5,9 +5,6 @@ const BbPromise = require('bluebird'); module.exports = { compileStage() { - // NOTE: right now we're only using a dedicated Stage resource if AWS X-Ray - // tracing is enabled. We'll change this in the future so that users can - // opt-in for other features as well const tracing = this.serverless.service.provider.tracing; const TracingEnabled = !_.isEmpty(tracing) && tracing.apiGateway; @@ -27,6 +24,11 @@ module.exports = { this.apiGatewayStageLogicalId = this.provider.naming .getStageLogicalId(); + // NOTE: right now we're only using a dedicated Stage resource + // - if AWS X-Ray tracing is enabled + // - if Tags are provided + // We'll change this in the future so that users can + // opt-in for other features as well if (TracingEnabled || Tags.length > 0) { _.merge(this.serverless.service.provider.compiledCloudFormationTemplate.Resources, { [this.apiGatewayStageLogicalId]: { From d45884d1b238cbc07c0c599d8b8b2028e3d27c28 Mon Sep 17 00:00:00 2001 From: exoego Date: Wed, 17 Apr 2019 12:01:00 +0900 Subject: [PATCH 08/12] Remove unnecessary code. --- lib/plugins/aws/package/compile/events/apiGateway/index.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/index.js b/lib/plugins/aws/package/compile/events/apiGateway/index.js index 380e0e9c8..f062a9c7a 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/index.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/index.js @@ -39,7 +39,6 @@ class AwsCompileApigEvents { compileCors, compileMethods, compileAuthorizers, - compileStage, compileDeployment, compilePermissions, compileStage, @@ -64,7 +63,6 @@ class AwsCompileApigEvents { .then(this.compileMethods) .then(this.compileAuthorizers) .then(this.compileDeployment) - .then(this.compileStage) .then(this.compileApiKeys) .then(this.compileUsagePlan) .then(this.compileUsagePlanKeys) From d4a6f2ce0e070adf31d108d5b21858c12a60715a Mon Sep 17 00:00:00 2001 From: exoego Date: Wed, 17 Apr 2019 13:15:08 +0900 Subject: [PATCH 09/12] Rename to express intent --- .../aws/package/compile/events/apiGateway/lib/stage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js index 9e6ac2801..f0d2ebdce 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js @@ -8,13 +8,13 @@ module.exports = { const tracing = this.serverless.service.provider.tracing; const TracingEnabled = !_.isEmpty(tracing) && tracing.apiGateway; - const tagsDict = [this.provider.stackTags, this.provider.tags].reduce((lastTags, newTags) => { + const tagsMerged = [this.provider.stackTags, this.provider.tags].reduce((lastTags, newTags) => { if (_.isPlainObject(newTags)) { return _.extend(lastTags, newTags); } return lastTags; }); - const Tags = _.entriesIn(tagsDict).map(pair => ({ + const Tags = _.entriesIn(tagsMerged).map(pair => ({ Key: pair[0], Value: pair[1], })); From 16424ec5f72cf9d9c8df3e24231b9a37f3890d90 Mon Sep 17 00:00:00 2001 From: Philipp Muens Date: Thu, 18 Apr 2019 14:05:41 +0200 Subject: [PATCH 10/12] Fix code to properly reference the provider property --- .../aws/package/compile/events/apiGateway/lib/stage.js | 10 +++++++--- .../compile/events/apiGateway/lib/stage.test.js | 8 ++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js index f0d2ebdce..79a2228ba 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.js @@ -5,15 +5,19 @@ const BbPromise = require('bluebird'); module.exports = { compileStage() { - const tracing = this.serverless.service.provider.tracing; + const provider = this.serverless.service.provider; + + // TracingEnabled + const tracing = provider.tracing; const TracingEnabled = !_.isEmpty(tracing) && tracing.apiGateway; - const tagsMerged = [this.provider.stackTags, this.provider.tags].reduce((lastTags, newTags) => { + // Tags + const tagsMerged = [provider.stackTags, provider.tags].reduce((lastTags, newTags) => { if (_.isPlainObject(newTags)) { return _.extend(lastTags, newTags); } return lastTags; - }); + }, {}); const Tags = _.entriesIn(tagsMerged).map(pair => ({ Key: pair[0], Value: pair[1], diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js index 124f8a8d0..d2e33d1df 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/stage.test.js @@ -96,7 +96,7 @@ describe('#compileStage()', () => { describe('tags', () => { it('should create a dedicated stage resource if provider.stackTags is configured', () => { - provider.stackTags = { + awsCompileApigEvents.serverless.service.provider.stackTags = { foo: '1', }; @@ -127,7 +127,7 @@ describe('#compileStage()', () => { }); it('should create a dedicated stage resource if provider.tags is configured', () => { - provider.tags = { + awsCompileApigEvents.serverless.service.provider.tags = { foo: '1', }; @@ -158,11 +158,11 @@ describe('#compileStage()', () => { }); it('should override provider.stackTags by provider.tags', () => { - provider.stackTags = { + awsCompileApigEvents.serverless.service.provider.stackTags = { foo: 'from-stackTags', bar: 'from-stackTags', }; - provider.tags = { + awsCompileApigEvents.serverless.service.provider.tags = { foo: 'from-tags', buz: 'from-tags', }; From 66e94e998b26d0be38297035e8d0b7880517e037 Mon Sep 17 00:00:00 2001 From: Philipp Muens Date: Thu, 18 Apr 2019 14:05:58 +0200 Subject: [PATCH 11/12] Update error message --- .../events/apiGateway/lib/checkForBreakingChanges.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/plugins/aws/package/compile/events/apiGateway/lib/checkForBreakingChanges.js b/lib/plugins/aws/package/compile/events/apiGateway/lib/checkForBreakingChanges.js index a22877860..7593525b9 100644 --- a/lib/plugins/aws/package/compile/events/apiGateway/lib/checkForBreakingChanges.js +++ b/lib/plugins/aws/package/compile/events/apiGateway/lib/checkForBreakingChanges.js @@ -2,10 +2,6 @@ const BbPromise = require('bluebird'); -// NOTE: the checks here are X-Ray specific. However the error messages can be updated -// to reflect the general problem which occurrs when upgrading / downgrading the -// Stage resource / Deplyment resource - module.exports = { checkForBreakingChanges() { const StackName = this.provider.naming.getStackName(); @@ -27,7 +23,7 @@ module.exports = { // the old state still uses the stage defined on the AWS::ApiGateway::Deployment resource if (oldResources[oldDeploymentLogicalId] && oldResources[oldDeploymentLogicalId].Properties.StageName && newResources[stageLogicalId]) { // eslint-disable-line max-len const msg = [ - 'NOTE: Enabling API Gateway X-Ray Tracing for existing ', + 'NOTE: Enabling API Gateway stage settings for existing ', 'deployments requires a remove and re-deploy of your API Gateway. ', '\n\n ', 'Please refer to our documentation for more information.', @@ -40,7 +36,7 @@ module.exports = { if (oldResources[stageLogicalId] && newResources[newDeploymentLogicalId] && newResources[newDeploymentLogicalId].Properties.StageName) { // eslint-disable-line if (!this.options.force) { const msg = [ - 'NOTE: Disabling API Gateway X-Ray Tracing for existing ', + 'NOTE: Disabling API Gateway stage settings for existing ', 'deployments might result in unexpected behavior.', '\n ', 'We recommend to remove and re-deploy your API Gateway. ', From e6b764734f2cb7ab7c79214d8c97860b3448199f Mon Sep 17 00:00:00 2001 From: Philipp Muens Date: Thu, 18 Apr 2019 14:17:11 +0200 Subject: [PATCH 12/12] Add documentation --- docs/providers/aws/events/apigateway.md | 29 ++++++++++++++++++++----- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/docs/providers/aws/events/apigateway.md b/docs/providers/aws/events/apigateway.md index 63ebd2902..0a50158a4 100644 --- a/docs/providers/aws/events/apigateway.md +++ b/docs/providers/aws/events/apigateway.md @@ -280,7 +280,7 @@ functions: maxAge: 86400 ``` -If you are using CloudFront or another CDN for your API Gateway, you may want to setup a `Cache-Control` header to allow for OPTIONS request to be cached to avoid the additional hop. +If you are using CloudFront or another CDN for your API Gateway, you may want to setup a `Cache-Control` header to allow for OPTIONS request to be cached to avoid the additional hop. To enable the `Cache-Control` header on preflight response, set the `cacheControl` property in the `cors` object: @@ -1282,7 +1282,7 @@ functions: events: - http: path: /users - ... + ... authorizer: # Provide both type and authorizerId type: COGNITO_USER_POOLS # TOKEN or REQUEST or COGNITO_USER_POOLS, same as AWS Cloudformation documentation @@ -1294,7 +1294,7 @@ functions: events: - http: path: /users/{userId} - ... + ... # Provide both type and authorizerId type: COGNITO_USER_POOLS # TOKEN or REQUEST or COGNITO_USER_POOLS, same as AWS Cloudformation documentation authorizerId: @@ -1349,11 +1349,13 @@ provider: minimumCompressionSize: 1024 ``` -## AWS X-Ray Tracing +## Stage specific setups -**IMPORTANT:** Due to CloudFormation limitations it's not possible to enable AWS X-Ray Tracing on existing deployments. Please remove your old API Gateway and re-deploy it with enabled tracing if you want to use AWS X-Ray Tracing for API Gateway. Once tracing is enabled you can re-deploy your service anytime without issues. +**IMPORTANT:** Due to CloudFormation limitations it's not possible to enable API Gateway stage settings on existing deployments. Please remove your old API Gateway and re-deploy with your new stage configuration. Once done, subsequent deployments should work without any issues. -Disabling tracing might result in unexpected behavior. We recommend to remove and re-deploy your service if you want to disable tracing. +Disabling settings might result in unexpected behavior. We recommend to remove and re-deploy your service without such stage settings. + +### AWS X-Ray Tracing API Gateway supports a form of out of the box distributed tracing via [AWS X-Ray](https://aws.amazon.com/xray/) though enabling [active tracing](https://docs.aws.amazon.com/apigateway/latest/developerguide/apigateway-xray.html). To enable this feature for your serverless application's API Gateway add the following to your `serverless.yml` @@ -1366,3 +1368,18 @@ provider: tracing: apiGateway: true ``` + +### Tags / Stack Tags + +API Gateway stages will be tagged with the `tags` and `stackTags` values defined at the `provider` level: + +```yml +# serverless.yml + +provider: + name: aws + stackTags: + stackTagKey: stackTagValue + tags: + tagKey: tagValue +```