feat(website): resolve pretty URLs via CloudFront KVS, drop per-page terraform apply#1900
Closed
ytallo wants to merge 1 commit into
Closed
feat(website): resolve pretty URLs via CloudFront KVS, drop per-page terraform apply#1900ytallo wants to merge 1 commit into
ytallo wants to merge 1 commit into
Conversation
…terraform apply Adding a new top-level page required editing the hardcoded htmlPretty map in the redirects CloudFront Function and running `terraform apply` to republish it — a separate, manual pipeline from the automatic content deploy. Until that apply ran, the clean URL 404'd, forcing links to the literal `.html` (MOT-3669). Move the pretty-URL -> .html map into a CloudFront KeyValueStore that deploy-website.yml syncs from website/*.html on every content deploy. The function reads the store at the edge (async). Adding a page becomes a content-only change: drop foo.html, link to /foo, merge — no terraform apply. - redirects.js: async handler; KVS lookup replaces the hardcoded map and unifies the pretty-URL rewrite with the real-404 fallback into one extensionless branch - redirects.test.js: inject a mock cf.kvs(); await handler; +6 KVS cases (46 pass) - routes-kvs.ts (+test): derive the desired map from *.html, diff vs the store - deploy-website.yml: batch update-keys step after the HTML sync (gated on CF_KVS_ARN) - terraform: aws_cloudfront_key_value_store + function association, IAM data-plane grant, routes_kvs_arn output (one-time apply; validates clean) Claude-Session: https://claude.ai/code/session_01N9Xi9vXLynC4JsaCwPJhVq
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe PR replaces the hardcoded pretty-URL map in the CloudFront edge function with a CloudFront KeyValueStore (KVS) lookup. A new ChangesCloudFront KVS-Backed Pretty-URL Routing
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
Note over GitHubActions,CloudFrontKVS: Deployment: sync pretty-URL routes
GitHubActions->>CloudFrontKVS: describe-key-value-store → ETag
GitHubActions->>CloudFrontKVS: list-keys → current JSON
GitHubActions->>routes-kvs.ts: tsx routes-kvs.ts --current <file>
routes-kvs.ts-->>GitHubActions: {Puts, Deletes}
GitHubActions->>CloudFrontKVS: update-keys --if-match ETag
end
rect rgba(60, 179, 113, 0.5)
Note over Viewer,CloudFrontKVS: Runtime: edge request routing
Viewer->>redirects.js: GET /manifesto
redirects.js->>CloudFrontKVS: routes.get("/manifesto")
alt key exists
CloudFrontKVS-->>redirects.js: "/manifesto.html"
redirects.js-->>Viewer: rewrite URI → /manifesto.html
else key missing
CloudFrontKVS-->>redirects.js: throws
redirects.js-->>Viewer: 404 notFound()
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Terraform plan —
|
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/scripts/routes-kvs.ts (1)
52-53: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMake the route source directory explicit instead of relying on cwd.
Line 53 ties behavior to
process.cwd(). If this command is run from an unexpected directory, it can compute the wrong desired set and emit unintended Deletes. Pass a directory explicitly (and wire it from the workflow) to harden this path.Suggested change
function main(): void { - const desired = desiredRoutes(listFiles(process.cwd())) + const dirFlagIdx = process.argv.indexOf('--dir') + const sourceDir = dirFlagIdx === -1 ? process.cwd() : process.argv[dirFlagIdx + 1] + if (!sourceDir) { + console.error('--dir requires a directory path') + process.exitCode = 1 + return + } + const desired = desiredRoutes(listFiles(sourceDir))-ops=$(pnpm --filter iii-website exec tsx scripts/routes-kvs.ts --current /tmp/kvs-current.json) +ops=$(pnpm --filter iii-website exec tsx scripts/routes-kvs.ts --dir . --current /tmp/kvs-current.json)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/scripts/routes-kvs.ts` around lines 52 - 53, The main function relies on process.cwd() which can produce incorrect results if the script is run from an unexpected directory. Modify the main function to accept a directory parameter explicitly and pass that parameter to the listFiles call instead of using process.cwd(). Update the function signature and the desiredRoutes call to use this explicit directory parameter, then wire the directory from the workflow or command line arguments so it is passed explicitly when main is invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@website/scripts/routes-kvs.ts`:
- Around line 52-53: The main function relies on process.cwd() which can produce
incorrect results if the script is run from an unexpected directory. Modify the
main function to accept a directory parameter explicitly and pass that parameter
to the listFiles call instead of using process.cwd(). Update the function
signature and the desiredRoutes call to use this explicit directory parameter,
then wire the directory from the workflow or command line arguments so it is
passed explicitly when main is invoked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 68ea8e61-0ff3-4b03-be76-00a8c100ef64
📒 Files selected for processing (8)
.github/workflows/deploy-website.ymlinfra/terraform/website/cloudfront.tfinfra/terraform/website/cloudfront_functions/redirects.jsinfra/terraform/website/cloudfront_functions/redirects.test.jsinfra/terraform/website/iam_github_oidc.tfinfra/terraform/website/outputs.tfwebsite/scripts/routes-kvs.test.tswebsite/scripts/routes-kvs.ts
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (MOT-3669)
Adding a top-level page like
/privacy-policyrequired two things on two different pipelines:privacy-policy.htmlwebsite/→ S3deploy-website.ymlaws s3 sync— automatic on push tomain/privacy-policy→/privacy-policy.htmlrewritehtmlPrettymap in the redirects CloudFront Functionterraform apply(publishes the function) — manual, no apply automationSo the
.htmlshipped automatically, but the clean-URL rewrite didn't. Until the manual apply ran,/privacy-policyfell through to the real-404 branch and 404'd — which is why the footer link had to be pointed at the literal/privacy-policy.htmlas a workaround.Fix
Move the pretty-URL →
.htmlmap out of the function source and into a CloudFront KeyValueStore (KVS). The function reads it at the edge (async);deploy-website.ymlsyncs it fromwebsite/*.htmlon every content deploy — same automatic pipeline as the HTML.Adding a page is now a content-only change: drop
foo.html, link to/foo, merge. The clean URL resolves on the next deploy. Noterraform apply.Changes
redirects.js—asynchandler; KVS lookup replaces the hardcoded map. The pretty-URL rewrite and the real-404 fallback are unified into a single extensionless-path branch, so/,/blog/*, and*.htmlnever incur a KVS read. ThenotFound()SEO behavior is preserved (KVS miss → real 404).redirects.test.js— loader strips theimport cf from 'cloudfront'line and injects a mockcf.kvs(); tests areawaited; +6 KVS cases (hit, miss→404, verbatim value, empty store, www precedence). 46/46 pass.routes-kvs.ts(+ test) — derives the desired map fromwebsite/*.html(excludesindex.html) and diffs it against the store into a batch{Puts, Deletes}. 6 unit tests.deploy-website.yml— after the HTML sync (so a key never points at a missing object), describe → list → diff →update-keys. Gated on theCF_KVS_ARNrepo var, so it no-ops until that's set.concurrency: deploy-websiteserializes deploys, keeping the describe-time ETag valid.aws_cloudfront_key_value_store.routes+ function association, a least-privilege IAM data-plane grant on the deploy role (Describe/ListKeys/UpdateKeys), and aroutes_kvs_arnoutput.terraform validatepasses. Terraform owns the store; the pipeline owns the data (the AWS provider has no key resource — data is decoupled by design).Rollout (one-time)
Terraform can't seed KVS data, so the first rollout populates the store out-of-band. To avoid a 404 window on
/manifesto+/privacy-policy, apply in two phases:key_value_store_associationsso the function keeps its current behavior).CF_KVS_ARN = terraform output -raw routes_kvs_arn, then triggerdeploy-website.yml(or run its KVS step locally) to seed/manifesto+/privacy-policy.key_value_store_associationsand apply; the function now reads a populated store. No 404 window./privacy-policy.htmlback to/privacy-policy.(If a brief 404 on those two secondary pages is acceptable, collapse to a single apply + immediate deploy.)
Test plan
redirects.test.js— 46/46 pass (40 existing behaviors unchanged + 6 KVS cases)routes-kvs.test.ts— 6/6 pass; generator verified against realwebsite/*.htmland against empty / stale-key storesterraform fmt(clean) +terraform validate(success)curl -I https://iii.dev/privacy-policy→ 200; unknown extensionless path → 404test-page.htmland confirm/test-pageresolves with no applyNote: an unrelated pre-existing test failure (
blog-posts.test.ts"finds hello-world") exists onmain— there's nohello-world.mdin the local blog content; this PR touches no blog files.Closes MOT-3669.
https://claude.ai/code/session_01N9Xi9vXLynC4JsaCwPJhVq
Summary by CodeRabbit
Release Notes
Chores
Tests