memory and timeout vars for lambda@edge #330
Conversation
📝 WalkthroughWalkthroughThe changes introduce two new configuration parameters, Changes
Sequence DiagramsequenceDiagram
participant User as Terraform User
participant Module as Lambda@Edge Module
participant AWS as AWS Lambda
User->>Module: Configure memory_size and timeout
Module->>AWS: Create Lambda Function
AWS-->>Module: Function Created
Module-->>User: Deployment Complete
Assessment against linked issues
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
examples/complete/lambda-at-edge.tf(1 hunks)modules/lambda@edge/README.md(1 hunks)modules/lambda@edge/main.tf(1 hunks)modules/lambda@edge/variables.tf(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
modules/lambda@edge/README.md
74-74: Element: a
Inline HTML
(MD033, no-inline-html)
75-75: Element: a
Inline HTML
(MD033, no-inline-html)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (1)
modules/lambda@edge/main.tf (1)
85-86: LGTM! Variables are properly integratedThe memory_size and timeout variables are correctly added to the aws_lambda_function resource.
modules/lambda@edge/variables.tf
Outdated
| type = number | ||
| description = "The amount of time the Lambda Function has to run in seconds." | ||
| default = 3 | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for Lambda@Edge timeout constraints
Lambda@Edge functions have stricter timeout limits compared to regular Lambda functions. The timeout varies based on the event type.
variable "timeout" {
type = number
description = "The amount of time the Lambda Function has to run in seconds."
default = 3
+ validation {
+ condition = var.timeout >= 1 && var.timeout <= 30
+ error_message = "Lambda@Edge timeout must be between 1 and 30 seconds."
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| variable "timeout" { | |
| type = number | |
| description = "The amount of time the Lambda Function has to run in seconds." | |
| default = 3 | |
| } | |
| variable "timeout" { | |
| type = number | |
| description = "The amount of time the Lambda Function has to run in seconds." | |
| default = 3 | |
| validation { | |
| condition = var.timeout >= 1 && var.timeout <= 30 | |
| error_message = "Lambda@Edge timeout must be between 1 and 30 seconds." | |
| } | |
| } |
modules/lambda@edge/variables.tf
Outdated
| type = number | ||
| description = "Amount of memory in MB the Lambda Function can use at runtime." | ||
| default = 128 | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding validation for Lambda@Edge memory constraints
Lambda@Edge functions have specific memory constraints. Consider adding a validation block to ensure the value stays within Lambda@Edge limits (128MB to 10240MB).
variable "memory_size" {
type = number
description = "Amount of memory in MB the Lambda Function can use at runtime."
default = 128
+ validation {
+ condition = var.memory_size >= 128 && var.memory_size <= 10240
+ error_message = "Lambda@Edge memory_size must be between 128 MB and 10240 MB."
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| variable "memory_size" { | |
| type = number | |
| description = "Amount of memory in MB the Lambda Function can use at runtime." | |
| default = 128 | |
| } | |
| variable "memory_size" { | |
| type = number | |
| description = "Amount of memory in MB the Lambda Function can use at runtime." | |
| default = 128 | |
| validation { | |
| condition = var.memory_size >= 128 && var.memory_size <= 10240 | |
| error_message = "Lambda@Edge memory_size must be between 128 MB and 10240 MB." | |
| } | |
| } |
examples/complete/lambda-at-edge.tf
Outdated
| memory_size = 128 | ||
| timeout = 3 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider documenting memory and timeout choices
While the values match the defaults, it would be helpful to document why these specific values were chosen for the example, especially since different Lambda@Edge event types might have different requirements.
+ # Minimum memory size for Lambda@Edge functions
memory_size = 128
+ # Conservative timeout for viewer-request/response functions
timeout = 3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| memory_size = 128 | |
| timeout = 3 | |
| # Minimum memory size for Lambda@Edge functions | |
| memory_size = 128 | |
| # Conservative timeout for viewer-request/response functions | |
| timeout = 3 |
modules/lambda@edge/README.md
Outdated
| | <a name="input_memory_size"></a> [memory\_size](#input\_memory\_size) | Amount of memory in MB the Lambda Function can use at runtime. | `number` | `128` | no | | ||
| | <a name="input_timeout"></a> [timeout](#input\_timeout) | The amount of time the Lambda Function has to run in seconds. | `number` | `3` | no | |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance documentation with Lambda@Edge specific details
The documentation should include information about Lambda@Edge specific constraints and recommendations for different event types.
-| <a name="input_memory_size"></a> [memory_size](#input_memory_size) | Amount of memory in MB the Lambda Function can use at runtime. | `number` | `128` | no |
-| <a name="input_timeout"></a> [timeout](#input_timeout) | The amount of time the Lambda Function has to run in seconds. | `number` | `3` | no |
+| <a name="input_memory_size"></a> [memory_size](#input_memory_size) | Amount of memory in MB the Lambda Function can use at runtime. Must be between 128 MB and 10240 MB for Lambda@Edge. | `number` | `128` | no |
+| <a name="input_timeout"></a> [timeout](#input_timeout) | The amount of time the Lambda Function has to run in seconds. Must be between 1 and 30 seconds for Lambda@Edge. Note that viewer-request and viewer-response functions have a 5-second timeout limit, while origin-request and origin-response functions have a 30-second limit. | `number` | `3` | no |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | <a name="input_memory_size"></a> [memory\_size](#input\_memory\_size) | Amount of memory in MB the Lambda Function can use at runtime. | `number` | `128` | no | | |
| | <a name="input_timeout"></a> [timeout](#input\_timeout) | The amount of time the Lambda Function has to run in seconds. | `number` | `3` | no | | |
| | <a name="input_memory_size"></a> [memory_size](#input_memory_size) | Amount of memory in MB the Lambda Function can use at runtime. Must be between 128 MB and 10240 MB for Lambda@Edge. | `number` | `128` | no | | |
| | <a name="input_timeout"></a> [timeout](#input_timeout) | The amount of time the Lambda Function has to run in seconds. Must be between 1 and 30 seconds for Lambda@Edge. Note that viewer-request and viewer-response functions have a 5-second timeout limit, while origin-request and origin-response functions have a 30-second limit. | `number` | `3` | no | |
🧰 Tools
🪛 Markdownlint (0.37.0)
74-74: Element: a
Inline HTML
(MD033, no-inline-html)
75-75: Element: a
Inline HTML
(MD033, no-inline-html)
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
examples/complete/lambda-at-edge.tf (1)
35-36: 🧹 Nitpick (assertive)Consider function-specific configurations.
While the current configuration works, consider adjusting memory and timeout based on function type:
- Viewer functions (request/response): Lower timeout (max 5s)
- Origin functions (request/response): Higher timeout if needed (max 30s)
Example configuration:
viewer_request = { ... memory_size = 128 - timeout = 3 + timeout = 5 # Maximum allowed for viewer functions ... } origin_response = { ... memory_size = 128 - timeout = 3 + timeout = 30 # Maximum allowed for origin functions ... }Also applies to: 45-46, 54-55, 88-89
modules/lambda@edge/README.md (1)
76-76: 🧹 Nitpick (assertive)Enhance documentation with Lambda@Edge constraints.
Consider adding Lambda@Edge specific constraints to the documentation:
| <a name="input_functions"></a> [functions](#input_functions) | Lambda@Edge functions to create.<br><br>The key of this map is the name label of the Lambda@Edge function.<br><br>`source.filename` and `source.content` dictate the name and content of the files that will make up the Lambda function<br>source, respectively.<br><br>`runtime`, `handler`, `memory_size`, and `timeout` correspond to the attributes of the same name in the [lambda_function](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_function)<br>resource.<br><br>`event_type` and `include_body` correspond to the attributes of the same name in the [Lambda Function association block<br>of the cloudfront_distribution](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudfront_distribution#lambda-function-association)<br>resource. | +<br><br>Lambda@Edge specific constraints:<br> +- `memory_size`: Must be between 128 MB and 10240 MB<br> +- `timeout`: Must be between 1 and 30 seconds. Note that viewer-request and viewer-response functions have a 5-second timeout limit, while origin-request and origin-response functions have a 30-second limit. |🧰 Tools
🪛 Markdownlint (0.37.0)
76-76: Element: a
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: pre
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
76-76: Element: br
Inline HTML(MD033, no-inline-html)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
README.md(1 hunks)README.yaml(1 hunks)examples/complete/lambda-at-edge.tf(3 hunks)modules/lambda@edge/README.md(1 hunks)modules/lambda@edge/main.tf(1 hunks)modules/lambda@edge/variables.tf(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
modules/lambda@edge/README.md
76-76: Element: a
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: pre
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
76-76: Element: br
Inline HTML
(MD033, no-inline-html)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (3)
modules/lambda@edge/variables.tf (1)
32-33: LGTM! Consider implementing the suggested validations.The addition of
memory_sizeandtimeoutas optional parameters is correct. However, as suggested in previous review comments, consider adding validation blocks to enforce Lambda@Edge specific constraints:
- Memory: 128MB to 10240MB
- Timeout: 1-30s (with 5s limit for viewer functions)
modules/lambda@edge/main.tf (1)
81-82: LGTM! Conservative default values.The implementation correctly sets conservative default values:
- memory_size = 128 (minimum allowed)
- timeout = 3 (safe for all function types)
README.yaml (1)
358-359: LGTM! Example is consistent.The example configuration is consistent with the implementation and uses safe default values.
| memory_size = 128 | ||
| timeout = 3 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider enhancing the documentation.
The addition of memory_size and timeout parameters is valuable for controlling Lambda@Edge function resources. The default values are appropriate for most use cases.
Consider adding documentation notes about:
- Maximum allowed values for memory and timeout
- How these parameters affect pricing
- Different timeout limits based on event types (viewer-facing events have 5s limit, origin-facing events have 30s limit)
what
Allow to configure memory size and timeout for Lambda@Edge module.
why
These fields are not configurable now.
references
Resolves #331
Summary by CodeRabbit
Summary by CodeRabbit