Revert "Merge pull request #190 from horike37/feature/intrinsic_functions"

theburningmonk · theburningmonk · commit f9e913f5c19c · 2019-04-30T21:56:23.000+01:00
This reverts commit bcdd812, reversing changes made to 2b006d6.
diff --git a/README.md b/README.md
@@ -15,13 +15,12 @@ plugins:
 ```
 
 ## Setup
-Specifies your statemachine definition using Amazon States Language in a `definition` statement in serverless.yml. You can use CloudFormation intrinsic functions such as `Ref` and `Fn::GetAtt` to reference Lambda functions, SNS topics, SQS queues and DynamoDB tables declared in the same `serverless.yml`.
-
-Alternatively, you can also provide the raw ARN, or SQS queue URL, or DynamoDB table name as a string. If you need to construct the ARN by hand, then we recommend to use the [serverless-pseudo-parameters](https://www.npmjs.com/package/serverless-pseudo-parameters) plugin together to make it easier.
+Specifies your statemachine definition using Amazon States Language in a `definition` statement in serverless.yml.
+We recommend to use [serverless-pseudo-parameters](https://www.npmjs.com/package/serverless-pseudo-parameters) plugin together so that it makes it easy to set up `Resource` section under `definition`.
 
 ```yml
 functions:
-  hello:
+  hellofunc:
     handler: handler.hello
 
 stepFunctions:
@@ -46,8 +45,7 @@ stepFunctions:
         States:
           HelloWorld1:
             Type: Task
-            Resource:
-              Ref: HelloLambdaFunction
+            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello
             End: true
       dependsOn: CustomIamRole
       tags:
@@ -70,8 +68,7 @@ stepFunctions:
         States:
           HelloWorld2:
             Type: Task
-            Resource:
-              Ref: HelloLambdaFunction
+            Resource: arn:aws:states:#{AWS::Region}:#{AWS::AccountId}:activity:myTask
             End: true
       dependsOn:
         - DynamoDBTable
@@ -518,8 +515,7 @@ functions:
             States:
               HelloWorld1:
                 Type: Task
-                Resource:
-                  Ref: HelloLambdaFunction
+                Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello
                 End: true
 
 
@@ -828,8 +824,7 @@ stepFunctions:
         States:
           FirstState:
             Type: Task
-            Resource:
-              Ref: HelloLambdaFunction
+            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello
             Next: wait_using_seconds
           wait_using_seconds:
             Type: Wait
@@ -849,8 +844,7 @@ stepFunctions:
             Next: FinalState
           FinalState:
             Type: Task
-            Resource:
-              Ref: HelloLambdaFunction
+            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello
             End: true
 plugins:
   - serverless-step-functions
@@ -872,8 +866,7 @@ stepFunctions:
         States:
           HelloWorld:
             Type: Task
-            Resource:
-              Ref: HelloLambdaFunction
+            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello
             Retry:
             - ErrorEquals:
               - HandledError
@@ -953,8 +946,7 @@ stepFunctions:
         States:
           HelloWorld:
             Type: Task
-            Resource:
-              Ref: HelloLambdaFunction
+            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello
             Catch:
             - ErrorEquals: ["HandledError"]
               Next: CustomErrorFallback
@@ -1002,8 +994,7 @@ stepFunctions:
         States:
           FirstState:
             Type: Task
-            Resource:
-              Ref: Hello1LambdaFunction
+            Resource: arn:aws:lambda:${opt:region}:${self:custom.accountId}:function:${self:service}-${opt:stage}-hello1
             Next: ChoiceState
           ChoiceState:
             Type: Choice
@@ -1017,21 +1008,18 @@ stepFunctions:
             Default: DefaultState
           FirstMatchState:
             Type: Task
-            Resource:
-              Ref: Hello2LambdaFunction
+            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello2
             Next: NextState
           SecondMatchState:
             Type: Task
-            Resource:
-              Ref: Hello3LambdaFunction
+            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello3
             Next: NextState
           DefaultState:
             Type: Fail
             Cause: "No Matches!"
           NextState:
             Type: Task
-            Resource:
-              Ref: Hello4LambdaFunction
+            Resource: arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${self:service}-${opt:stage}-hello4
             End: true
 plugins:
   - serverless-step-functions
diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js
@@ -2,7 +2,6 @@
 const _ = require('lodash');
 const BbPromise = require('bluebird');
 const path = require('path');
-const { isIntrinsic } = require('../../utils/aws');
 
 function getTaskStates(states) {
   return _.flatMap(states, state => {
@@ -29,12 +28,6 @@ function sqsQueueUrlToArn(serverless, queueUrl) {
     const accountId = match[2];
     const queueName = match[3];
     return `arn:aws:sqs:${region}:${accountId}:${queueName}`;
-  } else if (isIntrinsic(queueUrl) && queueUrl.Ref) {
-    // most likely we'll see a { Ref: LogicalId }, which we need to map to
-    // { Fn::GetAtt: [ LogicalId, Arn ] } to get the ARN
-    return {
-      'Fn::GetAtt': [queueUrl.Ref, 'Arn'],
-    };
   }
   serverless.cli.consoleLog(`Unable to parse SQS queue url [${queueUrl}]`);
   return [];
@@ -65,14 +58,6 @@ function getSnsPermissions(serverless, state) {
 }
 
 function getDynamoDBArn(tableName) {
-  if (isIntrinsic(tableName) && tableName.Ref) {
-    // most likely we'll see a { Ref: LogicalId }, which we need to map to
-    // { Fn::GetAtt: [ LogicalId, Arn ] } to get the ARN
-    return {
-      'Fn::GetAtt': [tableName.Ref, 'Arn'],
-    };
-  }
-
   return {
     'Fn::Join': [
       ':',
@@ -212,7 +197,7 @@ function getIamPermissions(serverless, taskStates) {
         return getEcsPermissions();
 
       default:
-        if (isIntrinsic(state.Resource) || state.Resource.startsWith('arn:aws:lambda')) {
+        if (state.Resource.startsWith('arn:aws:lambda')) {
           return [{
             action: 'lambda:InvokeFunction',
             resource: state.Resource,
diff --git a/lib/deploy/stepFunctions/compileIamRole.test.js b/lib/deploy/stepFunctions/compileIamRole.test.js
@@ -900,103 +900,4 @@ describe('#compileIamRole', () => {
       .Properties.Policies[0];
     expectDenyAllPolicy(policy);
   });
-
-  it('should respect CloudFormation intrinsic functions for Resource', () => {
-    serverless.service.stepFunctions = {
-      stateMachines: {
-        myStateMachine: {
-          name: 'stateMachine',
-          definition: {
-            StartAt: 'Lambda',
-            States: {
-              Lambda: {
-                Type: 'Task',
-                Resource: {
-                  Ref: 'MyFunction',
-                },
-                Next: 'Sns',
-              },
-              Sns: {
-                Type: 'Task',
-                Resource: 'arn:aws:states:::sns:publish',
-                Parameters: {
-                  Message: {
-                    'Fn::GetAtt': ['MyTopic', 'TopicName'],
-                  },
-                  TopicArn: {
-                    Ref: 'MyTopic',
-                  },
-                },
-                Next: 'Sqs',
-              },
-              Sqs: {
-                Type: 'Task',
-                Resource: 'arn:aws:states:::sqs:sendMessage',
-                Parameters: {
-                  QueueUrl: {
-                    Ref: 'MyQueue',
-                  },
-                  MessageBody: 'This is a static message',
-                },
-                Next: 'DynamoDB',
-              },
-              DynamoDB: {
-                Type: 'Task',
-                Resource: 'arn:aws:states:::dynamodb:putItem',
-                Parameters: {
-                  TableName: {
-                    Ref: 'MyTable',
-                  },
-                },
-                Next: 'Parallel',
-              },
-              Parallel: {
-                Type: 'Parallel',
-                End: true,
-                Branches: [
-                  {
-                    StartAt: 'Lambda2',
-                    States: {
-                      Lambda2: {
-                        Type: 'Task',
-                        Resource: {
-                          Ref: 'MyFunction2',
-                        },
-                        End: true,
-                      },
-                    },
-                  },
-                ],
-              },
-            },
-          },
-        },
-      },
-    };
-
-    serverlessStepFunctions.compileIamRole();
-    serverlessStepFunctions.compileStateMachines();
-    const policy = serverlessStepFunctions.serverless.service
-      .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution
-      .Properties.Policies[0];
-
-    const statements = policy.PolicyDocument.Statement;
-
-    const lambdaPermissions = statements.find(x => x.Action[0] === 'lambda:InvokeFunction');
-    expect(lambdaPermissions.Resource).to.be.deep.equal([
-      { Ref: 'MyFunction' }, { Ref: 'MyFunction2' }]);
-
-    const snsPermissions = statements.find(x => x.Action[0] === 'sns:Publish');
-    expect(snsPermissions.Resource).to.be.deep.equal([{ Ref: 'MyTopic' }]);
-
-    const sqsPermissions = statements.find(x => x.Action[0] === 'sqs:SendMessage');
-    expect(sqsPermissions.Resource).to.be.deep.equal([{
-      'Fn::GetAtt': ['MyQueue', 'Arn'],
-    }]);
-
-    const dynamodbPermissions = statements.find(x => x.Action[0] === 'dynamodb:PutItem');
-    expect(dynamodbPermissions.Resource).to.be.deep.equal([{
-      'Fn::GetAtt': ['MyTable', 'Arn'],
-    }]);
-  });
 });
diff --git a/lib/deploy/stepFunctions/compileStateMachines.js b/lib/deploy/stepFunctions/compileStateMachines.js
@@ -1,12 +1,10 @@
 'use strict';
 const _ = require('lodash');
 const BbPromise = require('bluebird');
-const { isIntrinsic } = require('../../utils/aws');
 
-function randomName() {
-  const chars = 'abcdefghijklmnopqrstufwxyzABCDEFGHIJKLMNOPQRSTUFWXYZ1234567890';
-  const pwd = _.sampleSize(chars, 10);
-  return pwd.join('');
+function isIntrinsic(obj) {
+  const isObject = typeof obj === 'object';
+  return isObject && Object.keys(obj).some((k) => k.startsWith('Fn::') || k.startsWith('Ref'));
 }
 
 function toTags(obj, serverless) {
@@ -28,40 +26,8 @@ function toTags(obj, serverless) {
   return tags;
 }
 
-// return an iterable of
-// [ ParamName,  IntrinsicFunction ]
-// e.g. [ 'mptFnX05Fb', { Ref: 'MyTopic' } ]
-// this makes it easy to use _.fromPairs to construct an object afterwards
-function* getIntrinsicFunctions(obj) {
-  // eslint-disable-next-line no-restricted-syntax
-  for (const key in obj) {
-    if (Object.prototype.hasOwnProperty.call(obj, key)) {
-      const value = obj[key];
-
-      if (Array.isArray(value)) {
-        // eslint-disable-next-line guard-for-in, no-restricted-syntax
-        for (const idx in value) {
-          const innerFuncs = Array.from(getIntrinsicFunctions(value[idx]));
-          for (const x of innerFuncs) {
-            yield x;
-          }
-        }
-      } else if (isIntrinsic(value)) {
-        const paramName = randomName();
-        // eslint-disable-next-line no-param-reassign
-        obj[key] = `\${${paramName}}`;
-        yield [paramName, value];
-      } else if (typeof value === 'object') {
-        const innerFuncs = Array.from(getIntrinsicFunctions(value));
-        for (const x of innerFuncs) {
-          yield x;
-        }
-      }
-    }
-  }
-}
-
 module.exports = {
+  isIntrinsic,
   compileStateMachines() {
     if (this.isStateMachines()) {
       this.getAllStateMachines().forEach((stateMachineName) => {
@@ -76,17 +42,7 @@ module.exports = {
             DefinitionString = JSON.stringify(stateMachineObj.definition)
               .replace(/\\n|\\r|\\n\\r/g, '');
           } else {
-            const functionMappings = Array.from(getIntrinsicFunctions(stateMachineObj.definition));
-            if (_.isEmpty(functionMappings)) {
-              DefinitionString = JSON.stringify(stateMachineObj.definition, undefined, 2);
-            } else {
-              DefinitionString = {
-                'Fn::Sub': [
-                  JSON.stringify(stateMachineObj.definition, undefined, 2),
-                  _.fromPairs(functionMappings),
-                ],
-              };
-            }
+            DefinitionString = JSON.stringify(stateMachineObj.definition, undefined, 2);
           }
         } else {
           const errorMessage = [
diff --git a/lib/deploy/stepFunctions/compileStateMachines.test.js b/lib/deploy/stepFunctions/compileStateMachines.test.js
diff --git a/lib/utils/aws.js b/lib/utils/aws.js
