From 490e714ccd924a85eb32a82768e409587864ffad Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 14:53:18 +0200 Subject: [PATCH 1/9] Obtain path to statements once --- .../aws/package/lib/mergeIamTemplates.js | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/lib/plugins/aws/package/lib/mergeIamTemplates.js b/lib/plugins/aws/package/lib/mergeIamTemplates.js index 2698be83f..cfa1893e1 100644 --- a/lib/plugins/aws/package/lib/mergeIamTemplates.js +++ b/lib/plugins/aws/package/lib/mergeIamTemplates.js @@ -86,24 +86,21 @@ module.exports = { const logGroupsPrefix = this.provider.naming .getLogGroupName(`${this.provider.serverless.service.service}-${this.provider.getStage()}`); - this.serverless.service.provider.compiledCloudFormationTemplate + const policyDocumentStatements = this.serverless.service.provider.compiledCloudFormationTemplate .Resources[this.provider.naming.getRoleLogicalId()] .Properties .Policies[0] .PolicyDocument - .Statement[0] + .Statement; + + policyDocumentStatements[0] .Resource .push({ 'Fn::Sub': 'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' + `:log-group:${logGroupsPrefix}*:*`, }); - this.serverless.service.provider.compiledCloudFormationTemplate - .Resources[this.provider.naming.getRoleLogicalId()] - .Properties - .Policies[0] - .PolicyDocument - .Statement[1] + policyDocumentStatements[1] .Resource .push({ 'Fn::Sub': 'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' + @@ -117,12 +114,8 @@ module.exports = { .Properties .Policies[0] .PolicyDocument - .Statement = this.serverless.service.provider.compiledCloudFormationTemplate - .Resources[this.provider.naming.getRoleLogicalId()] - .Properties - .Policies[0] - .PolicyDocument - .Statement.concat(this.serverless.service.provider.iamRoleStatements); + .Statement = policyDocumentStatements + .concat(this.serverless.service.provider.iamRoleStatements); } if (this.serverless.service.provider.iamManagedPolicies) { From d417a0fc4ef8ba699358668248d30d47ec249acd Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 14:58:24 +0200 Subject: [PATCH 2/9] Seclude canonicalFunctionNamePrefix var --- lib/plugins/aws/package/lib/mergeIamTemplates.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/plugins/aws/package/lib/mergeIamTemplates.js b/lib/plugins/aws/package/lib/mergeIamTemplates.js index cfa1893e1..7b35403e0 100644 --- a/lib/plugins/aws/package/lib/mergeIamTemplates.js +++ b/lib/plugins/aws/package/lib/mergeIamTemplates.js @@ -83,8 +83,10 @@ module.exports = { } ); + const canonicalFunctionNamePrefix = + `${this.provider.serverless.service.service}-${this.provider.getStage()}`; const logGroupsPrefix = this.provider.naming - .getLogGroupName(`${this.provider.serverless.service.service}-${this.provider.getStage()}`); + .getLogGroupName(canonicalFunctionNamePrefix); const policyDocumentStatements = this.serverless.service.provider.compiledCloudFormationTemplate .Resources[this.provider.naming.getRoleLogicalId()] From 6796ccf866d8ac58a2b86514ecfdc96d8a21c6e5 Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 15:38:16 +0200 Subject: [PATCH 3/9] Ensure tests reflect more closely a real world --- .../aws/package/lib/mergeIamTemplates.test.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/plugins/aws/package/lib/mergeIamTemplates.test.js b/lib/plugins/aws/package/lib/mergeIamTemplates.test.js index 59a41473c..7d0bee446 100644 --- a/lib/plugins/aws/package/lib/mergeIamTemplates.test.js +++ b/lib/plugins/aws/package/lib/mergeIamTemplates.test.js @@ -9,12 +9,15 @@ const AwsPackage = require('../index'); describe('#mergeIamTemplates()', () => { let awsPackage; let serverless; + const serviceName = 'new-service'; const functionName = 'test'; + const stage = 'dev'; + const resolvedFunctionName = `${serviceName}-${stage}-${functionName}`; beforeEach(() => { serverless = new Serverless(); const options = { - stage: 'dev', + stage, region: 'us-east-1', }; serverless.setProvider('aws', new AwsProvider(serverless, options)); @@ -23,14 +26,14 @@ describe('#mergeIamTemplates()', () => { awsPackage.serverless.service.provider.compiledCloudFormationTemplate = { Resources: {}, }; - awsPackage.serverless.service.service = 'new-service'; + awsPackage.serverless.service.service = serviceName; awsPackage.serverless.service.functions = { [functionName]: { - name: 'test', artifact: 'test.zip', handler: 'handler.hello', }, }; + serverless.service.setFunctionNames(); // Ensure to resolve function names }); it('should not merge if there are no functions', () => { @@ -291,7 +294,7 @@ describe('#mergeIamTemplates()', () => { { Type: 'AWS::Logs::LogGroup', Properties: { - LogGroupName: awsPackage.provider.naming.getLogGroupName(functionName), + LogGroupName: awsPackage.provider.naming.getLogGroupName(resolvedFunctionName), }, } ); @@ -310,7 +313,7 @@ describe('#mergeIamTemplates()', () => { { Type: 'AWS::Logs::LogGroup', Properties: { - LogGroupName: awsPackage.provider.naming.getLogGroupName(functionName), + LogGroupName: awsPackage.provider.naming.getLogGroupName(resolvedFunctionName), RetentionInDays: 5, }, } From 426393b2b3d8120235923d55b223107a10ad7822 Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 15:48:27 +0200 Subject: [PATCH 4/9] Configure test exposing issue with custom function name Related to #6236 --- .../aws/package/lib/mergeIamTemplates.test.js | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/lib/plugins/aws/package/lib/mergeIamTemplates.test.js b/lib/plugins/aws/package/lib/mergeIamTemplates.test.js index 7d0bee446..c659ab00f 100644 --- a/lib/plugins/aws/package/lib/mergeIamTemplates.test.js +++ b/lib/plugins/aws/package/lib/mergeIamTemplates.test.js @@ -139,6 +139,114 @@ describe('#mergeIamTemplates()', () => { }) ); + + it('should ensure IAM policies for custom named functions', () => { + const customFunctionName = 'foo-bar'; + awsPackage.serverless.service.functions = { + [functionName]: { + name: customFunctionName, + artifact: 'test.zip', + handler: 'handler.hello', + }, + }; + serverless.service.setFunctionNames(); // Ensure to resolve function names + + return awsPackage.mergeIamTemplates() + .then(() => { + const canonicalFunctionsPrefix = + `${awsPackage.serverless.service.service}-${awsPackage.provider.getStage()}`; + + expect(awsPackage.serverless.service.provider.compiledCloudFormationTemplate + .Resources[awsPackage.provider.naming.getRoleLogicalId()] + ).to.deep.equal({ + Type: 'AWS::IAM::Role', + Properties: { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Principal: { + Service: [ + 'lambda.amazonaws.com', + ], + }, + Action: [ + 'sts:AssumeRole', + ], + }, + ], + }, + Path: '/', + Policies: [ + { + PolicyName: { + 'Fn::Join': [ + '-', + [ + awsPackage.provider.getStage(), + awsPackage.serverless.service.service, + 'lambda', + ], + ], + }, + PolicyDocument: { + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Action: [ + 'logs:CreateLogStream', + ], + Resource: [ + { + 'Fn::Sub': 'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}:' + + `log-group:/aws/lambda/${canonicalFunctionsPrefix}*:*`, + }, + { + 'Fn::Sub': 'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}:' + + `log-group:/aws/lambda/${customFunctionName}:*`, + }, + ], + }, + { + Effect: 'Allow', + Action: [ + 'logs:PutLogEvents', + ], + Resource: [ + { + 'Fn::Sub': 'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}:' + + `log-group:/aws/lambda/${canonicalFunctionsPrefix}*:*:*`, + }, + { + 'Fn::Sub': 'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}:' + + `log-group:/aws/lambda/${customFunctionName}:*:*`, + }, + ], + }, + ], + }, + }, + ], + RoleName: { + 'Fn::Join': [ + '-', + [ + awsPackage.serverless.service.service, + awsPackage.provider.getStage(), + { + Ref: 'AWS::Region', + }, + 'lambdaRole', + ], + ], + }, + }, + }); + }); + }); + it('should add custom IAM policy statements', () => { awsPackage.serverless.service.provider.iamRoleStatements = [ { From 025d58579e20cb68f16e516f781f51479416482b Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 15:55:01 +0200 Subject: [PATCH 5/9] Fix setup of IAM policies for functions with custom names --- .../aws/package/lib/mergeIamTemplates.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/plugins/aws/package/lib/mergeIamTemplates.js b/lib/plugins/aws/package/lib/mergeIamTemplates.js index 7b35403e0..fddea5482 100644 --- a/lib/plugins/aws/package/lib/mergeIamTemplates.js +++ b/lib/plugins/aws/package/lib/mergeIamTemplates.js @@ -109,6 +109,30 @@ module.exports = { `:log-group:${logGroupsPrefix}*:*:*`, }); + this.serverless.service.getAllFunctions().forEach((functionName) => { + const { name: resolvedFunctionName } = this.serverless.service.getFunction(functionName); + if (!resolvedFunctionName || resolvedFunctionName.startsWith(canonicalFunctionNamePrefix)) { + return; + } + + const customFunctionNamelogGroupsPrefix = + this.provider.naming.getLogGroupName(resolvedFunctionName); + + policyDocumentStatements[0] + .Resource + .push({ + 'Fn::Sub': 'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' + + `:log-group:${customFunctionNamelogGroupsPrefix}:*`, + }); + + policyDocumentStatements[1] + .Resource + .push({ + 'Fn::Sub': 'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' + + `:log-group:${customFunctionNamelogGroupsPrefix}:*:*`, + }); + }); + if (this.serverless.service.provider.iamRoleStatements) { // add custom iam role statements this.serverless.service.provider.compiledCloudFormationTemplate From 093881b6a582392a1d318535597b4fb6e3d07d36 Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 16:00:29 +0200 Subject: [PATCH 6/9] Add code comments --- lib/plugins/aws/package/lib/mergeIamTemplates.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/plugins/aws/package/lib/mergeIamTemplates.js b/lib/plugins/aws/package/lib/mergeIamTemplates.js index fddea5482..030751af7 100644 --- a/lib/plugins/aws/package/lib/mergeIamTemplates.js +++ b/lib/plugins/aws/package/lib/mergeIamTemplates.js @@ -95,6 +95,7 @@ module.exports = { .PolicyDocument .Statement; + // Ensure general polices for functions with default name resolution policyDocumentStatements[0] .Resource .push({ @@ -109,6 +110,7 @@ module.exports = { `:log-group:${logGroupsPrefix}*:*:*`, }); + // Ensure policies for functions with custom name resolution this.serverless.service.getAllFunctions().forEach((functionName) => { const { name: resolvedFunctionName } = this.serverless.service.getFunction(functionName); if (!resolvedFunctionName || resolvedFunctionName.startsWith(canonicalFunctionNamePrefix)) { From 9ae66384007743190c758646e472bfdb3d3f10cd Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 16:10:17 +0200 Subject: [PATCH 7/9] Update package-lock.json --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index cd27db017..fb510dbd7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7521,9 +7521,9 @@ } }, "resolve": { - "version": "1.11.1", - "resolved": "https://registry.npmjs.org/resolve/-/resolve-1.11.1.tgz", - "integrity": "sha512-vIpgF6wfuJOZI7KKKSP+HmiKggadPQAdsp5HiC1mvqnfp0gF1vdwgBWZIdrVft9pgqoMFQN+R7BSWZiBxx+BBw==", + "version": "1.11.0", + "resolved": "https://registry.npmjs.org/resolve/-/resolve-1.11.0.tgz", + "integrity": "sha512-WL2pBDjqT6pGUNSUzMw00o4T7If+z4H2x3Gz893WoUQ5KW8Vr9txp00ykiP16VBaZF5+j/OcXJHZ9+PCvdiDKw==", "dev": true, "requires": { "path-parse": "^1.0.6" From 710bfa699a19839c89c1ed5fe9cb951cf43adb09 Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 16:10:40 +0200 Subject: [PATCH 8/9] Release v1.45.1 --- CHANGELOG.md | 8 ++++++++ package-lock.json | 2 +- package.json | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f38b8c87..b23756d6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# 1.45.1 (2019-06-12) + +- [Fix IAM policies setup for functions with custom name](https://github.com/serverless/serverless/pull/6240) + +## Meta +- [Comparison since last release](https://github.com/serverless/serverless/compare/v1.45.0...v1.45.1) + + # 1.45.0 (2019-06-12) - [Add `--config` option](https://github.com/serverless/serverless/pull/6216) diff --git a/package-lock.json b/package-lock.json index fb510dbd7..0e7462712 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "serverless", - "version": "1.45.0", + "version": "1.45.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 879eb991f..9fd83e12f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "serverless", - "version": "1.45.0", + "version": "1.45.1", "engines": { "node": ">=6.0" }, From 1d00c0440490723a37e558810a3cdecbe718642f Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 12 Jun 2019 16:21:58 +0200 Subject: [PATCH 9/9] Update changelog for release --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b23756d6b..7bf612b16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # 1.45.1 (2019-06-12) - [Fix IAM policies setup for functions with custom name](https://github.com/serverless/serverless/pull/6240) +- [Fix Travis CI deploy config](https://github.com/serverless/serverless/pull/6234) ## Meta - [Comparison since last release](https://github.com/serverless/serverless/compare/v1.45.0...v1.45.1)