Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DQL wrong SNS permissions while SQS resource set #59

Open
Enase opened this issue Nov 6, 2020 · 1 comment
Open

DQL wrong SNS permissions while SQS resource set #59

Enase opened this issue Nov 6, 2020 · 1 comment
Labels
bug Something isn't working

Comments

@Enase
Copy link
Collaborator

Enase commented Nov 6, 2020

Issue description:
Plugin adds sns:Publish permission in case if function has onError definition and doesn't take into account that it might be SQS resource arn

Steps to reproduce:

  1. Define any function with onError property mapped to SQS resource:
functions:
  function_name_here:
    handler: functions/function_name_here/index.handler
    iamRoleStatementsName: "function_name_here_lambda_role"
    iamRoleStatements:
      - ${file(../../function_name_here.yml)}
    onError:
      Fn::GetAtt: [QueueNameDeadLetterQueue, Arn]
  1. Define SQS Resource:
Resources:
  QueueNameDeadLetterQueue:
    Type: AWS::SQS::Queue
    Properties:
      QueueName: "SomeNameDeadLetterQueue"
  1. Deploy your code with sls deploy

Expected result:
No useless permissions should be added

Current result:
Useless permissions added:

        {
            "Action": [
                "sns:Publish"
            ],
            "Resource": "arn:aws:sqs:{region_here}:{accountId_here}:SomeNameDeadLetterQueue",
            "Effect": "Allow"
        }

Background:

Serverless added support for DQL setup with onError function property here. However it has some strange concurrency issue described in docs. But still community has some workarounds and moreover, it's hard to reproduce it (that's why it's not fixed yet), so that most people (like me) use it as is without any issues.

TBD:

As possible solution I recommend to delete additional policy definitions in case if onError property set in function definition.

In case if we want to keep backward compatibility we may add configuration property like addOnErrorPolicy: boolean.

Looking forward for your comments,
Thanks in advance.

@Enase Enase added the bug Something isn't working label Nov 24, 2020
@Enase
Copy link
Collaborator Author

Enase commented Dec 4, 2020

@glicht any comment on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant