S01 follow-up: reconcile spec + add server.probe_address for LabKit v2 dual-listener model

📫 Context

The S01 spec (docs/specs/S01-http-server-and-routing.md) was written against early LabKit v2, which served /-/liveness, /-/readiness, and /-/metrics directly on the application listener via Server.ServeHTTP. LabKit v2.6.1 introduced a dedicated probe listener (gitlab-org/labkit!402 (merged)) and the version pinned in go.mod (v2.8.0) inherits that split: the /-/* paths are now served on Config.ProbeAddr (default :9090), not on Config.Addr.

The Step 5 MR (!275 (merged)) ships a deriveProbeAddr helper that mirrors ephemeral ports under test and rejects an application Address whose port collides with LabKit's :9090 default. This is a contained stopgap so S01 tests stay deterministic and operators get a clear startup error instead of a confusing LabKit bind failure — but the spec still describes a single-listener model and operators cannot name the probe port explicitly.

🚒 Scope

Reconcile the S01 spec with LabKit v2.6.1+ and give operators an explicit knob for the probe port.

1. Spec amendment

  • Update docs/specs/S01-http-server-and-routing.md — specifically the LabKit v2 integration section and the acceptance criteria around /-/liveness, /-/readiness, /-/metrics — to describe the two-listener model.
  • Make explicit that /-/* paths land on the probe listener at ProbeAddr, and that the application listener at Addr serves only application traffic.
  • Acceptance criteria currently expecting health endpoints on the application listener (GET :8080/-/liveness) should be rewritten to target the probe listener (GET :9090/-/liveness).

1b. Probe-listener dispatch model — AC-7 and AC-18 drift

The S01 spec (AC-7, AC-18) describes LabKit dispatching health endpoints by "an exact r.Method == GET check in Server.ServeHTTP before any ServeMux matching." That is not how LabKit v2.8.0 actually works. httpserver/server.go:178-184 builds a http.ServeMux on the probe listener and registers the endpoints with Go 1.22's method-prefixed pattern syntax:

probeMux.HandleFunc("GET /-/liveness", s.livenessHandler)
probeMux.HandleFunc("GET /-/readiness", s.readinessHandler)

Concrete consequences observed via TestServer_ProbeListener_NonGETBehavior in !291 (merged):

  • POST /-/liveness on probe listener: Go 1.22 ServeMux returns 405 Method Not Allowed with Allow: GET, HEAD — not the spec's described 404-with-JSON-envelope (which would only apply if requests fell through to the application mux; the probe listener has its own mux, so there is no fall-through).
  • HEAD /-/liveness on probe listener: Go 1.22 ServeMux auto-routes HEAD to GET when only a GET handler is registered, so HEAD returns 200 — not the spec's described "not supported."

The AR-side test pins the current LabKit v2.8.0 behavior so a future LabKit dispatch change is visible. The spec text needs amendment to match either:

  • the model LabKit actually implements (rewrite AC-7 as 405-with-Allow, drop AC-18's "HEAD not supported" claim), or
  • a model that AR enforces above LabKit (e.g., a guard in the composition root that rejects non-GET on health paths before LabKit's ServeMux sees them — heavier and arguably out of scope, since Kubernetes only ever sends GET).

The expected resolution is the first option (align with LabKit). Capture the chosen reading in the AC-7 / AC-18 rewrite when this issue's spec-amendment MR opens.

2. ServerConfig field: probe_address

  • Add a probe_address field to the ServerConfig proto (Step 1's server_config.proto).
  • Wire YAML loading through the existing protovalidate path.
  • Default behavior: when unset, fall through to LabKit's :9090 (preserves today's behavior).
  • Add a protovalidate rule that rejects probe_address == address (port-level collision) at config load time — this replaces the constructor-level ErrProbePortCollision check that server.New carries today as a stopgap.

3. Constructor cleanup (rides for free)

Once probe_address exists in the config surface:

  • server.New reads it from cfg.Server.ProbeAddress and passes it through to httpserver.Config.ProbeAddr directly.
  • deriveProbeAddr shrinks back to a pure helper or is deleted entirely: the ephemeral-mirror branch becomes redundant because tests can set probe_address: "127.0.0.1:0" explicitly.
  • ErrProbePortCollision and the labkitDefaultProbePort constant are removed; collision validation moves into Step 1's protovalidate rules per the AR codebase pattern.

✏️ Notes for the picker

  • Best done as a single MR (spec amendment + proto field + constructor cleanup) since the three pieces are tightly coupled. If split, the spec amendment can land first as a docs-only MR; the proto + constructor changes follow.
  • Step 6's lifecycle integration test (composition root + binary lifecycle) is the natural place to exercise the new two-listener model end-to-end.
  • The AC-7 / AC-18 rewrite (section 1b) is purely docs and can land independently of sections 2 and 3 if convenient; TestServer_ProbeListener_NonGETBehavior is already pinning LabKit's actual behavior, so the spec amendment is the last loose thread.
  • Cross-link with #10 (the existing LabKit v2 follow-up issue) when the MR opens — both issues track residual LabKit-v2-vs-spec gaps but cover different surfaces.

🧭 Related

Edited by Suleimi Ahmed