-
Notifications
You must be signed in to change notification settings - Fork 165
feat: add public mode configuration to TUI installer #7638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| appproxy_itable["url"] = f"https://{public_facing_address}:{advertised_port}" | ||
| else: | ||
| appproxy_itable["url"] = ( | ||
| f"http://{service.appproxy_coordinator_addr.face.host}:{service.appproxy_coordinator_addr.face.port}" |
Check warning
Code scanning / devskim
An HTTP-based URL without TLS was detected. Warning
50bc8e5 to
4c66e36
Compare
9552aac to
4af738e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds public mode configuration support to the TUI installer, enabling Backend.AI to be deployed with TLS and wildcard domain support for public-facing installations. The changes introduce new CLI options and configuration logic to handle both local development and public deployment scenarios.
- Adds four new CLI options:
--public-mode,--fqdn-prefix,--tls-advertised, and--advertised-port - Enhances SetupLog widget to support non-interactive mode with stdout output
- Updates webserver and appproxy configuration generation to support public mode with custom domains
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/install/widgets.py | Adds non-interactive mode support to SetupLog, enabling output to stdout via Rich Console |
| src/ai/backend/install/types.py | Adds public mode fields to CliArgs and InstallVariable, with properties for generating domain names from FQDN prefix |
| src/ai/backend/install/context.py | Updates webserver and appproxy configuration logic to handle public mode, TLS settings, and domain-based routing |
| src/ai/backend/install/cli.py | Defines four new CLI options for controlling public mode configuration |
| src/ai/backend/install/app.py | Passes non_interactive flag to SetupLog instances and initializes InstallVariable with new public mode fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| data["proxy_worker"]["api_advertised_addr"] = { # type: ignore[index] | ||
| "host": public_facing_address, | ||
| "port": service.appproxy_worker_addr.bind.port, | ||
| } | ||
| data["proxy_worker"]["api_bind_addr"] = { # type: ignore[index] | ||
| "host": service.appproxy_worker_addr.bind.host, | ||
| "port": service.appproxy_worker_addr.bind.port, | ||
| } | ||
| data["proxy_worker"]["port_proxy"]["bind_port"] = service.appproxy_worker_addr.bind.port # type: ignore[index] | ||
| data["proxy_worker"]["port_proxy"]["bind_host"] = service.appproxy_worker_addr.bind.host # type: ignore[index] | ||
| data["proxy_worker"]["port_proxy"]["bind_host"] = "0.0.0.0" # type: ignore[index] | ||
| data["proxy_worker"]["port_proxy"]["advertised_host"] = public_facing_address # type: ignore[index] | ||
| data["secrets"]["api_secret"] = service.appproxy_api_secret # type: ignore[index] | ||
| data["secrets"]["jwt_secret"] = service.appproxy_jwt_secret # type: ignore[index] | ||
| data["permit_hash"]["permit_hash_secret"] = service.appproxy_permit_hash_secret # type: ignore[index] |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration set on lines 799-812 is redundant and will be immediately overwritten by the if/else block starting at line 814. In the public_mode branch (lines 814-835), these same fields are reassigned with different values, and in the else branch (lines 836-850), they are set again. This creates unnecessary code duplication and potential confusion. Consider removing lines 799-812 and ensuring all necessary configuration is set within the if/else branches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The settings in the else block appear identical to the defaults above, but they're not redundant. they overwrite the TOML template's placeholder values with runtime variables like public_facing_address and service.appproxy_worker_addr. This is necessary because the template file contains static defaults, not the actual runtime configuration values.
| f"http://{service.appproxy_coordinator_addr.face.host}:{service.appproxy_coordinator_addr.face.port}" | ||
| ) | ||
| if public_mode: | ||
| appproxy_itable["url"] = f"https://{public_facing_address}:{advertised_port}" |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When public_mode is enabled, the webserver configuration uses public_facing_address directly (line 615), but the appproxy configuration uses the apphub_address and app_address properties which incorporate the fqdn_prefix (lines 778, 817, 819). This inconsistency could lead to misconfiguration when fqdn_prefix is provided. Consider using the appropriate property instead of public_facing_address directly to ensure consistent domain usage across all components.
| appproxy_itable["url"] = f"https://{public_facing_address}:{advertised_port}" | |
| appproxy_host = service.apphub_address.face.host | |
| appproxy_itable["url"] = f"https://{appproxy_host}:{advertised_port}" |
| wildcard_table["domain"] = wildcard_domain | ||
| bind_addr_table = tomlkit.inline_table() | ||
| bind_addr_table["host"] = "0.0.0.0" | ||
| bind_addr_table["port"] = 10250 |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bind port for wildcard_domain is hardcoded to 10250. Consider whether this should be configurable or at least documented why this specific port is used. If this port conflicts with other services or needs to be different in certain deployments, it could cause issues.
| bind_addr_table["port"] = 10250 | |
| bind_addr_table["port"] = service.appproxy_worker_addr.bind.port |
src/ai/backend/install/widgets.py
Outdated
| self._stdout_console.print_exception() | ||
| else: | ||
| self._stdout_console.print(content) | ||
| except Exception: |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'except' clause does nothing but pass and there is no explanatory comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stdout output failure is intentionally silenced to prevent it from interrupting the installation process. The primary goal is completing the installation; logging is secondary.
Co-authored-by: Copilot <[email protected]>
resolves #7637 (BA-3596)
Checklist: (if applicable)
ai.backend.testdocsdirectory