amadan

carlos / platform Public

assets: set-assets and purge record the change message in the audit (task 10 fix round 1, part A)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
claude b80ecd736e24a5e07e4e0e35e166f47f16022ed6 parent 2307718
4 files changed, +205 −5
  • assets_cli_test.go +49 −0
  • internal/console/apiassets.go +20 −3
  • internal/console/apiassets_test.go +125 −0
  • internal/console/apiinstances.go +11 −2
diff --git a/assets_cli_test.go b/assets_cli_test.go
index cf5b44663..f86101e7e 100644
--- a/assets_cli_test.go
+++ b/assets_cli_test.go
@@ -31,6 +31,9 @@ func assetsDoor(t *testing.T, status int, reply string) (*apiSession, *struct{ M
// TestInstanceSetAssetsPatches pins instanceSetAssets's wire contract: a
// PATCH to the instance door carrying "assets" as the word "on" or "off",
// always sent, since the verb has no leave-it-alone spelling.
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestInstanceSetAssetsPatches(t *testing.T) {
sess, got := assetsDoor(t, http.StatusOK, `{"host":"shop.example.com"}`)
acct := apiAcct{Seg: "abc"}
@@ -58,6 +61,9 @@ func TestInstanceSetAssetsPatches(t *testing.T) {
// TestInstanceSetAssetsOlderConsole: a console that predates the field
// answers 400 "nothing to update" for a body of only "assets"; the verb says
// the console is older instead of passing that on.
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestInstanceSetAssetsOlderConsole(t *testing.T) {
sess, _ := assetsDoor(t, http.StatusBadRequest, `{"error":"nothing to update — send addrs, probe, steering, environment, gate, or a combination"}`)
_, err := sess.instanceSetAssets(context.Background(), apiAcct{Seg: "abc"}, "shop", "shop.example.com", false)
@@ -68,6 +74,9 @@ func TestInstanceSetAssetsOlderConsole(t *testing.T) {
// TestInstanceSetAssetsSurfacesRefusalVerbatim: any other refusal reaches the
// caller as the door worded it.
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestInstanceSetAssetsSurfacesRefusalVerbatim(t *testing.T) {
sess, _ := assetsDoor(t, http.StatusBadRequest, `{"error":"assets must be on or off."}`)
_, err := sess.instanceSetAssets(context.Background(), apiAcct{Seg: "abc"}, "shop", "shop.example.com", true)
@@ -78,6 +87,9 @@ func TestInstanceSetAssetsSurfacesRefusalVerbatim(t *testing.T) {
// TestSetAssetsResultPrintsBothSentences pins EAC-10 and EAC-11 as a person
// reads them.
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestSetAssetsResultPrintsBothSentences(t *testing.T) {
var b bytes.Buffer
printSetAssets(&b, "shop.example.com", true)
@@ -94,6 +106,9 @@ const purgePartialReply = `{"instances":[{"host":"a.shop.example.com","gen":"012
// TestAssetsPurgePrintsComplete pins the wire (POST to the purge door) and
// EAC-12.
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestAssetsPurgePrintsComplete(t *testing.T) {
sess, got := assetsDoor(t, http.StatusOK, purgeDoneReply)
res, err := sess.assetsPurge(context.Background(), apiAcct{Seg: "abc"}, "shop")
@@ -115,6 +130,9 @@ func TestAssetsPurgePrintsComplete(t *testing.T) {
// TestAssetsPurgePrintsPartial: a purge that stopped part way names the
// instance that did not change and says to run it again (EAC-13).
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestAssetsPurgePrintsPartial(t *testing.T) {
sess, _ := assetsDoor(t, http.StatusOK, purgePartialReply)
res, err := sess.assetsPurge(context.Background(), apiAcct{Seg: "abc"}, "shop")
@@ -134,6 +152,9 @@ func TestAssetsPurgePrintsPartial(t *testing.T) {
// TestAssetsPurgeOlderConsole: a 404 from the purge door is the mux of a
// console that has no such route, or an unknown app; the message says both,
// the form the restart verb uses.
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestAssetsPurgeOlderConsole(t *testing.T) {
sess, _ := assetsDoor(t, http.StatusNotFound, "404 page not found\n")
_, err := sess.assetsPurge(context.Background(), apiAcct{Seg: "abc"}, "shop")
@@ -155,6 +176,9 @@ const statusReply = `{"instances":[` +
// TestAssetsStatusPrintsTheBound pins the status wire (GET) and EAC-14: the
// column heads, a row per instance, and the bound sentence after them.
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestAssetsStatusPrintsTheBound(t *testing.T) {
sess, got := assetsDoor(t, http.StatusOK, statusReply)
st, err := sess.assetsStatus(context.Background(), apiAcct{Seg: "abc"}, "shop")
@@ -189,6 +213,9 @@ func TestAssetsStatusPrintsTheBound(t *testing.T) {
// TestAssetsStatusDecodesTheWire keeps the JSON field names the status door
// writes (internal/console assetStatusView) in step with the client's.
+//
+// Not acceptance: the console is a stand-in that returns canned JSON; the real
+// doors are tested in internal/console.
func TestAssetsStatusDecodesTheWire(t *testing.T) {
var st apiAssetsStatus
if err := json.Unmarshal([]byte(statusReply), &st); err != nil {
@@ -200,3 +227,25 @@ func TestAssetsStatusDecodesTheWire(t *testing.T) {
t.Fatalf("decoded = %+v", st)
}
}
+
+// TestAssetsVerbsSendTheMessage: a reason given to purge or set-assets rides
+// in the request body under "message", where the doors read it.
+//
+// Not acceptance: the console is a stand-in that records the request; the
+// real doors' audit rows are asserted in internal/console
+// (TestPurgeRecordsTheMessage, TestAPIInstanceUpdateRecordsTheMessage).
+func TestAssetsVerbsSendTheMessage(t *testing.T) {
+ sess, got := assetsDoor(t, http.StatusOK, purgeDoneReply)
+ if _, err := sess.assetsPurge(context.Background(), apiAcct{Seg: "abc"}, "shop", "after the logo change"); err != nil {
+ t.Fatal(err)
+ }
+ if got.Body != `{"message":"after the logo change"}` {
+ t.Fatalf("purge body = %q", got.Body)
+ }
+ if _, err := sess.instanceSetAssets(context.Background(), apiAcct{Seg: "abc"}, "shop", "shop.example.com", false, "debugging"); err != nil {
+ t.Fatal(err)
+ }
+ if got.Body != `{"assets":"off","message":"debugging"}` {
+ t.Fatalf("set-assets body = %q", got.Body)
+ }
+}
diff --git a/internal/console/apiassets.go b/internal/console/apiassets.go
index 43f3368b5..dc509d3ab 100644
--- a/internal/console/apiassets.go
+++ b/internal/console/apiassets.go
@@ -89,13 +89,27 @@ const assetPurgeWriteFailed = "could not update the instance record"
// record retired between the listing and its write is not an instance any
// more and is left out of the reply, placeAppTarget's rule.
//
-// One audit row names the instances changed and any not changed; the serial
-// is bumped once, after the writes, when any landed.
+// One audit row names the instances changed and any not changed, with the
+// caller's message; the serial is bumped once, after the writes, when any
+// landed.
func (s *Server) handleAPIAssetsPurge(w http.ResponseWriter, r *http.Request) {
principal, acct, app, ok := s.instanceGate(w, r)
if !ok {
return
}
+ // The body is optional (the door took none before): a client that sends
+ // one carries its reason as message, validated before any record is
+ // written.
+ var req struct {
+ Message string `json:"message"`
+ }
+ if r.ContentLength != 0 && !readJSON(w, r, &req) {
+ return
+ }
+ message, ok := checkedActivityMessage(w, req.Message)
+ if !ok {
+ return
+ }
ctx, cancel := context.WithTimeout(r.Context(), instanceStoreTimeout)
defer cancel()
hosts, err := appRecordHosts(ctx, s.cfg.Releases, acct.Sqid, app)
@@ -129,7 +143,10 @@ func (s *Server) handleAPIAssetsPurge(w http.ResponseWriter, r *http.Request) {
if len(failed) > 0 {
detail += "; not changed: " + strings.Join(failed, ", ")
}
- s.audit(acct, app, principal, "assets-purge", detail+" (cli)")
+ s.auditActivity(acct, app, principal, Activity{
+ Action: "assets-purge", TargetType: "app", Target: app, Message: message,
+ Detail: detail + " (cli)",
+ })
}
if len(done) > 0 {
s.bumpSerial(ctx)
diff --git a/internal/console/apiassets_test.go b/internal/console/apiassets_test.go
index 1c014c8b5..12afd62bb 100644
--- a/internal/console/apiassets_test.go
+++ b/internal/console/apiassets_test.go
@@ -582,3 +582,128 @@ func TestAssetsStatusJoinsPublishedGeneration(t *testing.T) {
t.Fatalf("shop-2.example.com = %+v", two)
}
}
+
+// ---- the change message -----------------------------------------------------
+
+// assetEvents is the app's audit rows for one action, newest first.
+func assetEvents(t *testing.T, fx *consoleFixture, action string) []Event {
+ t.Helper()
+ events, err := fx.st.Events(shipAssetsAccountID, assetsApp, 50)
+ if err != nil {
+ t.Fatal(err)
+ }
+ var out []Event
+ for _, e := range events {
+ if e.Action == action {
+ out = append(out, e)
+ }
+ }
+ return out
+}
+
+// TestAPIInstanceUpdateRecordsTheMessage: the reason a person types with
+// set-assets reaches the audit row, beside the same detail as before, and a
+// message that is not one short line is refused before anything is written.
+func TestAPIInstanceUpdateRecordsTheMessage(t *testing.T) {
+ fx, token := assetsFixture(t, "code-assets-patch-message")
+ const host = "shop.example.com"
+ putAssetRecord(t, fx, host, "0123456789abcdef")
+ pu := fx.ts.URL + "/api/cli/apps/" + shipAssetsAccount + "/" + assetsApp + "/instances/" + host
+ key := provision.RecordKey(shipAssetsAccount, assetsApp, host)
+
+ status, _, out := rawReq(t, http.MethodPatch, pu, token, `{"assets":"off","message":" debugging a stale logo "}`)
+ if status != http.StatusOK {
+ t.Fatalf("patch = %d %s", status, out)
+ }
+ rows := assetEvents(t, fx, "instance-update")
+ if len(rows) != 1 {
+ t.Fatalf("instance-update rows = %+v, want one", rows)
+ }
+ if rows[0].Message != "debugging a stale logo" || rows[0].Target != host || rows[0].TargetType != "instance" ||
+ rows[0].Detail != "updated instance shop.example.com (assets) (cli)" {
+ t.Fatalf("audit row = %+v, want the trimmed message, the instance target and the usual detail", rows[0])
+ }
+
+ // A bad message: refused with the activity sentence, record, serial and
+ // audit untouched.
+ raw, _, err := fx.rel.Get(context.Background(), key)
+ if err != nil {
+ t.Fatal(err)
+ }
+ before := serial(t, fx)
+ for _, bad := range []string{"two\\nlines", strings.Repeat("x", activityMessageMaxRunes+1)} {
+ status, _, out := rawReq(t, http.MethodPatch, pu, token, `{"assets":"on","message":"`+bad+`"}`)
+ var body map[string]string
+ _ = json.Unmarshal([]byte(out), &body)
+ if status != http.StatusBadRequest || body["error"] != activityText.InvalidMessage {
+ t.Fatalf("patch with message %.20q = %d %s, want 400 %q", bad, status, out, activityText.InvalidMessage)
+ }
+ }
+ after, _, err := fx.rel.Get(context.Background(), key)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if string(after) != string(raw) || serial(t, fx) != before || len(assetEvents(t, fx, "instance-update")) != 1 {
+ t.Fatal("a refused message changed the record, the serial or the audit")
+ }
+
+ // No message: still a row, with none.
+ if status, _, out := rawReq(t, http.MethodPatch, pu, token, `{"assets":"on"}`); status != http.StatusOK {
+ t.Fatalf("patch = %d %s", status, out)
+ }
+ if rows := assetEvents(t, fx, "instance-update"); len(rows) != 2 || rows[0].Message != "" {
+ t.Fatalf("rows = %+v, want a second row with no message", rows)
+ }
+}
+
+// TestPurgeRecordsTheMessage: the purge door reads its body, records the
+// message on the one audit row (partial or complete), refuses a bad message
+// before any record is written, and still takes no body at all.
+func TestPurgeRecordsTheMessage(t *testing.T) {
+ fx, token := assetsFixture(t, "code-assets-purge-message")
+ for i, host := range purgeHosts {
+ putAssetRecord(t, fx, host, strings.Repeat(string(rune('a'+i)), 16))
+ }
+ purge := func(body string) (int, string) {
+ status, _, out := rawReq(t, http.MethodPost, assetsURL(fx)+"/purge", token, body)
+ return status, out
+ }
+
+ // Refused first: nothing drawn, nothing audited.
+ before := serial(t, fx)
+ status, out := purge(`{"message":"two\nlines"}`)
+ var body map[string]string
+ _ = json.Unmarshal([]byte(out), &body)
+ if status != http.StatusBadRequest || body["error"] != activityText.InvalidMessage {
+ t.Fatalf("purge with a bad message = %d %s", status, out)
+ }
+ for i, host := range purgeHosts {
+ if got := assetRecord(t, fx, host).AssetGen; got != strings.Repeat(string(rune('a'+i)), 16) {
+ t.Fatalf("a refused purge changed %s to %q", host, got)
+ }
+ }
+ if serial(t, fx) != before || len(assetEvents(t, fx, "assets-purge")) != 0 {
+ t.Fatal("a refused purge bumped the serial or audited")
+ }
+
+ // A partial purge records the message too.
+ fs := failRecords(fx)
+ fs.fail(provision.RecordKey(shipAssetsAccount, assetsApp, purgeHosts[1]))
+ if status, out := purge(`{"message":"after the logo change"}`); status != http.StatusOK {
+ t.Fatalf("purge = %d %s", status, out)
+ }
+ rows := assetEvents(t, fx, "assets-purge")
+ if len(rows) != 1 || rows[0].Message != "after the logo change" || rows[0].TargetType != "app" || rows[0].Target != assetsApp ||
+ !strings.Contains(rows[0].Detail, "not changed: "+purgeHosts[1]) {
+ t.Fatalf("audit rows = %+v, want one with the message and the unchanged host", rows)
+ }
+
+ // No body at all, as before: a purge with no message.
+ fs.fail("")
+ if status, out := purge(``); status != http.StatusOK {
+ t.Fatalf("bodyless purge = %d %s", status, out)
+ }
+ if rows := assetEvents(t, fx, "assets-purge"); len(rows) != 2 || rows[0].Message != "" {
+ t.Fatalf("rows = %+v, want a second row with no message", rows)
+ }
+}
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go
index ab40380cb..aa5eeca82 100644
--- a/internal/console/apiinstances.go
+++ b/internal/console/apiinstances.go
@@ -2012,6 +2012,9 @@ func (s *Server) handleAPIInstanceUpdate(w http.ResponseWriter, r *http.Request)
// Absent leaves the record alone. assetsSwitch (apiassets.go) reads
// it and holds the refusal for anything else.
Assets *string `json:"assets"`
+ // Message is the caller's optional reason for the change, recorded on
+ // the audit row (checkedActivityMessage owns its refusal).
+ Message string `json:"message"`
}
if !readJSON(w, r, &req) {
return
@@ -2021,6 +2024,10 @@ func (s *Server) handleAPIInstanceUpdate(w http.ResponseWriter, r *http.Request)
"nothing to update — send addrs, probe, steering, environment, gate, or a combination")
return
}
+ message, ok := checkedActivityMessage(w, req.Message)
+ if !ok {
+ return
+ }
var probe *provision.Health
if req.Probe != nil {
if err := json.Unmarshal(req.Probe, &probe); err != nil {
@@ -2206,8 +2213,10 @@ func (s *Server) handleAPIInstanceUpdate(w http.ResponseWriter, r *http.Request)
if req.Assets != nil {
changed = append(changed, "assets")
}
- s.audit(acct, app, principal, "instance-update",
- "updated instance "+host+" ("+strings.Join(changed, ", ")+") (cli)")
+ s.auditActivity(acct, app, principal, Activity{
+ Action: "instance-update", TargetType: "instance", Target: host, Message: message,
+ Detail: "updated instance " + host + " (" + strings.Join(changed, ", ") + ") (cli)",
+ })
// Strictly after the write landed, and exactly once: the record is what a
// box's reconcile leg converges upstreams from (provision.convergeOwned),
// so this announcement is what makes the new set live in seconds instead