amadan
ship: record the superseded read-after-ship ruling, and the static assumption
Two comments, no behaviour change. At the flags read: this read sits BEFORE release.Ship and fails the ship closed, and that supersedes the earlier design where the build-probe echo read the same row AFTER the bytes were stored and degraded on failure. That ruling was correct in that position and its premise is gone with the move. One read cannot carry both policies, so the guard's dominates: a console that cannot read the row does not know whether this app is the control plane, and shipping anyway is a database blip becoming the way past the guard. The echo is a convenience and does not get to set a guard's failure policy. Exposure is column-level, not outage-level — apiAccountApp has already read the same table and migrate() runs at Open(). At the static exemption: both checks parse an ELF and a static release is a tarball, so `-kind static` skips them. No refusal is added — what a control-plane app may BE is not this guard's business — but the assumption is now written where someone making a control-plane app static will see that they are removing these checks by doing so.
1 file changed,
+40
−15
internal/console/apicli.go+40 −15
diff --git a/internal/console/apicli.go b/internal/console/apicli.go| index 76996e5ca..553534c94 100644 |
| --- a/internal/console/apicli.go |
| +++ b/internal/console/apicli.go |
| @@ -1539,18 +1539,32 @@ func (s *Server) handleAPIShip(w http.ResponseWriter, r *http.Request) { |
| // both success paths are two columns of the same row, and a ship has no |
| // reason to ask twice. |
| // |
| - // It fails CLOSED — an unreadable row refuses the ship — which is the |
| - // same ruling every other reader of this flag follows (controlplane.go): |
| - // treating an unreadable row as "not the control plane" would make a |
| - // database blip the way past the guard, and the guard here is the one |
| - // standing between a wrong-architecture binary and every box that runs |
| - // the console. |
| + // It sits deliberately BEFORE release.Ship, and it FAILS THE SHIP CLOSED: |
| + // an unreadable row is answered 503 with nothing written. |
| // |
| - // That the ECHO now refuses too, where task 6 had it degrade, is the |
| - // move rather than a reversal: that ruling rested on "the bytes are |
| - // already stored by the time this runs", and this read happens BEFORE |
| - // release.Ship writes anything. Refusing costs the caller a retry; there |
| - // is no half-done ship to be honest about. |
| + // THIS SUPERSEDES AN EARLIER DESIGN, and the history holds both shapes, |
| + // so which reasoning won is written here. The build-probe echo used to |
| + // read this row for itself, AFTER release.Ship had stored the bytes, and |
| + // the ruling there was that a failed read must not fail the ship — |
| + // correct in that position, because the ship had already succeeded and |
| + // refusing bought nothing. Reading once moves the read ahead of the |
| + // write, and the premise goes with it: there is no stored release to be |
| + // honest about. |
| + // |
| + // The two policies cannot be reconciled from ONE read, so one has to |
| + // dominate, and it is the GUARD's. If the console cannot read this row it |
| + // does not know whether this app is the control plane, and shipping |
| + // anyway is exactly "a database blip is the way past the guard" — the |
| + // failure every other reader of this flag holds closed (controlplane.go). |
| + // The echo is a convenience; the guard is what stands between a |
| + // wrong-architecture binary and every box that runs the console. A |
| + // convenience does not get to set the failure policy of a guard. |
| + // |
| + // The exposure this buys is small: apiAccountApp has already read the |
| + // same table (HasAppIn) before this line, and migrate() runs at Open(), |
| + // so a row unreadable HERE but readable THERE is a column-level fault on |
| + // a console that started successfully — reachable in a test by dropping |
| + // one column, not by an ordinary outage. |
| cp, probe, err := s.store.AppFlags(acct.ID, app) |
| if err != nil { |
| log.Printf("console: api ship %s/%s: read app flags: %v", acct.Sqid, app, err) |
| @@ -1559,8 +1573,17 @@ func (s *Server) handleAPIShip(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| // Read the bytes that actually arrived, before a single one is written. |
| - // A static release ships a tarball, not an ELF, so it is exempt — there |
| - // is nothing in it for either check to read. |
| + // |
| + // The static exemption is an ASSUMPTION, stated here because it is the |
| + // shape a bypass would take: both checks read an ELF (machine header, |
| + // embedded build info) and a static release ships one gzipped tarball, |
| + // which neither can parse — so `-kind static` skips them entirely. That |
| + // is sound only while no control-plane app ships static. Nothing refuses |
| + // one that does, deliberately: inventing constraints on what a |
| + // control-plane app may BE is not this guard's business. But anyone |
| + // making the console (or any control-plane app) a static release is |
| + // removing these two checks by doing so, and should replace them rather |
| + // than discover the gap later. |
| if cp && meta.Kind != "static" { |
| if err := checkControlPlaneArtifacts(files, ver); err != nil { |
| jsonError(w, http.StatusBadRequest, err.Error()) |
| @@ -1689,8 +1712,10 @@ func (s *Server) handleAPIShip(w http.ResponseWriter, r *http.Request) { |
| // It takes the probe rather than reading it because handleAPIShip already |
| // read that row, for the control-plane check that must happen before any |
| // bytes are written — so the read moved UP, ahead of release.Ship, and this |
| -// function is now only the echo. The read's failure is handled there, and |
| -// refuses: see the comment at the call. |
| +// function is now only the echo. With the move, this function's original |
| +// "a failed read must not fail the ship" ruling is SUPERSEDED: the read now |
| +// fails the ship closed, because the guard sharing it cannot fall through. |
| +// The reasoning is recorded in full at the read site. |
| // |
| // Additive: a client that does not read the key is unaffected, and an app |
| // with no probe carries no key at all, so every ship the fleet does today |