From 4c3285524f056c54687fa6925cf0335fe3ea2a98 Mon Sep 17 00:00:00 2001 From: Sergii Kovalev Date: Wed, 4 Nov 2020 18:15:28 +0200 Subject: [PATCH] test: validateStatements + getFunctionRoleName test cases added --- .eslintrc.js | 10 +++- src/lib/index.ts | 72 ++++++++++++------------- src/test/index.test.ts | 120 ++++++++++++++++++++++++++--------------- 3 files changed, 121 insertions(+), 81 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index d0c9226..46ae249 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,4 +1,5 @@ // @see https://eslint.org/docs/user-guide/configuring#configuring-rules + const OFF = 0; const WARN = 1; const ERROR = 2; @@ -25,6 +26,14 @@ module.exports = { rules: { '@typescript-eslint/no-explicit-any': OFF, '@typescript-eslint/explicit-module-boundary-types': OFF, + '@typescript-eslint/no-unused-vars': [ERROR, { argsIgnorePattern: '^_' }], + // eslint-disable-next-line max-len + // https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars.md + 'no-unused-vars': OFF, + 'camelcase': ERROR, + 'space-infix-ops': ERROR, + 'keyword-spacing': ERROR, + 'spaced-comment': ERROR, 'arrow-body-style': [ERROR, 'as-needed'], 'comma-dangle': [ERROR, 'always-multiline'], 'import/imports-first': ERROR, @@ -36,7 +45,6 @@ module.exports = { 'max-len': [ERROR, 120], 'newline-per-chained-call': ERROR, 'no-confusing-arrow': ERROR, - 'no-unused-vars': [ERROR, { argsIgnorePattern: '^_' }], 'no-use-before-define': ERROR, 'require-yield': ERROR, 'function-call-argument-newline': [ERROR, 'consistent'], diff --git a/src/lib/index.ts b/src/lib/index.ts index 58c3c59..7d0cdc9 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -21,7 +21,7 @@ class ServerlessIamPerFunctionPlugin { * @param {Serverless} serverless - serverless host object * @param {Object} _options */ - constructor(serverless: any) { + constructor(serverless: any, _options?: any) { this.serverless = serverless; if (this.serverless.service.provider.name !== 'aws') { @@ -66,18 +66,18 @@ class ServerlessIamPerFunctionPlugin { } /** - * Utility function which throws an error. The msg will be formated with args using util.format. + * Utility function which throws an error. The msg will be formatted with args using util.format. * Error message will be prefixed with ${PLUGIN_NAME}: ERROR: * @param {string} msg * @param {*[]} args * @returns void */ throwError(msg: string, ...args: any[]) { - if(!_.isEmpty(args)) { + if (!_.isEmpty(args)) { msg = util.format(msg, args); } - const err_msg = `${PLUGIN_NAME}: ERROR: ${msg}`; - throw new this.serverless.classes.Error(err_msg); + const errMsg = `${PLUGIN_NAME}: ERROR: ${msg}`; + throw new this.serverless.classes.Error(errMsg); } /** @@ -87,7 +87,7 @@ class ServerlessIamPerFunctionPlugin { validateStatements(statements: any): void { // Verify that iamRoleStatements (if present) is an array of { Effect: ..., // Action: ..., Resource: ... } objects. - if(_.isEmpty(statements)) { + if (_.isEmpty(statements)) { return; } let violationsFound; @@ -121,12 +121,12 @@ class ServerlessIamPerFunctionPlugin { } /** - * @param {*[]} name_parts + * @param {*[]} nameParts * @returns void */ - getRoleNameLength(name_parts: any[]) { - let length=0; //calculate the expected length. Sum the length of each part - for (const part of name_parts) { + getRoleNameLength(nameParts: any[]) { + let length = 0; // calculate the expected length. Sum the length of each part + for (const part of nameParts) { if (part.Ref) { if (part.Ref === 'AWS::Region') { length += this.serverless.service.provider.region.length; @@ -137,7 +137,7 @@ class ServerlessIamPerFunctionPlugin { length += part.length; } } - length += (name_parts.length - 1); //take into account the dashes between parts + length += (nameParts.length - 1); // take into account the dashes between parts return length; } @@ -148,15 +148,15 @@ class ServerlessIamPerFunctionPlugin { getFunctionRoleName(functionName: string) { const roleName = this.serverless.providers.aws.naming.getRoleName(); const fnJoin = roleName['Fn::Join']; - if(!_.isArray(fnJoin) || fnJoin.length !== 2 || !_.isArray(fnJoin[1]) || fnJoin[1].length < 2) { - this.throwError('Global Role Name is not in exepcted format. Got name: ' + JSON.stringify(roleName)); + if (!_.isArray(fnJoin) || fnJoin.length !== 2 || !_.isArray(fnJoin[1]) || fnJoin[1].length < 2) { + this.throwError('Global Role Name is not in expected format. Got name: ' + JSON.stringify(roleName)); } - fnJoin[1].splice(2, 0, functionName); //insert the function name - if(this.getRoleNameLength(fnJoin[1]) > 64 && fnJoin[1][fnJoin[1].length-1] === 'lambdaRole') { + fnJoin[1].splice(2, 0, functionName); // insert the function name + if (this.getRoleNameLength(fnJoin[1]) > 64 && fnJoin[1][fnJoin[1].length - 1] === 'lambdaRole') { // Remove lambdaRole from name to give more space for function name. fnJoin[1].pop(); } - if(this.getRoleNameLength(fnJoin[1]) > 64) { //aws limits to 64 chars the role name + if (this.getRoleNameLength(fnJoin[1]) > 64) { // aws limits to 64 chars the role name this.throwError(`auto generated role name for function: ${functionName} is too long (over 64 chars). Try setting a custom role name using the property: iamRoleStatementsName.`); } @@ -174,7 +174,7 @@ class ServerlessIamPerFunctionPlugin { const functionResource = this.serverless.service.provider .compiledCloudFormationTemplate.Resources[functionResourceName]; - if(_.isEmpty(functionResource) + if (_.isEmpty(functionResource) || _.isEmpty(functionResource.Properties) || _.isEmpty(functionResource.Properties.Role) || !_.isArray(functionResource.Properties.Role['Fn::GetAtt']) @@ -205,7 +205,7 @@ class ServerlessIamPerFunctionPlugin { Resource: [], }; for (const event of functionObject.events) { - if(event.sqs) { + if (event.sqs) { const sqsArn = event.sqs.arn || event.sqs; (sqsStatement.Resource as any[]).push(sqsArn); } @@ -220,7 +220,7 @@ class ServerlessIamPerFunctionPlugin { */ getStreamStatements(functionObject: any) { const res: any[] = []; - if(_.isEmpty(functionObject.events)) { //no events + if (_.isEmpty(functionObject.events)) { // no events return res; } const dynamodbStreamStatement: Statement = { @@ -244,7 +244,7 @@ class ServerlessIamPerFunctionPlugin { Resource: [], }; for (const event of functionObject.events) { - if(event.stream) { + if (event.stream) { const streamArn = event.stream.arn || event.stream; const streamType = event.stream.type || streamArn.split(':')[2]; switch (streamType) { @@ -277,24 +277,24 @@ class ServerlessIamPerFunctionPlugin { */ createRoleForFunction(functionName: string, functionToRoleMap: Map) { const functionObject = this.serverless.service.getFunction(functionName); - if(functionObject.iamRoleStatements === undefined) { + if (functionObject.iamRoleStatements === undefined) { return; } - if(functionObject.role) { + if (functionObject.role) { this.throwError( 'Define function with both \'role\' and \'iamRoleStatements\' is not supported. Function name: ' + functionName, ); } this.validateStatements(functionObject.iamRoleStatements); - //we use the configured role as a template + // we use the configured role as a template const globalRoleName = this.serverless.providers.aws.naming.getRoleLogicalId(); const globalIamRole = this.serverless.service.provider.compiledCloudFormationTemplate.Resources[globalRoleName]; const functionIamRole = _.cloneDeep(globalIamRole); - //remove the statements + // remove the statements const policyStatements: Statement[] = []; functionIamRole.Properties.Policies[0].PolicyDocument.Statement = policyStatements; - //set log statements + // set log statements policyStatements[0] = { Effect: 'Allow', Action: ['logs:CreateLogStream', 'logs:PutLogEvents'], @@ -307,7 +307,7 @@ class ServerlessIamPerFunctionPlugin { }; // remove managed policies functionIamRole.Properties.ManagedPolicyArns = []; - //set vpc if needed + // set vpc if needed if (!_.isEmpty(functionObject.vpc) || !_.isEmpty(this.serverless.service.provider.vpc)) { functionIamRole.Properties.ManagedPolicyArns = [{ 'Fn::Join': ['', @@ -319,16 +319,16 @@ class ServerlessIamPerFunctionPlugin { ], }]; } - for (const s of this.getStreamStatements(functionObject)) { //set stream statements (if needed) + for (const s of this.getStreamStatements(functionObject)) { // set stream statements (if needed) policyStatements.push(s); } - const sqsStatement = this.getSqsStatement(functionObject); //set sqs statement (if needed) + const sqsStatement = this.getSqsStatement(functionObject); // set sqs statement (if needed) if (sqsStatement) { policyStatements.push(sqsStatement); } // set sns publish for DLQ if needed // currently only sns is supported: https://serverless.com/framework/docs/providers/aws/events/sns#dlq-with-sqs - if (!_.isEmpty(functionObject.onError)) { // + if (!_.isEmpty(functionObject.onError)) { policyStatements.push({ Effect: 'Allow', Action: [ @@ -341,13 +341,13 @@ class ServerlessIamPerFunctionPlugin { const isInherit = functionObject.iamRoleStatementsInherit || (this.defaultInherit && functionObject.iamRoleStatementsInherit !== false); - if(isInherit && !_.isEmpty(this.serverless.service.provider.iamRoleStatements)) { //add global statements + if (isInherit && !_.isEmpty(this.serverless.service.provider.iamRoleStatements)) { // add global statements for (const s of this.serverless.service.provider.iamRoleStatements) { policyStatements.push(s); } } - //add iamRoleStatements - if(_.isArray(functionObject.iamRoleStatements)) { + // add iamRoleStatements + if (_.isArray(functionObject.iamRoleStatements)) { for (const s of functionObject.iamRoleStatements) { policyStatements.push(s); } @@ -369,14 +369,14 @@ class ServerlessIamPerFunctionPlugin { */ setEventSourceMappings(functionToRoleMap: Map) { for (const mapping of _.values(this.serverless.service.provider.compiledCloudFormationTemplate.Resources)) { - if(mapping.Type && mapping.Type === 'AWS::Lambda::EventSourceMapping') { + if (mapping.Type && mapping.Type === 'AWS::Lambda::EventSourceMapping') { const functionNameFn = _.get(mapping, 'Properties.FunctionName.Fn::GetAtt'); - if(!_.isArray(functionNameFn)) { + if (!_.isArray(functionNameFn)) { continue; } const functionName = functionNameFn[0]; const roleName = functionToRoleMap.get(functionName); - if(roleName) { + if (roleName) { mapping.DependsOn = roleName; } } @@ -388,7 +388,7 @@ class ServerlessIamPerFunctionPlugin { */ createRolesPerFunction() { const allFunctions = this.serverless.service.getAllFunctions(); - if(_.isEmpty(allFunctions)) { + if (_.isEmpty(allFunctions)) { return; } const functionToRoleMap: Map = new Map(); diff --git a/src/test/index.test.ts b/src/test/index.test.ts index 14c7292..4f6484b 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -21,7 +21,7 @@ describe('plugin tests', function(this: any) { try { fs.mkdirSync(dir); } catch (error) { - if(error.code !== 'EEXIST') { + if (error.code !== 'EEXIST') { console.log('failed to create dir: %s, error: ', dir, error); throw error; } @@ -42,11 +42,11 @@ describe('plugin tests', function(this: any) { }; serverless.config.servicePath = tempdir; serverless.pluginManager.loadAllPlugins(); - let compile_hooks: any[] = serverless.pluginManager.getHooks('package:setupProviderConfiguration'); - compile_hooks = compile_hooks.concat( + let compileHooks: any[] = serverless.pluginManager.getHooks('package:setupProviderConfiguration'); + compileHooks = compileHooks.concat( serverless.pluginManager.getHooks('package:compileFunctions'), serverless.pluginManager.getHooks('package:compileEvents')); - for (const ent of compile_hooks) { + for (const ent of compileHooks) { try { await ent.hook(); } catch (error) { @@ -102,61 +102,72 @@ describe('plugin tests', function(this: any) { }); it('should throw an error for invalid statement', () => { - const bad_statement = [{ //missing effect + const badStatement = [{ // missing effect Action: [ 'xray:PutTelemetryRecords', 'xray:PutTraceSegments', ], Resource: '*', }]; - assert.throws(() => {plugin.validateStatements(bad_statement);}); + assert.throws(() => {plugin.validateStatements(badStatement);}); + }); + + it('should throw an error for non array type of statement', () => { + const badStatement = { // missing effect + Action: [ + 'xray:PutTelemetryRecords', + 'xray:PutTraceSegments', + ], + Resource: '*', + }; + assert.throws(() => {plugin.validateStatements(badStatement);}); }); }); describe('#getRoleNameLength', () => { it('Should calculate the accurate role name length us-east-1', () => { serverless.service.provider.region = 'us-east-1'; - const function_name = 'a'.repeat(10); - const name_parts = [ + const functionName = 'a'.repeat(10); + const nameParts = [ serverless.service.service, // test-service , length of 12 serverless.service.provider.stage, // dev, length of 3 : 15 { Ref: 'AWS::Region' }, // us-east-1, length 9 : 24 - function_name, // 'a'.repeat(10), length 10 : 34 + functionName, // 'a'.repeat(10), length 10 : 34 'lambdaRole', // lambdaRole, length 10 : 44 ]; - const role_name_length = plugin.getRoleNameLength(name_parts); + const roleNameLength = plugin.getRoleNameLength(nameParts); const expected = 44; // 12 + 3 + 9 + 10 + 10 == 44 - assert.equal(role_name_length, expected + name_parts.length - 1); + assert.equal(roleNameLength, expected + nameParts.length - 1); }); it('Should calculate the accurate role name length ap-northeast-1', () => { serverless.service.provider.region = 'ap-northeast-1'; - const function_name = 'a'.repeat(10); - const name_parts = [ + const functionName = 'a'.repeat(10); + const nameParts = [ serverless.service.service, // test-service , length of 12 serverless.service.provider.stage, // dev, length of 3 { Ref: 'AWS::Region' }, // ap-northeast-1, length 14 - function_name, // 'a'.repeat(10), length 10 + functionName, // 'a'.repeat(10), length 10 'lambdaRole', // lambdaRole, length 10 ]; - const role_name_length = plugin.getRoleNameLength(name_parts); + const roleNameLength = plugin.getRoleNameLength(nameParts); const expected = 49; // 12 + 3 + 14 + 10 + 10 == 49 - assert.equal(role_name_length, expected + name_parts.length - 1); + assert.equal(roleNameLength, expected + nameParts.length - 1); }); it('Should calculate the actual length for a non AWS::Region ref to maintain backward compatibility', () => { serverless.service.provider.region = 'ap-northeast-1'; - const function_name = 'a'.repeat(10); - const name_parts = [ + const functionName = 'a'.repeat(10); + const nameParts = [ serverless.service.service, // test-service , length of 12 { Ref: 'bananas'}, // bananas, length of 7 { Ref: 'AWS::Region' }, // ap-northeast-1, length 14 - function_name, // 'a'.repeat(10), length 10 + functionName, // 'a'.repeat(10), length 10 'lambdaRole', // lambdaRole, length 10 ]; - const role_name_length = plugin.getRoleNameLength(name_parts); + const roleNameLength = plugin.getRoleNameLength(nameParts); const expected = 53; // 12 + 7 + 14 + 10 + 10 == 53 - assert.equal(role_name_length, expected + name_parts.length - 1); + assert.equal(roleNameLength, expected + nameParts.length - 1); }); }); @@ -165,35 +176,56 @@ describe('plugin tests', function(this: any) { const name = 'test-name'; const roleName = plugin.getFunctionRoleName(name); assertFunctionRoleName(name, roleName); - const name_parts = roleName['Fn::Join'][1]; - assert.equal(name_parts[name_parts.length - 1], 'lambdaRole'); + const nameParts = roleName['Fn::Join'][1]; + assert.equal(nameParts[nameParts.length - 1], 'lambdaRole'); }); it('should throw an error on long name', () => { - const long_name = 'long-long-long-long-long-long-long-long-long-long-long-long-long-name'; - assert.throws(() => {plugin.getFunctionRoleName(long_name);}); + const longName = 'long-long-long-long-long-long-long-long-long-long-long-long-long-name'; + assert.throws(() => {plugin.getFunctionRoleName(longName);}); try { - plugin.getFunctionRoleName(long_name); + plugin.getFunctionRoleName(longName); } catch (error) { // some validation that the error we throw is what we expect const msg: string = error.message; assert.isString(msg); assert.isTrue(msg.startsWith('serverless-iam-roles-per-function: ERROR:')); - assert.isTrue(msg.includes(long_name)); + assert.isTrue(msg.includes(longName)); assert.isTrue(msg.endsWith('iamRoleStatementsName.')); } }); + it('should throw with invalid Fn:Join statement', () => { + assert.throws(() => { + const longName = 'test-name'; + const invalidRoleName = { + 'Fn::Join': [], + }; + const slsMock = { + service: { + provider: { + name: 'aws', + }, + }, + providers: { + aws: { naming: { getRoleName: () => invalidRoleName } }, + }, + }; + (new Plugin(slsMock)).getFunctionRoleName(longName); + }); + }); + it('should return a name without "lambdaRole"', () => { let name = 'test-name'; let roleName = plugin.getFunctionRoleName(name); const len = plugin.getRoleNameLength(roleName['Fn::Join'][1]); - //create a name which causes role name to be longer than 64 chars by 1. Will cause then lambdaRole to be removed + // create a name which causes role name to be longer than 64 chars by 1. + // Will cause then lambdaRole to be removed name += 'a'.repeat(64 - len + 1); roleName = plugin.getFunctionRoleName(name); assertFunctionRoleName(name, roleName); - const name_parts = roleName['Fn::Join'][1]; - assert.notEqual(name_parts[name_parts.length - 1], 'lambdaRole'); + const nameParts = roleName['Fn::Join'][1]; + assert.notEqual(nameParts[nameParts.length - 1], 'lambdaRole'); }); }); @@ -218,20 +250,20 @@ describe('plugin tests', function(this: any) { ); const helloInheritRole = compiledResources.HelloInheritIamRoleLambdaExecution; assertFunctionRoleName('helloInherit', helloInheritRole.Properties.RoleName); - let policy_statements: any[] = helloInheritRole.Properties.Policies[0].PolicyDocument.Statement; + let policyStatements: any[] = helloInheritRole.Properties.Policies[0].PolicyDocument.Statement; assert.isObject( - policy_statements.find((s) => s.Action[0] === 'xray:PutTelemetryRecords'), + policyStatements.find((s) => s.Action[0] === 'xray:PutTelemetryRecords'), 'global statements imported upon inherit', ); assert.isObject( - policy_statements.find((s) => s.Action[0] === 'dynamodb:GetItem'), + policyStatements.find((s) => s.Action[0] === 'dynamodb:GetItem'), 'per function statements imported upon inherit', ); const streamHandlerRole = compiledResources.StreamHandlerIamRoleLambdaExecution; assertFunctionRoleName('streamHandler', streamHandlerRole.Properties.RoleName); - policy_statements = streamHandlerRole.Properties.Policies[0].PolicyDocument.Statement; + policyStatements = streamHandlerRole.Properties.Policies[0].PolicyDocument.Statement; assert.isObject( - policy_statements.find((s) => + policyStatements.find((s) => _.isEqual(s.Action, [ 'dynamodb:GetRecords', 'dynamodb:GetShardIterator', @@ -241,16 +273,16 @@ describe('plugin tests', function(this: any) { 'arn:aws:dynamodb:us-east-1:1234567890:table/test/stream/2017-10-09T19:39:15.151'])), 'stream statements included', ); - assert.isObject(policy_statements.find((s) => s.Action[0] === 'sns:Publish'), 'sns dlq statements included'); + assert.isObject(policyStatements.find((s) => s.Action[0] === 'sns:Publish'), 'sns dlq statements included'); const streamMapping = compiledResources.StreamHandlerEventSourceMappingDynamodbTest; assert.equal(streamMapping.DependsOn, 'StreamHandlerIamRoleLambdaExecution'); // verify sqsHandler should have SQS permissions const sqsHandlerRole = compiledResources.SqsHandlerIamRoleLambdaExecution; assertFunctionRoleName('sqsHandler', sqsHandlerRole.Properties.RoleName); - policy_statements = sqsHandlerRole.Properties.Policies[0].PolicyDocument.Statement; - JSON.stringify(policy_statements); + policyStatements = sqsHandlerRole.Properties.Policies[0].PolicyDocument.Statement; + JSON.stringify(policyStatements); assert.isObject( - policy_statements.find((s) => + policyStatements.find((s) => _.isEqual(s.Action, [ 'sqs:ReceiveMessage', 'sqs:DeleteMessage', @@ -260,7 +292,7 @@ describe('plugin tests', function(this: any) { 'arn:aws:sqs:us-east-1:1234567890:MyOtherQueue'])), 'sqs statements included', ); - assert.isObject(policy_statements.find((s) => s.Action[0] === 'sns:Publish'), 'sns dlq statements included'); + assert.isObject(policyStatements.find((s) => s.Action[0] === 'sns:Publish'), 'sns dlq statements included'); const sqsMapping = compiledResources.SqsHandlerEventSourceMappingSQSMyQueue; assert.equal(sqsMapping.DependsOn, 'SqsHandlerIamRoleLambdaExecution'); // verify helloNoPerFunction should have global role @@ -302,7 +334,7 @@ describe('plugin tests', function(this: any) { for (const key in compiledResources) { if (key !== 'IamRoleLambdaExecution' && Object.prototype.hasOwnProperty.call(compiledResources, key)) { const resource = compiledResources[key]; - if(resource.Type === 'AWS::IAM::Role') { + if (resource.Type === 'AWS::IAM::Role') { assert.fail(resource, undefined, 'There shouldn\'t be extra roles beyond IamRoleLambdaExecution'); } } @@ -338,9 +370,9 @@ describe('plugin tests', function(this: any) { let plugin: Plugin; beforeEach(() => { - //set defaultInherit + // set defaultInherit _.set(serverless.service, 'custom.serverless-iam-roles-per-function.defaultInherit', true); - //change helloInherit to false for testing + // change helloInherit to false for testing _.set(serverless.service, 'functions.helloInherit.iamRoleStatementsInherit', false); plugin = new Plugin(serverless); }); @@ -358,7 +390,7 @@ describe('plugin tests', function(this: any) { const helloRole = compiledResources.HelloIamRoleLambdaExecution; assert.isNotEmpty(helloRole); assertFunctionRoleName('hello', helloRole.Properties.RoleName); - //check depends and role is set properlly + // check depends and role is set properlly const helloFunctionResource = compiledResources.HelloLambdaFunction; assert.isTrue( helloFunctionResource.DependsOn.indexOf('HelloIamRoleLambdaExecution') >= 0,