diff --git a/.github/workflows/roster.yml b/.github/workflows/roster.yml new file mode 100644 index 0000000..f501d0a --- /dev/null +++ b/.github/workflows/roster.yml @@ -0,0 +1,75 @@ +# The comparison between the roster this repository pins and the one published +# where the machine-readable data lives. +# +# It reads a file published somewhere else, so its verdict moves when somebody +# else commits rather than when this tree changes, which is why it is not a leg +# of the gate and why the tokens comparison beside it is not either. The half +# that needs no network is the build, which reads the pinned copy on every run. +# +# It reports and does not write. A run that quietly pulled the published rows +# into the tree would destroy the difference, and the difference is the evidence +# for the change somebody makes. The last step is what holds it to that. +# +# THIS WORKFLOW CARRIES NO SCHEDULE YET, AND THAT IS THE STATE RATHER THAN AN +# OVERSIGHT. Nothing is published at the address the comparison reads, so every +# run today reports OFF and reds. That is the right verdict - a freshness check +# that passed having compared nothing is the exact failure it exists to prevent +# - and a weekly red for a state nobody can repair from this repository would be +# an entry the failure watchdog opens every day and a red somebody learns to +# ignore. Entry 6 of #7 is answered `the site builds against fixtures now and +# vendors the published roster file once it lands in the data repository; until +# then the freshness checks stay disabled by name`, and this file is that +# sentence carried out: the comparison exists, it is named, it can be asked for +# by hand, and its timer is off. +# +# What turns the timer on is one condition and it is written here rather than +# remembered: a roster published at the address `internal/freshness` reads. +# On that day this file gains +# +# schedule: +# - cron: "37 5 * * 3" +# +# on the weekly cadence the other comparisons of a pinned copy use, so the whole +# set is looked at on one rhythm rather than on several nobody can hold in their +# head. +name: Roster + +on: + workflow_dispatch: + +# Deny everything at the top level; the job below grants the one scope it needs. +permissions: {} + +jobs: + roster: + name: Compare the pinned roster against the published file + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read # checkout only; nothing here writes + steps: + - name: Checkout Repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install the toolchain go.mod pins + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + cache: false + + - name: Compare every pinned row against the published one + run: go run . roster + + - name: Prove the run wrote nothing + # After the comparison whether it passed or failed, because a run that + # rewrote a row on its way to a red verdict is the failure this step + # exists for and a red verdict is exactly when it would be missed. + if: always() + shell: bash + run: | + set -euo pipefail + git status --porcelain=v1 + git diff --exit-code + test -z "$(git status --porcelain=v1)" diff --git a/docs/roster-schema.md b/docs/roster-schema.md index c912271..74e0b33 100644 --- a/docs/roster-schema.md +++ b/docs/roster-schema.md @@ -16,6 +16,13 @@ once that file exists in the repository holding the machine-readable data. So later, and nothing about the build moves on that day. What moves is where the bytes come from, which is #24. +`go run . roster` is what says the copy has fallen behind. It compares the +pinned rows against the published ones, names every row that differs, writes +nothing, and reports OFF rather than green where nothing is published to compare +against. Nothing is published there today, so every run reports OFF; the address +it reads and what turns its schedule on are in `internal/freshness` and in the +workflow beside it. + Beside it is `data/releases.json`, which is not part of this shape. It carries what each repository the roster names has published, taken once by `go run . releases` and written down with the request that took it. The build diff --git a/internal/freshness/freshness.go b/internal/freshness/freshness.go new file mode 100644 index 0000000..361be0a --- /dev/null +++ b/internal/freshness/freshness.go @@ -0,0 +1,218 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package freshness says when the pinned roster in this tree has fallen behind +// the published one. +// +// The build reads a copy so that it is reproducible and works with no network, +// which is the same reason the release data is recorded rather than fetched. A +// copy nobody checks is a copy that silently goes stale, so this compares it +// against what is published and reds on a difference. It writes nothing: the +// difference is the evidence for the change somebody makes, and a run that +// quietly resolved it would destroy that. +// +// It is a package of its own rather than a function in internal/roster, and the +// reason is the dependency graph rather than tidiness. The parser is on the path +// every build takes, this reaches the network, and a build that linked a client +// it never calls is a build whose offline property rests on nobody calling it. +// +// It fails closed in both directions a comparison can be absent. A fetch that +// could not be made is reported as unresolved and reds, because a copy that was +// not compared is not a current one. And nothing published at the address is +// reported as OFF and reds too, rather than passing: a freshness check that +// passed having compared nothing is the exact failure it exists to prevent. +package freshness + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/Flowfin/site/internal/roster" + "github.com/Flowfin/site/internal/site" +) + +// Published is where the roster is expected to be published. +// +// The address is derived rather than decided here, and what it is derived from +// is where the two files already published for machines sit: the catalogue +// manifest and the design token file are both in the served directory of the +// repository that holds machine-readable data, and +// decisions/0001-where-the-plugin-list-comes-from.md puts the roster beside the +// manifest. The file name is settled nowhere, which is why the comparison below +// reports OFF rather than green when nothing answers here: a name guessed wrong +// costs a corrected constant and can never cost a false pass. +const Published = "https://raw.githubusercontent.com/Flowfin/hub/HEAD/docs/roster.json" + +// ErrNotPublished is what a fetcher answers with when there is no file at the +// address. It is a state of its own rather than an error, because a file that +// has not been published yet and a file that could not be read are different +// things to report and only one of them will end on its own. +var ErrNotPublished = errors.New("no file is published at that address") + +// Fetcher reads the published file. It is a parameter so that the suite over +// this package reaches no network. +type Fetcher func() ([]byte, error) + +// Difference is one row the two files disagree about. It carries the identifier +// rather than a position, because the identifier is the thing the two sides +// share and a row's position is not a fact about the roster. +type Difference struct { + ID string + // Says is what differs, in words, so a run names the repair rather than + // leaving somebody to diff two files to find it. + Says string +} + +// Rows reads a roster leniently, applying none of the rules the parser applies. +// +// The comparison is about two files agreeing, not about either being valid: a +// published file this tree would refuse is still a published file the copy has +// fallen behind, and reporting it as unreadable would hide the difference behind +// a second problem. What refuses a roster the build may not use is the parser, +// on the build's own path. +func Rows(body []byte) ([]roster.Entry, error) { + var rows []roster.Entry + if err := json.Unmarshal(body, &rows); err != nil { + return nil, fmt.Errorf("the bytes are not the array of rows a roster is: %w", err) + } + return rows, nil +} + +// Compare returns every row the two files disagree about, in identifier order so +// that two runs over one pair report in the same sequence. +func Compare(pinned, published []roster.Entry) []Difference { + here := index(pinned) + there := index(published) + + names := map[string]bool{} + for id := range here { + names[id] = true + } + for id := range there { + names[id] = true + } + var sorted []string + for id := range names { + sorted = append(sorted, id) + } + sort.Strings(sorted) + + var out []Difference + for _, id := range sorted { + a, inHere := here[id] + b, inThere := there[id] + switch { + case !inHere: + out = append(out, Difference{id, "the published file carries this row and this tree does not"}) + case !inThere: + out = append(out, Difference{id, "this tree carries this row and the published file does not"}) + case a != b: + out = append(out, Difference{id, saysWhatDiffers(a, b)}) + } + } + return out +} + +// index keys the rows by identifier. A row with no identifier is keyed by the +// empty string rather than dropped, so a file carrying one is reported as a +// difference instead of quietly matching nothing. +func index(rows []roster.Entry) map[string]roster.Entry { + out := make(map[string]roster.Entry, len(rows)) + for _, r := range rows { + out[r.ID] = r + } + return out +} + +// saysWhatDiffers names each field the two rows disagree about, with both +// values. A message saying only that a row differs sends the next reader back +// to the two files to find out which field it was. +func saysWhatDiffers(a, b roster.Entry) string { + var said []string + if a.Repository != b.Repository { + said = append(said, fmt.Sprintf("this tree says the repository is %q and the published file says %q", a.Repository, b.Repository)) + } + if a.Summary != b.Summary { + said = append(said, fmt.Sprintf("this tree says the sentence is %q and the published file says %q", a.Summary, b.Summary)) + } + if a.State != b.State { + said = append(said, fmt.Sprintf("this tree says the state is %q and the published file says %q", a.State, b.State)) + } + return strings.Join(said, ", ") +} + +// Run compares the pinned copy against the published file and reports what it +// found. +func Run(root string, fetch Fetcher, out io.Writer) error { + body, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(site.RosterFile))) + if err != nil { + return fmt.Errorf("roster: reading the pinned copy at %s: %w", site.RosterFile, err) + } + pinned, err := Rows(body) + if err != nil { + return fmt.Errorf("roster: the pinned copy at %s could not be read: %w", site.RosterFile, err) + } + fmt.Fprintf(out, "roster: %d row(s) pinned in %s\n", len(pinned), site.RosterFile) + + fetched, err := fetch() + switch { + case errors.Is(err, ErrNotPublished): + fmt.Fprintf(out, " OFF, nothing answered at %s, so this run compared nothing\n", Published) + fmt.Fprintf(out, " what ends this state is that file being published; until then the copy in this tree is held to nothing\n") + return fmt.Errorf("roster: no file is published at %s, so this run compared nothing and is not a run that found the copy current", Published) + case err != nil: + fmt.Fprintf(out, " UNRESOLVED, %s could not be read: %v\n", Published, err) + return fmt.Errorf("roster: the published file could not be read, and a copy that was not compared is not a current one") + } + + published, err := Rows(fetched) + if err != nil { + fmt.Fprintf(out, " UNRESOLVED, what %s answered %v\n", Published, err) + return fmt.Errorf("roster: the published file could not be read, and a copy that was not compared is not a current one") + } + + differing := Compare(pinned, published) + for _, d := range differing { + fmt.Fprintf(out, " %s: BEHIND, %s\n", d.ID, d.Says) + } + + fmt.Fprintf(out, "%d row(s) pinned, %d published, %d differing.\n", len(pinned), len(published), len(differing)) + if len(differing) > 0 { + return fmt.Errorf("roster: %d row(s) differ from what is published, and %s is the authority for them", len(differing), Published) + } + return nil +} + +// Publisher is the fetcher the scheduled run uses. A plain GET over a file +// somebody else publishes, which is why this needs no client library and no +// dependency. +func Publisher() ([]byte, error) { + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Get(Published) + if err != nil { + return nil, err + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusNotFound { + return nil, ErrNotPublished + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("%s answered %s", Published, resp.Status) + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 4<<20)) + if err != nil { + return nil, err + } + if len(bytes.TrimSpace(body)) == 0 { + return nil, fmt.Errorf("%s answered with an empty body", Published) + } + return body, nil +} diff --git a/internal/freshness/freshness_test.go b/internal/freshness/freshness_test.go new file mode 100644 index 0000000..f118b8a --- /dev/null +++ b/internal/freshness/freshness_test.go @@ -0,0 +1,228 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +// The suite over the comparison between the pinned roster and the published +// one. +// +// No case reaches the network. The door is a parameter, and every case below +// hands the run something it wrote itself, including the two failures that +// cannot be produced on demand from a real address: nothing published there, and +// a fetch that did not come back. +package freshness + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Flowfin/site/internal/roster" + "github.com/Flowfin/site/internal/site" +) + +const twoRows = `[ + {"id":"alpha","repository":"Flowfin/jellyfin-plugin-alpha","summary":"What alpha does","state":"build-up"}, + {"id":"beta","repository":"Flowfin/jellyfin-plugin-beta","summary":"What beta does","state":"shell"} +]` + +// tree writes a root carrying the pinned copy and returns it. +func tree(t *testing.T, pinned string) string { + t.Helper() + root := t.TempDir() + name := filepath.Join(root, filepath.FromSlash(site.RosterFile)) + if err := os.MkdirAll(filepath.Dir(name), 0o755); err != nil { + t.Fatalf("preparing the tree: %v", err) + } + if err := os.WriteFile(name, []byte(pinned), 0o644); err != nil { + t.Fatalf("writing the pinned copy: %v", err) + } + return root +} + +func answers(body string) Fetcher { + return func() ([]byte, error) { return []byte(body), nil } +} + +// The neighbour of every case below: two files that agree pass, and the run +// says what it compared rather than only that it was happy. +func TestACopyLevelWithWhatIsPublishedPasses(t *testing.T) { + var log strings.Builder + if err := Run(tree(t, twoRows), answers(twoRows), &log); err != nil { + t.Fatalf("the run refused two files that agree: %v\n%s", err, log.String()) + } + if !strings.Contains(log.String(), "2 row(s) pinned, 2 published, 0 differing.") { + t.Errorf("the run does not say what it compared; it said:\n%s", log.String()) + } +} + +// Each shape a difference arrives in, one at a time, and the run names the row +// rather than saying the files differ. The three field cases are separate +// because a message that named the row and not the field would send the next +// reader back to diff two files by hand. +func TestADifferenceRedsTheRunAndNamesTheRow(t *testing.T) { + for name, tc := range map[string]struct{ published, says string }{ + "a sentence somebody edited where it is published": { + strings.Replace(twoRows, "What alpha does", "What alpha really does", 1), + `alpha: BEHIND`}, + "a state that moved where it is published": { + strings.Replace(twoRows, `"state":"shell"`, `"state":"build-up"`, 1), + `this tree says the state is "shell" and the published file says "build-up"`}, + "a repository that moved where it is published": { + strings.Replace(twoRows, `"repository":"Flowfin/jellyfin-plugin-beta"`, + `"repository":"Elsewhere/jellyfin-plugin-beta"`, 1), + `this tree says the repository is "Flowfin/jellyfin-plugin-beta" and the published file says "Elsewhere/jellyfin-plugin-beta"`}, + "a row published that this tree does not carry": { + strings.Replace(twoRows, "\n]", + `,{"id":"gamma","repository":"Flowfin/jellyfin-plugin-gamma","summary":"What gamma does","state":"shell"}]`, 1), + "the published file carries this row and this tree does not"}, + "a row this tree carries that is not published": { + `[{"id":"alpha","repository":"Flowfin/jellyfin-plugin-alpha","summary":"What alpha does","state":"build-up"}]`, + "this tree carries this row and the published file does not"}, + } { + var log strings.Builder + err := Run(tree(t, twoRows), answers(tc.published), &log) + if err == nil { + t.Errorf("%s: the run passed:\n%s", name, log.String()) + continue + } + if !strings.Contains(err.Error(), "differ from what is published") { + t.Errorf("%s: the run failed with %q, which is not the difference", name, err) + } + if !strings.Contains(log.String(), tc.says) { + t.Errorf("%s: the run does not name what differs; it said:\n%s", name, log.String()) + } + } +} + +// A sentence edited on one side names the row it is about, which the case above +// asserts only loosely for that one shape. It is separated because the sentence +// is the field most likely to be edited where it is published and least likely +// to be noticed here. +func TestAnEditedSentenceNamesTheRowAndBothSentences(t *testing.T) { + published := strings.Replace(twoRows, "What alpha does", "What alpha really does", 1) + + var log strings.Builder + if err := Run(tree(t, twoRows), answers(published), &log); err == nil { + t.Fatalf("the run passed over an edited sentence:\n%s", log.String()) + } + for _, want := range []string{ + "alpha: BEHIND", + `this tree says the sentence is "What alpha does"`, + `the published file says "What alpha really does"`, + } { + if !strings.Contains(log.String(), want) { + t.Errorf("the run does not say %q; it said:\n%s", want, log.String()) + } + } +} + +// A fetch that did not come back reds the run rather than passing it. A copy +// that was not compared is not a current one, and this is the failure the whole +// comparison exists against. +func TestAFetchThatFailedRedsTheRun(t *testing.T) { + boom := errors.New("dial tcp: lookup failed") + + var log strings.Builder + err := Run(tree(t, twoRows), func() ([]byte, error) { return nil, boom }, &log) + if err == nil { + t.Fatalf("the run passed over a fetch that failed:\n%s", log.String()) + } + if !strings.Contains(err.Error(), "could not be read") { + t.Errorf("the run failed with %q, which does not say the file was not read", err) + } + if !strings.Contains(log.String(), "UNRESOLVED") || !strings.Contains(log.String(), boom.Error()) { + t.Errorf("the run does not report the failure it met; it said:\n%s", log.String()) + } +} + +// Nothing published at the address reds the run and reports it as off, which is +// the state this comparison is in today and the one it must not report as +// green. It is a state of its own rather than a fetch failure, because it is +// the one of the two that ends by somebody publishing a file. +func TestNothingPublishedIsReportedAsOffRatherThanGreen(t *testing.T) { + var log strings.Builder + err := Run(tree(t, twoRows), func() ([]byte, error) { return nil, ErrNotPublished }, &log) + if err == nil { + t.Fatalf("the run passed having compared nothing:\n%s", log.String()) + } + if !strings.Contains(err.Error(), "compared nothing") { + t.Errorf("the run failed with %q, which does not say it compared nothing", err) + } + for _, want := range []string{ + "OFF, nothing answered at " + Published, + "what ends this state is that file being published", + } { + if !strings.Contains(log.String(), want) { + t.Errorf("the run does not say %q; it said:\n%s", want, log.String()) + } + } +} + +// A published file this tree's parser would refuse is still a published file the +// copy has fallen behind, so the comparison reads it leniently and reports the +// difference rather than hiding it behind a second problem. Bytes that are not +// an array of rows at all are the other case, and that one is unresolved. +func TestThePublishedFileIsReadLenientlyAndUnreadableBytesAreUnresolved(t *testing.T) { + // A row the parser refuses: its identifier does not match its repository + // name. The comparison still reports it as a row this tree does not + // carry. + odd := `[{"id":"alpha","repository":"Flowfin/jellyfin-plugin-alpha","summary":"What alpha does","state":"build-up"}, + {"id":"beta","repository":"Flowfin/jellyfin-plugin-beta","summary":"What beta does","state":"shell"}, + {"id":"wishes","repository":"Flowfin/jellyfin-plugin-elsewhere","summary":"What it does","state":"shell"}]` + var log strings.Builder + if err := Run(tree(t, twoRows), answers(odd), &log); err == nil { + t.Fatalf("the run passed over a published row this tree does not carry:\n%s", log.String()) + } + if !strings.Contains(log.String(), "wishes: BEHIND") { + t.Errorf("the run does not name the published row; it said:\n%s", log.String()) + } + + var second strings.Builder + err := Run(tree(t, twoRows), answers(`{"id":"alpha"}`), &second) + if err == nil { + t.Fatalf("the run passed over bytes that are not a roster:\n%s", second.String()) + } + if !strings.Contains(second.String(), "UNRESOLVED") { + t.Errorf("the run does not report the read as unresolved; it said:\n%s", second.String()) + } +} + +// A tree with no pinned copy is refused rather than compared against nothing. +func TestATreeWithNoPinnedCopyIsRefused(t *testing.T) { + var log strings.Builder + err := Run(t.TempDir(), answers(twoRows), &log) + if err == nil { + t.Fatal("the run passed over a tree carrying no pinned copy") + } + if !strings.Contains(err.Error(), site.RosterFile) { + t.Errorf("the run failed with %q, which does not name the copy it could not read", err) + } +} + +// What the comparison leaves alone, in the direction that costs if it is wrong. +// The order of the rows is what the site presents them in and is not a fact the +// two files have to agree about row by row, so a published file carrying the +// same rows in another order is not behind. +func TestTheOrderOfTheRowsIsNotADifference(t *testing.T) { + reversed := `[ + {"id":"beta","repository":"Flowfin/jellyfin-plugin-beta","summary":"What beta does","state":"shell"}, + {"id":"alpha","repository":"Flowfin/jellyfin-plugin-alpha","summary":"What alpha does","state":"build-up"} + ]` + var log strings.Builder + if err := Run(tree(t, twoRows), answers(reversed), &log); err != nil { + t.Fatalf("the run refused the same rows in another order: %v\n%s", err, log.String()) + } +} + +// Compare over the rows themselves, so the shapes above are held to the +// function rather than to the wording of a run. +func TestCompareReportsInIdentifierOrder(t *testing.T) { + pinned := []roster.Entry{{ID: "zulu"}, {ID: "alpha"}, {ID: "mike"}} + got := Compare(pinned, nil) + if len(got) != 3 { + t.Fatalf("Compare reported %d difference(s) for three rows against none: %v", len(got), got) + } + if got[0].ID != "alpha" || got[1].ID != "mike" || got[2].ID != "zulu" { + t.Errorf("Compare reported %v, which is not identifier order", got) + } +} diff --git a/main.go b/main.go index 6fb000d..8661f50 100644 --- a/main.go +++ b/main.go @@ -17,6 +17,7 @@ import ( "github.com/Flowfin/site/internal/blockers" "github.com/Flowfin/site/internal/bom" "github.com/Flowfin/site/internal/changelog" + "github.com/Flowfin/site/internal/freshness" "github.com/Flowfin/site/internal/gate" "github.com/Flowfin/site/internal/hygiene" "github.com/Flowfin/site/internal/invariant" @@ -102,6 +103,18 @@ func run(args []string, out, errOut io.Writer) error { // when this tree does. What the build needs from the copy is // decided on every run, because the build reads it. return tokens.Run(".", tokens.Publisher, out) + case "roster": + if len(args) != 1 { + usage(errOut) + return errors.New("roster takes no argument") + } + // Deliberately not a leg of the gate, for the reason the pins and + // tokens comparisons beside it are not: what it reads is a file + // published somewhere else, so its verdict moves when somebody + // else commits rather than when this tree changes. The half that + // needs no network is the build, which reads the pinned copy on + // every run. + return freshness.Run(".", freshness.Publisher, out) case "releases": if len(args) != 1 { usage(errOut) @@ -182,6 +195,9 @@ func usage(w io.Writer) { that publishes it, and report without writing anything go run . tokens compare the pinned copy in `+tokens.File+` against the published token file, naming every value that differs + go run . roster compare the pinned roster against the published one, naming + every row that differs, and report OFF rather than green + where nothing is published to compare against go run . releases ask each repository the roster names what it has published and write `+releases.File+`, which is what the shipping state