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

Cannot recognize provider.iam as an object given an arn for iam role #350

Closed
lhyleo opened this issue Jul 16, 2021 · 8 comments · Fixed by #434
Closed

Cannot recognize provider.iam as an object given an arn for iam role #350

lhyleo opened this issue Jul 16, 2021 · 8 comments · Fixed by #434
Labels

Comments

@lhyleo
Copy link

lhyleo commented Jul 16, 2021

When adding an iam role to the serverless.yml, it can only be done in this way

provider:
  name: aws
  iam:
    - role: arn:aws:iam::some-role-id

However, the best practice for setting the iam role should be setting the role as an object (with the warning from serverless Configuration warning at 'provider.iam': should be object)

provider:
  name: aws
  iam:
    role: arn:aws:iam::some-role-id

But this will throw an error

  TypeError: Cannot read property 'Properties' of undefined
      at ServerlessSnsSqsLambda.addLambdaSqsPermissions (/usr/local/lib/node_modules/@agiledigital/serverless-sns-sqs-lambda/dist/serverless-sns-sqs-lambda.js:333:51)
      at /usr/local/lib/node_modules/@agiledigital/serverless-sns-sqs-lambda/dist/serverless-sns-sqs-lambda.js:175:13
      at Array.reduce (<anonymous>)
      at ServerlessSnsSqsLambda.addSnsSqsResources (/usr/local/lib/node_modules/@agiledigital/serverless-sns-sqs-lambda/dist/serverless-sns-sqs-lambda.js:174:11)
      at /usr/local/lib/node_modules/@agiledigital/serverless-sns-sqs-lambda/dist/serverless-sns-sqs-lambda.js:151:31
      at Array.forEach (<anonymous>)
      at /usr/local/lib/node_modules/@agiledigital/serverless-sns-sqs-lambda/dist/serverless-sns-sqs-lambda.js:146:29
      at Array.forEach (<anonymous>)
      at ServerlessSnsSqsLambda.modifyTemplate (/usr/local/lib/node_modules/@agiledigital/serverless-sns-sqs-lambda/dist/serverless-sns-sqs-lambda.js:143:32)
      at PluginManager.invoke (/usr/local/lib/node_modules/serverless/lib/classes/PluginManager.js:576:20)
      at async PluginManager.spawn (/usr/local/lib/node_modules/serverless/lib/classes/PluginManager.js:598:5)
@NoxHarmonium
Copy link
Collaborator

Thanks for the report @lhyleo! Seems like a simple issue to fix. I might get a chance to look at it over the next week

@vicary
Copy link

vicary commented Oct 22, 2021

@NoxHarmonium When users are specifying ARNs, they are responsible to add the necessary permissions on their own. I think addLambdaSqsPermissions can be safely skipped when template.Resources.IamRoleLambdaExecution is undefined.

@NoxHarmonium
Copy link
Collaborator

Good point. Sorry I'm taking so long to get around to this. Things are bit hectic. I'll try and have a look this weekend.

@vicary
Copy link

vicary commented Oct 23, 2021

@NoxHarmonium Tested with an actual deployment, I found the following change is also required in my case. It's also trivial but it's your call if it should be split into another PR.

When iam.role is specified ARN style, there will be no IamRoleLambdaExecution for addEventSourceMapping to depends on. Since the FunctionName is already using Fn::GetAtt, and the function itself should already be depending on that role, it should not requires an explicit DependsOn from the event mapping. I think we can also safely remove this to increase compatibility.

NoxHarmonium added a commit that referenced this issue Oct 23, 2021
Fixes #350

- Instead of getting serverless to generate a role for the lambdas, you can create one yourself and specify the ARN in the serverless config
- In this case, serverless-sns-sqs-lambda should just skip process that adds policies to the lambda role
@NoxHarmonium
Copy link
Collaborator

@vicary PR is up will all your suggestions if you would like to review it:
#434

@vicary
Copy link

vicary commented Oct 23, 2021

@NoxHarmonium LGTM. Tested the branch with an actual deployment and it works. I am having some runtime errors about evanw/esbuild#495 but I don't think it's related, I'd say this is good to go.

NoxHarmonium pushed a commit that referenced this issue Oct 23, 2021
## [0.9.1](v0.9.0...v0.9.1) (2021-10-23)

### Bug Fixes

* remove unneccessary 'DependsOn' block ([9276523](9276523))
* support custom role ARNs ([563c202](563c202)), closes [#350](#350)
@NoxHarmonium
Copy link
Collaborator

🎉 This issue has been resolved in version 0.9.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@NoxHarmonium
Copy link
Collaborator

Thanks for your help @vicary !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants