-
Notifications
You must be signed in to change notification settings - Fork 0
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
Lambda docker image #1
base: master
Are you sure you want to change the base?
Conversation
fix: AWS parsing env vars as string, forcing int
…da code provided by the module
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.
PR Summary
This PR introduces support for using an externally provided Lambda Docker image as an alternative to the built-in Lambda code, addressing issue meltwater#61.
- Added new
lambda_image_uri
variable invariables.tf
for specifying external Lambda image - Modified
main.tf
to conditionally use either the provided image or built-in code for Lambda function - Updated
autoscale.py
to convertROUTE53_TTL
environment variable to integer - Implemented logic in
main.tf
to handle different Lambda package types (Image or Zip) - Consider adding documentation for the new
lambda_image_uri
variable and its usage
3 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings
@@ -93,7 +93,7 @@ def update_record(zone_id, ip, hostname, operation): | |||
'ResourceRecordSet': { | |||
'Name': hostname, | |||
'Type': 'A', | |||
'TTL': os.environ['ROUTE53_TTL'], | |||
'TTL': int(os.environ['ROUTE53_TTL']), |
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.
logic: Ensure ROUTE53_TTL is always set in the environment variables to avoid potential KeyError
@@ -93,7 +93,7 @@ def update_record(zone_id, ip, hostname, operation): | |||
'ResourceRecordSet': { | |||
'Name': hostname, | |||
'Type': 'A', | |||
'TTL': os.environ['ROUTE53_TTL'], | |||
'TTL': int(os.environ['ROUTE53_TTL']), |
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.
style: Consider adding a try-except block to handle potential ValueError when converting ROUTE53_TTL to int
@@ -98,6 +98,7 @@ data "aws_iam_policy_document" "lifecycle_policy" { | |||
} | |||
|
|||
data "archive_file" "autoscale" { | |||
count = var.lambda_image_uri != null ? 0 : 1 |
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.
style: Consider adding a descriptive comment explaining the purpose of this count parameter
filename = var.lambda_image_uri != null ? null : data.archive_file.autoscale[0].output_path | ||
function_name = "${var.vpc_name}-${var.autoscale_handler_unique_identifier}" | ||
role = aws_iam_role.autoscale_handling.arn | ||
handler = "autoscale.lambda_handler" | ||
runtime = "python3.8" | ||
source_code_hash = filebase64sha256(data.archive_file.autoscale.output_path) | ||
handler = var.lambda_image_uri != null ? null : "autoscale.lambda_handler" | ||
runtime = var.lambda_image_uri != null ? null : "python3.8" | ||
source_code_hash = var.lambda_image_uri != null ? null : filebase64sha256(data.archive_file.autoscale[0].output_path) | ||
image_uri = var.lambda_image_uri != null ? var.lambda_image_uri : null | ||
package_type = var.lambda_image_uri != null ? "Image" : "Zip" |
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.
style: These conditional expressions may impact readability. Consider using separate resources for Image and Zip package types
source_code_hash = filebase64sha256(data.archive_file.autoscale.output_path) | ||
handler = var.lambda_image_uri != null ? null : "autoscale.lambda_handler" | ||
runtime = var.lambda_image_uri != null ? null : "python3.8" | ||
source_code_hash = var.lambda_image_uri != null ? null : filebase64sha256(data.archive_file.autoscale[0].output_path) |
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.
logic: Ensure that source_code_hash is properly set when using a Docker image to trigger updates
variable "lambda_image_uri" { | ||
default = null | ||
type = string | ||
description = "The URI of the Lambda image" | ||
} |
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.
style: Add a note in the description about when to use this variable, e.g., 'Provide this URI to use a custom Lambda image instead of the default Python code'
Please read CONTRIBUTING.md for details on our code of conduct, and the process for submitting pull requests to us.
Fixes #
Proposed Changes
lambda_image_uri
) and logic to decide beteen using an externally provided lambda or the builtin lambda code.Description
This should address meltwater#61
Checklist
Unreleased
section of CHANGELOG.