Linkry: read on redirect from DynamoDB instead of cloudfront KV store #341
Conversation
WalkthroughMigration from CloudFront Key-Value Store to a Lambda@Edge-based redirect handler: removes CloudFront KV integration and related API code, adds a Python Lambda@Edge that queries region-appropriate DynamoDB replicas, updates Terraform (removes KV resource, adds edge lambda and region configs), and simplifies API to use only DynamoDB transact writes. Changes
Sequence Diagram(s)sequenceDiagram
participant CF as CloudFront
participant LE as Lambda@Edge
participant DR as DynamoDB Replica
participant FB as Fallback URL
CF->>LE: Request (path)
LE->>LE: select_replica(current_region)
LE->>DR: Query table for slug (PK) & owner prefix
alt Found with redirect
DR-->>LE: redirect URL
LE->>CF: 302 Redirect (Location: redirect URL)
else Not found or error
LE->>CF: 307 Redirect (Location: FALLBACK_URL)
Note right of CF: caching headers applied
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Comment |
💰 Infracost reportMonthly estimate increased by $0.31 📈
*Usage costs were estimated using infracost-usage.yml, see docs for other options. Estimate details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
terraform/modules/frontend/variables.tf (1)
46-48: Add description for the renamed variable.The variable rename from
LinkryKvArntoLinkryEdgeFunctionArncorrectly reflects the architectural change. However, adding a description would improve clarity.variable "LinkryEdgeFunctionArn" { type = string + description = "ARN of the Lambda@Edge function for Linkry redirects" }terraform/modules/archival/variables.tf (1)
6-8: Add description for better documentation.The
BucketPrefixvariable lacks a description, making its purpose unclear.variable "BucketPrefix" { type = string + description = "Prefix for S3 bucket names, typically account-id-region" }terraform/envs/qa/main.tf (1)
35-35: Consistent replication configuration across environments.The QA environment uses the same replication region (
us-west-2) as prod, which is good for testing consistency. However, consider whether QA needs replication at all, as it may add unnecessary cost.For cost optimization in QA, consider:
locals { - LinkryReplicationRegions = toset(["us-west-2"]) + LinkryReplicationRegions = toset([]) # No replication in QA to reduce costs }src/linkryEdgeFunction/main.py (1)
18-33: Consider clarifying the region selection logic.The function works correctly but could be more explicit about its fallback strategy. The logic prioritizes exact replica matches, then attempts region-prefix matching, and finally falls back to the default region.
Consider adding docstring details or inline comments to clarify the priority order:
def select_replica(lambda_region): - """Determine which DynamoDB replica to use based on Lambda execution region""" + """ + Determine which DynamoDB replica to use based on Lambda execution region. + Priority: 1) Exact match in AVAILABLE_REPLICAS + 2) Region prefix match (e.g., eu-* regions) + 3) "us" regions → DEFAULT_AWS_REGION + 4) All others → DEFAULT_AWS_REGION + """ # First check if Lambda is running in a replica region if lambda_region in AVAILABLE_REPLICAS: return lambda_region
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
src/api/functions/cloudfrontKvStore.ts(0 hunks)src/api/package.json(0 hunks)src/api/routes/linkry.ts(1 hunks)src/api/types.d.ts(0 hunks)src/common/config.ts(0 hunks)src/linkryEdgeFunction/main.py(1 hunks)terraform/envs/prod/main.tf(3 hunks)terraform/envs/qa/main.tf(3 hunks)terraform/modules/archival/main.tf(2 hunks)terraform/modules/archival/variables.tf(1 hunks)terraform/modules/dynamo/main.tf(1 hunks)terraform/modules/frontend/main.tf(1 hunks)terraform/modules/frontend/variables.tf(1 hunks)terraform/modules/lambdas/main.tf(3 hunks)terraform/modules/lambdas/variables.tf(1 hunks)tests/unit/linkry.test.ts(0 hunks)
💤 Files with no reviewable changes (5)
- tests/unit/linkry.test.ts
- src/common/config.ts
- src/api/types.d.ts
- src/api/package.json
- src/api/functions/cloudfrontKvStore.ts
🧰 Additional context used
🪛 Checkov (3.2.334)
terraform/modules/lambdas/main.tf
[high] 489-500: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
🪛 Ruff (0.14.1)
src/linkryEdgeFunction/main.py
43-43: Unused function argument: context
(ARG001)
102-102: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Unit Tests
- GitHub Check: Build Application
🔇 Additional comments (19)
terraform/modules/lambdas/variables.tf (1)
36-39: LGTM! Variable addition is well-documented.The
LinkryReplicationRegionsvariable is properly typed as a set and includes a clear description. This correctly supports multi-region DynamoDB replication for Lambda@Edge.terraform/modules/frontend/main.tf (2)
379-379: Good choice:include_body = falseoptimizes redirect performance.Setting
include_body = falseis appropriate for redirect operations, as it reduces latency by not forwarding the request body to Lambda@Edge.
375-380: Cache policy ID verified as valid.The cache policy 83da9c7e-98b4-4e11-a168-04f0df8e2c65 is the UseOriginCacheControlHeaders managed cache policy, which instructs CloudFront to honor the origin's Cache-Control headers. The configuration is correct for Lambda@Edge viewer-request events.
terraform/envs/qa/main.tf (1)
109-119: LGTM! QA module configuration matches prod structure.The module inputs are consistent with the prod environment, ensuring feature parity for testing.
terraform/envs/prod/main.tf (2)
104-114: LGTM! Module inputs correctly updated.The frontend module inputs are properly updated to use
LinkryEdgeFunctionArnfrom the lambdas module output, and all other inputs are preserved. Verified that the lambdas module exportslinkry_redirect_function_arnatterraform/modules/lambdas/main.tf:529.
40-40: Add DynamoDB replicas closer to Lambda@Edge deployment regions for optimal global performance.The Lambda@Edge function correctly implements region-aware DynamoDB client routing (selecting us-west-2 when available, otherwise falling back to us-east-2). However, with global CloudFront edge location deployment but only one replica region (us-west-2), edge locations in Europe, Asia, and other regions will route all queries cross-continentally to us-east-2 (Ohio).
Consider expanding LinkryReplicationRegions to include replicas in regions closer to your Lambda@Edge deployment footprint (e.g., eu-west-1 for Europe, ap-southeast-1 for Asia-Pacific) to reduce latency for global users.
terraform/modules/lambdas/main.tf (5)
13-17: LGTM!The archive configuration correctly sources the Python Lambda@Edge function code and outputs to the expected location.
444-461: LGTM!The IAM role correctly includes both
lambda.amazonaws.comandedgelambda.amazonaws.comservice principals, which is required for Lambda@Edge functions that replicate to CloudFront edge locations.
468-487: LGTM! Wildcard region enables multi-region DynamoDB access.The IAM policy correctly uses a wildcard region (
arn:aws:dynamodb:*:...) to allow the Lambda@Edge function to query DynamoDB replicas in multiple regions. The permissions are appropriately restricted to read-only operations (GetItem, Query) on the specific Linkry table.
529-531: LGTM! Correctly uses qualified_arn for Lambda@Edge.The output correctly uses
qualified_arninstead ofarn, which is required for Lambda@Edge associations as they must reference a specific version, not$LATEST.
489-500: Disregard the timeout concern—5 seconds is the AWS-enforced maximum for Lambda@Edge viewer-request events and cannot be increased.The linkry_edge function is correctly configured. Lambda@Edge viewer-request triggers have a hard 5-second timeout limit set by AWS, not a configurable setting. Cross-region DynamoDB latency is a valid design consideration, but if the function needs longer execution time, the trigger type would need to change to origin-request (30-second limit), which would alter the CloudFront architecture.
The code-signing suggestion remains valid as an optional security best practice, though not critical.
Likely an incorrect or invalid review comment.
src/linkryEdgeFunction/main.py (5)
1-15: LGTM! Well-structured constants and configuration.The constants are clearly defined with appropriate defaults. The DynamoDB global table configuration with us-east-2 as default and us-west-2 as replica aligns with the wildcard region IAM policy in the Terraform configuration.
36-40: LGTM! Efficient module-level initialization.The module-level initialization of the DynamoDB client is correct and follows Lambda best practices by reusing the client across invocations within the same container lifecycle. The routing log statement aids debugging.
43-59: LGTM! Appropriate handling of empty path.The 301 (Moved Permanently) redirect to the Linkry home page for empty paths is correct, and the caching configuration is appropriate for a permanent redirect.
95-115: Error handling is pragmatic for edge function constraints.The error handling appropriately falls back to a temporary redirect on any failure. While static analysis flags the broad
Exceptioncatch, it's acceptable in this Lambda@Edge context where:
- All errors should gracefully degrade to the fallback URL
- CloudWatch logs capture the error details for debugging
- The function must not raise exceptions to CloudFront
The 307 (Temporary Redirect) status correctly signals that the redirect is not permanent, encouraging clients to retry.
Based on static analysis
62-94: DynamoDB table schema verified—code implementation is correct.The query assumes partition key
slugand sort keyaccess, which matches the actual table definition atterraform/modules/dynamo/main.tf:274-292. Thebegins_with()operation on the sort key is appropriate for the schema. The 30-second cache TTL is reasonable for temporary redirects; no action needed.terraform/modules/archival/main.tf (3)
210-210: Wildcard region expands IAM permissions scope.The IAM policy now uses a wildcard region (
arn:aws:logs:*:...) which allows Firehose to write logs in any region. While this supports region-agnostic deployment, it grants broader permissions than region-specific ARNs.This appears intentional based on the broader refactoring to remove region constraints. Verify this aligns with your security requirements and deployment model.
32-32: BucketPrefix variable is properly defined and passed from parent modules.Verification confirms the variable is correctly defined in
terraform/modules/archival/variables.tf(line 6) withtype = string. Both qa and prod environments passlocal.bucket_prefixwhich is computed from AWS account ID and region, ensuring consistent naming across deployments. The implementation is complete and correct.
8-9: No action needed—the removal of regional suffixes is correct for this deployment model.The archival module is instantiated exactly once per environment (qa/prod), with no
countorfor_each. SinceProjectIdis environment-specific, resource names are already globally unique per environment. The named concern about multi-region naming conflicts cannot occur in this architecture.Likely an incorrect or invalid review comment.
1 participant
Summary by CodeRabbit
New Features
Refactor
Chores