Skip to content

Commit

Permalink
Merge e967f04 into 2c9a17c
Browse files Browse the repository at this point in the history
  • Loading branch information
@theburningmonk
theburningmonk committed Aug 18, 2019
2 parents 2c9a17c + e967f04 commit 01289fa
Show file tree
Hide file tree
Showing 6 changed files with 272 additions and 87 deletions.
62 changes: 43 additions & 19 deletions lib/deploy/stepFunctions/compileIamRole.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,46 +152,66 @@ function getLambdaPermissions(state) {
if (_.isString(functionName)) {
const segments = functionName.split(':');

let functionArn;
let functionArns;
if (functionName.startsWith('arn:aws:lambda')) {
// full ARN
functionArn = functionName;
functionArns = [
functionName,
`${functionName}:*`,
];
} else if (segments.length === 3 && segments[0].match(/^\d+$/)) {
// partial ARN
functionArn = {
'Fn::Sub': `arn:aws:lambda:\${AWS::Region}:${functionName}`,
};
functionArns = [
{ 'Fn::Sub': `arn:aws:lambda:\${AWS::Region}:${functionName}` },
{ 'Fn::Sub': `arn:aws:lambda:\${AWS::Region}:${functionName}:*` },
];
} else {
// name-only (with or without alias)
functionArn = {
'Fn::Sub': `arn:aws:lambda:\${AWS::Region}:\${AWS::AccountId}:function:${functionName}`,
};
functionArns = [
{
'Fn::Sub': `arn:aws:lambda:\${AWS::Region}:\${AWS::AccountId}:function:${functionName}`,
},
{
'Fn::Sub': `arn:aws:lambda:\${AWS::Region}:\${AWS::AccountId}:function:${functionName}:*`,
},
];
}

return [{
action: 'lambda:InvokeFunction',
resource: functionArn,
resource: functionArns,
}];
} if (_.has(functionName, 'Fn::GetAtt')) {
// because the FunctionName parameter can be either a name or ARN
// so you should be able to use Fn::GetAtt here to get the ARN
const functionArn = translateLocalFunctionNames.bind(this)(functionName);
return [{
action: 'lambda:InvokeFunction',
resource: translateLocalFunctionNames.bind(this)(functionName),
resource: [
functionArn,
{ 'Fn::Sub': ['${functionArn}:*', { functionArn }] },
],
}];
} if (_.has(functionName, 'Ref')) {
// because the FunctionName parameter can be either a name or ARN
// so you should be able to use Ref here to get the function name
const functionArn = translateLocalFunctionNames.bind(this)(functionName);
return [{
action: 'lambda:InvokeFunction',
resource: {
'Fn::Sub': [
'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${FunctionName}',
{
FunctionName: translateLocalFunctionNames.bind(this)(functionName),
},
],
},
resource: [
{
'Fn::Sub': [
'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${functionArn}',
{ functionArn },
],
},
{
'Fn::Sub': [
'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${functionArn}:*',
{ functionArn },
],
},
],
}];
}

Expand Down Expand Up @@ -278,9 +298,13 @@ function getIamPermissions(taskStates) {

default:
if (isIntrinsic(state.Resource) || state.Resource.startsWith('arn:aws:lambda')) {
const functionArn = translateLocalFunctionNames.bind(this)(state.Resource);
return [{
action: 'lambda:InvokeFunction',
resource: translateLocalFunctionNames.bind(this)(state.Resource),
resource: [
functionArn,
{ 'Fn::Sub': ['${functionArn}:*', { functionArn }] },
],
}];
}
this.serverless.cli.consoleLog('Cannot generate IAM policy statement for Task state', state);
Expand Down
41 changes: 27 additions & 14 deletions lib/deploy/stepFunctions/compileIamRole.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe('#compileIamRole', () => {
const helloLambda = 'arn:aws:lambda:123:*:function:hello';
const worldLambda = 'arn:aws:lambda:*:*:function:world';
const fooLambda = 'arn:aws:lambda:us-west-2::function:foo_';
const barLambda = 'arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:bar';
const barLambda = 'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:bar';

const genStateMachine = (name, lambda1, lambda2) => ({
name,
Expand Down Expand Up @@ -131,8 +131,21 @@ describe('#compileIamRole', () => {
const policy = serverlessStepFunctions.serverless.service
.provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution
.Properties.Policies[0];
expect(policy.PolicyDocument.Statement[0].Resource)
.to.be.deep.equal([helloLambda, worldLambda, fooLambda, barLambda]);
expect(policy.PolicyDocument.Statement[0].Action).to.deep.equal(['lambda:InvokeFunction']);

const resources = policy.PolicyDocument.Statement[0].Resource;
expect(resources).to.have.lengthOf(8);

expect(resources).to.include.members([helloLambda, worldLambda, fooLambda, barLambda]);

const versionResources = resources.filter(x => x['Fn::Sub']);
versionResources.forEach((x) => {
const template = x['Fn::Sub'][0];
expect(template).to.equal('${functionArn}:*');
});

const versionedArns = versionResources.map(x => x['Fn::Sub'][1].functionArn);
expect(versionedArns).to.deep.equal([helloLambda, worldLambda, fooLambda, barLambda]);
});

it('should give sns:Publish permission for only SNS topics referenced by state machine', () => {
Expand Down Expand Up @@ -786,7 +799,7 @@ describe('#compileIamRole', () => {

const lambdaPermissions = statements.filter(s => _.isEqual(s.Action, ['lambda:InvokeFunction']));
expect(lambdaPermissions).to.have.lengthOf(1);
expect(lambdaPermissions[0].Resource).to.deep.eq([lambda1, lambda2]);
expect(lambdaPermissions[0].Resource).to.include.members([lambda1, lambda2]);

const snsPermissions = statements.filter(s => _.isEqual(s.Action, ['sns:Publish']));
expect(snsPermissions).to.have.lengthOf(1);
Expand Down Expand Up @@ -969,7 +982,7 @@ describe('#compileIamRole', () => {
const statements = policy.PolicyDocument.Statement;

const lambdaPermissions = statements.find(x => x.Action[0] === 'lambda:InvokeFunction');
expect(lambdaPermissions.Resource).to.be.deep.equal([
expect(lambdaPermissions.Resource).to.deep.include.members([
{ Ref: 'MyFunction' }, { Ref: 'MyFunction2' }]);

const snsPermissions = statements.find(x => x.Action[0] === 'sns:Publish');
Expand Down Expand Up @@ -1130,7 +1143,7 @@ describe('#compileIamRole', () => {
'arn:aws:lambda:us-west-2:1234567890:function:c',
{ 'Fn::Sub': 'arn:aws:lambda:${AWS::Region}:1234567890:function:d' },
];
expect(lambdaPermissions[0].Resource).to.deep.eq(lambdaArns);
expect(lambdaPermissions[0].Resource).to.deep.include.members(lambdaArns);
});

it('should support lambda::invoke resource type', () => {
Expand Down Expand Up @@ -1183,7 +1196,7 @@ describe('#compileIamRole', () => {
'arn:aws:lambda:us-west-2:1234567890:function:c',
{ 'Fn::Sub': 'arn:aws:lambda:${AWS::Region}:1234567890:function:d' },
];
expect(lambdaPermissions[0].Resource).to.deep.eq(lambdaArns);
expect(lambdaPermissions[0].Resource).to.deep.include.members(lambdaArns);
});

it('should support intrinsic functions for lambda::invoke resource type', () => {
Expand Down Expand Up @@ -1238,8 +1251,8 @@ describe('#compileIamRole', () => {
const lambdaArns = [
{
'Fn::Sub': [
'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${FunctionName}',
{ FunctionName: lambda1 },
'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${functionArn}',
{ functionArn: lambda1 },
],
},
{
Expand All @@ -1257,7 +1270,7 @@ describe('#compileIamRole', () => {
],
},
];
expect(lambdaPermissions[0].Resource).to.deep.eq(lambdaArns);
expect(lambdaPermissions[0].Resource).to.deep.include.members(lambdaArns);
});

it('should support local function names', () => {
Expand Down Expand Up @@ -1305,7 +1318,7 @@ describe('#compileIamRole', () => {
],
},
];
expect(lambdaPermissions[0].Resource).to.deep.eq(lambdaArns);
expect(lambdaPermissions[0].Resource).to.deep.include.members(lambdaArns);
});

it('should support local function names for lambda::invoke resource type', () => {
Expand Down Expand Up @@ -1356,8 +1369,8 @@ describe('#compileIamRole', () => {
const lambdaArns = [
{
'Fn::Sub': [
'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${FunctionName}',
{ FunctionName: { Ref: 'HelloDashworldLambdaFunction' } },
'arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${functionArn}',
{ functionArn: { Ref: 'HelloDashworldLambdaFunction' } },
],
},
{
Expand All @@ -1367,6 +1380,6 @@ describe('#compileIamRole', () => {
],
},
];
expect(lambdaPermissions[0].Resource).to.deep.eq(lambdaArns);
expect(lambdaPermissions[0].Resource).to.deep.include.members(lambdaArns);
});
});
78 changes: 26 additions & 52 deletions lib/deploy/stepFunctions/compileStateMachines.js
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
'use strict';

const _ = require('lodash');
const BbPromise = require('bluebird');
const Joi = require('@hapi/joi');
const Chance = require('chance');
const { isIntrinsic, translateLocalFunctionNames } = require('../../utils/aws');
const BbPromise = require('bluebird');
const schema = require('./compileStateMachines.schema');
const { isIntrinsic, translateLocalFunctionNames, convertToFunctionVersion } = require('../../utils/aws');

const chance = new Chance();

Expand All @@ -14,22 +16,14 @@ function randomName() {
});
}

function toTags(obj, serverless) {
function toTags(obj) {
const tags = [];

if (!obj) {
return tags;
}

if (_.isPlainObject(obj)) {
_.forEach(
obj,
(Value, Key) => tags.push({ Key, Value: Value.toString() }),
);
} else {
throw new serverless.classes
.Error('Unable to parse tags, it should be an object.');
}
_.forEach(obj, (Value, Key) => tags.push({ Key, Value: Value.toString() }));

return tags;
}
Expand Down Expand Up @@ -75,7 +69,15 @@ module.exports = {
let DefinitionString;
let RoleArn;
let DependsOn = [];
const Tags = toTags(this.serverless.service.provider.tags, this.serverless);
const Tags = toTags(this.serverless.service.provider.tags);

const { error } = Joi.validate(stateMachineObj, schema, { allowUnknown: false });
if (error) {
const errorMessage = `State machine [${stateMachineName}] is malformed. `
+ 'Please check the README for more info. '
+ `${error}`;
throw new this.serverless.classes.Error(errorMessage);
}

if (stateMachineObj.definition) {
if (typeof stateMachineObj.definition === 'string') {
Expand All @@ -98,38 +100,17 @@ module.exports = {
};
}
}
} else {
const errorMessage = [
`Missing "definition" property in stateMachine ${stateMachineName}`,
' Please check the README for more info.',
].join('');
throw new this.serverless.classes
.Error(errorMessage);
}

if (stateMachineObj.useExactVersion === true && DefinitionString['Fn::Sub']) {
const params = DefinitionString['Fn::Sub'][1];
const f = convertToFunctionVersion.bind(this);
const converted = _.mapValues(params, f);
DefinitionString['Fn::Sub'][1] = converted;
}

if (stateMachineObj.role) {
if (typeof stateMachineObj.role === 'string') {
if (stateMachineObj.role.startsWith('arn:aws')) {
RoleArn = stateMachineObj.role;
} else {
const errorMessage = [
`role property in stateMachine "${stateMachineName}" is not ARN`,
' Please check the README for more info.',
].join('');
throw new this.serverless.classes
.Error(errorMessage);
}
} else if (isIntrinsic(stateMachineObj.role)) {
RoleArn = stateMachineObj.role;
} else {
const errorMessage = [
`role property in stateMachine "${stateMachineName}" is neither a string`,
' nor a CloudFormation intrinsic function',
' Please check the README for more info.',
].join('');
throw new this.serverless.classes
.Error(errorMessage);
}
RoleArn = stateMachineObj.role;
} else {
RoleArn = {
'Fn::GetAtt': [
Expand All @@ -143,22 +124,15 @@ module.exports = {
if (stateMachineObj.dependsOn) {
const dependsOn = stateMachineObj.dependsOn;

if (_.isArray(dependsOn) && _.every(dependsOn, _.isString)) {
if (_.isArray(dependsOn)) {
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);
DependsOn.push(dependsOn);
}
}

if (stateMachineObj.tags) {
const stateMachineTags = toTags(stateMachineObj.tags, this.serverless);
const stateMachineTags = toTags(stateMachineObj.tags);
_.forEach(stateMachineTags, tag => Tags.push(tag));
}

Expand Down
44 changes: 44 additions & 0 deletions lib/deploy/stepFunctions/compileStateMachines.schema.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
const Joi = require('@hapi/joi');

const arn = Joi.alternatives().try(
Joi.string().regex(/^arn:aws/, 'ARN'),
Joi.object().keys({
Ref: Joi.string(),
}),
Joi.object().keys({
'Fn::GetAtt': Joi.array().items(Joi.string()),
}),
);

const definition = Joi.alternatives().try(
Joi.string(),
Joi.object(),
);

const dependsOn = Joi.alternatives().try(
Joi.string(),
Joi.array().items(Joi.string()),
);

const id = Joi.string();
const tags = Joi.object();
const name = Joi.string();
const events = Joi.array();
const alarms = Joi.object();
const notifications = Joi.object();
const useExactVersion = Joi.boolean().default(false);

const schema = Joi.object().keys({
id,
events,
name,
role: arn,
useExactVersion,
definition: definition.required(),
dependsOn,
tags,
alarms,
notifications,
});

module.exports = schema;

0 comments on commit 01289fa

Please sign in to comment.