ci: share CloudFront cache policy + KV store across preview stages#859
ci: share CloudFront cache policy + KV store across preview stages#859willwashburn wants to merge 3 commits into
Conversation
Each sst.aws.Nextjs preview stage created its own CloudFront CachePolicy and KeyValueStore. The per-account quotas (20 cache policies / 5 KV stores) capped concurrent previews and produced TooManyCachePolicies / EntityLimitExceeded once leaked stages accumulated. - web/scripts/bootstrap-preview-infra.sh: one-shot, idempotent script that creates a shared CachePolicy (matching SST's internal default config) and KV store, then publishes their IDs to SSM (/relay-web/preview/cache-policy-id, /relay-web/preview/kv-store-arn). - web/sst.config.ts: non-prod stages now read those SSM params and pass them to sst.aws.Nextjs as `cachePolicy` and `edge.viewerRequest.kvStore`. Production keeps creating its own resources for isolation. SST namespaces KV entries by md5(app + stage + componentName), so the shared store is safe — keys from different preview stages don't collide. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an idempotent Bash bootstrap script that provisions (or reuses) a shared CloudFront cache policy and key-value store and stores their identifiers in SSM. sst.config.ts now reads those SSM parameters for non-production stages and conditionally injects them into the Next.js deployment. ChangesPreview Infrastructure with Shared CloudFront Resources
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
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)
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: 2
🤖 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.
Inline comments:
In `@web/scripts/bootstrap-preview-infra.sh`:
- Around line 35-37: The read_ssm function currently suppresses all aws ssm
errors; change it to only treat a missing parameter as non-fatal by calling aws
ssm get-parameter and inspecting its stderr/exit status: if the error message
contains "ParameterNotFound" return empty, otherwise propagate the failure (log
the error and exit non-zero or return a non-empty failure) so
AccessDenied/throttling/transport errors are not masked; update the read_ssm
implementation to capture stderr, check for the "ParameterNotFound" string and
handle only that case silently, while letting other errors surface.
- Around line 17-19: The script only checks AWS_ACCESS_KEY_ID before setting
AWS_PROFILE, which overrides valid OIDC/assumed-role credentials; change the
conditional that sets AWS_PROFILE to only run when none of the common credential
sources are present — e.g. check that AWS_ACCESS_KEY_ID,
AWS_WEB_IDENTITY_TOKEN_FILE, and AWS_ROLE_ARN (and optionally AWS_SESSION_TOKEN)
are all empty/unset before exporting AWS_PROFILE="${AWS_PROFILE:-ar_preview}" so
role-based or web-identity authentication is not overridden.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e6659cc-8551-4a9c-bb73-2ce303350b9c
📒 Files selected for processing (2)
web/scripts/bootstrap-preview-infra.shweb/sst.config.ts
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| # 2. KV store. SST namespaces keys by md5(app + stage + componentName) so sharing | ||
| # the store across stages is safe — entries don't collide. | ||
| existing_kv_arn="$(read_ssm "$SSM_KV_STORE_PARAM")" | ||
| if [ -n "$existing_kv_arn" ] && aws cloudfront describe-key-value-store --kvs-arn "$existing_kv_arn" >/dev/null 2>&1; then |
There was a problem hiding this comment.
🔴 Wrong CLI flag --kvs-arn for describe-key-value-store breaks script idempotency
The AWS CLI command aws cloudfront describe-key-value-store accepts --name (which can be a name or an ARN), not --kvs-arn. The --kvs-arn flag is only valid for data-plane KVS operations like list-keys-in-key-value-store, get-key-in-key-value-store, etc. Because --kvs-arn is unrecognized, the command on line 85 always fails (silently, due to 2>&1), causing the if to evaluate false and the script to fall into the else branch. On a re-run where the KV store already exists, create-key-value-store will fail with an AlreadyExists conflict, and set -euo pipefail will abort the script. This directly breaks the script's stated idempotency guarantee.
| if [ -n "$existing_kv_arn" ] && aws cloudfront describe-key-value-store --kvs-arn "$existing_kv_arn" >/dev/null 2>&1; then | |
| if [ -n "$existing_kv_arn" ] && aws cloudfront describe-key-value-store --name "$existing_kv_arn" >/dev/null 2>&1; then |
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfa4b2cb1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| existing_policy_id="$(read_ssm "$SSM_CACHE_POLICY_PARAM")" | ||
| if [ -n "$existing_policy_id" ] && aws cloudfront get-cache-policy --id "$existing_policy_id" >/dev/null 2>&1; then |
There was a problem hiding this comment.
This idempotency check only trusts the SSM value, so if the first run creates the policy but fails before put-parameter (or if the SSM param is later deleted), a rerun will execute create-cache-policy with the same fixed name and fail. AWS CloudFront requires cache policy names to be unique (CachePolicyAlreadyExists), so the script cannot recover automatically from partial failures even though it is documented as safe to rerun.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/scripts/bootstrap-preview-infra.sh">
<violation number="1" location="web/scripts/bootstrap-preview-infra.sh:23">
P2: The script requires `jq` but never uses it. All JSON handling uses heredocs and AWS CLI's `--query`/`--output text`. Remove `jq` from the dependency check to avoid unnecessarily blocking execution.</violation>
<violation number="2" location="web/scripts/bootstrap-preview-infra.sh:36">
P1: `read_ssm()` suppresses all errors (`2>/dev/null || true`), not just `ParameterNotFound`. If the call fails due to AccessDenied, throttling, or network issues, the function returns an empty string and the script interprets it as "parameter missing" — then incorrectly attempts to create duplicate resources. Only `ParameterNotFound` should be treated as an empty result; other failures should abort the script.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Re-trigger cubic
|
@copilot fix issues raised by comments in the PR thread |
Agent-Logs-Url: https://github.com/AgentWorkforce/relay/sessions/d51b31a7-d2d9-43d9-a551-0f8567032f05 Co-authored-by: willwashburn <957608+willwashburn@users.noreply.github.com>
All issues from the review comments have been addressed in commit
|
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/scripts/bootstrap-preview-infra.sh">
<violation number="1" location="web/scripts/bootstrap-preview-infra.sh:110">
P2: The KV-store existence check passes an ARN into `--name`, so the check never succeeds. This bypasses the intended SSM fast-path and can cause unnecessary fallback calls/failures.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Fix all with cubic | Re-trigger cubic
| # 2. KV store. SST namespaces keys by md5(app + stage + componentName) so sharing | ||
| # the store across stages is safe — entries don't collide. | ||
| existing_kv_arn="$(read_ssm "$SSM_KV_STORE_PARAM")" | ||
| if [ -n "$existing_kv_arn" ] && aws cloudfront describe-key-value-store --name "$existing_kv_arn" >/dev/null 2>&1; then |
There was a problem hiding this comment.
P2: The KV-store existence check passes an ARN into --name, so the check never succeeds. This bypasses the intended SSM fast-path and can cause unnecessary fallback calls/failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/scripts/bootstrap-preview-infra.sh, line 110:
<comment>The KV-store existence check passes an ARN into `--name`, so the check never succeeds. This bypasses the intended SSM fast-path and can cause unnecessary fallback calls/failures.</comment>
<file context>
@@ -72,27 +96,37 @@ else
# the store across stages is safe — entries don't collide.
existing_kv_arn="$(read_ssm "$SSM_KV_STORE_PARAM")"
-if [ -n "$existing_kv_arn" ] && aws cloudfront describe-key-value-store --kvs-arn "$existing_kv_arn" >/dev/null 2>&1; then
+if [ -n "$existing_kv_arn" ] && aws cloudfront describe-key-value-store --name "$existing_kv_arn" >/dev/null 2>&1; then
echo "==> KV store already exists: $existing_kv_arn"
kv_store_arn="$existing_kv_arn"
</file context>
Summary
web/scripts/bootstrap-preview-infra.sh— one-shot, idempotent script that creates a shared CloudFrontCachePolicy(matching SST's internal default config) andKeyValueStore, then writes their IDs to SSM (/relay-web/preview/cache-policy-id,/relay-web/preview/kv-store-arn). Safe to re-run; only creates if SSM-recorded resources don't exist.web/sst.config.ts— non-prod stages now read those SSM params and pass them tosst.aws.NextjsascachePolicyandedge.viewerRequest.kvStore. Production keeps its own resources for isolation.Why
Each
sst.aws.Nextjspreview stage was creating its ownWebServerCachePolicyandWebKvStore. Per-account CloudFront quotas — 20 cache policies, 5 KV stores — capped how many concurrent previews could exist, and Deploy Preview was failing withTooManyCachePolicies/EntityLimitExceededonce leaked stages accumulated.After this change, preview stages contribute 0 cache policies and 0 KV stores. Only the one shared pair counts against the quota, regardless of how many preview PRs are open.
Safety: shared KV store is OK
SST's router code namespaces every key by
md5($app.name + $app.stage + componentName):So
pr-839andpr-778reading/writing the same KV store can't collide.Rollout
web/scripts/bootstrap-preview-infra.shonce (withAWS_PROFILE=ar_previewor ambient creds) to provision the shared resources and populate SSM.Pairs with #858 (the stale-stage cleanup workflow) — together they remove the cap-and-leak problem entirely.
Test plan
web/scripts/bootstrap-preview-infra.sh— runs cleanly, SSM params populated, AWS CachePolicy + KVStore visible in console.web/scripts/bootstrap-preview-infra.sh— idempotent, doesn't create duplicates.TooManyCachePolicies/EntityLimitExceeded, no per-stageWebServerCachePolicyorWebKvStoreresources appear in the SST output.🤖 Generated with Claude Code