From 6d7103da02dcbc4f89949dcebdf4ac6745b91776 Mon Sep 17 00:00:00 2001
From: Mariusz Nowak <medyk@medikoo.com>
Date: Wed, 2 Sep 2020 10:33:22 +0200
Subject: [PATCH] refactor: Seclude IAM role resource name resolution logic

---
 .../aws/package/compile/events/sqs/index.js   | 33 ++-----------------
 .../package/compile/events/stream/index.js    | 31 +----------------
 lib/plugins/aws/provider/awsProvider.js       | 30 +++++++++++++++++
 3 files changed, 34 insertions(+), 60 deletions(-)

diff --git a/lib/plugins/aws/package/compile/events/sqs/index.js b/lib/plugins/aws/package/compile/events/sqs/index.js
index 7eec169ff..edaaf6b79 100644
--- a/lib/plugins/aws/package/compile/events/sqs/index.js
+++ b/lib/plugins/aws/package/compile/events/sqs/index.js
@@ -96,36 +96,9 @@ class AwsCompileSQSEvents {
 
             const queueLogicalId = this.provider.naming.getQueueLogicalId(functionName, queueName);
 
-            const funcRole = functionObj.role || this.serverless.service.provider.role;
-            let dependsOn = '"IamRoleLambdaExecution"';
-            if (funcRole) {
-              if (
-                // check whether the custom role is an ARN
-                typeof funcRole === 'string' &&
-                funcRole.indexOf(':') !== -1
-              ) {
-                dependsOn = '[]';
-              } else if (
-                // otherwise, check if we have an in-service reference to a role ARN
-                typeof funcRole === 'object' &&
-                'Fn::GetAtt' in funcRole &&
-                Array.isArray(funcRole['Fn::GetAtt']) &&
-                funcRole['Fn::GetAtt'].length === 2 &&
-                typeof funcRole['Fn::GetAtt'][0] === 'string' &&
-                typeof funcRole['Fn::GetAtt'][1] === 'string' &&
-                funcRole['Fn::GetAtt'][1] === 'Arn'
-              ) {
-                dependsOn = `"${funcRole['Fn::GetAtt'][0]}"`;
-              } else if (
-                // otherwise, check if we have an import
-                typeof funcRole === 'object' &&
-                'Fn::ImportValue' in funcRole
-              ) {
-                dependsOn = '[]';
-              } else if (typeof funcRole === 'string') {
-                dependsOn = `"${funcRole}"`;
-              }
-            }
+            const dependsOn = JSON.stringify(
+              this.provider.resolveFunctionIamRoleResourceName(functionObj) || []
+            );
             const sqsTemplate = `
               {
                 "Type": "AWS::Lambda::EventSourceMapping",
diff --git a/lib/plugins/aws/package/compile/events/stream/index.js b/lib/plugins/aws/package/compile/events/stream/index.js
index bbc1dfb97..36ca5cc98 100644
--- a/lib/plugins/aws/package/compile/events/stream/index.js
+++ b/lib/plugins/aws/package/compile/events/stream/index.js
@@ -192,36 +192,7 @@ class AwsCompileStreamEvents {
               streamName
             );
 
-            const funcRole = functionObj.role || this.serverless.service.provider.role;
-            let dependsOn = 'IamRoleLambdaExecution';
-            if (funcRole) {
-              if (
-                // check whether the custom role is an ARN
-                typeof funcRole === 'string' &&
-                funcRole.indexOf(':') !== -1
-              ) {
-                dependsOn = [];
-              } else if (
-                // otherwise, check if we have an in-service reference to a role ARN
-                typeof funcRole === 'object' &&
-                'Fn::GetAtt' in funcRole &&
-                Array.isArray(funcRole['Fn::GetAtt']) &&
-                funcRole['Fn::GetAtt'].length === 2 &&
-                typeof funcRole['Fn::GetAtt'][0] === 'string' &&
-                typeof funcRole['Fn::GetAtt'][1] === 'string' &&
-                funcRole['Fn::GetAtt'][1] === 'Arn'
-              ) {
-                dependsOn = funcRole['Fn::GetAtt'][0];
-              } else if (
-                // otherwise, check if we have an import or parameters ref
-                typeof funcRole === 'object' &&
-                ('Fn::ImportValue' in funcRole || 'Ref' in funcRole)
-              ) {
-                dependsOn = [];
-              } else if (typeof funcRole === 'string') {
-                dependsOn = funcRole;
-              }
-            }
+            const dependsOn = this.provider.resolveFunctionIamRoleResourceName(functionObj) || [];
             const streamResource = {
               Type: 'AWS::Lambda::EventSourceMapping',
               DependsOn: dependsOn,
diff --git a/lib/plugins/aws/provider/awsProvider.js b/lib/plugins/aws/provider/awsProvider.js
index 2181e799b..18f54e87c 100644
--- a/lib/plugins/aws/provider/awsProvider.js
+++ b/lib/plugins/aws/provider/awsProvider.js
@@ -724,6 +724,36 @@ class AwsProvider {
     throw new Error(`Unrecognized function address ${functionAddress}`);
   }
 
+  resolveFunctionIamRoleResourceName(functionObj) {
+    const customRole = functionObj.role || this.serverless.service.provider.role;
+    if (customRole) {
+      if (typeof customRole === 'string') {
+        // check whether the custom role is an ARN
+        if (customRole.includes(':')) return null;
+        return customRole;
+      }
+      if (
+        // otherwise, check if we have an in-service reference to a role ARN
+        customRole['Fn::GetAtt'] &&
+        Array.isArray(customRole['Fn::GetAtt']) &&
+        customRole['Fn::GetAtt'].length === 2 &&
+        typeof customRole['Fn::GetAtt'][0] === 'string' &&
+        typeof customRole['Fn::GetAtt'][1] === 'string' &&
+        customRole['Fn::GetAtt'][1] === 'Arn'
+      ) {
+        return customRole['Fn::GetAtt'][0];
+      }
+      if (
+        // otherwise, check if we have an import or parameters ref
+        customRole['Fn::ImportValue'] ||
+        customRole.Ref
+      ) {
+        return null;
+      }
+    }
+    return 'IamRoleLambdaExecution';
+  }
+
   getAlbTargetGroupPrefix() {
     const provider = this.serverless.service.provider;
     if (!provider.alb || !provider.alb.targetGroupPrefix) {