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

memory and timeout vars for lambda@edge #330

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions examples/complete/lambda-at-edge.tf
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ module "lambda_at_edge" {
# A destruction delay is always enabled due to automated tests (see variable description for more information).
destruction_delay = "20m"

memory_size = 128
timeout = 3
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
memory_size = 128
timeout = 3
# Minimum memory size for Lambda@Edge functions
memory_size = 128
# Conservative timeout for viewer-request/response functions
timeout = 3


providers = {
aws = aws.us-east-1
}
Expand Down
2 changes: 2 additions & 0 deletions modules/lambda@edge/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ module "lambda_at_edge" {
| <a name="input_delimiter"></a> [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.<br>Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no |
| <a name="input_descriptor_formats"></a> [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.<br>Map of maps. Keys are names of descriptors. Values are maps of the form<br>`{<br> format = string<br> labels = list(string)<br>}`<br>(Type is `any` so the map values can later be enhanced to provide additional options.)<br>`format` is a Terraform format string to be passed to the `format()` function.<br>`labels` is a list of labels, in order, to pass to `format()` function.<br>Label values will be normalized before being passed to `format()` so they will be<br>identical to how they appear in `id`.<br>Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no |
| <a name="input_destruction_delay"></a> [destruction\_delay](#input\_destruction\_delay) | The delay, in [Golang ParseDuration](https://pkg.go.dev/time#ParseDuration) format, to wait before destroying the Lambda@Edge<br>functions.<br><br>This delay is meant to circumvent Lambda@Edge functions not being immediately deletable following their dissociation from<br>a CloudFront distribution, since they are replicated to CloudFront Edge servers around the world.<br><br>If set to `null`, no delay will be introduced.<br><br>By default, the delay is 20 minutes. This is because it takes about 3 minutes to destroy a CloudFront distribution, and<br>around 15 minutes until the Lambda@Edge function is available for deletion, in most cases.<br><br>For more information, see: https://github.com/hashicorp/terraform-provider-aws/issues/1721. | `string` | `"20m"` | no |
| <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 |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
| <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)

| <a name="input_enabled"></a> [enabled](#input\_enabled) | Set to false to prevent the module from creating any resources | `bool` | `null` | no |
| <a name="input_environment"></a> [environment](#input\_environment) | ID element. Usually used for region e.g. 'uw2', 'us-west-2', OR role 'prod', 'staging', 'dev', 'UAT' | `string` | `null` | no |
| <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` and `handler` 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. | <pre>map(object({<br> source = list(object({<br> filename = string<br> content = string<br> }))<br> runtime = string<br> handler = string<br> event_type = string<br> include_body = bool<br> }))</pre> | n/a | yes |
Expand Down
2 changes: 2 additions & 0 deletions modules/lambda@edge/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ resource "aws_lambda_function" "default" {
filename = each.value.source_zip != null ? data.local_file.lambda_zip[each.key].filename : data.archive_file.lambda_zip[each.key].output_path
source_code_hash = each.value.source_zip != null ? sha256(data.local_file.lambda_zip[each.key].content_base64) : data.archive_file.lambda_zip[each.key].output_base64sha256
publish = true
memory_size = var.memory_size
timeout = var.timeout
}

resource "aws_lambda_permission" "allow_cloudfront" {
Expand Down
14 changes: 13 additions & 1 deletion modules/lambda@edge/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,16 @@ variable "destruction_delay" {
For more information, see: https://github.com/hashicorp/terraform-provider-aws/issues/1721.
EOT
default = "20m"
}
}

variable "memory_size" {
type = number
description = "Amount of memory in MB the Lambda Function can use at runtime."
default = 128
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
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."
}
}


variable "timeout" {
type = number
description = "The amount of time the Lambda Function has to run in seconds."
default = 3
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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."
}
}