Skip to content

fix(modal): Support non-main envs in Modal#629

Open
kadams54 wants to merge 1 commit into
ColeMurray:mainfrom
CompanyCam:kadams54/fix-modal-workspaces
Open

fix(modal): Support non-main envs in Modal#629
kadams54 wants to merge 1 commit into
ColeMurray:mainfrom
CompanyCam:kadams54/fix-modal-workspaces

Conversation

@kadams54
Copy link
Copy Markdown
Contributor

@kadams54 kadams54 commented May 12, 2026

Summary

Modal has a hierarchy of workspaces -> environments. The default environment is main. If you stick with that, the environment doesn't have any impact on Modal's URLs; however, if you use a non-default environment, e.g., prod, that environment name is added into the URL. These changes support generating Modal's URL slug properly to support both main and non-default environments.

Backwards compatibility

  • The new modal_environment Terraform variable defaults to main so projects that don't set it will not be impacted.

Docs updated

  • docs/GETTING_STARTED.md — updated terraform.tfvars block
  • terraform/environments/production/terraform.tfvars.example — updated example to include the new variable

Summary by CodeRabbit

  • Documentation

    • Updated getting started guide with expanded Modal prerequisites, now requiring both Workspace and Environment names with guidance on locating them.
  • New Features

    • Terraform configuration now includes environment variable support for Modal, enabling flexible workspace configuration across different deployment environments.

Review Change Stack

modal_workspace held the full URL slug rather than just the
workspace name. Added modal_environment and a modal_workspace_slug
local so each variable holds its correct semantic value.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

📝 Walkthrough

Walkthrough

This PR extends Modal workspace configuration to support environment-specific naming. A new modal_environment variable is introduced alongside the existing modal_workspace setting, then conditionally merged into a workspace slug that is applied to both the Modal app module and worker control-plane binding.

Changes

Modal Environment Variable Integration

Layer / File(s) Summary
Input variable definition, example configuration, and user documentation
terraform/environments/production/variables.tf, terraform/environments/production/terraform.tfvars.example, docs/GETTING_STARTED.md
The modal_environment input variable defaults to "main" with validation requiring a non-empty value when sandbox provider is modal. The example tfvars file and getting-started guide are updated to document both workspace and environment configuration.
Conditional workspace slug computation
terraform/environments/production/locals.tf
A new modal_workspace_slug local computes the effective workspace name: for the "main" environment, it uses the raw workspace value; for other environments, it appends the environment name as a hyphenated suffix.
Modal app and worker control-plane integration
terraform/environments/production/modal.tf, terraform/environments/production/workers-control-plane.tf
Both the modal app module workspace argument and the control-plane worker MODAL_WORKSPACE binding are updated to use the computed modal_workspace_slug instead of the raw workspace variable.

Sequence Diagram

Not applicable; this PR contains configuration and infrastructure variable updates without multi-component runtime interaction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • ColeMurray/background-agents#529: Related — both PRs modify the same terraform module call in terraform/environments/production/modal.tf but with different purposes.

Poem

🐰 Environments awaken, workspaces align,
Main and satellites dance in a slug design,
Modal now knows where it's meant to be,
Configuration blooms—what unity!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly and clearly summarizes the main change: adding support for non-main Modal environments through a new modal_environment variable and conditional workspace slug formatting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/production/variables.tf`:
- Around line 75-84: Update the validation block for the Terraform variable
"modal_environment" to also reject colon and slash characters: modify the
existing validation condition in the variable "modal_environment" (currently
referencing var.sandbox_provider and length(var.modal_environment)) to
incorporate a regex check that ensures the value does not contain ":" or "/"
(e.g., using can(regex("^[^:/]+$", var.modal_environment)) in the condition when
sandbox_provider == "modal"), and update the error_message to something like
"modal_environment must not contain colons or slashes." so invalid endpoint
slugs are prevented.
🪄 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: 29cf18c9-edff-40e9-938f-637f10338315

📥 Commits

Reviewing files that changed from the base of the PR and between b52ab9a and 45e3a23.

📒 Files selected for processing (6)
  • docs/GETTING_STARTED.md
  • terraform/environments/production/locals.tf
  • terraform/environments/production/modal.tf
  • terraform/environments/production/terraform.tfvars.example
  • terraform/environments/production/variables.tf
  • terraform/environments/production/workers-control-plane.tf

Comment thread terraform/environments/production/variables.tf

app_name = "open-inspect"
workspace = var.modal_workspace
workspace = local.modal_workspace_slug
Copy link
Copy Markdown
Owner

@ColeMurray ColeMurray May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Automated Review]
The slug is plumbed into URL construction here, but modal deploy / modal secret create in terraform/modules/modal-app/scripts/{deploy,create-secrets}.sh are invoked without --env (and no MODAL_ENVIRONMENT is exported), so they deploy to the workspace's default environment regardless of var.modal_environment.

Concretely, with modal_environment = "production":

  • This module's outputs and the worker's MODAL_WORKSPACEmyworkspace-production--…modal.run
  • But modal deploy lands the app at the workspace default (typically main) → myworkspace--…modal.run
  • Result: every endpoint 404s.

Suggested fix:

  1. Add a modal_environment input to the modal-app module.
  2. Pass it as MODAL_ENVIRONMENT in the environment blocks of null_resource.modal_secrets and null_resource.modal_deploy in modules/modal-app/main.tf — Modal CLI reads that var natively.
    (Or alternatively, append --env "${MODAL_ENVIRONMENT}" in both scripts.)

Without this, the feature is half-wired for non-main envs — URLs point one place, deployment lands somewhere else.

Copy link
Copy Markdown
Owner

@ColeMurray ColeMurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment regarding modal script

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants