feat: Add support for custom Lambda@Edge policies #333
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new IAM policy document ( Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Attribute renamed from `policy_documents` to `additional_policy`. Add `LambdaWriteCloudWatchLogs` SID to default CloudWatch policy to make overrrides possible.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
modules/lambda@edge/README.md (1)
76-80: Clarify and Enhance Documentation for New Function AttributesThe updated
functionsinput block now includes the mutually exclusive source options (source,source_dir,source_zip) and introduces theadditional_policyattribute. While the description is comprehensive, consider the following improvements:
- Detail the
additional_policyFormat: Explicitly state thatadditional_policyshould be provided as a valid JSON string representing IAM policy statements. Explain how the substitution of$${lambda_arn}works within these policies.- Reference Custom Policy Usage: Mention how overriding the default policy statement (by specifying a custom statement with the
LambdaWriteCloudWatchLogssid) affects the execution role.- Clarify Mutual Exclusivity: Although the mutually exclusive nature of
source,source_dir, andsource_zipis mentioned, a brief example or note clarifying the expected usage might help users avoid configuration mistakes.These enhancements will improve readability and assist users in correctly configuring their Lambda@Edge functions with custom IAM policies.
🧰 Tools
🪛 LanguageTool
[grammar] ~77-~77: The verb ‘keep’ seems to be in the wrong form here.
Context: ... for unlimited length.
Set tonullfor keep the existing setting, which defaults to...(FOR_VB)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/lambda@edge/README.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
Dynamic Lambda ARN repalcements bring no value for Lambda IAM execution roles, as typically you'd like to list other AWS resources/services there, not the Lambda itself.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
modules/lambda@edge/README.md (1)
76-77: Enhance Documentation for the UpdatedfunctionsInput ObjectThe updated
functionsinput object now includes new attributes (source_dir,source_zip, andadditional_policy) to accommodate multiple source options and additional IAM policies. The description clearly explains that one of these source options must be specified and that ifadditional_policyis used, the variable$${lambda_arn}will be substituted.Suggestion: Consider adding a brief example snippet demonstrating how to override the default
LambdaWriteCloudWatchLogspolicy viaadditional_policy. This extra context could help users understand the practical usage of the new feature.🧰 Tools
🪛 LanguageTool
[grammar] ~77-~77: The verb ‘keep’ seems to be in the wrong form here.
Context: ... for unlimited length.
Set tonullfor keep the existing setting, which defaults to...(FOR_VB)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
modules/lambda@edge/README.md(1 hunks)modules/lambda@edge/variables.tf(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/lambda@edge/variables.tf
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
modules/lambda@edge/README.md (1)
76-77: Enhanced Input Definition for Lambda@Edge FunctionsThe updated
functionsinput clearly documents the new, mutually exclusive options (source,source_dir,source_zip) and introduces theadditional_policyfield with a sensible default ("{}"). The detailed description guides users on how to provide the Lambda function source and extra IAM policies, aligning well with the PR objectives. For further clarity, consider adding a small example (or a reference to one) that illustrates howadditional_policycan be used in practice.🧰 Tools
🪛 LanguageTool
[grammar] ~77-~77: The verb ‘keep’ seems to be in the wrong form here.
Context: ... for unlimited length.
Set tonullfor keep the existing setting, which defaults to...(FOR_VB)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/lambda@edge/README.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
|
@goruha @Gowiem @jamengual can someone start the tests please? Thanks in advance. |
|
/terratest |
what
Execution role associated with Lambda@Edge comes with a hardcoded policy that enables write access to CloudWatch logs. This PR adds support for additional policies. It was implemented in a similar fashion to
additional_bucket_policyfrom the parent module.why
It's a fairly common situation that a Lambda@Edge function needs access to other AWS services/resources than CloudWatch logs.
aws_lambda_function'sroleargument expects a single role ARN, therefore the only reasonable option is to append new policy statements to the IAM role created in scope of this module.references
closes #261