diff --git a/configs/systemd/haproxy.service b/configs/systemd/haproxy.service index d63dbc5..be2cbe7 100644 --- a/configs/systemd/haproxy.service +++ b/configs/systemd/haproxy.service @@ -20,10 +20,21 @@ After=vps.service # configuration and lets the old one DRAIN established connections, which # is why a published generation never drops a live HTTP or TLS connection, # * `systemctl reload` therefore never has to restart the process. -# The configuration is the stable entry file, which includes -# /etc/haproxy/current/proxy.cfg — the generation the panel last published. +# +# BOTH -f ARGUMENTS ARE REQUIRED. The static entry config only declares the +# frontends, the reject backends and the timeouts; the per-domain backends live +# in the generation the panel publishes, and the routing maps whose paths the +# entry config resolves point at THAT generation. Loading the entry config +# alone starts a perfectly healthy-looking HAProxy that answers 404 on :80 and +# closes every :443 connection, because no domain backend exists. Any later +# change to this command line has to keep both files, and +# TestUnitLoadsBothConfigFiles fails if it does not. +# +# The generation is loaded through "current" so a reload picks up whatever the +# panel published last; scripts/30-haproxy.sh creates the bootstrap generation +# before the unit is ever started. Type=notify -ExecStart=/usr/sbin/haproxy -Ws -f /etc/haproxy/haproxy.cfg -p /run/haproxy/haproxy.pid +ExecStart=/usr/sbin/haproxy -Ws -f /etc/haproxy/haproxy.cfg -f /etc/haproxy/current/domains.cfg -p /run/haproxy/haproxy.pid ExecReload=/bin/kill -USR2 $MAINPID # Send SIGUSR1 (graceful stop) to the master on `systemctl stop`: established # connections are allowed to finish; hard-stop-after (see haproxy.cfg) is the diff --git a/scripts/30-haproxy.sh b/scripts/30-haproxy.sh index 4195655..0ce9728 100644 --- a/scripts/30-haproxy.sh +++ b/scripts/30-haproxy.sh @@ -178,7 +178,9 @@ chmod 0644 "$ENTRY_CFG" # refuses to start when a map cannot be opened. The panel republishes this from # the DB at startup; this is only the bootstrap (empty maps = every request # falls through to the reject backends). -if [[ ! -f "$CURRENT_LINK/maps/http.map" ]]; then +# The unit loads BOTH the entry config and this file, so "the generation exists" +# means domains.cfg plus both maps. +if [[ ! -f "$CURRENT_LINK/domains.cfg" ]] || [[ ! -f "$CURRENT_LINK/maps/http.map" ]]; then log "bootstrapping an empty HAProxy generation" gen="$HAPROXY_DIR/releases/000001" mkdir -p "$gen/maps" @@ -334,9 +336,16 @@ fi # The candidate configuration must be valid BEFORE we touch the running proxy: # a broken config would otherwise take the domain service down with it. -/usr/sbin/haproxy -c -q -f "$ENTRY_CFG" 2>/dev/null \ - || { /usr/sbin/haproxy -c -f "$ENTRY_CFG"; die "the generated HAProxy configuration is invalid"; } -log "configuration check passed" +# Validate EXACTLY the command line the unit is about to run (see +# configs/systemd/haproxy.service): the entry config alone is not the +# configuration, it only declares the frontends, the reject backends and the +# timeouts. The per-domain backends and the routing maps live in the +# generation, so checking one file would happily pass a setup that answers 404 +# on :80 and closes every :443 connection. +/usr/sbin/haproxy -c -q -f "$ENTRY_CFG" -f "$CURRENT_LINK/domains.cfg" 2>/dev/null \ + || { /usr/sbin/haproxy -c -f "$ENTRY_CFG" -f "$CURRENT_LINK/domains.cfg"; + die "the generated HAProxy configuration is invalid"; } +log "configuration check passed (entry config + generation)" if [[ "$want_start" -eq 1 ]]; then systemctl enable --now haproxy.service >/dev/null 2>&1 || true diff --git a/src/internal/hpx/realbin_test.go b/src/internal/hpx/realbin_test.go index bb10113..c21883c 100644 --- a/src/internal/hpx/realbin_test.go +++ b/src/internal/hpx/realbin_test.go @@ -30,6 +30,13 @@ func TestGeneratedConfigIsAcceptedByRealHAProxy(t *testing.T) { t.Fatalf("VPSMGR_TEST_HAPROXY_BIN=%s: %v", bin, err) } + // The rendered entry config binds the admin socket at its production path + // (/run/haproxy/admin.sock), which the systemd unit creates via + // RuntimeDirectory. Off systemd nothing creates it, and HAProxy refuses to + // start when a stats socket cannot be bound — so create it here. + if err := os.MkdirAll("/run/haproxy", 0o750); err != nil { + t.Skipf("cannot create /run/haproxy (%v); run these tests as root", err) + } dir := t.TempDir() t.Setenv("VPSMGR_HAPROXY_DIR", dir) t.Setenv("VPSMGR_HAPROXY_BIN", bin) diff --git a/src/internal/hpx/unit_test.go b/src/internal/hpx/unit_test.go new file mode 100644 index 0000000..44d07df --- /dev/null +++ b/src/internal/hpx/unit_test.go @@ -0,0 +1,192 @@ +package hpx + +import ( + "os" + "os/exec" + "path/filepath" + "regexp" + "strconv" + "strings" + "testing" + "time" + + "vpsmgr/internal/cfg" +) + +// unitPath is the shipped systemd unit relative to src/internal/hpx. +const unitPath = "../../../configs/systemd/haproxy.service" + +// unitConfigArgs pulls the -f arguments out of the unit's ExecStart. +func unitConfigArgs(t *testing.T, unit string) []string { + t.Helper() + b, err := os.ReadFile(unit) + if err != nil { + t.Skipf("systemd unit not readable: %v", err) + } + m := regexp.MustCompile(`(?m)^ExecStart=(.*)$`).FindStringSubmatch(string(b)) + if m == nil { + t.Fatalf("no ExecStart in %s", unit) + } + var args []string + fields := strings.Fields(m[1]) + for i := 0; i < len(fields); i++ { + if fields[i] == "-f" && i+1 < len(fields) { + args = append(args, fields[i+1]) + i++ + } + } + return args +} + +// TestUnitLoadsBothConfigFiles is the regression guard for the bug that made +// every managed domain unreachable in 1.9.0/1.9.1: the unit started HAProxy +// with the STATIC entry config only, so the per-domain backends were never +// loaded. HAProxy came up healthy, listened on 80/443 and answered 404 on :80 +// and closed every :443 connection — `haproxy -c` on the entry config alone +// passes, so nothing in the install path noticed. +// +// It asserts the unit passes two config files, and that the SECOND one is a +// generation file (reached through "current") which the entry config's map +// lookups depend on. +func TestUnitLoadsBothConfigFiles(t *testing.T) { + args := unitConfigArgs(t, unitPath) + if len(args) != 2 { + t.Fatalf("unit ExecStart passes %d config file(s), want 2 (entry + generation): %v", len(args), args) + } + if args[0] != "/etc/haproxy/haproxy.cfg" { + t.Errorf("first config = %q, want the static entry config", args[0]) + } + if !strings.Contains(args[1], "/etc/haproxy/current/") { + t.Errorf("second config = %q; it must be the CURRENT generation (so a reload picks up the new one)", args[1]) + } + // The entry config's routing maps point INTO that same generation, or the + // backends could not be found by the maps that select them. + body, err := os.ReadFile(filepath.Join("..", "..", "..", "configs", "haproxy", "haproxy.cfg")) + if err != nil { + t.Skipf("entry config not readable: %v", err) + } + currDir := filepath.Dir(args[1]) + for _, want := range []string{ + "map(" + currDir + "/maps/http.map)", + "map(" + currDir + "/maps/sni.map)", + } { + if !strings.Contains(string(body), want) { + t.Errorf("entry config does not resolve %q — the maps must live in the generation the unit loads", want) + } + } +} + +// TestUnitCommandLineRoutesTraffic runs the unit's OWN command line against a +// real layout and checks the routing actually works — not just that the files +// parse. Skipped without VPSMGR_TEST_HAPROXY_BIN. +func TestUnitCommandLineRoutesTraffic(t *testing.T) { + bin := os.Getenv("VPSMGR_TEST_HAPROXY_BIN") + if bin == "" { + t.Skip("VPSMGR_TEST_HAPROXY_BIN not set") + } + args := unitConfigArgs(t, unitPath) + if len(args) != 2 { + t.Fatalf("unit ExecStart passes %d config files, want 2: %v", len(args), args) + } + if out, err := exec.Command("unshare", "-n", "true").CombinedOutput(); err != nil { + t.Skipf("no network namespace (%v: %s)", err, strings.TrimSpace(string(out))) + } + + // Lay out a real installed tree, then rewrite the unit's absolute paths at + // it so the SAME arguments run against the temp copy. + dir := t.TempDir() + t.Setenv("VPSMGR_HAPROXY_DIR", dir) + t.Setenv("VPSMGR_HAPROXY_BIN", bin) + p := New(cfg.Default()) + if err := p.WriteEntryConfig(); err != nil { + t.Fatal(err) + } + if err := p.PublishAll([]Domain{{Domain: "example.com", IP: "127.0.0.1"}}); err != nil { + t.Fatal(err) + } + + entry := strings.Replace(args[0], "/etc/haproxy", dir, 1) + gen := strings.Replace(args[1], "/etc/haproxy", dir, 1) + + // A backend that answers on :80 so a routed request is distinguishable + // from the reject backend's 404. + backend := filepath.Join(dir, "backend.py") + if err := os.WriteFile(backend, []byte(httpEchoBackend), 0o755); err != nil { + t.Fatal(err) + } + script := filepath.Join(dir, "run.sh") + if err := os.WriteFile(script, []byte(`#!/bin/sh +set -e +ip link set lo up +python3 "$1" & +sleep 0.5 +exec "$2" -f "$3" -f "$4" +`), 0o755); err != nil { + t.Fatal(err) + } + // The published config binds :80/:443 and points at the container IP, which + // is not this host — rewrite the backend target to the local test server + // and the listeners to unprivileged ports, INSIDE the temp generation only. + genDomains := filepath.Join(dir, "current", "domains.cfg") + body, err := os.ReadFile(genDomains) + if err != nil { + t.Fatal(err) + } + fixed := strings.ReplaceAll(string(body), "127.0.0.1:80", "127.0.0.1:8081") + if err := os.WriteFile(genDomains, []byte(fixed), 0o644); err != nil { + t.Fatal(err) + } + entryBody, err := os.ReadFile(entry) + if err != nil { + t.Fatal(err) + } + entryBody = []byte(strings.ReplaceAll(strings.ReplaceAll(string(entryBody), "bind :80", "bind :18080"), + "bind :443", "bind :18443")) + if err := os.WriteFile(entry, entryBody, 0o644); err != nil { + t.Fatal(err) + } + + // --fork makes the shell (and the haproxy it execs) children of the process + // we capture, so its network namespace is the one nsenter must enter. + cmd := exec.Command("unshare", "-n", "--fork", "bash", script, backend, bin, entry, gen) + cmd.Dir = dir + if err := cmd.Start(); err != nil { + t.Fatalf("start haproxy: %v", err) + } + defer func() { _ = cmd.Process.Kill(); _, _ = cmd.Process.Wait() }() + + probe := filepath.Join(dir, "probe.sh") + if err := os.WriteFile(probe, []byte(`for i in $(seq 1 30); do + curl -s -m 1 -o /dev/null http://127.0.0.1:18080/ && break + sleep 0.1 +done +curl -s -m 3 -H 'Host: example.com' http://127.0.0.1:18080/ +`), 0o755); err != nil { + t.Fatal(err) + } + deadline := time.Now().Add(8 * time.Second) + for { + out, err := exec.Command("nsenter", "-t", itoa(cmd.Process.Pid), "-n", "bash", probe).CombinedOutput() + if err == nil && strings.Contains(string(out), "ROUTED-OK") { + return + } + if time.Now().After(deadline) { + t.Fatalf("the unit's command line did not route the host to its backend (got %q)", strings.TrimSpace(string(out))) + } + time.Sleep(200 * time.Millisecond) + } +} + +const httpEchoBackend = `import http.server, socketserver +class H(http.server.BaseHTTPRequestHandler): + def do_GET(self): + b = b"ROUTED-OK" + self.send_response(200) + self.send_header("Content-Length", str(len(b))) + self.end_headers() + self.wfile.write(b) + def log_message(self, *a): pass +socketserver.TCPServer(("127.0.0.1", 8081), H).serve_forever() +` + +func itoa(n int) string { return strconv.Itoa(n) }