feat: Lambda와 CloudFront를 TF로 관리하도록 Migrate #13
Conversation
Hexeong
requested review from
Gyuhyeok99,
JAEHEE25,
lsy1307,
sukangpunch,
whqtker and
wibaek
as code owners
📝 WalkthroughWalkthroughThis pull request migrates Lambda and CloudFront infrastructure to Terraform management. A new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant S3 as S3 Bucket
participant Event as S3 Event
participant ResizeLambda as Resize Lambda
participant Sharp as Image Library
participant S3Out as S3 Output
User->>S3: Upload image to original/*
S3->>Event: Trigger PutObject event
Event->>ResizeLambda: Invoke with event
ResizeLambda->>S3: GetObject (original image)
S3-->>ResizeLambda: Image stream
ResizeLambda->>Sharp: Convert to WEBP, width 600px
Sharp-->>ResizeLambda: Processed buffer
ResizeLambda->>S3Out: PutObject (resize/*.webp)
S3Out-->>User: Resized image stored
sequenceDiagram
actor User
participant S3 as S3 Bucket
participant Event as S3 Event
participant ThumbLambda as Thumbnail Lambda
participant Sharp as Image Library
participant S3Out as S3 Output
User->>S3: Upload image to chat/images/*
S3->>Event: Trigger PutObject event
Event->>ThumbLambda: Invoke with event
ThumbLambda->>S3: GetObject (original image)
S3-->>ThumbLambda: Image stream
ThumbLambda->>ThumbLambda: Convert stream to buffer
ThumbLambda->>Sharp: Resize to 200x200, JPEG quality 85
Sharp-->>ThumbLambda: Thumbnail buffer
ThumbLambda->>S3Out: PutObject (chat/thumbnails/*_thumb.jpg)
S3Out-->>User: Thumbnail stored with metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
modules/shared_resources/src/img_resizing/index.js (3)
29-33: Inconsistent supported image types between handlers.This handler only supports
jpgandpng, while the thumbnail handler (modules/shared_resources/src/thumbnail/index.js) supportsjpg,jpeg,png, andwebp. Consider aligning the supported formats for consistency.🔎 Proposed fix
- const imageType = typeMatch[1].toLowerCase(); - if (imageType != "jpg" && imageType != "png") { - console.log(`Unsupported image type: ${imageType}`); - return; - } + const imageType = typeMatch[1].toLowerCase(); + const supportedTypes = ["jpg", "jpeg", "png"]; + if (!supportedTypes.includes(imageType)) { + console.log(`Unsupported image type: ${imageType}`); + return; + }
40-47: Preferconst/letovervar.Using
varis outdated and can lead to scoping issues. Modern JavaScript best practice recommendsconstfor immutable bindings andletfor mutable ones.🔎 Proposed fix
- var response = await s3.send(new GetObjectCommand(params)); - var stream = response.Body; + const response = await s3.send(new GetObjectCommand(params)); + const stream = response.Body; if (stream instanceof Readable) { - var content_buffer = Buffer.concat(await stream.toArray()); + const content_buffer = Buffer.concat(await stream.toArray()); } else { throw new Error("Unknown object stream type"); }Also update line 56:
- var output_buffer = await sharp(content_buffer) + const output_buffer = await sharp(content_buffer)
48-51: Silent error returns may complicate debugging.The handler returns
undefinedon errors, unlike the thumbnail handler which returns a structured response withstatusCode: 500. For S3 event-triggered Lambdas, this may not be critical, but consistent error responses improve observability.modules/shared_resources/provider.tf (1)
4-9: Provider version constraint inconsistency.This module uses
>= 5.0whileenvironment/global/provider.tfuses~> 5.0. The>=constraint allows future major versions (6.0+) which could introduce breaking changes. Consider using~> 5.0for consistency and to avoid unexpected breakages.🔎 Proposed fix
required_providers { aws = { source = "hashicorp/aws" - version = ">= 5.0" + version = "~> 5.0" } }modules/shared_resources/lambda.tf (2)
15-26: Consider specifying memory_size for image processing Lambda.The default Lambda memory is 128MB, which may be insufficient for sharp-based image processing, especially for larger images. Sharp is memory-intensive during image manipulation.
🔎 Proposed fix
resource "aws_lambda_function" "resizing_img_func" { function_name = var.resizing_img_func_name role = var.resizing_img_func_role handler = var.resizing_img_func_handler runtime = var.resizing_img_func_runtime filename = data.archive_file.resizing_zip.output_path source_code_hash = data.archive_file.resizing_zip.output_base64sha256 layers = var.resizing_img_func_layers timeout = 15 + memory_size = 512 # Adjust based on expected image sizes }Apply the same change to
thumbnail_generating_func.
15-39: Consider enabling Lambda code-signing for enhanced security.Static analysis flagged that code-signing validation is not configured (CKV_AWS_272). While not critical for internal tooling, code-signing ensures only trusted code is deployed. This can be deferred if the Lambda functions are only updated through this Terraform pipeline.
If code-signing is required by your security policies, you would need to create a signing configuration and add:
code_signing_config_arn = aws_lambda_code_signing_config.example.arnmodules/shared_resources/cloudfront.tf (2)
17-17: Hard-coded origin_id suffix reduces reproducibility.The origin_id contains a hard-coded random suffix (
-mjo1g7tk2w8) that appears to be from the console-created resource. This makes the infrastructure less reproducible across environments.🔎 Recommended simplification
origin { domain_name = "${var.s3_default_bucket_name}.s3.ap-northeast-2.amazonaws.com" - origin_id = "${var.s3_default_bucket_name}.s3.ap-northeast-2.amazonaws.com-mjo1g7tk2w8" + origin_id = "S3-${var.s3_default_bucket_name}" origin_access_control_id = "E14M8OP55A3YO7" # ... } default_cache_behavior { - target_origin_id = "${var.s3_default_bucket_name}.s3.ap-northeast-2.amazonaws.com-mjo1g7tk2w8" + target_origin_id = "S3-${var.s3_default_bucket_name}"Also applies to: 25-25
66-66: Hard-coded origin_id suffix reduces reproducibility.Same issue as default_cdn - the origin_id contains a hard-coded random suffix (
-mjo1jpx6rvc) from the console-created resource.🔎 Recommended simplification
origin { domain_name = "${var.s3_upload_bucket_name}.s3.ap-northeast-2.amazonaws.com" - origin_id = "${var.s3_upload_bucket_name}.s3.ap-northeast-2.amazonaws.com-mjo1jpx6rvc" + origin_id = "S3-${var.s3_upload_bucket_name}" origin_access_control_id = "E1ZBB5RMSBZQ4I" # ... } default_cache_behavior { - target_origin_id = "${var.s3_upload_bucket_name}.s3.ap-northeast-2.amazonaws.com-mjo1jpx6rvc" + target_origin_id = "S3-${var.s3_upload_bucket_name}"Also applies to: 74-74
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.gitignoreREADME.mdconfig/secretsenvironment/global/main.tfenvironment/global/provider.tfenvironment/global/variables.tfenvironment/prod/main.tfenvironment/prod/variables.tfenvironment/stage/main.tfenvironment/stage/variables.tfmodules/app_stack/variables.tfmodules/shared_resources/cloudfront.tfmodules/shared_resources/lambda.tfmodules/shared_resources/provider.tfmodules/shared_resources/s3.tfmodules/shared_resources/src/img_resizing/index.jsmodules/shared_resources/src/thumbnail/index.jsmodules/shared_resources/variables.tf
💤 Files with no reviewable changes (5)
- modules/app_stack/variables.tf
- environment/stage/variables.tf
- environment/prod/variables.tf
- environment/prod/main.tf
- environment/stage/main.tf
🧰 Additional context used
🧬 Code graph analysis (1)
modules/shared_resources/src/img_resizing/index.js (1)
modules/shared_resources/src/thumbnail/index.js (3)
handler(6-99)handler(6-99)response(39-39)
🪛 Checkov (3.2.334)
modules/shared_resources/lambda.tf
[high] 15-26: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
[high] 28-39: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
modules/shared_resources/cloudfront.tf
[medium] 2-48: Ensure CloudFront distribution has a default root object configured
(CKV_AWS_305)
[medium] 2-48: Ensure CloudFront distributions should have origin failover configured
(CKV_AWS_310)
[medium] 51-97: Ensure CloudFront distribution has a default root object configured
(CKV_AWS_305)
[medium] 51-97: Ensure CloudFront distributions should have origin failover configured
(CKV_AWS_310)
[medium] 2-48: Ensure AWS CloudFront attached WAFv2 WebACL is configured with AMR for Log4j Vulnerability
(CKV2_AWS_47)
[medium] 51-97: Ensure AWS CloudFront attached WAFv2 WebACL is configured with AMR for Log4j Vulnerability
(CKV2_AWS_47)
🪛 Gitleaks (8.30.0)
modules/shared_resources/cloudfront.tf
[high] 18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (7)
config/secrets (1)
1-1: Verify secrets submodule includes all required configurations for new Lambda and CloudFront resources.The submodule reference has been updated, which aligns with the introduction of shared_resources module for Lambda, CloudFront, and S3 management. Since the secrets repository is not visible in this review, please verify that:
- All necessary environment variables and credentials for the new Lambda functions are present (execution role credentials, layer configurations, etc.)
- CloudFront-related secrets (origin access identities, WAF configurations) are properly configured
- S3 bucket credentials and access policies are appropriately set for the prod/stage environments
- No sensitive credentials are accidentally exposed in the main repository
.gitignore (1)
7-7: LGTM!The ignore rule correctly excludes Lambda deployment ZIP archives generated during Terraform plan/apply. This keeps build artifacts out of version control.
modules/shared_resources/s3.tf (1)
5-6: LGTM!Explicitly setting
force_destroy = false(even though it's the default) provides clear intent and works well with the existingprevent_destroy = truelifecycle rule to protect against accidental data loss.Also applies to: 16-17
environment/global/provider.tf (1)
1-21: LGTM!Provider configuration is well-structured with appropriate version constraints and default tags for resource tracking.
modules/shared_resources/src/thumbnail/index.js (1)
10-11: Only first S3 event record is processed.If S3 sends batch events (multiple records), only
Records[0]is processed. S3 notifications typically send one record per invocation, but it's safer to iterate over all records.Consider whether batch processing is needed based on your S3 notification configuration. If batching is enabled, a loop would be required:
for (const record of event.Records) { // process each record }modules/shared_resources/lambda.tf (1)
62-72: Verify event trigger difference is intentional.The resizing function triggers on both
s3:ObjectCreated:Putands3:ObjectCreated:Post, while the thumbnail function only triggers ons3:ObjectCreated:Put. If multipart uploads are used for chat images, they may useCompleteMultipartUploadwhich doesn't match either event.environment/global/main.tf (1)
1-25: LGTM!The module invocation is well-structured with all required variables properly passed through from the global environment to the shared_resources module.
| variable "thumbnail_generating_func_layers" { | ||
| description = "Layers For Image Resizing func" | ||
| type = list(string) | ||
| } |
There was a problem hiding this comment.
Copy-paste error in variable description.
The description says "Layers For Image Resizing func" but this variable is for thumbnail_generating_func_layers, not the resizing function.
🔎 Recommended fix
variable "thumbnail_generating_func_layers" {
- description = "Layers For Image Resizing func"
+ description = "Layers For Thumbnail Generating func"
type = list(string)
}📝 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 "thumbnail_generating_func_layers" { | |
| description = "Layers For Image Resizing func" | |
| type = list(string) | |
| } | |
| variable "thumbnail_generating_func_layers" { | |
| description = "Layers For Thumbnail Generating func" | |
| type = list(string) | |
| } |
🤖 Prompt for AI Agents
In environment/global/variables.tf around lines 58 to 61, the variable
description incorrectly references the image resizing function ("Layers For
Image Resizing func") while the variable is named
thumbnail_generating_func_layers; update the description to accurately describe
this variable (e.g., mention thumbnail generating function layers or thumbnail
generation) so the description matches the variable's purpose and intent.
| origin { | ||
| domain_name = "${var.s3_default_bucket_name}.s3.ap-northeast-2.amazonaws.com" | ||
| origin_id = "${var.s3_default_bucket_name}.s3.ap-northeast-2.amazonaws.com-mjo1g7tk2w8" # 기존 ID 유지 | ||
| origin_access_control_id = "E14M8OP55A3YO7" |
There was a problem hiding this comment.
Hard-coded Origin Access Control ID prevents Terraform management.
The origin_access_control_id is hard-coded, which means this OAC resource was created outside of Terraform. This prevents proper infrastructure-as-code management and could lead to drift.
🔎 Recommended fix: Import or create OAC resource
Option 1: Import existing OAC (recommended for migration)
Add a data source to reference the existing OAC:
+data "aws_cloudfront_origin_access_control" "default_oac" {
+ id = "E14M8OP55A3YO7"
+}
+
resource "aws_cloudfront_distribution" "default_cdn" {
# ...
origin {
domain_name = "${var.s3_default_bucket_name}.s3.ap-northeast-2.amazonaws.com"
origin_id = "${var.s3_default_bucket_name}.s3.ap-northeast-2.amazonaws.com-mjo1g7tk2w8"
- origin_access_control_id = "E14M8OP55A3YO7"
+ origin_access_control_id = data.aws_cloudfront_origin_access_control.default_oac.idOption 2: Create new OAC resource
resource "aws_cloudfront_origin_access_control" "default_oac" {
name = "solid-connection-default-oac"
origin_access_control_origin_type = "s3"
signing_behavior = "always"
signing_protocol = "sigv4"
}Then reference it:
- origin_access_control_id = "E14M8OP55A3YO7"
+ origin_access_control_id = aws_cloudfront_origin_access_control.default_oac.idCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| viewer_certificate { | ||
| cloudfront_default_certificate = true | ||
| minimum_protocol_version = "TLSv1" | ||
| } |
There was a problem hiding this comment.
TLSv1 is deprecated and insecure.
The minimum TLS protocol version is set to TLSv1, which has known security vulnerabilities and is deprecated. AWS recommends TLSv1.2 as the minimum.
🔎 Recommended security fix
viewer_certificate {
cloudfront_default_certificate = true
- minimum_protocol_version = "TLSv1"
+ minimum_protocol_version = "TLSv1.2_2021"
}📝 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.
| viewer_certificate { | |
| cloudfront_default_certificate = true | |
| minimum_protocol_version = "TLSv1" | |
| } | |
| viewer_certificate { | |
| cloudfront_default_certificate = true | |
| minimum_protocol_version = "TLSv1.2_2021" | |
| } |
🤖 Prompt for AI Agents
In modules/shared_resources/cloudfront.tf around lines 44 to 47, the
viewer_certificate block sets minimum_protocol_version = "TLSv1", which is
deprecated; update it to use TLSv1.2 by changing minimum_protocol_version to
"TLSv1.2_2018" (or the latest TLSv1.2 policy) and, if you require a custom
certificate, set cloudfront_default_certificate = false and reference an ACM
certificate via acm_certificate_arn; ensure the certificate option and protocol
string are valid for the CloudFront distribution.
| origin { | ||
| domain_name = "${var.s3_upload_bucket_name}.s3.ap-northeast-2.amazonaws.com" | ||
| origin_id = "${var.s3_upload_bucket_name}.s3.ap-northeast-2.amazonaws.com-mjo1jpx6rvc" | ||
| origin_access_control_id = "E1ZBB5RMSBZQ4I" |
There was a problem hiding this comment.
Hard-coded Origin Access Control ID prevents Terraform management.
Same issue as the default_cdn distribution - the OAC ID is hard-coded. This should be managed through Terraform for consistency and infrastructure-as-code best practices.
🔎 Recommended fix: Import or create OAC resource
Option 1: Import existing OAC
+data "aws_cloudfront_origin_access_control" "upload_oac" {
+ id = "E1ZBB5RMSBZQ4I"
+}
+
resource "aws_cloudfront_distribution" "upload_cdn" {
# ...
origin {
domain_name = "${var.s3_upload_bucket_name}.s3.ap-northeast-2.amazonaws.com"
origin_id = "${var.s3_upload_bucket_name}.s3.ap-northeast-2.amazonaws.com-mjo1jpx6rvc"
- origin_access_control_id = "E1ZBB5RMSBZQ4I"
+ origin_access_control_id = data.aws_cloudfront_origin_access_control.upload_oac.idOption 2: Create new OAC resource
resource "aws_cloudfront_origin_access_control" "upload_oac" {
name = "solid-connection-upload-oac"
origin_access_control_origin_type = "s3"
signing_behavior = "always"
signing_protocol = "sigv4"
}Committable suggestion skipped: line range outside the PR's diff.
| viewer_certificate { | ||
| cloudfront_default_certificate = true | ||
| minimum_protocol_version = "TLSv1" | ||
| } |
There was a problem hiding this comment.
TLSv1 is deprecated and insecure.
Same security issue as default_cdn - TLSv1 should be upgraded to at least TLSv1.2_2021.
🔎 Recommended security fix
viewer_certificate {
cloudfront_default_certificate = true
- minimum_protocol_version = "TLSv1"
+ minimum_protocol_version = "TLSv1.2_2021"
}🤖 Prompt for AI Agents
In modules/shared_resources/cloudfront.tf around lines 93 to 96, the
viewer_certificate block sets minimum_protocol_version = "TLSv1" which is
deprecated; change that value to "TLSv1.2_2021" (or a later supported value) so
CloudFront requires TLS 1.2+, and if you need a custom cert switch to
acm_certificate_arn + ssl_support_method accordingly; update any related
tests/plan expectations and re-run terraform plan/apply to verify.
| // 썸네일 파일명 생성 | ||
| const fileName = key.split('/').pop(); // 파일명만 추출 | ||
| const nameWithoutExt = fileName.substring(0, fileName.lastIndexOf('.')); | ||
| const extension = fileName.substring(fileName.lastIndexOf('.')); | ||
| const thumbnailKey = `chat/thumbnails/${nameWithoutExt}_thumb${extension}`; | ||
|
|
||
| console.log(`Uploading thumbnail to: ${thumbnailKey}`); | ||
|
|
||
| // 썸네일을 S3에 업로드 (AWS SDK v3 방식) | ||
| const putCommand = new PutObjectCommand({ | ||
| Bucket: bucket, | ||
| Key: thumbnailKey, | ||
| Body: thumbnailBuffer, | ||
| ContentType: 'image/jpeg', |
There was a problem hiding this comment.
File extension and ContentType mismatch.
The thumbnail preserves the original file extension (e.g., .png) but always outputs JPEG format with ContentType: 'image/jpeg'. This causes a mismatch where a file named image_thumb.png contains JPEG data.
🔎 Proposed fix - use consistent .jpg extension
// 썸네일 파일명 생성
const fileName = key.split('/').pop(); // 파일명만 추출
const nameWithoutExt = fileName.substring(0, fileName.lastIndexOf('.'));
- const extension = fileName.substring(fileName.lastIndexOf('.'));
- const thumbnailKey = `chat/thumbnails/${nameWithoutExt}_thumb${extension}`;
+ const thumbnailKey = `chat/thumbnails/${nameWithoutExt}_thumb.jpg`;🤖 Prompt for AI Agents
modules/shared_resources/src/thumbnail/index.js lines 54-67: the code builds a
thumbnail filename using the original file extension but always writes JPEG data
with ContentType 'image/jpeg', causing mismatched file contents; change the
thumbnailKey creation to force a .jpg extension (e.g., const thumbnailKey =
`chat/thumbnails/${nameWithoutExt}_thumb.jpg`) and ensure PutObjectCommand uses
ContentType: 'image/jpeg' so filename and stored data/type are consistent
(alternatively, if you actually produce PNGs, adjust the ContentType and
extension to match the real output format).
| variable "thumbnail_generating_func_layers" { | ||
| description = "Layers For Image Resizing func" | ||
| type = list(string) | ||
| } |
There was a problem hiding this comment.
Copy-paste error in variable description.
Same issue as in environment/global/variables.tf - the description incorrectly refers to "Image Resizing func" instead of "Thumbnail Generating func".
🔎 Recommended fix
variable "thumbnail_generating_func_layers" {
- description = "Layers For Image Resizing func"
+ description = "Layers For Thumbnail Generating func"
type = list(string)
}📝 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 "thumbnail_generating_func_layers" { | |
| description = "Layers For Image Resizing func" | |
| type = list(string) | |
| } | |
| variable "thumbnail_generating_func_layers" { | |
| description = "Layers For Thumbnail Generating func" | |
| type = list(string) | |
| } |
🤖 Prompt for AI Agents
In modules/shared_resources/variables.tf around lines 58 to 61, the variable
description is a copy-paste error referencing "Image Resizing func" instead of
the correct "Thumbnail Generating func"; update the description string to
accurately read "Layers For Thumbnail Generating func" (or similar precise
wording) so it correctly describes the variable purpose and mirrors the fix
applied in environment/global/variables.tf.
| ├── monitoring/ # [부하테스트 환경] | ||
| │ ├── main.tf | ||
| │ ├── provider.tf | ||
| │ └── variables.tf |
There was a problem hiding this comment.
Incorrect comment for monitoring environment.
The comment "부하테스트 환경" (load test environment) is duplicated from load_test/. The monitoring/ directory should have its own description, likely "모니터링 환경" or similar.
🔎 Proposed fix
- ├── monitoring/ # [부하테스트 환경]
+ ├── monitoring/ # [모니터링 환경]📝 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.
| ├── monitoring/ # [부하테스트 환경] | |
| │ ├── main.tf | |
| │ ├── provider.tf | |
| │ └── variables.tf | |
| ├── monitoring/ # [모니터링 환경] | |
| │ ├── main.tf | |
| │ ├── provider.tf | |
| │ └── variables.tf |
🤖 Prompt for AI Agents
In README.md around lines 42 to 45, the inline comment for the monitoring/
directory incorrectly says "부하테스트 환경" (load test environment); update that
comment to accurately describe the monitoring directory (e.g., "모니터링 환경") so it
no longer duplicates load_test/ and reflects the correct purpose of monitoring/.
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.