From 6d04fb91563c528173f76f428503637630ffe4c3 Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Wed, 30 Jan 2019 17:05:05 +0000 Subject: [PATCH 1/6] - add support for a `dependsOn` option proposed by #158 --- README.md | 31 ++++++- .../stepFunctions/compileStateMachines.js | 21 ++++- .../compileStateMachines.test.js | 90 +++++++++++++++++-- 3 files changed, 130 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 591bf8b1..aa75b8e1 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,8 @@ stepFunctions: Type: Task Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello End: true + dependsOn: + Ref: DynamoDBTable hellostepfunc2: definition: StartAt: HelloWorld2 @@ -55,6 +57,10 @@ stepFunctions: Type: Task Resource: arn:aws:states:#{AWS::Region}:#{AWS::AccountId}:activity:myTask End: true + dependsOn: + - Ref: DynamoDBTable + - Ref: KinesisStream + - Ref: CUstomIamRole activities: - myTask - yourTask @@ -110,6 +116,23 @@ plugins: You can then `Ref: SendMessageStateMachine` in various parts of CloudFormation or serverless.yml +#### Depending on another logical id +If your state machine depends on another resource defined in your `serverless.yml` then you can add a `dependsOn` field to the state machine `definition`. This would add the `DependsOn`clause to the generated CloudFormation template. + +This `dependsOn` field can be either a string, or an array of strings. + +```yaml +stepFunctions: + stateMachines: + myStateMachine: + dependsOn: myDB + + myOtherStateMachine: + dependsOn: + - myOtherDB + - myStream +``` + #### Current Gotcha Please keep this gotcha in mind if you want to reference the `name` from the `resources` section. To generate Logical ID for CloudFormation, the plugin transforms the specified name in serverless.yml based on the following scheme. @@ -329,11 +352,11 @@ stepFunctions: events: - http: path: /users - ... + ... authorizer: # Provide both type and authorizerId type: COGNITO_USER_POOLS # TOKEN, CUSTOM or COGNITO_USER_POOLS, same as AWS Cloudformation documentation - authorizerId: + authorizerId: Ref: ApiGatewayAuthorizer # or hard-code Authorizer ID ``` @@ -581,7 +604,7 @@ stepFunctions: state: - pending definition: - ... + ... ``` ## Specifying a Name @@ -654,7 +677,7 @@ resources: Resources: StateMachineRole: Type: AWS::IAM::Role - Properties: + Properties: ... ``` diff --git a/lib/deploy/stepFunctions/compileStateMachines.js b/lib/deploy/stepFunctions/compileStateMachines.js index 77a20aa3..07c5e902 100644 --- a/lib/deploy/stepFunctions/compileStateMachines.js +++ b/lib/deploy/stepFunctions/compileStateMachines.js @@ -15,7 +15,7 @@ module.exports = { const stateMachineObj = this.getStateMachine(stateMachineName); let DefinitionString; let RoleArn; - let DependsOn; + let DependsOn = []; if (stateMachineObj.definition) { if (typeof stateMachineObj.definition === 'string') { @@ -63,7 +63,24 @@ module.exports = { 'Arn', ], }; - DependsOn = 'IamRoleStateMachineExecution'; + DependsOn.push('IamRoleStateMachineExecution'); + } + + if (stateMachineObj.dependsOn) { + const dependsOn = stateMachineObj.dependsOn; + + if (_.isArray(dependsOn) && _.every(dependsOn, _.isString)) { + DependsOn = _.concat(DependsOn, dependsOn); + } else if (_.isString(dependsOn)) { + DependsOn.push(dependsOn); + } else { + const errorMessage = [ + `dependsOn property in stateMachine "${stateMachineName}" is neither a string`, + ' nor an array of strings', + ].join(''); + throw new this.serverless.classes + .Error(errorMessage); + } } const stateMachineLogicalId = this.getStateMachineLogicalId(stateMachineName, diff --git a/lib/deploy/stepFunctions/compileStateMachines.test.js b/lib/deploy/stepFunctions/compileStateMachines.test.js index 3a23fd4e..8c5666d0 100644 --- a/lib/deploy/stepFunctions/compileStateMachines.test.js +++ b/lib/deploy/stepFunctions/compileStateMachines.test.js @@ -64,11 +64,11 @@ describe('#compileStateMachines', () => { expect(serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources .StateMachineBeta1.DependsOn - ).to.equal('IamRoleStateMachineExecution'); + ).to.deep.eq(['IamRoleStateMachineExecution']); expect(serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources .StateMachineBeta2.DependsOn - ).to.equal('IamRoleStateMachineExecution'); + ).to.deep.eq(['IamRoleStateMachineExecution']); expect(serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Outputs .StateMachineBeta1Arn.Value.Ref @@ -119,11 +119,11 @@ describe('#compileStateMachines', () => { expect(serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources .MyStateMachine1StepFunctionsStateMachine.DependsOn - ).to.equal('IamRoleStateMachineExecution'); + ).to.deep.eq(['IamRoleStateMachineExecution']); expect(serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources .MyStateMachine2StepFunctionsStateMachine.DependsOn - ).to.equal('IamRoleStateMachineExecution'); + ).to.deep.eq(['IamRoleStateMachineExecution']); expect(serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Outputs .MyStateMachine1StepFunctionsStateMachineArn.Value.Ref @@ -176,11 +176,11 @@ describe('#compileStateMachines', () => { expect(serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources .StateMachineBeta1.DependsOn - ).to.equal('IamRoleStateMachineExecution'); + ).to.deep.eq(['IamRoleStateMachineExecution']); expect(serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources .StateMachineBeta2.DependsOn - ).to.equal('IamRoleStateMachineExecution'); + ).to.deep.eq(['IamRoleStateMachineExecution']); }); it('should create corresponding resources when definition and role property are given', () => { @@ -405,4 +405,82 @@ describe('#compileStateMachines', () => { expect(actual).to.equal(JSON.stringify(definition, undefined, 2)); }); + + it('should add dependsOn resources', () => { + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + definition: 'definition1', + name: 'stateMachineBeta1', + dependsOn: 'DynamoDBTable', + }, + myStateMachine2: { + definition: 'definition2', + name: 'stateMachineBeta2', + dependsOn: [ + 'DynamoDBTable', + 'KinesisStream', + ], + }, + }, + }; + + serverlessStepFunctions.compileStateMachines(); + expect(serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources + .StateMachineBeta1.Type + ).to.equal('AWS::StepFunctions::StateMachine'); + expect(serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources + .StateMachineBeta2.Type + ).to.equal('AWS::StepFunctions::StateMachine'); + expect(serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources + .StateMachineBeta1.Properties.DefinitionString + ).to.equal('"definition1"'); + expect(serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources + .StateMachineBeta2.Properties.DefinitionString + ).to.equal('"definition2"'); + expect(serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources + .StateMachineBeta1.Properties.RoleArn['Fn::GetAtt'][0] + ).to.equal('IamRoleStateMachineExecution'); + expect(serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources + .StateMachineBeta2.Properties.RoleArn['Fn::GetAtt'][0] + ).to.equal('IamRoleStateMachineExecution'); + expect(serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources + .StateMachineBeta1.DependsOn + ).to.deep.eq(['IamRoleStateMachineExecution', 'DynamoDBTable']); + expect(serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources + .StateMachineBeta2.DependsOn + ).to.deep.eq(['IamRoleStateMachineExecution', 'DynamoDBTable', 'KinesisStream']); + }); + + it('should throw error when dependsOn property is neither string nor [string]', () => { + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + definition: 'definition1', + name: 'stateMachineBeta1', + dependsOn: { Ref: 'ss' }, + }, + }, + }; + expect(() => serverlessStepFunctions.compileStateMachines()).to.throw(Error); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + definition: 'definition1', + name: 'stateMachineBeta1', + dependsOn: [{ Ref: 'ss' }], + }, + }, + }; + expect(() => serverlessStepFunctions.compileStateMachines()).to.throw(Error); + }); }); From 31efd440a67e92beb3c092cb15609ab91b58cc32 Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Wed, 30 Jan 2019 17:11:16 +0000 Subject: [PATCH 2/6] - updated ReadMe --- README.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index aa75b8e1..03f2fdbc 100644 --- a/README.md +++ b/README.md @@ -47,8 +47,7 @@ stepFunctions: Type: Task Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello End: true - dependsOn: - Ref: DynamoDBTable + dependsOn: CustomIamRole hellostepfunc2: definition: StartAt: HelloWorld2 @@ -58,9 +57,9 @@ stepFunctions: Resource: arn:aws:states:#{AWS::Region}:#{AWS::AccountId}:activity:myTask End: true dependsOn: - - Ref: DynamoDBTable - - Ref: KinesisStream - - Ref: CUstomIamRole + - DynamoDBTable + - KinesisStream + - CUstomIamRole activities: - myTask - yourTask From 29ca84cc625925a127afed127a113595b28a04eb Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Wed, 30 Jan 2019 20:18:07 +0000 Subject: [PATCH 3/6] - fixed node4/5 error --- lib/deploy/stepFunctions/compileIamRole.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index b330a67e..0ae8a1e9 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -130,7 +130,7 @@ function consolidatePermissionsByAction(permissions) { .mapValues(perms => { // find the unique resources let resources = _.uniqWith(_.flatMap(perms, p => p.resource), _.isEqual); - if (resources.includes('*')) { + if (_.includes(resources, '*')) { resources = '*'; } From 50dfa5c52140c86bef24b62030832ec7fbf3c1db Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Wed, 30 Jan 2019 20:19:21 +0000 Subject: [PATCH 4/6] - fixed node4/5 error --- lib/deploy/stepFunctions/compileIamRole.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index b330a67e..0ae8a1e9 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -130,7 +130,7 @@ function consolidatePermissionsByAction(permissions) { .mapValues(perms => { // find the unique resources let resources = _.uniqWith(_.flatMap(perms, p => p.resource), _.isEqual); - if (resources.includes('*')) { + if (_.includes(resources, '*')) { resources = '*'; } From 4ca4a5c0b0244b2cac9739d18a3d27d8eb30f5f2 Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Wed, 30 Jan 2019 20:42:55 +0000 Subject: [PATCH 5/6] - issue a Deny all policy statement when there are no Task states in the state machine --- lib/deploy/stepFunctions/compileIamRole.js | 25 +++++++-- .../stepFunctions/compileIamRole.test.js | 52 ++++++++++++++++--- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index 0ae8a1e9..aab18e9c 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -198,6 +198,25 @@ function getIamPermissions(serverless, taskStates) { }); } +function getIamStatements(iamPermissions) { + // when the state machine doesn't define any Task states, and therefore doesn't need ANY + // permission, then we should follow the behaviour of the AWS console and return a policy + // that denies access to EVERYTHING + if (_.isEmpty(iamPermissions)) { + return [{ + Effect: 'Deny', + Action: '*', + Resource: '*', + }]; + } + + return iamPermissions.map(p => ({ + Effect: 'Allow', + Action: p.action.split(','), + Resource: p.resource, + })); +} + module.exports = { compileIamRole() { const customRolesProvided = []; @@ -223,11 +242,7 @@ module.exports = { iamPermissions = consolidatePermissionsByAction(iamPermissions); iamPermissions = consolidatePermissionsByResource(iamPermissions); - const iamStatements = iamPermissions.map(p => ({ - Effect: 'Allow', - Action: p.action.split(','), - Resource: p.resource, - })); + const iamStatements = getIamStatements(iamPermissions); const iamRoleJson = iamRoleStateMachineExecutionTemplate diff --git a/lib/deploy/stepFunctions/compileIamRole.test.js b/lib/deploy/stepFunctions/compileIamRole.test.js index b95712e0..15f42179 100644 --- a/lib/deploy/stepFunctions/compileIamRole.test.js +++ b/lib/deploy/stepFunctions/compileIamRole.test.js @@ -24,6 +24,14 @@ describe('#compileIamRole', () => { serverlessStepFunctions = new ServerlessStepFunctions(serverless, options); }); + const expectDenyAllPolicy = (policy) => { + const statements = policy.PolicyDocument.Statement; + expect(statements).to.have.lengthOf(1); + expect(statements[0].Effect).to.equal('Deny'); + expect(statements[0].Action).to.equal('*'); + expect(statements[0].Resource).to.equal('*'); + }; + it('should do nothing when role property exists in all statmachine properties', () => { serverless.service.stepFunctions = { stateMachines: { @@ -243,7 +251,7 @@ describe('#compileIamRole', () => { const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution .Properties.Policies[0]; - expect(policy.PolicyDocument.Statement).to.have.lengthOf(0); + expectDenyAllPolicy(policy); }); it('should give sqs:SendMessage permission for only SQS referenced by state machine', () => { @@ -362,7 +370,7 @@ describe('#compileIamRole', () => { const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution .Properties.Policies[0]; - expect(policy.PolicyDocument.Statement).to.have.lengthOf(0); + expectDenyAllPolicy(policy); }); it('should not give sqs:SendMessage permission if QueueUrl is invalid', () => { @@ -789,10 +797,10 @@ describe('#compileIamRole', () => { }; serverlessStepFunctions.compileIamRole(); - const statements = serverlessStepFunctions.serverless.service + const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution - .Properties.Policies[0].PolicyDocument.Statement; - expect(statements).to.have.lengthOf(0); + .Properties.Policies[0]; + expectDenyAllPolicy(policy); }); it('should not generate any permissions for Task states not yet supported', () => { @@ -818,9 +826,37 @@ describe('#compileIamRole', () => { }; serverlessStepFunctions.compileIamRole(); - const statements = serverlessStepFunctions.serverless.service + const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution - .Properties.Policies[0].PolicyDocument.Statement; - expect(statements).to.have.lengthOf(0); + .Properties.Policies[0]; + expectDenyAllPolicy(policy); + }); + + it('should generate a Deny all statement if state machine has no tasks', () => { + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Pass', + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1'), + myStateMachine2: genStateMachine('stateMachineBeta2'), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + expectDenyAllPolicy(policy); }); }); From 1b2d7885c9a1af1cd592dcaa1c541771d6b337b6 Mon Sep 17 00:00:00 2001 From: horike37 Date: Thu, 31 Jan 2019 11:11:43 +0900 Subject: [PATCH 6/6] change support version of node --- .travis.yml | 8 ++++---- package-lock.json | 2 +- package.json | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index dac3490e..1ecec069 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,10 +2,10 @@ language: node_js matrix: include: - - node_js: '4.4' - - node_js: '5.11' - - node_js: '6.2' - node_js: '6.2' + - node_js: '8.9' + - node_js: '10.6' + - node_js: '10.6' env: - DISABLE_TESTS=true - LINTING=true @@ -20,4 +20,4 @@ script: - if [[ ! -z "$DISABLE_TESTS" && ! -z "$LINTING" ]]; then npm run lint; fi after_success: - - cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage \ No newline at end of file + - cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js && rm -rf ./coverage diff --git a/package-lock.json b/package-lock.json index e60f1612..15ce10c3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "serverless-step-functions", - "version": "1.10.0", + "version": "1.11.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index cfa168c3..f706f7b1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "serverless-step-functions", - "version": "1.10.0", + "version": "1.11.0", "description": "The module is AWS Step Functions plugin for Serverless Framework", "main": "lib/index.js", "scripts": {