Skip to content

Generating random variable names for intrinsic functions breaks Serverless caching #304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 9, 2020
Merged

Generating random variable names for intrinsic functions breaks Serverless caching #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 40 additions & 34 deletions lib/deploy/stepFunctions/compileNotifications.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@

const _ = require('lodash');
const Joi = require('@hapi/joi');
const Chance = require('chance');
const crypto = require('crypto');
const BbPromise = require('bluebird');
const schema = require('./compileNotifications.schema');

const chance = new Chance();

const executionStatuses = [
'ABORTED', 'FAILED', 'RUNNING', 'SUCCEEDED', 'TIMED_OUT',
];
Expand All @@ -25,65 +23,72 @@ const targetPermissions = {
stepFunctions: 'states:StartExecution',
};

function randomTargetId(stateMachineName, status) {
const suffix = chance.string({
length: 5,
pool: 'abcdefghijklmnopqrstufwxyzABCDEFGHIJKLMNOPQRSTUFWXYZ1234567890',
});
function generateTargetId(target, index, stateMachineName, status) {
const suffix = crypto
.createHash('md5')
.update(JSON.stringify({ target, index }))
.digest('hex')
.substr(0, 5);

return `${stateMachineName}-${status}-${suffix}`;
}

function randomLogicalId(prefix) {
const suffix = chance.string({
length: 5,
pool: 'ABCDEFGHIJKLMNOPQRSTUFWXYZ',
});
function generateLogicalId(prefix, index, resource) {
const suffix = crypto
.createHash('md5')
.update(JSON.stringify({ index, resource }))
.digest('hex')
.substr(0, 5);
return `${prefix}${suffix}`;
}

function randomPolicyName(status, targetType) {
const suffix = chance.string({
length: 5,
pool: 'abcdefghijklmnopqrstufwxyzABCDEFGHIJKLMNOPQRSTUFWXYZ',
});
function generatePolicyName(status, targetType, action, resource) {
const suffix = crypto
.createHash('md5')
.update(JSON.stringify({ action, resource }))
.digest('hex')
.substr(0, 5);
return `${status}-${targetType}-${suffix}`;
}

function compileTarget(stateMachineName, status, targetObj, iamRoleLogicalId) {
function compileTarget(stateMachineName, status, targetObj, targetIndex, iamRoleLogicalId) {
// SQS and Kinesis are special cases as they can have additional props
if (_.has(targetObj, 'sqs.arn')) {
return {
const target = {
Arn: targetObj.sqs.arn,
Id: randomTargetId(stateMachineName, status),
SqsParameters: {
MessageGroupId: targetObj.sqs.messageGroupId,
},
};
target.Id = generateTargetId(target, targetIndex, stateMachineName, status);
return target;
} if (_.has(targetObj, 'kinesis.arn')) {
return {
const target = {
Arn: targetObj.kinesis.arn,
Id: randomTargetId(stateMachineName, status),
KinesisParameters: {
PartitionKeyPath: targetObj.kinesis.partitionKeyPath,
},
};
target.Id = generateTargetId(target, targetIndex, stateMachineName, status);
return target;
} if (_.has(targetObj, 'stepFunctions')) {
return {
const target = {
Arn: targetObj.stepFunctions,
Id: randomTargetId(stateMachineName, status),
RoleArn: {
'Fn::GetAtt': [iamRoleLogicalId, 'Arn'],
},
};
target.Id = generateTargetId(target, targetIndex, stateMachineName, status);
return target;
}

const targetType = supportedTargets.find(t => _.has(targetObj, t));
const arn = _.get(targetObj, targetType);
return {
const target = {
Arn: arn,
Id: randomTargetId(stateMachineName, status),
};
target.Id = generateTargetId(target, targetIndex, stateMachineName, status);
return target;
}

function compileSnsPolicy(status, snsTarget) {
Expand All @@ -93,7 +98,7 @@ function compileSnsPolicy(status, snsTarget) {
PolicyDocument: {
Version: '2012-10-17',
Statement: {
Sid: randomPolicyName(status, 'sns'),
Sid: generatePolicyName(status, 'sns', 'sns:Publish', snsTarget),
Principal: {
Service: 'events.amazonaws.com',
},
Expand Down Expand Up @@ -135,7 +140,7 @@ function compileSqsPolicy(status, sqsTarget) {
PolicyDocument: {
Version: '2012-10-17',
Statement: {
Sid: randomPolicyName(status, 'sqs'),
Sid: generatePolicyName(status, 'sqs', 'sqs:SendMessage', sqsTarget),
Principal: {
Service: 'events.amazonaws.com',
},
Expand Down Expand Up @@ -232,18 +237,19 @@ function bootstrapIamRole() {
function* compilePermissionResources(stateMachineLogicalId, iamRoleLogicalId, targets) {
const { iamRole, addPolicy } = bootstrapIamRole();

for (const { status, target } of targets) {
for (let index = 0; index < targets.length; index++) {
const { status, target } = targets[index];
const perm = compilePermissionForTarget(status, target);
if (perm.type === 'iam') {
const targetType = _.keys(target)[0];
addPolicy(
randomPolicyName(status, targetType),
generatePolicyName(status, targetType, perm.action, perm.resource),
perm.action,
perm.resource,
);
} else if (perm.type === 'policy') {
yield {
logicalId: randomLogicalId(`${stateMachineLogicalId}ResourcePolicy`),
logicalId: generateLogicalId(`${stateMachineLogicalId}ResourcePolicy`, index, perm.resource),
resource: perm.resource,
};
}
Expand Down Expand Up @@ -277,8 +283,8 @@ function* compileResources(stateMachineLogicalId, stateMachineName, notification
for (const status of executionStatuses) {
const targets = notificationsObj[status];
if (!_.isEmpty(targets)) {
const cfnTargets = targets.map(t => compileTarget(stateMachineName,
status, t, iamRoleLogicalId));
const cfnTargets = targets.map((t, index) => compileTarget(stateMachineName,
status, t, index, iamRoleLogicalId));

const eventRuleLogicalId = `${stateMachineLogicalId}Notifications${status.replace('_', '')}EventRule`;
const eventRule = {
Expand Down
49 changes: 49 additions & 0 deletions lib/deploy/stepFunctions/compileNotifications.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,55 @@ describe('#compileNotifications', () => {
expect(consoleLogSpy.callCount).equal(0);
});

it('should do deterministic compilation of CloudWatch Event Rules', () => {
const snsArn = { Ref: 'MyTopic' };
const sqsArn = { 'Fn::GetAtt': ['MyQueue', 'Arn'] };
const sqsNestedArn = { 'Fn::GetAtt': ['MyNestedQueue', 'Arn'] };
const lambdaArn = { 'Fn::GetAtt': ['MyFunction', 'Arn'] };
const kinesisArn = { 'Fn::GetAtt': ['MyStream', 'Arn'] };
const kinesisNestedArn = { 'Fn::GetAtt': ['MyNestedStream', 'Arn'] };
const firehoseArn = { 'Fn::GetAtt': ['MyDeliveryStream', 'Arn'] };
const stepFunctionsArn = { Ref: 'MyStateMachine' };
const targets = [
{ sns: snsArn },
{ sqs: sqsArn },
{ sqs: { arn: sqsNestedArn, messageGroupId: '12345' } },
{ lambda: lambdaArn },
{ kinesis: kinesisArn },
{ kinesis: { arn: kinesisNestedArn, partitionKeyPath: '$.id' } },
{ firehose: firehoseArn },
{ stepFunctions: stepFunctionsArn },
{ sns: 'SNS_TOPIC_ARN' },
{ sqs: 'SQS_QUEUE_ARN' },
{ sqs: 'arn:aws:sqs:#{AWS::Region}:#{AWS::AccountId}:MyQueue' },
{ sqs: { arn: 'SQS_QUEUE_NESTED_ARN', messageGroupId: '12345' } },
{ lambda: 'LAMBDA_FUNCTION_ARN' },
{ kinesis: 'KINESIS_STREAM_ARN' },
{ kinesis: { arn: 'KINESIS_STREAM_NESTED_ARN', partitionKeyPath: '$.id' } },
{ firehose: 'FIREHOSE_STREAM_ARN' },
{ stepFunctions: 'STATE_MACHINE_ARN' },
];

const definition = {
stateMachines: {
beta1: genStateMachineWithTargets('Beta1', targets),
beta2: genStateMachineWithTargets('Beta2', targets),
},
};

serverless.service.stepFunctions = _.cloneDeep(definition);
serverlessStepFunctions.compileNotifications();
const resources1 = _.cloneDeep(serverlessStepFunctions.serverless.service
.provider.compiledCloudFormationTemplate.Resources);

serverless.service.stepFunctions = _.cloneDeep(definition);
serverlessStepFunctions.compileNotifications();
const resources2 = _.cloneDeep(serverlessStepFunctions.serverless.service
.provider.compiledCloudFormationTemplate.Resources);

expect(resources1).to.deep.equal(resources2);
});

it('should not generate resources when no notifications are defined', () => {
const genStateMachine = name => ({
name,
Expand Down
18 changes: 8 additions & 10 deletions lib/deploy/stepFunctions/compileStateMachines.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,16 @@
const _ = require('lodash');
const Joi = require('@hapi/joi');
const aslValidator = require('asl-validator');
const Chance = require('chance');
const BbPromise = require('bluebird');
const crypto = require('crypto');
const schema = require('./compileStateMachines.schema');
const { isIntrinsic, translateLocalFunctionNames, convertToFunctionVersion } = require('../../utils/aws');

const chance = new Chance();

function randomName() {
return chance.string({
length: 10,
pool: 'abcdefghijklmnopqrstufwxyzABCDEFGHIJKLMNOPQRSTUFWXYZ1234567890',
});
function generateSubVariableName(element) {
return crypto
.createHash('md5')
.update(JSON.stringify(element))
.digest('hex');
}

function toTags(obj) {
Expand Down Expand Up @@ -44,7 +42,7 @@ function* getIntrinsicFunctions(obj) {
for (const idx in value) {
const element = value[idx];
if (isIntrinsic(element)) {
const paramName = randomName();
const paramName = generateSubVariableName(element);
value[idx] = `\${${paramName}}`;
yield [paramName, element];
} else {
Expand All @@ -55,7 +53,7 @@ function* getIntrinsicFunctions(obj) {
}
}
} else if (isIntrinsic(value)) {
const paramName = randomName();
const paramName = generateSubVariableName(value);
// eslint-disable-next-line no-param-reassign
obj[key] = `\${${paramName}}`;
yield [paramName, value];
Expand Down
61 changes: 61 additions & 0 deletions lib/deploy/stepFunctions/compileStateMachines.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,67 @@ describe('#compileStateMachines', () => {
});
});

it('should do deterministic compilcation', () => {
const definition = {
stateMachines: {
myStateMachine: {
name: 'stateMachine',
definition: {
StartAt: 'LambdaA',
States: {
LambdaA: {
Type: 'Task',
Resource: {
Ref: 'MyFunction',
},
Next: 'LambdaB',
},
LambdaB: {
Type: 'Task',
Resource: {
Ref: 'MyFunction2',
},
Next: 'Parallel',
},
Parallel: {
Type: 'Parallel',
End: true,
Branches: [
{
StartAt: 'Lambda2',
States: {
Lambda2: {
Type: 'Task',
Resource: {
Ref: 'MyFunction',
},
End: true,
},
},
},
],
},
},
},
},
},
};

serverless.service.stepFunctions = _.cloneDeep(definition);
serverlessStepFunctions.compileStateMachines();
const stateMachine1 = _.cloneDeep(serverlessStepFunctions.serverless.service
.provider.compiledCloudFormationTemplate.Resources
.StateMachine);

serverless.service.stepFunctions = _.cloneDeep(definition);
serverlessStepFunctions.compileStateMachines();
const stateMachine2 = _.cloneDeep(serverlessStepFunctions.serverless.service
.provider.compiledCloudFormationTemplate.Resources
.StateMachine);

expect(stateMachine1).to.deep.equal(stateMachine2);
});

it('should allow null values #193', () => {
serverless.service.stepFunctions = {
stateMachines: {
Expand Down
5 changes: 0 additions & 5 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
"aws-sdk": "^2.282.1",
"bluebird": "^3.4.0",
"chalk": "^1.1.1",
"chance": "^1.0.18",
"lodash": "^4.17.11",
"serverless": "^1.46.1"
},
Expand Down