S13: scope allow/deny rule evaluation to the fields a request addresses; fix the Retry-After value
Problem
Two gaps in the S13 foundation that both virtual slices hit independently.
1. target_field scoping has no owner
A request does not always carry every field its format defines: an npm packument request carries no version, a container request addressed by digest carries no tag. Rule evaluation has to be scoped to the fields the request does address, or an allow rule on an unaddressed field matches nothing and excludes the upstream from every request that omits the field.
virtual.Eligible currently fails closed when a rule targets a field absent from the Coordinate (internal/virtual/rules.go), which is correct for a corrupt target_field but wrong for a field the request simply does not address. Both slices worked around it the same way — filter the rule set before calling the matcher:
- S31 npm virtual: !862 (merged)
- S32 container virtual: !972 (merged)
Two slices implementing one filter is the signal it belongs below both. A slice-side filter also cannot keep the fail-closed arm: a rule dropped before the matcher runs is a rule the matcher cannot reject, so a corrupted deny stops denying with nothing logged.
2. No spec fixed the Retry-After value
S13, S15, and docs/dev/api-style.md each require Retry-After on a 503 upstream_unavailable, and until the S13 amendment in !1502 (merged) none stated a value, so no slice could write an acceptance criterion that asserts one. !1502 (merged) states it in S13's Retry-After on an unavailable upstream section, for the three verdicts that section names.
Proposal
- S13 takes the scoping: a slice supplies its format's field set together with the values the request carries, and the foundation decides which rules that leaves in play. A
target_fieldnaming no field its format defines stays a fail-closed data error, distinct from an unaddressed one. Retry-Afterishealth_check.scheduled_intervalrounded up to the next whole second, never below1:300under the default configuration, and1for any legal interval below one second. The knob's validation bounds it only as positive and below one hour, so500msis a legal setting and truncation would answerRetry-After: 0. A client honoring the value retries no sooner than the next probe sweep is due. The interval does not name the moment recovery becomes possible: a manual probe changes a status on demand, and theUpstreamUnavailableverdict answers a live transport failure rather than a stored status. One value across the virtual and standalone cases lets a client treat both kinds alike, and deriving it from an existing knob avoids a second knob that could contradict the first.
Scope
- Spec amendment to
docs/specs/S13-virtual-remote-foundation.md. internal/virtual/rules.go: makeCoordinatedistinguish an unaddressed field from a corrupttarget_field, so the matcher can skip the first and reject the second.Coordinatehas no production caller outsideinternal/virtualtoday, so the change is cheap now and gets more expensive once npm and container both wire it.- The derivation helper in
internal/remote, and then the assertion where the503responses are built. !1678 (merged) lands the helper on its own. While no slice or fill-handler MR that renders one of these503s has merged, no site onmainwrites one, so there is no header to assert; the first of those MRs to merge is where the assertion belongs.
A 503 that relays a status the upstream itself produced is outside this item. The merged specs disagree on what it carries, and #687 tracks reconciling them.
Both S31 and S32 drop their slice-side filter once this lands.