DX: Throw if missing RoleName in Custom function role

[DavidWells]

If RoleName is missing in a custom function role this passes through undefined and single sls deploy -f funcName fails with a hard to decipher error:

  Serverless Error ---------------------------------------

  ServerlessError: The role defined for the function cannot be assumed by Lambda.
      at /service/node_modules/serverless/lib/plugins/aws/provider/awsProvider.js:330:27
      at processTicksAndRejections (internal/process/task_queues.js:85:5)

After turning on SLS_DEBUG I found the issue where the IAM role name was undefined

Serverless: [AWS lambda 400 0.417s 0 retries] updateFunctionConfiguration({
  FunctionName: 'service-dev-addNewUser',
  Description: 'Add user test function. Gated by imported Managed policy',
  Handler: 'examples/new-user.handler',
  Role: 'arn:aws:iam::01200101001:role/undefined'
})

This PR will throw an error if this value is missing (aka undefined) & the updateFunctionConfiguration won't be called with incorrect values

---

[codecov-commenter]

Codecov Report

Merging #8219 into master will decrease coverage by 0.00%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #8219      +/-   ##
==========================================
- Coverage   88.16%   88.16%   -0.01%     
==========================================
  Files         248      250       +2     
  Lines        9430     9393      -37     
==========================================
- Hits         8314     8281      -33     
+ Misses       1116     1112       -4     

Impacted Files	Coverage Δ
lib/plugins/aws/deployFunction/index.js	89.56% <50.00%> (-0.71%)	⬇️
...ins/aws/package/compile/events/alexaSkill/index.js	96.29% <0.00%> (-3.71%)	⬇️
lib/plugins/aws/package/compile/functions/index.js	96.64% <0.00%> (-0.61%)	⬇️
...plugins/aws/package/compile/events/stream/index.js	98.00% <0.00%> (-0.60%)	⬇️
.../package/compile/events/websockets/lib/validate.js	97.22% <0.00%> (-0.51%)	⬇️
lib/plugins/aws/info/display.js	81.92% <0.00%> (-0.22%)	⬇️
lib/utils/analytics/generatePayload.js	96.00% <0.00%> (-0.16%)	⬇️
lib/configSchema.js	100.00% <0.00%> (ø)
lib/plugins/print/print.js	100.00% <0.00%> (ø)
lib/utils/getCommandSuggestion.js	100.00% <0.00%> (ø)
... and 27 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4af57a5...8bfc5ba. Read the comment docs.

[medikoo]

@DavidWells great thanks for stepping in. Still it's not clear to me what exactly why is the issue.

Why exactly we should require RoleName to be defined on custom defined role? (It's not required on AWS side)

Also note that change you propose introduce ReferenceError (roleProperties variable is used before it actually is defined)

[DavidWells]

Without a RoleName defined, serverless passes through an undefined variable here. This results in 'arn:aws:iam::01200101001:role/undefined'

Serverless: [AWS lambda 400 0.417s 0 retries] updateFunctionConfiguration({
  FunctionName: 'service-dev-addNewUser',
  Description: 'Add user test function. Gated by imported Managed policy',
  Handler: 'examples/new-user.handler',
  Role: 'arn:aws:iam::01200101001:role/undefined'
})

Example yml to reproduce. If you remove RoleName it will break the deployment with 'arn:aws:iam::01200101001:role/undefined'

service: xyz

provider:
  name: aws
  runtime: nodejs12.x

functions:
  # Function with role assigned
  myFunc:
    handler: func.handler
    role: { Fn::GetAtt: [ MyCustomRole, Arn ] }

resources:
  Resources:
    MyCustomRole:
      Type: AWS::IAM::Role
      Properties:
        RoleName: MyCustomRole-${self:provider.stage}
        Description: IAM function for function
        AssumeRolePolicyDocument:
          Version: '2012-10-17'
          Statement:
          - Effect: Allow
            Principal:
              Service:
              - lambda.amazonaws.com
            Action: sts:AssumeRole

[medikoo]

Great thanks @DavidWells for explanations, all clear now!

Intention looks legitimate. Still patch is placed at wrong line (producing ReferenceError), and I believe we should throw ServerlessError (it's designated for operational errors, so errors that signal problem with how user use the Framework, and not Framework itself)

[medikoo]

Thank you @DavidWells !