feat(terraform): support custom domains for Cloudflare workers#604
Open
donnfelker wants to merge 6 commits into
Open
feat(terraform): support custom domains for Cloudflare workers#604donnfelker wants to merge 6 commits into
donnfelker wants to merge 6 commits into
Conversation
…trol plane Adds two optional input variables — web_app_domain and control_plane_domain — that override the default workers.dev URLs in cross-service configuration (NEXTAUTH_URL, NEXT_PUBLIC_WS_URL, CONTROL_PLANE_URL, Modal allowed hosts) and in outputs. Both default to empty so existing deployments produce no diff. The variables describe the URL/identity layer only. Binding the hostname to the Cloudflare worker (cloudflare_workers_custom_domain or dashboard) is out of scope and left to the operator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds Step 9b to GETTING_STARTED.md walking through binding a hostname to the Cloudflare worker, setting web_app_domain / control_plane_domain in terraform.tfvars, updating the GitHub App callback, and smoke-testing. Includes a troubleshooting table for the common cutover failure modes (NEXTAUTH_URL mismatch, GitHub callback typo, WSS failure, 522/525 on the new domain). Updates the example tfvars block in the same step to include the two new variables. Adds a brief pointer in SETUP_GUIDE.md's Path C section so contributors deploying their own stack are aware of the option without duplicating the full procedure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds tests/custom_domains.tftest.hcl with five plan-mode runs that exercise the override and fallback logic in locals.tf: - defaults_use_workers_dev: empty vars resolve to workers.dev fallbacks - web_app_domain_overrides_only_web: web override leaves control plane alone - control_plane_domain_overrides_only_control_plane: inverse case, also asserts control_plane_url and ws_url propagation - both_domains_set: both overrides applied - vercel_platform_ignores_web_app_domain: web_app_domain has no effect when web_platform = vercel Tests use mock_provider blocks for cloudflare and vercel, so they require no real credentials and create no resources. They run in seconds. Wires `terraform test -no-color` into the existing validate job in .github/workflows/terraform.yml and surfaces the result in the PR comment status table. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n is set Adds a validation block to web_app_domain that fails at plan time if web_platform is anything other than "cloudflare". Vercel deployments use Vercel-managed domains and ignored web_app_domain silently — that silent no-op was the kind of thing that bites you on a redeploy. Replaces the existing "vercel_platform_ignores_web_app_domain" tftest run with two cases: - vercel_platform_rejects_web_app_domain — uses expect_failures to assert the validation fires when both are set - vercel_platform_with_empty_web_app_domain_is_allowed — empty value still works on Vercel, so the validation isn't overzealous Cross-variable validation is supported on Terraform >= 1.9; the project already requires >= 1.14.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updates the variables.tf comment block, variable descriptions, terraform.tfvars.example, GETTING_STARTED.md Step 9b, and SETUP_GUIDE.md Path C to be explicit about three things: 1. The feature is Cloudflare-only. web_app_domain only takes effect when web_platform = "cloudflare" (now rejected at plan time when set with web_platform = "vercel"). control_plane_domain always applies because the control plane is a Cloudflare Worker regardless of web platform. 2. Modal vs Daytona behavior. When sandbox_provider = "modal", control_plane_domain flows into Modal's ALLOWED_CONTROL_PLANE_HOSTS automatically. Daytona has no analogue and needs no wiring — the control plane calls Daytona's REST API, not the reverse, so there's no host allowlist to maintain. 3. Variable descriptions tightened to encode the platform/sandbox constraints inline so they show up in `terraform console` and IDE hover tooltips, not just in the comment block. 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds optional Cloudflare custom-domain variables, resolves derived URLs/hosts with workers.dev fallbacks, updates outputs and verification commands, adds Terraform tests and CI reporting for tests, and documents setup and troubleshooting. ChangesCloudflare Custom Domain Support
Sequence Diagram(s)sequenceDiagram
participant Operator
participant GH as "GitHub Actions"
participant TF as "Terraform (validate job)"
participant Cloudflare
Operator->>GH: push PR / set tfvars
GH->>TF: run format, init, validate, test
TF->>Cloudflare: resolve/derive host URLs (using locals/vars)
TF->>GH: report outcomes (format, init, validate, test)
GH->>Operator: post PR comment with outcomes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🧹 Nitpick comments (1)
terraform/environments/production/variables.tf (1)
449-464: ⚡ Quick winAdd format guard to reject scheme-prefixed domain inputs.
Neither
web_app_domainnorcontrol_plane_domainvalidates that the value is a bare hostname. A user who supplies"https://app.example.com"instead of"app.example.com"will silently get broken URLs (https://https://app.example.com) baked into the worker bundles with no plan-time feedback.🛡️ Proposed validation additions
variable "web_app_domain" { description = "Custom hostname for the Cloudflare web Worker. Drives NEXTAUTH_URL. Has no effect when web_platform = \"vercel\" (rejected at plan time when both are set)." type = string default = "" validation { condition = var.web_app_domain == "" || var.web_platform == "cloudflare" error_message = "web_app_domain only takes effect when web_platform = \"cloudflare\". Vercel deployments use Vercel-managed domains and ignore this variable. Either remove web_app_domain or switch web_platform to \"cloudflare\"." } + + validation { + condition = var.web_app_domain == "" || !startswith(var.web_app_domain, "http") + error_message = "web_app_domain must be a bare hostname (e.g. \"app.example.com\"), not a URL. Remove the scheme prefix." + } } variable "control_plane_domain" { description = "Custom hostname for the Cloudflare control plane Worker. Drives CONTROL_PLANE_URL, NEXT_PUBLIC_WS_URL, and (when sandbox_provider = \"modal\") Modal's ALLOWED_CONTROL_PLANE_HOSTS. No-op for Daytona's REST-pull model." type = string default = "" + + validation { + condition = var.control_plane_domain == "" || !startswith(var.control_plane_domain, "http") + error_message = "control_plane_domain must be a bare hostname (e.g. \"api.example.com\"), not a URL. Remove the scheme prefix." + } }🤖 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 `@terraform/environments/production/variables.tf` around lines 449 - 464, Add validation to ensure web_app_domain and control_plane_domain are bare hostnames by rejecting values containing a URL scheme; update the existing validation block for variable "web_app_domain" (inside its validation.condition) to also require that var.web_app_domain does not contain "://", and add a new validation block for variable "control_plane_domain" that requires var.control_plane_domain == "" || !contains(var.control_plane_domain, "://") with an error_message telling users to provide a bare hostname (e.g. "app.example.com") rather than a scheme-prefixed value like "https://app.example.com".
🤖 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 `@docs/GETTING_STARTED.md`:
- Around line 710-712: Add a language identifier to the fenced code block that
currently contains the single-line URL
("https://app.example.com/api/auth/callback/github") to satisfy MD040 and enable
proper rendering; update the fenced block around that URL to use a plaintext
language tag (e.g., "text") so the block becomes ```text ... ``` while leaving
the URL content unchanged.
---
Nitpick comments:
In `@terraform/environments/production/variables.tf`:
- Around line 449-464: Add validation to ensure web_app_domain and
control_plane_domain are bare hostnames by rejecting values containing a URL
scheme; update the existing validation block for variable "web_app_domain"
(inside its validation.condition) to also require that var.web_app_domain does
not contain "://", and add a new validation block for variable
"control_plane_domain" that requires var.control_plane_domain == "" ||
!contains(var.control_plane_domain, "://") with an error_message telling users
to provide a bare hostname (e.g. "app.example.com") rather than a
scheme-prefixed value like "https://app.example.com".
🪄 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: 9d81a347-63d6-4548-b7cf-0a8cfbc90968
📒 Files selected for processing (8)
.github/workflows/terraform.ymldocs/GETTING_STARTED.mddocs/SETUP_GUIDE.mdterraform/environments/production/locals.tfterraform/environments/production/outputs.tfterraform/environments/production/terraform.tfvars.exampleterraform/environments/production/tests/custom_domains.tftest.hclterraform/environments/production/variables.tf
Satisfies markdownlint MD040 and ensures consistent rendering. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ColeMurray
requested changes
May 14, 2026
Owner
ColeMurray
left a comment
ColeMurray
left a comment
There was a problem hiding this comment.
[Automated Review] Requesting changes for the custom-domain deployment issues noted inline.
| # | ||
| # Set the hostname only (no scheme): "app.example.com", not "https://...". | ||
|
|
||
| variable "web_app_domain" { |
Owner
There was a problem hiding this comment.
[Automated Review] For deployments that rely on the provided GitHub Actions Terraform plan/apply path, terraform.tfvars is ignored and variables are supplied as TF_VAR_* secrets in the workflow. These new variables default to empty, but the workflow never exports TF_VAR_web_app_domain or TF_VAR_control_plane_domain, so CI-driven applies keep publishing the workers.dev URLs even after operators configure custom-domain secrets.
Concretely, please add the two exports to both workflow env blocks:
TF_VAR_web_app_domain: ${{ secrets.WEB_APP_DOMAIN }}
TF_VAR_control_plane_domain: ${{ secrets.CONTROL_PLANE_DOMAIN }}Places to update:
- Terraform Plan env block in
.github/workflows/terraform.ymlaround lines 188-230:background-agents/.github/workflows/terraform.yml
Lines 188 to 230 in 452f090
- Terraform Apply env block in
.github/workflows/terraform.ymlaround lines 319-361:background-agents/.github/workflows/terraform.yml
Lines 319 to 361 in 452f090
- CI/CD secrets table in
docs/GETTING_STARTED.mdaround lines 760-800, documentingWEB_APP_DOMAINandCONTROL_PLANE_DOMAIN:background-agents/docs/GETTING_STARTED.md
Lines 760 to 800 in 452f090
Comment on lines
+12
to
+18
| control_plane_host = var.control_plane_domain != "" ? var.control_plane_domain : local.default_control_plane_host | ||
| control_plane_url = "https://${local.control_plane_host}" | ||
| ws_url = "wss://${local.control_plane_host}" | ||
|
|
||
| # Web app URL depends on deployment platform | ||
| # Web app URL: custom domain on Cloudflare path, or workers.dev / Vercel default | ||
| web_app_url = var.web_platform == "cloudflare" ? ( | ||
| "https://open-inspect-web-${local.name_suffix}.${var.cloudflare_worker_subdomain}.workers.dev" | ||
| var.web_app_domain != "" ? "https://${var.web_app_domain}" : "https://${local.default_web_app_host}" |
Owner
There was a problem hiding this comment.
[Automated Review] When an operator provides a common URL-form value such as https://app.example.com for either custom domain, these locals prefix another scheme and bake values like https://https://... or wss://https://... into NextAuth, WebSocket config, and Modal allowlists. Since the variables require bare hostnames, please add plan-time validation rejecting scheme/path input so the deployment fails clearly instead of generating broken URLs.
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
2 participants
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.
Summary
web_app_domainandcontrol_plane_domain— that override the default*.workers.devURLs in cross-service config (NEXTAUTH_URL,NEXT_PUBLIC_WS_URL,CONTROL_PLANE_URL, Modal allowed hosts) and outputs.web_app_domaintakes effect only whenweb_platform = "cloudflare"— Vercel deployments use Vercel-managed domains and ignore the variable.control_plane_domainalways applies since the control plane is on Cloudflare regardless of web platform.cloudflare_workers_custom_domain) is out of scope and left to the operator.docs/GETTING_STARTED.mdStep 9b (binding, GitHub App callback update, apply, smoke-test, troubleshooting table) plus a pointer indocs/SETUP_GUIDE.md. Both are flagged "Cloudflare only".terraform testcoverage with five plan-mode runs (includingvercel_platform_ignores_web_app_domainto lock the no-op behavior on the Vercel path) and wiresterraform testinto the existingvalidateCI job.Summary by CodeRabbit
New Features
Tests
Documentation