Allow Role 'Fn::GetAtt' for Lambda role

[erikerikson]

What did you implement:

Fixes #3081 by allowing the role defined for a Lambda to be a reference (e.g. { 'Fn::GetAtt', [ 'LambdaLogicalId', 'Arn'] }) to a role in the current service. If the role attribute is defined, check whether it is a role reference and if it is, fill the DependsOn attribute for the event mapping to be that logical ID.
Add tests that make sure this use case is covered in future incarnations of the code.

How did you implement it:

evaluate the role attribute (from either function or provider) and if it is a Fn::GetAtt to an ARN (which uses a LogicalId) use that logical ID as the value for DependsOn.

How can we verify it:

See tests in this PR. Alternatively, define a role in your service. Then use that role by reference by defining role on the provider and subsequently on the function. Deploy with both configurations.

Todos:

• Write tests
• Write documentation
• Fix linting errors
• Make sure code coverage hasn't dropped
• Provide verification config/commands/resources
• Enable "Allow edits from maintainers" for this PR
• Change ready for review message below

Note that with regards to the documentation, this shouldn't be a change. I didn't check.

Is this ready for review?: YES
Is it a breaking change?: NO

[erikerikson]

@pmuens @eahefnawy - Can we please consider this for the next release? I've been using it for a deploy and don't want my team to fall behind or off the official build. Thanks!

[pmuens]

@erikerikson Thanks for the PR 👍 . Putting this into the v1.6 milestone...

[erikerikson]

Thanks @pmuens!

[pmuens]

Hey @erikerikson thanks for the PR!

I just gave it a spin today. Unfortunately I got the following error:

     An error occurred while provisioning your stack: HelloEventSourceMappingDynamodbTest
     - Cannot access stream arn:aws:dynamodb:us-east-1:XXXXX:table/test/stream/2017-01-25T13:33:32.098.
     Please ensure the role can perform the GetRecords, GetShardIterator,
     DescribeStream, and ListStreams Actions on your stream
     in IAM..

Here's the serverless.yml I'm using:

service: stream-test

provider:
  name: aws
  runtime: nodejs4.3
  cfLogs: true
  role:
    'Fn::GetAtt': [ 'test123', Arn ]

functions:
  hello:
    handler: handler.hello
    events:
      - stream: arn:aws:dynamodb:us-east-1:XXXXX:table/test/stream/2017-01-25T13:33:32.098
      - stream: arn:aws:kinesis:us-east-1:XXXXX:stream/test

resources:
  Resources:
    test123:
      Type: AWS::IAM::Role
      Properties:
        RoleName: test123
        Path: "/"
        AssumeRolePolicyDocument:
          Version: '2012-10-17'
          Statement:
            - Effect: Allow
              Principal:
                Service:
                  - lambda.amazonaws.com
              Action: sts:AssumeRole
        Policies:
          - PolicyName: myPolicyName
            PolicyDocument:
              Version: '2012-10-17'
              Statement:
                - Effect: Allow
                  Action:
                    - logs:CreateLogGroup
                    - logs:CreateLogStream
                    - logs:PutLogEvents
                  Resource: arn:aws:logs:us-east-1:XXXXX:log-group:/aws/lambda/*:*:*

Am I missing something? 🤔
Do you have a serverless.yml at hand we can use to validate this real quick?

[erikerikson]

This is the serverless.yml that I've needed this change to deploy:

https://github.com/Nordstrom/hello-retail/blob/initial-formation/product-catalog/serverless.yml

Line 20 is specifically the allowance of this change.

You would also need to deploy its dependency (following) to successfully deploy the serverless.yml above.

https://github.com/Nordstrom/hello-retail/blob/initial-formation/retail-stream/serverless.yml

As for your serverless.yml, you are defining a role that is applied to your function. The error you see happens because your declaration of a role bypasses the usual automated rights generation that would build IamPolicyLambdaExecution (1.). You also omit the standard set of Lambda rights for reading off those streams from the custom role you have defined. Given your serverless.yml, you would want to add the following policies:

          - PolicyName: ReadFromDynamoDb
            PolicyDocument:
              Version: '2012-10-17'
              Statement:
                - Effect: 'Allow'
                  Action:
                    - 'dynamodb:GetRecords'
                    - 'dynamodb:GetShardIterator'
                    - 'dynamodb:DescribeStream'
                    - 'dynamodb:ListStreams'
                  Resource: arn:aws:dynamodb:us-east-1:XXXXX:table/test/stream/2017-01-25T13:33:32.098
          - PolicyName: ReadFromKinesis
            PolicyDocument:
              Version: '2012-10-17'
              Statement:
                - Effect: 'Allow'
                  Action:
                    - 'kinesis:GetRecords'
                    - 'kinesis:GetShardIterator'
                    - 'kinesis:DescribeStream'
                    - 'kinesis:ListStreams'
                  Resource: arn:aws:kinesis:us-east-1:XXXXX:stream/test

1. Note that modifying a role provided at the provider or function level is considered inappropriate. Those means of supplying a role assert that the role is to be externally managed. The referenced role can be defined in the same service but it could also be imported or managed by a different team.

[pmuens]

Whoops. That's embarassing. Sorry for wasting your time @erikerikson 😞 I copied the role resource definition from some other location w/o thinking about it (damn it "copy and paste"!)...

You're 100% correct. Here's the serverless.yml I used to validate this PR (you can move the definition to the function level or provider level:

service: stream-test

provider:
  name: aws
  runtime: nodejs4.3
  cfLogs: true

functions:
  hello:
    handler: handler.hello
    role:
      'Fn::GetAtt': [ testRole, Arn ]
    events:
      - stream: arn:aws:dynamodb:us-east-1:XXXXX:table/test/stream/2017-01-26T12:11:48.130
      - stream: arn:aws:kinesis:us-east-1:XXXXX:stream/test

resources:
  Resources:
    testRole:
      Type: AWS::IAM::Role
      Properties:
        RoleName: testRole
        Path: "/"
        AssumeRolePolicyDocument:
          Version: '2012-10-17'
          Statement:
            - Effect: Allow
              Principal:
                Service:
                  - lambda.amazonaws.com
              Action: sts:AssumeRole
        Policies:
          - PolicyName: ReadFromDynamoDb
            PolicyDocument:
              Version: '2012-10-17'
              Statement:
                - Effect: 'Allow'
                  Action:
                    - 'dynamodb:GetRecords'
                    - 'dynamodb:GetShardIterator'
                    - 'dynamodb:DescribeStream'
                    - 'dynamodb:ListStreams'
                  Resource: arn:aws:dynamodb:us-east-1:XXXX:table/test/stream/2017-01-26T12:11:48.130
          - PolicyName: ReadFromKinesis
            PolicyDocument:
              Version: '2012-10-17'
              Statement:
                - Effect: 'Allow'
                  Action:
                    - 'kinesis:GetRecords'
                    - 'kinesis:GetShardIterator'
                    - 'kinesis:DescribeStream'
                    - 'kinesis:ListStreams'
                  Resource: arn:aws:kinesis:us-east-1:XXXX:stream/test

The PR works great. Thanks for the fix! 💪 💯

[erikerikson]

Hah! Not at all. You've totally helped me out with exactly the same sort of thing. There's a lot of complexity and being right every time (tm) is for the computers! 😄

[erikerikson]

Thanks for helping me feel a bit better about being human myself!

[pmuens]

Thanks @erikerikson 👍 😃

[eahefnawy]

Works great @erikerikson 👍 ... G2M 😊

[eahefnawy]

Robust!!! 💯 💪