fix(terraform/aws): add dashboard_url override for Lambda-Function-URL deployments (follow-up to #355) - #363
Conversation
…L deployments (#355) local.dashboard_url at terraform/environments/aws/main.tf only fell back to "" when var.frontend_domain_names was empty — there was no tier for deployments served from a raw Lambda Function URL. Result: DASHBOARD_URL reached the Lambda as empty, the auth Service refused to send invite / password-reset emails (the guard introduced earlier in this branch), and every operator using a dev / scratch deployment without a custom domain saw silent email loss. A direct local.dashboard_url = module.compute_lambda[0].function_url fallback creates a dependency cycle (compute_lambda consumes additional_env_vars which contains local.dashboard_url; we'd then have local.dashboard_url → compute_lambda → compute_lambda). Adding a tfvar bootstraps cleanly: 1. var.dashboard_url — explicit override (NEW) 2. var.frontend_domain_names[0] (existing) — custom domain path 3. empty — the app's startup WARN + per-send guards already surface the misconfiguration Operators of Lambda-Function-URL deployments now: - `terraform apply` once (DASHBOARD_URL empty in the env) - `terraform output frontend_url` prints the just-provisioned URL - set var.dashboard_url to that value in tfvars - `terraform apply` again — DASHBOARD_URL is now persisted Matches outputs.tf's frontend_url chain (which already prioritises the Lambda Function URL for Lambda compute platforms) modulo the cycle-avoidance constraint. Doc updated in github-dev.tfvars to call out the bootstrap pattern.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a ChangesDashboard URL Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@terraform/environments/aws/variables.tf`:
- Around line 441-443: Validation for variable dashboard_url should be tightened
to reject query strings, fragments and userinfo: update the validation.condition
(the block validating var.dashboard_url) to still allow empty string or a full
origin but replace the current regex with one that requires an http or https
scheme, then a host portion that does NOT allow user@ (no userinfo) and accepts
either a hostname/FQDN or bracketed IPv6, followed optionally by a colon and
numeric port, and then anchors to end-of-string so no path/query/fragment are
allowed; keep the existing can() wrapping and the var.dashboard_url == ""
short-circuit.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 519959aa-1255-4f53-9769-e199cf5842c5
📒 Files selected for processing (3)
terraform/environments/aws/github-dev.tfvarsterraform/environments/aws/main.tfterraform/environments/aws/variables.tf
Address CR feedback on PR #363: the previous regex `^https?://[^/]+(?::[0-9]+)?$` was too permissive — `[^/]+` matched anything except `/`, so URLs containing `@` (userinfo), `?` (query), or `#` (fragment) silently slipped through and would have produced invalid CORS origins downstream. New pattern: ^https?:// scheme: http or https (?: [A-Za-z0-9.-]+ hostname / FQDN |\[[0-9A-Fa-f:]+\] OR bracketed IPv6 literal ) (?::[0-9]+)? optional numeric port $ end-of-string Rejects userinfo (`https://user@example.com`), path (`https://example.com/foo`), query (`https://example.com?q=1`), fragment (`https://example.com#frag`), and trailing slash. Accepts plain hostnames (`http://localhost`), FQDNs (`https://app.example.com`), IPv4 (`http://192.168.1.1`), and IPv6 literals (`https://[::1]:8443`). Empty-string short-circuit preserved (the value is optional; leaving it blank falls through to frontend_domain_names → empty as before). Verified against 13 sample inputs (positive + negative).
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…(follow-up to #363) (#368) * chore(ci): pass TF_VAR_dashboard_url into AWS Lambda deploy workflow Follow-up to #363 (which added var.dashboard_url to the Terraform config). Without this commit the variable stays unset in CI and the DASHBOARD_URL env var on the deployed Lambda gets reset to "" on every apply — symptom: invite / password-reset emails skip the send with the "DashboardURL is not configured" error. Adds TF_VAR_dashboard_url to both the Terraform Plan and Terraform Apply env blocks in .github/workflows/deploy-aws-lambda.yml, sourced from a new repo secret named DASHBOARD_URL. Matches the existing pattern used by TF_VAR_admin_email and TF_VAR_from_email — operator sets the secret once per repository (or per environment), CI threads it through to every deploy. For dev (Lambda Function URL, no custom domain), the value is the Function URL itself: e.g. DASHBOARD_URL = https://<random>.lambda-url.<region>.on.aws For deployments with frontend_domain_names already configured (custom domain + Route53 + ACM), the secret can be left empty — the Terraform local falls back to "https://${frontend_domain_names[0]}" via the priority chain added in #363. Caller action: set the GitHub repo secret DASHBOARD_URL to the dashboard origin for each environment that uses this workflow, then re-run the deploy. The very next apply persists the value into the Lambda env permanently (no more manual `aws lambda update-function-configuration` pokes that get clobbered). * chore(ci): bind build-and-deploy to GitHub Environment (CR #368) CR on PR #368 flagged that adding TF_VAR_dashboard_url without binding the job to a GitHub Environment leaves DASHBOARD_URL repo-scoped. Result: dev/staging/prod would all read the same secret value and inject the wrong origin into customer-facing email links for two of the three environments. Adds environment: ${{ needs.prepare.outputs.environment }} to the build-and-deploy job so secrets.* resolve to the named environment's secrets when defined, falling back to repo-scoped secrets when an env-level override isn't present. Backward- compatible with the current repo-level DASHBOARD_URL secret — operator can move it to env-level secrets per environment incrementally without breaking the workflow. Same protection now applies to TF_VAR_admin_email, TF_VAR_from_email, TF_BACKEND_AWS, and any other secret the job reads — they pick up environment overrides automatically once defined.
Summary
Follow-up to #355: even with PR #362's app-layer guards in place, the dev Lambda deployment had
DASHBOARD_URL=""becauselocal.dashboard_url(used to build the env var) only fell back to custom domains, not to the Lambda Function URL itself. Operators of any Lambda-Function-URL deployment without a custom domain configured got silent email loss.This PR adds a
var.dashboard_urltfvar that takes priority overvar.frontend_domain_names, so operators can bootstrap the env var manually for Function-URL deployments.Why not just read
module.compute_lambda[0].function_urldirectly?Dependency cycle. The same module consumes
additional_env_vars(which containslocal.dashboard_url), so referencing its output from the local would mean:The workable Terraform-only path is a manual bootstrap:
terraform apply—DASHBOARD_URL=""in the env, app refuses to send.terraform output frontend_url— gives the function URL.dashboard_url = "<that value>"in tfvars.DASHBOARD_URLis now persisted across deploys.The trace was:
local.dashboard_urlpriority chain (main.tf:58) was shallower than thefrontend_urloutput's chain (outputs.tf:195-200), which already handles all four tiers correctly. This PR closes the gap with a single override variable (the cycle prevents a direct mirror).Changes
terraform/environments/aws/variables.tf— newdashboard_urlvariable with validation (must include scheme, no trailing slash, no path).terraform/environments/aws/main.tf—local.dashboard_urlchecks the new var first, then falls back to the existingfrontend_domain_namespath. Comment refreshed to spell out why the Lambda-Function-URL tier can't live here.terraform/environments/aws/github-dev.tfvars— comment updated to call out the new tfvar + the bootstrap pattern. The CI/CDTF_VAR_dashboard_urlline is documented but not hard-coded (it's per-environment).Test plan
terraform fmt -check -recursiveclean.terraform validate— not run locally; this PR's CI will exercise it against the prod state backend.DASHBOARD_URLin the Lambda env survives subsequent applies (no longer reset to empty byadditional_env_varsreconciliation).Out of scope
dashboard_urlfor CDN / Fargate / custom-domain deployments — those already work via the existing chain. The new tfvar is the Lambda-Function-URL escape hatch.aws lambda update-function-configurationenv-var poke applied directly to the live Lambda earlier today gets clobbered by the nextterraform applyuntil this PR'sdashboard_urltfvar value is set in the dev CI/CD pipeline. That's the operational follow-up.Sister PRs
DASHBOARD_URLis empty + HTML CTA-button templates. The code change works as designed; this PR fills in the IaC half.Summary by CodeRabbit