Serve the owner's own answer on the per-certname routes
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was canceled

Unanimity is the right rule for a fan-out of peers, but the per-certname
routes are not one: a backend that does not hold the certname answers 404
to say so, not to disagree, so requiring it to agree turned the owner's
real 500 into a 502 that described neither backend.

- Add askOrder, which says whether a set of backends was asked as peers
  or owner-first, and resolve each round's replies under its own rule.
- Serve the first backend that answered on owner-routed paths, so an
  unreachable owner still falls back rather than collapsing to 502.
- Keep unanimity for the merged, meta, metrics and pass-through routes.
- Cover the owner routes: owner errors against a non-owner 404, both
  erroring differently, an unreachable owner, and a non-owner error
  behind the owner's 200.
- Record what clientRefusal's 4xx exemption assumes about client certs.
This commit is contained in:
2026-09-13 14:11:44 +10:00
parent 121bfacc2f
commit 72380f27e6
6 changed files with 182 additions and 21 deletions
+9 -1
View File
@@ -34,7 +34,7 @@ not PQL) is forwarded verbatim.
| `GET /pdb/query/v4/event-counts` | Fan out to all and **sum** each subject's counts into one row per subject. | | `GET /pdb/query/v4/event-counts` | Fan out to all and **sum** each subject's counts into one row per subject. |
| `GET /pdb/query/v4/aggregate-event-counts` | Fan out to all and **sum** the summary object's counts. | | `GET /pdb/query/v4/aggregate-event-counts` | Fan out to all and **sum** the summary object's counts. |
| `GET /pdb/query/v4/reports/<hash>/{events,logs,metrics}` | Ask every backend; serve the answer from whichever backend actually holds that report. `404` when none does. | | `GET /pdb/query/v4/reports/<hash>/{events,logs,metrics}` | Ask every backend; serve the answer from whichever backend actually holds that report. `404` when none does. |
| `GET /pdb/query/v4/{nodes,factsets,catalogs}/<certname>[/...]` | Ask the backend that owns that `certname` — the same owner the `/facts` merge attributes records to — and serve its reply verbatim. The remaining backends are tried after it, so a node only one backend holds is still served, and openvoxdb's own `404` body is replayed when none holds it. | | `GET /pdb/query/v4/{nodes,factsets,catalogs}/<certname>[/...]` | Ask the backend that owns that `certname` — the same owner the `/facts` merge attributes records to — and serve its reply verbatim, success or error. The remaining backends are tried after it, so a node only one backend holds is still served, and openvoxdb's own `404` body is replayed when none holds it. |
| `GET /pdb/query/v4/*` (any other) | No merge rule, so backends are tried in configured order and the first success is streamed back verbatim; if all reject it, the first upstream error response is replayed. | | `GET /pdb/query/v4/*` (any other) | No merge rule, so backends are tried in configured order and the first success is streamed back verbatim; if all reject it, the first upstream error response is replayed. |
| `GET /pdb/meta/v1/version` | Fan out to all and report the **lowest** version any backend runs. | | `GET /pdb/meta/v1/version` | Fan out to all and report the **lowest** version any backend runs. |
| `GET /pdb/meta/v1/server-time` | Fan out to all and serve the first reachable backend's clock. | | `GET /pdb/meta/v1/server-time` | Fan out to all and serve the first reachable backend's clock. |
@@ -61,6 +61,14 @@ describes: backends **disagreeing** on the status, or a backend that answered
nothing at all. The rule is the same on `/pdb/query`, `/pdb/meta` and `/metrics`, nothing at all. The rule is the same on `/pdb/query`, `/pdb/meta` and `/metrics`,
so no route answers a failure differently from any other. so no route answers a failure differently from any other.
Unanimity is asked for only where the backends are peers answering the same
question. The per-certname routes are not: one backend **owns** the node and the
others are fallbacks, so the owner's reply is the answer whatever they said — a
backend that does not hold the certname answers `404` to say the node is not its
own, not to disagree about the request. When the owner answers nothing at all the
first fallback that did answer stands in, so an unreachable owner does not take
the node with it, and `502` is left for a request no backend answered.
A unanimous `4xx` blames the request, so it is not counted as a partial round on A unanimous `4xx` blames the request, so it is not counted as a partial round on
`/healthz` and nothing about it is cached. A unanimous `5xx` is the backends `/healthz` and nothing about it is cached. A unanimous `5xx` is the backends
reporting their own fault, so it is replayed just as faithfully but still counts reporting their own fault, so it is replayed just as faithfully but still counts
+5 -1
View File
@@ -50,8 +50,12 @@ func isCertnameRoute(path string) bool { return certnameFor(path) != "" }
// which is the record array the merges are built on. The remaining backends are // which is the record array the merges are built on. The remaining backends are
// tried after it, so a node only one backend holds is still served and a // tried after it, so a node only one backend holds is still served and a
// certname no backend holds still answers with openvoxdb's own 404 body. // certname no backend holds still answers with openvoxdb's own 404 body.
//
// askOwnerFirst is what makes the owner's reply the answer even when it is an
// error. A backend that does not hold the certname 404s to say so, so it has no
// opinion to weigh against the owner's.
func (s *Server) serveByOwner(w http.ResponseWriter, r *http.Request) { func (s *Server) serveByOwner(w http.ResponseWriter, r *http.Request) {
s.proxyOrdered(w, r, s.ownerFirst(r.Context(), certnameFor(r.URL.Path))) s.proxyOrdered(w, r, s.ownerFirst(r.Context(), certnameFor(r.URL.Path)), askOwnerFirst)
} }
// ownerFirst puts the backend holding a certname's newest report ahead of the // ownerFirst puts the backend holding a certname's newest report ahead of the
+91
View File
@@ -55,6 +55,13 @@ type voxBackend struct {
// nodes maps a certname it holds to that node's report_timestamp. // nodes maps a certname it holds to that node's report_timestamp.
nodes map[string]string nodes map[string]string
fail bool fail bool
// perNodeStatus and perNodeBody answer the per-certname routes with a fixed
// reply, leaving /nodes alone so the freshness map still resolves an owner.
perNodeStatus int
perNodeBody string
// perNodeDead aborts the per-certname routes without a status, standing in
// for a backend that is unreachable only for that request.
perNodeDead bool
mu sync.Mutex mu sync.Mutex
gotParams map[string]url.Values gotParams map[string]url.Values
@@ -73,6 +80,17 @@ func newVoxBackend(t *testing.T, name string, nodes map[string]string) *voxBacke
} }
w.Header().Set("Content-Type", "application/json;charset=utf-8") w.Header().Set("Content-Type", "application/json;charset=utf-8")
if isCertnameRoute(r.URL.Path) {
if vb.perNodeDead {
panic(http.ErrAbortHandler)
}
if vb.perNodeStatus != 0 {
w.WriteHeader(vb.perNodeStatus)
_, _ = io.WriteString(w, vb.perNodeBody)
return
}
}
if certname, ok := strings.CutPrefix(r.URL.Path, factsetsPath+"/"); ok { if certname, ok := strings.CutPrefix(r.URL.Path, factsetsPath+"/"); ok {
ts, held := vb.nodes[certname] ts, held := vb.nodes[certname]
if !held { if !held {
@@ -342,3 +360,76 @@ func TestCertnameRoutes_ClaimedByTheRouteTable(t *testing.T) {
} }
} }
} }
// The owner holds the node, so its answer is the answer: a non-owner's 404 only
// says the node is not its own, and demanding it agree turns the owner's real
// error into a 502 that describes neither backend.
func TestCertnameRoutes_OwnerAnswerWins(t *testing.T) {
const certname = "h1.example.net"
for _, tc := range []struct {
name string
// setUp configures the owner (a, which reported later) and the backend
// that does not hold the node (b).
setUp func(a, b *voxBackend)
wantCode int
wantBody string
}{
{
name: "owner errors while the non-owner 404s",
setUp: func(a, b *voxBackend) { a.perNodeStatus, a.perNodeBody = http.StatusInternalServerError, "boom" },
wantCode: http.StatusInternalServerError,
wantBody: "boom",
},
{
name: "owner and non-owner error differently",
setUp: func(a, b *voxBackend) {
a.perNodeStatus, a.perNodeBody = http.StatusInternalServerError, "boom"
b.perNodeStatus, b.perNodeBody = http.StatusServiceUnavailable, "busy"
},
wantCode: http.StatusInternalServerError,
wantBody: "boom",
},
{
// The fallback chain is there so an unreachable owner does not take the
// node with it; a backend that did answer explains more than a 502.
name: "owner unreachable, fallback answers",
setUp: func(a, b *voxBackend) {
a.perNodeDead = true
b.perNodeStatus, b.perNodeBody = http.StatusServiceUnavailable, "busy"
},
wantCode: http.StatusServiceUnavailable,
wantBody: "busy",
},
{
name: "owner succeeds while the non-owner errors",
setUp: func(a, b *voxBackend) {
b.perNodeStatus, b.perNodeBody = http.StatusInternalServerError, "boom"
},
wantCode: http.StatusOK,
wantBody: `"a"`,
},
{
// Nothing answered at all, so there is no upstream reply to replay.
name: "every backend unreachable",
setUp: func(a, b *voxBackend) { a.perNodeDead, b.perNodeDead = true, true },
wantCode: http.StatusBadGateway,
},
} {
t.Run(tc.name, func(t *testing.T) {
a := newVoxBackend(t, "a", map[string]string{certname: "2026-07-20T00:00:00Z"})
b := newVoxBackend(t, "b", map[string]string{})
tc.setUp(a, b)
srv := newTestServer(testConfig(a.srv.URL, b.srv.URL, mergeFreshness))
for _, rt := range perNodePaths {
rec := doGet(t, srv.Handler(), rt.path(certname), "")
if rec.Code != tc.wantCode {
t.Fatalf("%s: status = %d, want %d: %s", rt.name, rec.Code, tc.wantCode, rec.Body.String())
}
if tc.wantBody != "" && !strings.Contains(rec.Body.String(), tc.wantBody) {
t.Errorf("%s: body = %q, want it to contain %q", rt.name, rec.Body.String(), tc.wantBody)
}
}
})
}
}
+1 -1
View File
@@ -86,7 +86,7 @@ func (s *Server) aliveRaw(w http.ResponseWriter, results []rawResult, path strin
alive = append(alive, res) alive = append(alive, res)
} }
if len(alive) == 0 { if len(alive) == 0 {
s.writeUpstreamError(w, upstreamOutcome(rawUpstreamErrors(results))) s.writeUpstreamError(w, peerOutcome(rawUpstreamErrors(results)))
return nil, false return nil, false
} }
return alive, true return alive, true
+11 -9
View File
@@ -535,7 +535,7 @@ func (s *Server) serveFirstHolder(w http.ResponseWriter, r *http.Request) {
if len(alive) == 0 { if len(alive) == 0 {
// Unanimity is the whole answer here too: every backend saying 404 means // Unanimity is the whole answer here too: every backend saying 404 means
// nobody holds the report, while one silent backend leaves that unknown. // nobody holds the report, while one silent backend leaves that unknown.
s.writeUpstreamError(w, upstreamOutcome(backendUpstreamErrors(results))) s.writeUpstreamError(w, peerOutcome(backendUpstreamErrors(results)))
return return
} }
for _, res := range alive { for _, res := range alive {
@@ -922,19 +922,21 @@ func (s *Server) queryBackend(ctx context.Context, b Backend, path string, param
return recs, total, err return recs, total, err
} }
// The record shape is unknown, so a union would be guesswork: the first 2xx wins and the first error response is replayed when none succeeds. // The record shape is unknown, so a union would be guesswork: the first 2xx wins, and every backend was asked the same question, so only a reply they all gave is replayed.
func (s *Server) proxyUnmerged(w http.ResponseWriter, r *http.Request) { func (s *Server) proxyUnmerged(w http.ResponseWriter, r *http.Request) {
s.proxyOrdered(w, r, s.cfg.Backends) s.proxyOrdered(w, r, s.cfg.Backends, askPeers)
} }
// proxyOrdered asks backends in the given order, which is what decides the answer when more than one of them holds the path. // proxyOrdered asks backends in the given order, which is what decides the
func (s *Server) proxyOrdered(w http.ResponseWriter, r *http.Request, backends []Backend) { // answer when more than one of them holds the path. order says why they are in
refusals := make([]*upstreamError, 0, len(backends)) // that order, which is what decides whose reply is served when none answers 2xx.
func (s *Server) proxyOrdered(w http.ResponseWriter, r *http.Request, backends []Backend, order askOrder) {
replies := make([]*upstreamError, 0, len(backends))
for _, b := range backends { for _, b := range backends {
resp, err := s.passThrough(r, b) resp, err := s.passThrough(r, b)
if err != nil { if err != nil {
s.log.Printf("warning: backend %q pass-through failed for %s: %v", b.Name, r.URL.Path, err) s.log.Printf("warning: backend %q pass-through failed for %s: %v", b.Name, r.URL.Path, err)
refusals = append(refusals, nil) replies = append(replies, nil)
continue continue
} }
if resp.StatusCode >= 200 && resp.StatusCode < 300 { if resp.StatusCode >= 200 && resp.StatusCode < 300 {
@@ -946,9 +948,9 @@ func (s *Server) proxyOrdered(w http.ResponseWriter, r *http.Request, backends [
} }
body, _ := io.ReadAll(resp.Body) body, _ := io.ReadAll(resp.Body)
_ = resp.Body.Close() _ = resp.Body.Close()
refusals = append(refusals, newUpstreamError(resp.StatusCode, resp.Header.Get("Content-Type"), body)) replies = append(replies, newUpstreamError(resp.StatusCode, resp.Header.Get("Content-Type"), body))
} }
s.writeUpstreamError(w, upstreamOutcome(refusals)) s.writeUpstreamError(w, order.outcome(replies))
} }
func (s *Server) passThrough(r *http.Request, b Backend) (*http.Response, error) { func (s *Server) passThrough(r *http.Request, b Backend) (*http.Response, error) {
+65 -9
View File
@@ -82,17 +82,66 @@ func unanimousUpstreamError(errs []*upstreamError) *upstreamError {
return first return first
} }
// upstreamOutcome is the error to answer a fan-out with when it produced // askOrder says why a set of backends was asked, which is what decides whose
// nothing: the backends' own unanimous reply where there is one, and pdbmux's // reply becomes the client's answer when none of them answered 2xx. The two
// gateway error otherwise. Every handler ends here, so no route answers a // cases are not settings on one rule, they are different questions, and reading
// failure differently from any other. // a reply under the wrong one is how a real answer turns into a 502.
func upstreamOutcome(errs []*upstreamError) error { type askOrder int
const (
// askPeers: every backend was asked the same question and any of them could
// have answered it, so each reply is an opinion about that question. Only a
// reply they all gave is the estate's answer; anything else leaves a backend
// whose reply is evidence about the backend, which is what 502 reports.
askPeers askOrder = iota
// askOwnerFirst: the path names one entity, the backend holding it is asked
// first and the rest only as fallbacks in case it does not answer. A backend
// that does not hold the entity replies 404 to say exactly that, which is not
// a dissent from the holder's reply, so requiring the two to agree asks a
// question nobody was posed. The first backend that answered is the one that
// knows.
askOwnerFirst
)
// outcome turns a round's replies — one entry per backend in the order they were
// asked, nil where a backend produced no HTTP status at all — into the error to
// answer with.
func (o askOrder) outcome(replies []*upstreamError) error {
if o == askOwnerFirst {
return ownerOutcome(replies)
}
return peerOutcome(replies)
}
// peerOutcome is the error to answer a fan-out of peers with when it produced
// nothing: their unanimous reply where there is one, and pdbmux's gateway error
// otherwise.
func peerOutcome(errs []*upstreamError) error {
if ue := unanimousUpstreamError(errs); ue != nil { if ue := unanimousUpstreamError(errs); ue != nil {
return ue return ue
} }
return errAllBackendsFailed return errAllBackendsFailed
} }
// ownerOutcome is the error to answer an owner-routed path with: the reply of
// the first backend that answered. The owner is asked first, so that is the
// owner's own reply — it holds the entity, so its 500 is the truth about this
// request whatever a backend that does not hold the entity said.
//
// An owner that produced no status at all leaves a nil entry and the next
// backend's reply stands instead. The fallback chain exists so a node whose
// owner is unreachable is still served, and a backend that did answer explains
// more than a 502 that describes neither. Only a round where nothing answered is
// pdbmux's own gateway error.
func ownerOutcome(replies []*upstreamError) error {
for _, ue := range replies {
if ue != nil && replayableStatus(ue.status) {
return ue
}
}
return errAllBackendsFailed
}
// backendUpstreamErrors reduces a merged fan-out's results to one entry per // backendUpstreamErrors reduces a merged fan-out's results to one entry per
// backend, nil where the backend answered or failed without a status. // backend, nil where the backend answered or failed without a status.
func backendUpstreamErrors(results []backendResult) []*upstreamError { func backendUpstreamErrors(results []backendResult) []*upstreamError {
@@ -111,14 +160,21 @@ func backendUpstreamErrors(results []backendResult) []*upstreamError {
// which is why they are neither counted as degraded service nor answered from // which is why they are neither counted as degraded service nor answered from
// the cache; a unanimous 5xx is replayed just the same but is the backends // the cache; a unanimous 5xx is replayed just the same but is the backends
// reporting their own fault. // reporting their own fault.
//
// Every 4xx openvoxdb answers a fan-out with is about the request, including the
// 403 on /metrics/v2/list, which is that backend's own policy rather than an
// authentication failure. That holds only while pdbmux presents no client
// certificate: authenticate to backends and a rejected certificate becomes a
// 403 on every route at once, which this would read as a healthy estate refusing
// a bad query and hide a total outage behind a replayed 403.
func clientRefusal(err error) bool { func clientRefusal(err error) bool {
var ue *upstreamError var ue *upstreamError
return errors.As(err, &ue) && ue != nil && ue.status < 500 return errors.As(err, &ue) && ue != nil && ue.status < 500
} }
// writeUpstreamError answers a fan-out that produced no records. A unanimous // writeUpstreamError answers a round that produced no records. An upstream
// upstream error is replayed with the backend's own status, content type and // error the askOrder resolved to is replayed with the backend's own status,
// explanation; anything else is reported as a gateway failure. // content type and explanation; anything else is reported as a gateway failure.
func (s *Server) writeUpstreamError(w http.ResponseWriter, err error) { func (s *Server) writeUpstreamError(w http.ResponseWriter, err error) {
var ue *upstreamError var ue *upstreamError
if !errors.As(err, &ue) { if !errors.As(err, &ue) {
@@ -129,7 +185,7 @@ func (s *Server) writeUpstreamError(w http.ResponseWriter, err error) {
if len(strings.TrimSpace(string(body))) == 0 { if len(strings.TrimSpace(string(body))) == 0 {
setContentType(w, "text/plain; charset=utf-8") setContentType(w, "text/plain; charset=utf-8")
w.WriteHeader(ue.status) w.WriteHeader(ue.status)
_, _ = fmt.Fprintf(w, "every backend answered %d %s\n", ue.status, http.StatusText(ue.status)) _, _ = fmt.Fprintf(w, "upstream answered %d %s\n", ue.status, http.StatusText(ue.status))
return return
} }
setContentType(w, ue.contentType) setContentType(w, ue.contentType)