-
Notifications
You must be signed in to change notification settings - Fork 4k
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
docs(lambda): document adding execution permissions to provided IAM roles #8041
docs(lambda): document adding execution permissions to provided IAM roles #8041
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this is the correct behaviour of this construct. There are users of this library who want to have full control over the permissions of their role. The property role
is provided to satisfy this.
What is the exact use case you are trying to achieve here?
This doesn't unlock any new use cases -- this was intended to make Lambda creation foolproof.
I get the full control portion. However, without the basic execution role, or equivalent permissions, Lambda does not work correctly (cannot log output). In a VPC, the Lambda does not even initialize without the EC2 permissions. Is this the desired outcome for users who want full control? At the very least, it should be clearly documented that those permissions must be added yourself when providing your own role. I've hit that issue, as have co-workers, which was the impetus for this PR. |
37f410e
to
c6c8a8c
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
How about a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Me love some good clean documentation and writing. Some minor adjustments called out below.
Let me know if you think it's a good idea and would like to pursue the LambdaRole
option in this PR.
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
I like the idea -- it's just a Role with the "assumedBy" always being Lambda. Once concern I have is I'd prefer not to create a class like "LambdaRoleProps" because I worry about keeping it's features in sync with regular RoleProps. Do you have any ideas as to how to achieve this? |
What if |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Commit Message
docs(lambda): document adding execution permissions to provided IAM roles
If I am providing a Role for a Lambda function, it currently isn't given the basic execution permissions, so the function cannot log anything or, in the case of a VPC Lambda, it cannot create the network interfaces. The user has to add those permissions themselves, but it isn't clear from the documentation that that needs to happen.
This commit adds documentation showing CDK users how to add the required permissions for execution.
End Commit Message
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license