Repository navigation
feat(api): read OPENSTATUS_API_URL to target self-hosted instances - #38
Conversation
|
@claude review this |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Package initialization occurs before dotenv loading, causing .env API URLs to be ignored.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds self-hosted OpenStatus API support through OPENSTATUS_API_URL.
Changes:
- Derives REST and RPC endpoints from a configurable API origin.
- Configures generated Terraform providers for self-hosted instances.
- Updates documentation, errors, and tests.
| File | Description |
|---|---|
README.md |
Documents self-hosted configuration. |
internal/api/client.go |
Resolves configurable API endpoints. |
internal/api/client_test.go |
Tests endpoint normalization. |
internal/cli/errors.go |
Generalizes connection errors. |
internal/terraform/generate.go |
Passes the configured origin to generation. |
internal/terraform/generate_test.go |
Tests self-hosted provider output. |
internal/terraform/hcl.go |
Emits Terraform base_url. |
internal/terraform/smoke_test.go |
Updates provider generation usage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const ConnectBaseURL = "https://api.openstatus.dev/rpc" | ||
| // BaseURL is the API origin. Set OPENSTATUS_API_URL to target a self-hosted | ||
| // instance. | ||
| var BaseURL = resolveBaseURL(os.Getenv("OPENSTATUS_API_URL")) |
There was a problem hiding this comment.
Good catch. Fixed in 1af50f5: the endpoints now default to Cloud and api.LoadBaseURL() applies OPENSTATUS_API_URL in main right after godotenv.Load(). TestLoadBaseURL_ReadsEnvAtCallTime covers it, and a built binary picks the value up from .env.
| // resolveBaseURL also accepts a trailing /rpc, the form the Node SDK | ||
| // documents for the same variable. | ||
| func resolveBaseURL(v string) string { | ||
| v = strings.TrimSuffix(strings.TrimRight(v, "/"), "/rpc") |
There was a problem hiding this comment.
[low] resolveBaseURL does not validate or normalize the value beyond trailing / and /rpc. Two easy misconfigurations get through:
- No scheme:
OPENSTATUS_API_URL=openstatus.example.comgivesAPIBaseURL = "openstatus.example.com/v1". Every command then fails with Go's rawunsupported protocol scheme ""error.FormatErrordoes not map that error to the new OPENSTATUS_API_URL hint.terraform generatealso writes the same scheme-lessbase_urlintoprovider.tf. - Surrounding whitespace or a newline, for example from
export OPENSTATUS_API_URL=$(cat url.txt)or a.envfile with CRLF line endings: the value is used verbatim and requests fail withinvalid control character in URL.
Suggested fix: strings.TrimSpace the value, parse it with url.Parse, and require an http or https scheme and a non-empty host. If that check fails, return a clear error that names OPENSTATUS_API_URL.
There was a problem hiding this comment.
Done in 1af50f5, as suggested: TrimSpace, then url.Parse, requiring an http/https scheme and a host. A bad value now stops the CLI at startup with OPENSTATUS_API_URL must be an http(s) URL such as https://api.openstatus.example.com, got "…", so the scheme-less value never reaches provider.tf. Tests cover whitespace, CRLF, no scheme, a non-http scheme and an empty host.
thibaultleouay
left a comment
thibaultleouay
left a comment
There was a problem hiding this comment.
Review: 2 findings (1 medium, 1 low), inline below.
Checked and fine: every use of APIBaseURL/ConnectBaseURL is read at command run time, and the Terraform base_url written by the generator (/rpc form) matches what the provider expects.
|
|
||
| func main() { | ||
| _ = godotenv.Load() | ||
| if err := api.LoadBaseURL(); err != nil { |
There was a problem hiding this comment.
Medium — a .env in the working directory can redirect the saved token to another server.
godotenv.Load() reads .env from whatever directory the CLI runs in, and LoadBaseURL() then honours OPENSTATUS_API_URL from it. The token still falls back to ~/.config/openstatus/token when the .env only sets the URL.
Scenario: clone a repo whose .env contains OPENSTATUS_API_URL=https://evil.example, run openstatus whoami or openstatus monitors list there → the real token is sent to evil.example in the auth header. Before this PR, a .env could only change which token was used, not where it was sent.
Possible fixes: read the URL from the real process environment before godotenv.Load(), or warn when the URL came from .env and differs from the default.
| // LoadBaseURL applies OPENSTATUS_API_URL. Call it after the .env file is | ||
| // loaded so a value set there is honoured. | ||
| func LoadBaseURL() error { | ||
| base, err := resolveBaseURL(os.Getenv("OPENSTATUS_API_URL")) |
There was a problem hiding this comment.
Low — an invalid OPENSTATUS_API_URL stops every command.
LoadBaseURL runs in main before the app is built and its error goes to log.Fatal. With OPENSTATUS_API_URL=api.example.com (no scheme), --help, --version, login and check all exit with the URL error — including check, which the README says always uses the public speed checker.
Consider validating only when a command actually calls the API.
The CLI hard-codes
https://api.openstatus.dev, so it cannot talk to a self-hosted server. The SDKs already readOPENSTATUS_API_URL; this makes the CLI do the same.OPENSTATUS_API_URLsets the API origin;/v1and/rpchang off it. Unset = unchanged behaviour./rpc(the Node SDK's form of the same variable) is accepted.terraform generatewritesbase_urlintoprovider.tfwhen the origin isn't Cloud, since the provider has no env var for it.api.openstatus.dev.checkstill uses the public speed checker.Tested:
go test -race ./...,golangci-lint run, andwhoami/monitors listagainst a self-hosted server.