PCSM-345 Make HTTP server bind host configurable - #68
rasika-chivate wants to merge 16 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The documentation contains a broken API link and inaccurate Kubernetes verification guidance.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Documents configurable HTTP bind hosts for Kubernetes and other deployments.
Changes:
- Adds configuration, validation, verification, and security guidance.
- Documents the new CLI option and environment variable.
File summaries
| File | Description |
|---|---|
docs/install/start-pcsm.md |
Adds HTTP listen-address guidance. |
docs/install/parameters.md |
Lists the new configuration options. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Render
Destroyed
Render
Destroyed
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Render
Destroyed
Render
Destroyed
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Docker and IPv6 scenarios are currently documented with commands that can remain unreachable or fail validation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
docs/install/start-pcsm.md:78
- This check produces an invalid URL when the first pod address is IPv6 (for example,
http://2001:db8::1:2242/status), because IPv6 literals in URLs require brackets. Since the section explicitly supports IPv6, show a family-safe check or separate IPv4 and IPv6 commands so a valid listener is not reported as failing.
$ curl -s "http://$(hostname -i | awk '{print $1}'):2242/status"
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
Co-authored-by: rasika-chivate <95711051+rasika-chivate@users.noreply.github.com>
Render
Destroyed
Render
Destroyed
Co-authored-by: rasika-chivate <95711051+rasika-chivate@users.noreply.github.com>
| For an IPv6 pod, enclose the address in brackets: | ||
|
|
||
| ```{.bash data-prompt="$"} | ||
| $ curl -g -s "http://[$(hostname -i | awk '{for (i=1;i<=NF;i++) if ($i ~ /:/) {print $i; exit}}')]:2242/status" |
There was a problem hiding this comment.
curl -s exits with no output when the connection is refused however the next paragraph tells the reader to look for that diagnostic. We should use the command with -sS flags in both examples so the error messages and status are actually visible.
There was a problem hiding this comment.
Removing the curl examples addresses the original concern, but this paragraph still describes a response and connection refusal. The check now only reads the startup log. Can we remove that leftover paragraph too?
| A response means the bind address took effect. Connection refused means the server is still on loopback, so check that the environment variable or option reached the process. | ||
|
|
||
| !!! warning | ||
| Binding to `0.0.0.0` (IPv4) or `::` (IPv6) exposes the control endpoints `/start`, `/pause`, `/resume`, and `/finalize`, along with the `pprof` profiling endpoints, on every network interface of the host or pod. None of them require authentication, so anything that can route to the pod can start, pause, or finalize replication. |
There was a problem hiding this comment.
/status and /metrics are exposed also however the warning doesn't include those. Could we include all exposed endpoints please?
Co-authored-by: Adnan <chupe@chupe.ba>
Co-authored-by: Adnan <chupe@chupe.ba>
…tps://github.com/percona/pcsm-docs into PCSM-345-Make-HTTP-server-bind-host-configurable
There was a problem hiding this comment.
🟡 Changes recommended
The Docker example exposes an unauthenticated control API on all host interfaces.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…tps://github.com/percona/pcsm-docs into PCSM-345-Make-HTTP-server-bind-host-configurable
| | A DNS name | Resolved when the server binds | A named interface. The name is accepted without being resolved first, so validation doesn't catch an unresolvable name. | | ||
| | Anything containing a port | Rejected at startup | Not supported. Values such as `localhost:2242`, `127.0.0.1:2242`, and `[::1]:2242` fail. Use `--port` to set the port. | | ||
|
|
||
| Changing the bind host doesn't affect the CLI. Subcommands such as `pcsm status` always connect to `localhost`. |
There was a problem hiding this comment.
The sentence is true but stops short of the consequence, and the CLI target is hard-coded.
| Changing the bind host doesn't affect the CLI. Subcommands such as `pcsm status` always connect to `localhost`. | |
| Changing the bind host doesn't affect the CLI. Subcommands such as `pcsm status` always connect to `localhost`. Keep the loopback address reachable or call the HTTP API directly at the address you configured. |
nit: this sentence also appears at line 102, and the new table repeats the DNS and port guidance right below it. Can we consolidate these sections?
Make the HTTP server bind host configurable. For details, see:
https://perconadev.atlassian.net/browse/PCSM-345