feat: Smart online installer — auto-detect RVSconfig.yml, URL/paste/wizard#8
feat: Smart online installer — auto-detect RVSconfig.yml, URL/paste/wizard#8PhilippWu wants to merge 1 commit into
Conversation
… embedded wizard Closes #4 install.sh now resolves RVSconfig.yml through a four-path priority chain instead of always requiring a manual YAML paste: Path 1 — existing file on disk (/etc/dev-server/RVSconfig.yml or ./RVSconfig.yml) → used directly, no prompts shown (enables idempotent re-runs) Path 2 — URL entered at the prompt (http/https prefix) → downloaded with curl, validated, written to disk Path 3 — YAML content pasted at the prompt, Ctrl-D to end → preserved from previous behaviour Path 4 — empty Enter at the prompt → launches an embedded Bash wizard (Step 1-4: network/domain, admin password, AI agents, API keys); writes RVSconfig.yml to disk before proceeding so re-runs use Path 1 All paths persist the resolved config to /etc/dev-server/RVSconfig.yml (mode 0600) before parsing, making every subsequent run fully automatic. Also fixes inherited bugs from main: - validators.py: restore missing 'def validate_api_key_optional' declaration - test_validators.py: add coder_admin_password to _full_config() base fixture
![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: c2dbfcb2b6
ℹ️ 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".
| # We read interactive input from /dev/tty so the user can still paste config. | ||
| echo -e "${BOLD}Paste the contents of your RVSconfig.yml below.${RESET}" | ||
| echo -e "When done, press ${BOLD}Ctrl-D${RESET} (EOF) on an empty line.\n" | ||
| exec 3</dev/tty |
There was a problem hiding this comment.
Opening fd 3 with exec 3</dev/tty at startup makes the script fail immediately in non-interactive contexts (/dev/tty: No such device or address), even when RVSconfig.yml already exists and Path 1 should run with zero prompts. This breaks the new automated re-run/pre-provision flow for cloud-init, CI, or cron executions that lack a controlling TTY.
Useful? React with 👍 / 👎.
| use_cloudflare="false" | ||
| warn "Manual DNS required: after boot, point ${subdomain:-<subdomain>}.${domain:-<domain>} → this server's public IP." | ||
| fi | ||
| else | ||
| ip_only="true" |
There was a problem hiding this comment.
The wizard now allows use_cloudflare=false and ip_only=true, but the provisioning pipeline still hard-requires DOMAIN, SUBDOMAIN, EMAIL, CLOUDFLARE_API_TOKEN, and CLOUDFLARE_ZONE_ID and always runs DNS setup (dev-server-provision/setup.sh required-vars block and unconditional dns.sh call). Choosing either branch therefore generates a config that predictably aborts later in setup, so these options are currently non-functional.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Implements a smarter install.sh flow that auto-resolves RVSconfig.yml via on-disk detection, URL download, YAML paste, or an interactive wizard—reducing friction for bare-server installs and enabling more automated re-runs.
Changes:
- Updated
install.shto resolve config via a 4-path priority chain and persist the resolved config to/etc/dev-server/RVSconfig.yml. - Expanded bare-server installation documentation to describe all config resolution paths with examples.
- Fixed/adjusted configurator validation plumbing and tests (restored missing validator symbol; updated preflight fixture).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| install.sh | Adds multi-path config resolution, TTY read helpers, YAML validation, and wizard-based config generation. |
| dev-server-provision/docs/deployment.md | Documents the new installer config resolution paths and usage examples. |
| dev-server-provision/configurator/validators.py | Restores validate_api_key_optional() symbol. |
| dev-server-provision/configurator/tests/test_validators.py | Updates the full-config fixture to include coder_admin_password. |
| # TTY helpers | ||
| # All interactive I/O goes through /dev/tty (fd 3) so the script works | ||
| # correctly even when stdin is a pipe (curl ... | bash). | ||
| # --------------------------------------------------------------------------- | ||
| # When the script is piped (curl ... | sudo bash), stdin is the curl stream. | ||
| # We read interactive input from /dev/tty so the user can still paste config. | ||
| echo -e "${BOLD}Paste the contents of your RVSconfig.yml below.${RESET}" | ||
| echo -e "When done, press ${BOLD}Ctrl-D${RESET} (EOF) on an empty line.\n" | ||
| exec 3</dev/tty | ||
|
|
There was a problem hiding this comment.
exec 3</dev/tty is unguarded under set -euo pipefail, so the script will exit immediately when there is no controlling TTY (e.g., cloud-init, CI, or any non-interactive re-run that relies on Path 1 with an on-disk config). Please make opening /dev/tty conditional (only when interactive input is needed) and/or provide a fallback to read from stdin when /dev/tty is unavailable.
| if _read_confirm " Do you have a domain name for this server? [y/N]: " "n"; then | ||
| _read_line " Domain (e.g. example.com): " | ||
| domain="$_TTY_LINE" | ||
|
|
||
| _read_line " Subdomain (e.g. dev): " | ||
| subdomain="$_TTY_LINE" | ||
|
|
||
| _read_line " Email (for TLS certificates & alerts): " | ||
| email="$_TTY_LINE" | ||
|
|
||
| ip_only="false" | ||
|
|
||
| if _read_confirm " Use Cloudflare for automatic DNS? [Y/n]: " "y"; then | ||
| use_cloudflare="true" | ||
| echo -e " ${CYAN}Tip: create a scoped token at https://dash.cloudflare.com/profile/api-tokens${RESET}" >/dev/tty | ||
| _read_secret " Cloudflare API Token: " | ||
| cloudflare_api_token="$_TTY_SECRET" | ||
| _read_line " Cloudflare Zone ID: " | ||
| cloudflare_zone_id="$_TTY_LINE" | ||
| else | ||
| use_cloudflare="false" | ||
| warn "Manual DNS required: after boot, point ${subdomain:-<subdomain>}.${domain:-<domain>} → this server's public IP." | ||
| fi | ||
| else | ||
| ip_only="true" | ||
| use_cloudflare="false" | ||
| echo -e " ${CYAN}IP-only mode — Coder will be served over plain HTTP on port 80.${RESET}" >/dev/tty | ||
| fi | ||
|
|
There was a problem hiding this comment.
The wizard’s IP-only branch generates ip_only: true and leaves domain/subdomain/email empty, but the provisioning pipeline still executes dev-server-provision/setup.sh, which currently hard-requires DOMAIN, SUBDOMAIN, EMAIL, and Cloudflare vars and will abort if they’re missing. Either remove/disable IP-only mode in the installer until the backend supports it, or update the provisioning scripts to handle IP-only mode end-to-end (proxy, DNS, required var validation, and CODER_ACCESS_URL).
| if _read_confirm " Do you have a domain name for this server? [y/N]: " "n"; then | |
| _read_line " Domain (e.g. example.com): " | |
| domain="$_TTY_LINE" | |
| _read_line " Subdomain (e.g. dev): " | |
| subdomain="$_TTY_LINE" | |
| _read_line " Email (for TLS certificates & alerts): " | |
| email="$_TTY_LINE" | |
| ip_only="false" | |
| if _read_confirm " Use Cloudflare for automatic DNS? [Y/n]: " "y"; then | |
| use_cloudflare="true" | |
| echo -e " ${CYAN}Tip: create a scoped token at https://dash.cloudflare.com/profile/api-tokens${RESET}" >/dev/tty | |
| _read_secret " Cloudflare API Token: " | |
| cloudflare_api_token="$_TTY_SECRET" | |
| _read_line " Cloudflare Zone ID: " | |
| cloudflare_zone_id="$_TTY_LINE" | |
| else | |
| use_cloudflare="false" | |
| warn "Manual DNS required: after boot, point ${subdomain:-<subdomain>}.${domain:-<domain>} → this server's public IP." | |
| fi | |
| else | |
| ip_only="true" | |
| use_cloudflare="false" | |
| echo -e " ${CYAN}IP-only mode — Coder will be served over plain HTTP on port 80.${RESET}" >/dev/tty | |
| fi | |
| while true; do | |
| if _read_confirm " Do you have a domain name for this server? [y/N]: " "n"; then | |
| _read_line " Domain (e.g. example.com): " | |
| domain="$_TTY_LINE" | |
| _read_line " Subdomain (e.g. dev): " | |
| subdomain="$_TTY_LINE" | |
| _read_line " Email (for TLS certificates & alerts): " | |
| email="$_TTY_LINE" | |
| ip_only="false" | |
| if _read_confirm " Use Cloudflare for automatic DNS? [Y/n]: " "y"; then | |
| use_cloudflare="true" | |
| echo -e " ${CYAN}Tip: create a scoped token at https://dash.cloudflare.com/profile/api-tokens${RESET}" >/dev/tty | |
| _read_secret " Cloudflare API Token: " | |
| cloudflare_api_token="$_TTY_SECRET" | |
| _read_line " Cloudflare Zone ID: " | |
| cloudflare_zone_id="$_TTY_LINE" | |
| else | |
| use_cloudflare="false" | |
| warn "Manual DNS required: after boot, point ${subdomain:-<subdomain>}.${domain:-<domain>} → this server's public IP." | |
| fi | |
| break | |
| fi | |
| warn "IP-only mode is not supported by the current provisioning pipeline. Please provide a domain name and try again." | |
| done |
| # Add derived CODER variables | ||
| DOMAIN="$(grep '^DOMAIN=' "$ENV_FILE" | cut -d= -f2-)" | ||
| SUBDOMAIN="$(grep '^SUBDOMAIN=' "$ENV_FILE" | cut -d= -f2-)" | ||
| IP_ONLY_VAL="$(grep '^IP_ONLY=' "$ENV_FILE" | cut -d= -f2- || true)" | ||
| if [[ -n "$DOMAIN" && -n "$SUBDOMAIN" ]]; then | ||
| CODER_FQDN="https://${SUBDOMAIN}.${DOMAIN}" | ||
| grep -q '^CODER_URL=' "$ENV_FILE" || echo "CODER_URL=${CODER_FQDN}" >> "$ENV_FILE" | ||
| grep -q '^CODER_URL=' "$ENV_FILE" || echo "CODER_URL=${CODER_FQDN}" >> "$ENV_FILE" | ||
| grep -q '^CODER_ACCESS_URL=' "$ENV_FILE" || echo "CODER_ACCESS_URL=${CODER_FQDN}" >> "$ENV_FILE" | ||
| fi |
There was a problem hiding this comment.
The derived CODER_FQDN logic reads DOMAIN/SUBDOMAIN by grepping the env file, but values are written with %q. Empty values become '', which is non-empty to [[ -n ... ]], so the script can incorrectly generate CODER_URL=https://''.'' and also miss “missing required values” warnings. Consider sourcing the env file (or normalizing '' back to empty) before checking emptiness / building URLs.
| elif [[ "$first_line" =~ ^https?:// ]]; then | ||
| # ── Path 2: URL download ──────────────────────────────────────────────── | ||
| _download_config "$first_line" | ||
| RVS_SOURCE="url:${first_line}" |
There was a problem hiding this comment.
URL config downloads currently accept http:// as well as https:// (^https?://). Because the URL may point to a file containing secrets, allowing plaintext HTTP creates a credential exposure risk. Restrict this path to https:// (and optionally reject URLs with embedded credentials / obvious token query params).
| local url="$1" | ||
| log "Downloading config from: $url" | ||
| local tmp curl_err | ||
| tmp="$(mktemp)" | ||
| curl_err="$(mktemp)" | ||
| if ! curl -fsSL --max-time 60 "$url" -o "$tmp" 2>"$curl_err"; then | ||
| local msg | ||
| msg="$(cat "$curl_err" 2>/dev/null || true)" | ||
| rm -f "$tmp" "$curl_err" | ||
| die "Failed to download config from ${url}${msg:+ — $msg}" |
There was a problem hiding this comment.
The installer logs the full config download URL (log "Downloading config from: $url"). If users provide signed URLs or URLs containing tokens/query secrets, those credentials will be written to /var/log/dev-server-provision.log. Consider redacting the query string (or logging only the host/path) to avoid leaking secrets into logs.
| RVS_CONTENT="# RVSconfig.yml — generated by install.sh wizard | ||
| domain: \"${domain}\" | ||
| subdomain: \"${subdomain}\" | ||
| email: \"${email}\" | ||
| cloudflare_api_token: \"${cloudflare_api_token}\" | ||
| cloudflare_zone_id: \"${cloudflare_zone_id}\" | ||
| coder_admin_password: \"${coder_admin_password}\" | ||
| ip_only: ${ip_only} | ||
| use_cloudflare: ${use_cloudflare} |
There was a problem hiding this comment.
Wizard-generated YAML interpolates user input directly into double-quoted scalars without escaping. Inputs containing ", backslashes, or newlines will produce invalid YAML (and could also change parsing semantics). Reuse the configurator’s YAML escaping approach (escape backslashes/quotes/newlines) or avoid quoting by emitting YAML-safe scalars.
|
|
||
|
|
||
| def validate_api_key_optional(value: str) -> str | bool: | ||
| """Accept empty or any non-whitespace string.""" |
There was a problem hiding this comment.
validate_api_key_optional() claims to accept “empty or any non-whitespace string”, but it currently returns True for whitespace-only inputs as well. Either update the docstring to match behavior or implement the intended value.strip() check so whitespace-only isn’t treated as a valid key.
| """Accept empty or any non-whitespace string.""" | |
| """Accept empty or any non-whitespace string.""" | |
| if value == "": | |
| return True | |
| if not value.strip(): | |
| return "API key cannot be whitespace-only." |
Closes #4
Summary
Replaces the single mandatory YAML-paste step in
install.shwith a smart four-path config resolution chain. The installer always picks the fastest available path automatically — no flags or arguments needed.Config resolution priority
Path 1 — file on disk (zero prompts)
If
/etc/dev-server/RVSconfig.ymlor./RVSconfig.ymlexists the installer uses it directly. Enables fully automated re-runs and SCP-based pre-provisioning.Path 2 — URL download
User pastes an
https://URL at the prompt (private Gist, object-storage bucket, etc.). Installer downloads withcurl --max-time 60, validates, proceeds.Path 3 — YAML paste (preserved from before)
User pastes the full
RVSconfig.ymlcontent directly into the terminal and presses Ctrl-D. This is the previous default behavior, now the third option rather than the only one.Path 4 — embedded Bash wizard
Empty Enter at the prompt launches a four-step interactive wizard:
The wizard writes
RVSconfig.ymlto disk before proceeding, so the next run uses Path 1 automatically (idempotent).Other changes
/etc/dev-server/RVSconfig.yml(mode 0600) before YAML→env parsing_validate_yaml()applied consistently across all paths_read_line,_read_secret,_read_confirm) useexec 3</dev/ttyso all interactive I/O works correctly even when stdin is a pipe (curl | bash)_read_secret) save/restore fullsttysettingsBug fixes (inherited from
main)configurator/validators.py: restored missingdef validate_api_key_optional()declaration (orphaned function body)configurator/tests/test_validators.py: addedcoder_admin_passwordto_full_config()base fixtureDocumentation
docs/deployment.md— Bare-Server Install section rewritten to document all four paths with examples for each.Tests
245/245 passed ✅