Skip to content

Commit a4982a8

Browse files
committed
admin: integrate LeaderForwarder into dynamo handler (Task #26 phase 2)
Wire the follower-side forwarder from phase 1 into the dynamo HTTP handler so writes hitting a follower transparently dispatch to the leader. Acceptance criteria 2 + 3 from design Section 3.3.2. Changes: - DynamoHandler gains a forwarder field plus WithLeaderForwarder option. nil keeps the previous "503 leader_unavailable" behaviour, so a build that ships only the leader can still deploy unchanged. - handleCreate / handleDelete catch ErrTablesNotLeader from the source and route to the forwarder. The principal already in context flows through to the leader's audit log unchanged. - writeForwardResult re-emits the leader's structured response verbatim — status, payload, and content-type all come from the gRPC response so a forwarded request looks identical to a leader-direct call from the SPA's perspective. - writeForwardFailure handles the two non-success paths: ErrLeaderUnavailable (election in flight, criterion 3) and generic gRPC transport errors. Both produce 503 + Retry-After: 1 so the SPA's retry policy is uniform regardless of where in the chain the 503 came from. ErrLeaderUnavailable is NOT logged as an error since elections are routine; transport errors are logged at LevelError so operators can investigate. - A 503 surfacing via the leader's structured response (e.g., leader stepped down mid-request) also gets Retry-After: 1 added on the way out so the wire contract is the same. Tests cover criteria 2 (transparent forwarding for both create and delete), 3 (election-period 503 + Retry-After), the leader-direct error pass-through (409 conflict), the no-forwarder fallback, and confirm the role / body validations short-circuit *before* the forwarder is invoked. Stub LeaderForwarder + a not-leader source make the tests deterministic without spinning up a real Raft cluster.
1 parent 1b9c692 commit a4982a8

2 files changed

Lines changed: 354 additions & 5 deletions

File tree

internal/admin/dynamo_handler.go

Lines changed: 121 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -168,15 +168,22 @@ func (e *ValidationError) Error() string {
168168
// — the JWT freezes the role at login time, and tokens last one
169169
// hour. Codex P1 on PR #635 flagged the gap on the HTTP path;
170170
// the forward server already does this re-evaluation on its side.
171+
//
172+
// When the source returns ErrTablesNotLeader and a LeaderForwarder
173+
// is configured, write requests are forwarded to the leader
174+
// transparently — the SPA sees a leader-direct response shape
175+
// regardless of which node it hit (design Section 3.3 criterion 2).
171176
type DynamoHandler struct {
172-
source TablesSource
173-
roles RoleStore
174-
logger *slog.Logger
177+
source TablesSource
178+
roles RoleStore
179+
forwarder LeaderForwarder
180+
logger *slog.Logger
175181
}
176182

177183
// NewDynamoHandler binds the source and seeds logging with
178-
// slog.Default(). Use WithLogger to attach a tagged logger and
179-
// WithRoleStore to plug in the live access-key role lookup.
184+
// slog.Default(). Use WithLogger to attach a tagged logger,
185+
// WithRoleStore to plug in the live access-key role lookup, and
186+
// WithLeaderForwarder to plug in the follower→leader forwarder.
180187
func NewDynamoHandler(source TablesSource) *DynamoHandler {
181188
return &DynamoHandler{source: source, logger: slog.Default()}
182189
}
@@ -201,6 +208,16 @@ func (h *DynamoHandler) WithRoleStore(r RoleStore) *DynamoHandler {
201208
return h
202209
}
203210

211+
// WithLeaderForwarder enables transparent follower→leader
212+
// forwarding. Without it, write requests on a follower fall back
213+
// to the standard 503 leader_unavailable response. Production
214+
// wires this to the gRPCForwardClient in main_admin.go; tests
215+
// inject a stub.
216+
func (h *DynamoHandler) WithLeaderForwarder(f LeaderForwarder) *DynamoHandler {
217+
h.forwarder = f
218+
return h
219+
}
220+
204221
// ServeHTTP routes /tables and /tables/{name}. We do not use
205222
// http.ServeMux because the admin router already guards the
206223
// /admin/api/v1/* prefix — adding another mux here would just
@@ -313,12 +330,48 @@ func (h *DynamoHandler) handleCreate(w http.ResponseWriter, r *http.Request) {
313330
}
314331
summary, err := h.source.AdminCreateTable(r.Context(), principal, body)
315332
if err != nil {
333+
// On a follower, the source returns ErrTablesNotLeader. If
334+
// a forwarder is wired, dispatch to the leader and re-emit
335+
// the leader's response verbatim — the SPA cannot tell the
336+
// difference between a leader-direct call and a forwarded
337+
// one. Without a forwarder, fall through to the standard
338+
// 503 leader_unavailable response.
339+
if h.tryForwardCreate(w, r, principal, body, err) {
340+
return
341+
}
316342
h.writeTablesError(w, r, "create", err)
317343
return
318344
}
319345
writeAdminJSONStatus(w, r.Context(), h.logger, http.StatusCreated, summary)
320346
}
321347

348+
// tryForwardCreate handles the follower→leader forwarding path
349+
// for POST /tables. Returns true when the response has been
350+
// written (regardless of forward success/failure); the caller
351+
// should then return without further processing.
352+
//
353+
// The "fall through to 503" path runs only when:
354+
// - the source error is something other than ErrTablesNotLeader,
355+
// - or no LeaderForwarder was configured,
356+
// - or the forwarder itself returned ErrLeaderUnavailable
357+
// (election in progress on the leader, criterion 3).
358+
//
359+
// Any other forwarder failure (gRPC transport error, etc.) is
360+
// also surfaced as 503 + Retry-After: 1 so the SPA can re-issue.
361+
// We log the raw error for operators and never echo it to clients.
362+
func (h *DynamoHandler) tryForwardCreate(w http.ResponseWriter, r *http.Request, principal AuthPrincipal, body CreateTableRequest, sourceErr error) bool {
363+
if !errors.Is(sourceErr, ErrTablesNotLeader) || h.forwarder == nil {
364+
return false
365+
}
366+
res, err := h.forwarder.ForwardCreateTable(r.Context(), principal, body)
367+
if err != nil {
368+
h.writeForwardFailure(w, r, "create", err)
369+
return true
370+
}
371+
h.writeForwardResult(w, r, res)
372+
return true
373+
}
374+
322375
// handleDelete is the DELETE /tables/{name} handler. Success is
323376
// 204 No Content; the body is intentionally empty so the SPA can
324377
// treat both 200 and 204 as success without parsing.
@@ -332,6 +385,9 @@ func (h *DynamoHandler) handleDelete(w http.ResponseWriter, r *http.Request, nam
332385
return
333386
}
334387
if err := h.source.AdminDeleteTable(r.Context(), principal, name); err != nil {
388+
if h.tryForwardDelete(w, r, principal, name, err) {
389+
return
390+
}
335391
h.writeTablesError(w, r, "delete", err)
336392
return
337393
}
@@ -390,6 +446,66 @@ func (h *DynamoHandler) principalForWrite(w http.ResponseWriter, r *http.Request
390446
return principal, true
391447
}
392448

449+
// tryForwardDelete is the DELETE counterpart of tryForwardCreate.
450+
// Same semantics: only the ErrTablesNotLeader source error
451+
// triggers forwarding, and only when a forwarder is configured.
452+
func (h *DynamoHandler) tryForwardDelete(w http.ResponseWriter, r *http.Request, principal AuthPrincipal, name string, sourceErr error) bool {
453+
if !errors.Is(sourceErr, ErrTablesNotLeader) || h.forwarder == nil {
454+
return false
455+
}
456+
res, err := h.forwarder.ForwardDeleteTable(r.Context(), principal, name)
457+
if err != nil {
458+
h.writeForwardFailure(w, r, "delete", err)
459+
return true
460+
}
461+
h.writeForwardResult(w, r, res)
462+
return true
463+
}
464+
465+
// writeForwardResult re-emits the leader's structured response
466+
// verbatim. Status, payload, and content-type all come from the
467+
// gRPC response so a forwarded request looks identical to a
468+
// leader-direct call from the SPA's point of view.
469+
func (h *DynamoHandler) writeForwardResult(w http.ResponseWriter, r *http.Request, res *ForwardResult) {
470+
w.Header().Set("Content-Type", res.ContentType)
471+
w.Header().Set("Cache-Control", "no-store")
472+
// 503 from the leader (e.g. it stepped down mid-request) must
473+
// carry Retry-After so the client retries; preserve the
474+
// criterion-3 contract on the wire whether the 503 originated
475+
// here or at the leader.
476+
if res.StatusCode == http.StatusServiceUnavailable {
477+
w.Header().Set("Retry-After", "1")
478+
}
479+
w.WriteHeader(res.StatusCode)
480+
if len(res.Payload) > 0 {
481+
if _, err := w.Write(res.Payload); err != nil {
482+
h.logger.LogAttrs(r.Context(), slog.LevelWarn, "admin forward response write failed",
483+
slog.String("error", err.Error()),
484+
)
485+
}
486+
}
487+
}
488+
489+
// writeForwardFailure handles forwarder errors that did not
490+
// produce a structured leader response: ErrLeaderUnavailable
491+
// (election in flight) and gRPC transport errors. Both surface as
492+
// 503 + Retry-After: 1 — the SPA's retry contract is identical
493+
// regardless of whether the leader is briefly absent or the
494+
// network hiccupped.
495+
func (h *DynamoHandler) writeForwardFailure(w http.ResponseWriter, r *http.Request, op string, err error) {
496+
if !errors.Is(err, ErrLeaderUnavailable) {
497+
// Not the "no leader known" case — log the raw error so
498+
// operators can investigate. Client still sees the same
499+
// 503 + Retry-After so they retry uniformly.
500+
h.logger.LogAttrs(r.Context(), slog.LevelError, "admin dynamo "+op+" forward failed",
501+
slog.String("error", err.Error()),
502+
)
503+
}
504+
w.Header().Set("Retry-After", "1")
505+
writeJSONError(w, http.StatusServiceUnavailable, "leader_unavailable",
506+
"raft leader currently unavailable; retry shortly")
507+
}
508+
393509
// writeTablesError translates a TablesSource error into the
394510
// appropriate HTTP response. Internal-server-error fallthrough logs
395511
// the raw err.Error() but never sends it to the client, matching

0 commit comments

Comments
 (0)