diff --git a/server/Dockerfile b/server/Dockerfile index f2c0e07..3dc0c86 100644 --- a/server/Dockerfile +++ b/server/Dockerfile @@ -22,4 +22,7 @@ VOLUME ["/state"] # the data plane must see real client source addresses/TTLs, and Docker's # userland NAT would falsify exactly what this server exists to observe. EXPOSE 8441/tcp 8442/udp 8443/tcp +# The verb is explicit here too, so `docker run ` serves and `docker run --help` +# still works by overriding the command. ENTRYPOINT ["/echolot-server"] +CMD ["--serve"] diff --git a/server/cmd/echolot-server/main.go b/server/cmd/echolot-server/main.go index fe11270..8e923c2 100644 --- a/server/cmd/echolot-server/main.go +++ b/server/cmd/echolot-server/main.go @@ -72,6 +72,10 @@ func run() error { control.Version = Version switch { + case actions.Help: + config.Usage(os.Stderr) + os.Exit(2) + return nil case actions.Version: fmt.Println(Version) return nil diff --git a/server/internal/config/config.go b/server/internal/config/config.go index 6779475..48dd18f 100644 --- a/server/internal/config/config.go +++ b/server/internal/config/config.go @@ -10,6 +10,7 @@ package config import ( "flag" "fmt" + "io" "os" "strconv" "strings" @@ -136,6 +137,9 @@ func Load(args []string) (*Config, *Actions, error) { fs.BoolVar(&a.InstallSystemd, "install-systemd", false, "install a systemd unit for this binary and exit") fs.BoolVar(&a.UninstallSystemd, "uninstall-systemd", false, "remove the systemd unit and exit") + var daemon bool + fs.BoolVar(&a.Serve, "serve", false, "run the server (bind listeners and answer requests)") + fs.BoolVar(&daemon, "daemon", false, "alias for --serve") fs.BoolVar(&a.SetAdminPassword, "set-admin-password", false, "set the break-glass admin password (username as --admin-user, password read from stdin) and exit") fs.StringVar(&c.AdminUser, "admin-user", envOr("ADMIN_USER", "admin"), "username for the break-glass admin") @@ -148,12 +152,42 @@ func Load(args []string) (*Config, *Actions, error) { if !c.Docker { c.Docker = inContainer() } + a.Serve = a.Serve || daemon + // No verb at all means the caller has not said what they want. Usage is the answer, and it + // is a usage error rather than success — otherwise a service manager sees a clean exit and + // concludes the server ran and finished. + if !a.Serve && !a.InstallSystemd && !a.UninstallSystemd && !a.SelfUpdate && + !a.SetAdminPassword && !a.Version { + a.Help = true + } if c.Docker && (a.InstallSystemd || a.UninstallSystemd || a.SelfUpdate) { return nil, nil, fmt.Errorf("systemd/self-update actions are native-mode only (container detected; override with ECHOLOT_DOCKER=0 if this is wrong)") } return c, a, nil } +// Usage prints the verbs first and the tuning flags second, because the question someone has +// when they run this by name is "what does it do", not "what can I set". +func Usage(w io.Writer) { + fmt.Fprint(w, `echolot-server — the Echolot probe server + +USAGE + echolot-server --serve run the server + echolot-server --version print the version + echolot-server --install-systemd install and enable a systemd unit + echolot-server --uninstall-systemd remove it + echolot-server --self-update replace this binary with the latest release + echolot-server --set-admin-password set the break-glass admin password (stdin) + echolot-server --help full flag list + +Every flag can also be set as an environment variable: --control-listen becomes +ECHOLOT_CONTROL_LISTEN. In a container, configuration comes from the environment. + +Running with no verb prints this and exits non-zero: starting to serve the internet +should be something you asked for. +`) +} + // Addrs splits a comma-separated listen spec into individual addresses. // Explicit per-address binds matter on multi-IP hosts: a wildcard bind // (":8443") would also claim addresses reserved for other purposes (e.g. an @@ -168,8 +202,15 @@ func Addrs(spec string) []string { return out } -// Actions are one-shot verbs that exit instead of serving. +// Actions are the verbs. Serving is one of them, and it is explicit: running the binary with no +// arguments prints usage rather than binding a dozen ports and starting to answer the internet. +// Someone typing the name of an unfamiliar program on a terminal should be told what it does, not +// have it start doing it. type Actions struct { + Serve bool + // Help is set when there is nothing to do: no verb was given. + Help bool + InstallSystemd bool UninstallSystemd bool SelfUpdate bool diff --git a/server/internal/selfupdate/selfupdate.go b/server/internal/selfupdate/selfupdate.go index 7f44782..cb42e60 100644 --- a/server/internal/selfupdate/selfupdate.go +++ b/server/internal/selfupdate/selfupdate.go @@ -9,6 +9,7 @@ package selfupdate import ( "crypto/sha256" + "echo-lot.app/server/internal/system" "encoding/hex" "encoding/json" "fmt" @@ -132,6 +133,15 @@ func Run(api, currentVersion string) error { os.Remove(tmp) return fmt.Errorf("atomic replace failed (filesystem boundaries?): %w", err) } + // Serving became an explicit verb, and a unit written before that change starts this binary + // with no arguments - which now prints usage and exits non-zero. The unit is not part of what + // an update replaces, so it is repaired here rather than left to fail at the next restart, + // which might be a reboot months from now. + if repaired, err := system.RepairExecStart(); err != nil { + fmt.Println("WARNING: could not update the systemd unit for --serve:", err) + } else if repaired { + fmt.Println("updated the systemd unit to pass --serve (serving is now an explicit verb)") + } fmt.Printf("updated %s -> %s (%s); restart to run it\n", currentVersion, rel.TagName, self) return nil } diff --git a/server/internal/system/systemd.go b/server/internal/system/systemd.go index 4aa6fa1..1b3b3f3 100644 --- a/server/internal/system/systemd.go +++ b/server/internal/system/systemd.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strings" ) const ( @@ -29,7 +30,7 @@ Wants=network-online.target [Service] Type=simple -ExecStart=%s +ExecStart=%s --serve Restart=on-failure RestartSec=5 StateDirectory=echolot-server @@ -144,3 +145,41 @@ func UninstallSystemd() error { fmt.Println("removed echolot-server units (state dir and env file left in place)") return nil } + +// RepairExecStart brings an already-installed unit up to date with the current invocation. +// +// Serving became an explicit verb (--serve), which means every unit written before that change +// would start the binary with no arguments — and the binary now answers that with usage and a +// non-zero exit. A self-update replaces the binary but never the unit, so without this a routine +// update would leave a service that cannot start, discovered whenever the host next reboots. +// +// Only a unit this program wrote is touched, identified by its description line. Editing an +// operator's hand-written unit would be overreach; leaving ours broken would be negligence. +func RepairExecStart() (repaired bool, err error) { + b, err := os.ReadFile(unitPath) + if err != nil { + return false, nil // no unit installed: nothing to repair, and not an error + } + text := string(b) + if !strings.Contains(text, "Echolot probe server") { + return false, nil // somebody else's unit + } + lines := strings.Split(text, "\n") + changed := false + for i, ln := range lines { + t := strings.TrimSpace(ln) + // Only the serving unit's ExecStart; the timer's own line already carries its verb. + if strings.HasPrefix(t, "ExecStart=") && !strings.Contains(t, "--") { + lines[i] = ln + " --serve" + changed = true + } + } + if !changed { + return false, nil + } + if err := os.WriteFile(unitPath, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + return false, fmt.Errorf("updating %s: %w", unitPath, err) + } + _ = exec.Command("systemctl", "daemon-reload").Run() + return true, nil +}