Merge pull request #304 from purple-technology/master

Generating random variable names for intrinsic functions breaks Serverless caching

Author: theburningmonk

lib/deploy/stepFunctions/compileNotifications.js

@@ -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',
 ];
@@ -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) {
@@ -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',
           },
@@ -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',
           },
@@ -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,
       };
     }
@@ -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 = {

lib/deploy/stepFunctions/compileNotifications.test.js

@@ -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,

lib/deploy/stepFunctions/compileStateMachines.js

@@ -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) {
@@ -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 {
@@ -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];

lib/deploy/stepFunctions/compileStateMachines.test.js

@@ -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: {

package-lock.json

@@ -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"
   },