diff --git a/health-undeclared b/health-undeclared index 0383faf4..9ce896e4 100644 --- a/health-undeclared +++ b/health-undeclared @@ -1,4 +1,4 @@ -59 +58 The long-running resources of this catalogue that do not say how they are ready (`health`, novox/hq ADR 0240 rule 8), as `module check` counts them. It may only go down: merge-check.sh fails a change that raises it, and one that lowers it writes the new number on the first line. From 2026-11-18, or once it is 0, a long-running diff --git a/modules/gitea/merge.ts b/modules/gitea/merge.ts index 09bdcee6..1bac4620 100644 --- a/modules/gitea/merge.ts +++ b/modules/gitea/merge.ts @@ -12,7 +12,7 @@ import { mergeableUnder, newestByContext } from "./checks.js"; import { MERGE_METHODS, Refused, checkName } from "./guard.js"; import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "./pulls.js"; import { approvedHeadsOf, groupRefusal, memberKey, namesSiblings, notApproved, readSiblings, siblingOutOfDate, siblingsFile } from "./siblings.js"; -import { announcementOf, askedWords, outOfDateWords, sameCommit, tookFile, tookWords, twelve, verdictAtTrunk } from "./trunk.js"; +import { announcementOf, askedWords, outOfDateWords, sameCommit, tookFile, tookWords, twelve, verdictAtTrunk, waitsForComposed } from "./trunk.js"; /** A head named for a merge: at least 12 hex characters of its sha (novox/hq issue 381), up to a sha-256 commit's 64. */ const REVIEWED_HEAD = /^[0-9a-f]{12,64}$/; @@ -117,6 +117,14 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition : at.judged ? `was judged against trunk ${twelve(at.judged)}` : "names no trunk commit it was judged against"; const took = tookWords(tookFile(deps.stateDir), `${owner}/${repo}`); const head = twelve(pull.head_sha), now = twelve(trunkHead); + // **A member that waits for its group's composed check is not announced again** (novox/hq issue 1106): its own + // check can only say "composed" once more, so an ask here brought a verdict naming no trunk and the next merge was + // refused the same way for ever. The composed check is mesh-delivery's to ask; this says whose verdict is awaited. + if (waitsForComposed(checks[CHECK_CONTEXT])) { + throw new Refused(`${owner}/${repo}#${number} is not merged: its merge gate ${checks[CHECK_CONTEXT]?.description}; the ` + + `verdict that names its trunk commit is its group's composed check's, and mesh-delivery asks it, this tool asks no check ` + + `(main is now ${now}). Ask again once the group's composed check has passed. Nothing was merged`); + } if (at.onItsWay) { throw new Refused(`${owner}/${repo}#${number} is not merged: its verdict ${judged}, and ${pull.base} is now ` + `${now}. Its check is already asked and not yet answered (${checks[CHECK_CONTEXT]?.description}), so it is not ` + diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index 4e01f415..cdd6a7ff 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -226,6 +226,10 @@ function repoWrong(repo: Layer | undefined, facts: RepoCheckFacts): boolean { return !!repo && repoStatus(repo, facts).state !== "success"; } +/** What a group member's gate says while the group's composed check is the one that judges it: the member's own check can say + * nothing more of it, so asking that check again brings no verdict that names a trunk (novox/hq issue 1106). */ +export const WAITS_FOR_COMPOSED = "waits for its group's composed check"; + /** The forge's status for the gate. */ export function statusFor(c: Checked): CommitStatus { const gate = c.gate ?? { verdict: c.verdict, summary: c.summary }; @@ -260,7 +264,7 @@ export function statusesFor(c: Checked, facts: RepoCheckFacts = {}, gateNow?: st // set. The gate stays pending, saying it waits for the group, unless a composed verdict already stands on it. const out: CommitStatus[] = []; if (!(gateNow ?? "").includes(" composed: ")) { - out.push({ state: "pending", context: CHECK_CONTEXT, description: "waits for its group's composed check" }); + out.push({ state: "pending", context: CHECK_CONTEXT, description: WAITS_FOR_COMPOSED }); } const repo = c["repo-check"]; if (repo) out.push(judgedFirst(c, repoStatus(repo, facts))); diff --git a/modules/gitea/test/guard.test.ts b/modules/gitea/test/guard.test.ts index a654393a..c7605f3c 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -339,6 +339,30 @@ test("gitea_merge_pull_request refuses a verdict judged against an earlier trunk assert.equal(state.emitted.length, 4, "a merge asks no check"); }); +// **A group member whose gate waits for its group's composed check is not announced again by the merge tool** (novox/hq issue +// 1106): the member's own check can only say "composed" again, so each refusal's ask brought a verdict that names no trunk and +// the next merge was refused the same way, for ever. The tool says whose verdict is awaited and asks nothing. +test("gitea_merge_pull_request asks no check of a member that waits for its group's composed check", async () => { + const merge = () => tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED }); + state.trunk = TRUNK; + const before = state.emitted.length; + state.statuses = [{ context: "mesh/merge-gate", state: "pending", description: "waits for its group's composed check" }, + { context: "mesh/repo-check", state: "success", description: "pass: y" }]; + for (let i = 0; i < 3; i++) { + await assert.rejects(merge(), (err: Error) => { + assert.match(err.message, /waits for its group's composed check/); + assert.match(err.message, /mesh-delivery asks it, this tool asks no check/); + assert.match(err.message, /Nothing was merged/); + return true; + }); + } + assert.equal(state.emitted.length, before, "no check is announced again and again"); + assert.equal(state.merged, false); + // Once the composed verdict names the member's unmoved trunk, it merges. + state.statuses = judgedAt(TRUNK); + assert.equal(((await merge()) as Record).merged, true); +}); + // A merge lands only the head that was reviewed (novox/hq issue 381): mesh-controller#197 was reviewed at one head, // force-pushed, checked green at the new one, and merged there, because the tool merged whatever the head was. test("gitea_merge_pull_request merges only the head it is named, and refuses one that is not the head", async () => { diff --git a/modules/gitea/trunk.ts b/modules/gitea/trunk.ts index 1555fab1..e2ed8572 100644 --- a/modules/gitea/trunk.ts +++ b/modules/gitea/trunk.ts @@ -17,7 +17,7 @@ import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; import { dirname } from "node:path"; import type { CommitStatus, GiteaClient, GiteaPull, HeadCheck } from "./client.js"; -import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "./pulls.js"; +import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, WAITS_FOR_COMPOSED } from "./pulls.js"; /** A commit as a verdict names it: its first twelve characters. */ export function twelve(sha: string | undefined): string { @@ -58,6 +58,11 @@ export function checkOnItsWay(gate: HeadCheck | undefined): boolean { return d.startsWith("waiting in the build queue") || d.startsWith("building on ") || d.startsWith("checking again"); } +/** Whether a gate says it waits for its group's composed check. */ +export function waitsForComposed(gate: HeadCheck | undefined): boolean { + return !!gate && gate.state === "pending" && (gate.description ?? "").startsWith(WAITS_FOR_COMPOSED); +} + /** * The statuses a trunk's move puts back to pending on one open pull request's head, from what the forge holds on it * now: none when it has no merge check yet, when a check is pending (on its way, or already out of date — its diff --git a/modules/mailu/README.md b/modules/mailu/README.md index e87c4f61..eceeef7d 100644 --- a/modules/mailu/README.md +++ b/modules/mailu/README.md @@ -16,6 +16,18 @@ installation's are settings on the assignment, `settings set mailu `: header from — the address the proxy forwards with. The front's own hostname is the name of its `web` route, told to it by the mesh. +## Mail client autoconfiguration + +`mailu-automx` answers what mail clients ask for their settings (Thunderbird, Outlook, Apple) for the domains in +`AMX_MAIL_DOMAINS`, naming the IMAP and SMTP servers and ports of mailu's environment (`AMX_IMAP_ADDRESS`, +`AMX_IMAP_PORT`, `AMX_SMTP_ADDRESS`, `AMX_SMTP_PORT`). It is `cmd/automx`, a Go program of the standard library +only in this module's `go.mod` (ADR 0480 clauses 1, 2, 9). It runs in the mirrored alpine image the route proxy's +`trust` step runs, from the `program-automx` archive mounted read-only at `/opt/mesh`; the container is restarted +with the archive and with `mailu.env`, which the program reads at each start. It keeps no database. The file the +Python automx kept in `data-automx` is left where it is on the machine, not deleted by the mesh (ADR 0480 +clause 10). The answers it gave before are the fixtures in `cmd/automx/testdata/captured`, with how they were +taken (`CAPTURED`) and what the program answers differently (`DIFFERENCES`). + ## Files it does not know Each directory or archive this module declares says how it treats a file it did not write (ADR 0422, ADR 0416). @@ -23,7 +35,7 @@ Each directory or archive this module declares says how it treats a file it did - `${dir:mesh-state}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:state}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:grants}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). -- `${dir:data-automx}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- the archive `program-automx` (`${dir:state}/automx`): everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:data-certs}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:data-data}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:data-dkim}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). diff --git a/modules/mailu/automx.ts b/modules/mailu/automx.ts deleted file mode 100644 index 1512f0b9..00000000 --- a/modules/mailu/automx.ts +++ /dev/null @@ -1,87 +0,0 @@ -// What the autoconfiguration module (automx2) holds, as stored: its provider, server, domain and server_domain -// rows, read from its SQLite file inside its container with the sqlite3 program the image carries, opened -// read-only. Nothing is written and nothing is interpreted: the rows are the evidence for what automx answers -// to mail clients (novox/hq issue 1630, ADR 0480), and the reason the answer may be empty. - -import type { Exec } from "./health.js"; - -export const AUTOMX_DATABASE = "/data/db.sqlite"; - -/** The tables read, each with its columns and its ordering. Fixed text: the tool takes no query from its caller. */ -export const AUTOMX_TABLES: Record = { - provider: { columns: ["id", "name", "short_name"], order: "id" }, - server: { columns: ["id", "prio", "name", "port", "type", "socket_type", "user_name", "authentication"], order: "id" }, - domain: { columns: ["id", "name", "provider_id", "ldapserver_id"], order: "id" }, - server_domain: { columns: ["server_id", "domain_id"], order: "domain_id, server_id" }, -}; - -/** - * The one statement that reads a table. The sqlite3 in the mailu-automx image does not know `-json` (it answers - * "sqlite3: Error: unknown option: -json", issue 1630), so each column is printed as `:`, which - * the oldest sqlite3 prints in its default list mode: no quoting, no separator or newline inside a cell, and a NULL - * (`null:`) apart from an empty text (`text:`). - */ -export function selectOf(table: string): string { - const t = AUTOMX_TABLES[table]; - return `SELECT ${t.columns.map((c) => `typeof(${c}) || ':' || hex(${c})`).join(", ")} FROM ${table} ORDER BY ${t.order}`; -} - -export type Row = Record; - -export type AutomxRows = { - database: string; - tables: Record; - /** servers whose name is empty: the client is told an empty host name for them */ - empty_names: { id: number; type: string }[]; - /** tables that could not be read, each with what sqlite3 or docker said on one line */ - unreadable: Record; -}; - -/** One cell as selectOf prints it: `:`; text is the bytes read as UTF-8, a number is read from its text. */ -function cellOf(cell: string): string | number | null { - const at = cell.indexOf(":"); - if (at < 0 || cell.length - at - 1 > 0 && !/^([0-9A-Fa-f]{2})+$/.test(cell.slice(at + 1))) throw new Error(`sqlite3 printed ${JSON.stringify(cell.slice(0, 40))}, not a cell`); - const type = cell.slice(0, at); - const bytes = Buffer.from(cell.slice(at + 1), "hex").toString("utf8"); - switch (type) { - case "null": return null; - case "integer": case "real": return Number(bytes); - default: return bytes; - } -} - -/** sqlite3 prints one line per row, cells apart by `|`, and nothing at all for a table without rows. */ -export function parseRows(table: string, output: string): Row[] { - const columns = AUTOMX_TABLES[table].columns; - const rows: Row[] = []; - for (const line of output.split("\n")) { - if (line.trim() === "") continue; - const cells = line.replace(/\r$/, "").split("|"); - if (cells.length !== columns.length) throw new Error(`sqlite3 printed ${cells.length} cells for ${columns.length} columns`); - rows.push(Object.fromEntries(columns.map((c, i) => [c, cellOf(cells[i])]))); - } - return rows; -} - -/** The command that reads one table: read-only, one fixed statement. */ -export function readCommand(table: string): string[] { - return ["sqlite3", "-readonly", AUTOMX_DATABASE, selectOf(table)]; -} - -/** Read every table; one that fails is said in `unreadable` and does not hide the others. */ -export async function readAutomx(exec: Exec, container: string): Promise { - const answer: AutomxRows = { database: AUTOMX_DATABASE, tables: {}, empty_names: [], unreadable: {} }; - for (const table of Object.keys(AUTOMX_TABLES)) { - try { - answer.tables[table] = parseRows(table, await exec(container, readCommand(table))); - } catch (e) { - const err = e as { stderr?: string; message?: string }; - const text = (err?.stderr ?? "").trim() || (err?.message ?? String(e)).trim(); - answer.unreadable[table] = text.split("\n").map((l) => l.trim()).filter(Boolean)[0] ?? "it failed and said nothing"; - } - } - for (const s of answer.tables.server ?? []) { - if (String(s.name ?? "").trim() === "") answer.empty_names.push({ id: Number(s.id), type: String(s.type) }); - } - return answer; -} diff --git a/modules/mailu/automx/Dockerfile b/modules/mailu/automx/Dockerfile deleted file mode 100644 index c48cab29..00000000 --- a/modules/mailu/automx/Dockerfile +++ /dev/null @@ -1,44 +0,0 @@ -# automx2 — the autoconfig/autodiscover responder, carried by the mailu module as its own -# artifact: it is a config-baked sidecar of this mail server, not a standalone application -# (novox/hq ADR 0015 draws that line at applications). -# -# The base is named rather than pinned (novox/hq issue 044): declared in module.json's -# `build.on`. The build context is the module's own directory; every ADD says so. -ARG PYTHON_BASE - -FROM ${PYTHON_BASE} -RUN apk add --no-cache bash sqlite -WORKDIR /automx2 - -ADD automx/files/setupvenv.sh /automx2/setupvenv.sh -ADD automx/files/start /automx2/start -ADD automx/files/setup /automx2/setup -ADD automx/files/setup-db /automx2/setup-db -ADD automx/files/add-domains /automx2/add-domains -RUN chmod u+x setupvenv.sh start add-domains setup setup-db - -RUN ./setupvenv.sh \ - && . .venv/bin/activate \ - && pip install automx2==2021.6 - -# The launcher `start` expects. In the predecessor's image this wrapper appeared during a build -# step that never made it into the files this module carries — the image worked and the recipe -# could not reproduce it. Written here explicitly, verbatim from the proven image, so the build -# is the whole truth about the image again. -RUN mkdir -p .venv/scripts && printf '%s\n' \ - '#!/usr/bin/env bash' \ - 'set -euo pipefail' \ - '. .venv/bin/activate' \ - "export FLASK_ENV='production'" \ - "export FLASK_APP='automx2.server:app'" \ - 'flask "$@"' > .venv/scripts/flask.sh && chmod +x .venv/scripts/flask.sh - -ENV AUTOMX2_CONF=/etc/automx2.conf -ADD automx/files/automx2.conf /etc/automx2.conf - -# VOLUME deliberately absent: the anonymous /data volume is exactly what lost db.sqlite on -# every recreate (measured on novox 2026-08-10). The manifest binds a real directory instead. -ENTRYPOINT ["/bin/sh"] -CMD ["./start"] - -EXPOSE 4243 diff --git a/modules/mailu/automx/files/add-domains b/modules/mailu/automx/files/add-domains deleted file mode 100644 index e4136236..00000000 --- a/modules/mailu/automx/files/add-domains +++ /dev/null @@ -1,49 +0,0 @@ -#!/usr/bin/env bash -set -e - -echo "${MAIL_DOMAINS}" - -# Split domains into array -IFS=', ' read -r -a array <<< "${AMX_MAIL_DOMAINS}" - -# User configurable section -- START -PROVIDER_ID=001 -SQL_CMD=""; - -# Iterate domains resulting from split on second arg -for element in "${array[@]}" -do - # Set vars - DOMAIN=$element - PROVIDER_NAME=$DOMAIN - PROVIDER_SHORTNAME=$DOMAIN - - # Optional LDAP server - #LDAP_SERVER="ldap.${DOMAIN}" - # User configurable section -- END - s1_id=$((PROVIDER_ID + 1)) - s2_id=$((PROVIDER_ID + 2)) - s3_id=$((PROVIDER_ID + 3)) - dom_id=$((PROVIDER_ID + 4)) - - s3_id='NULL' - - SQL_CMD=$(cat <&2 "Directory '${dir}' already exists, exiting." - exit 1 -fi -python3 -m venv "${dir}" -source "${dir}/bin/activate" - -set +e -pip install -U pip setuptools wheel || true - -#set -e -## vim:tabstop=4:noexpandtab -## -## Creates a Python 3 virtual environment. The target directory can be passed -## as a parameter. The default path is 'venv' in the current directory. -# -#dir="${1:-venv}" -# -#set -e -#if [ -d "${dir}" ]; then -# echo "Directory '${dir}' already exists, exiting." >&2 -# exit 1 -#fi -#python3 -m venv "${dir}" -#. "${dir}/bin/activate" -# -#set +e -#pip install -U pip setuptools || true diff --git a/modules/mailu/automx/files/start b/modules/mailu/automx/files/start deleted file mode 100644 index d23943b6..00000000 --- a/modules/mailu/automx/files/start +++ /dev/null @@ -1,8 +0,0 @@ -#!/usr/bin/env bash -set -e - -# Setup -./setup - -# Start -./.venv/scripts/flask.sh run --host=0.0.0.0 --port=4243 diff --git a/modules/mailu/cmd/automx/automx_test.go b/modules/mailu/cmd/automx/automx_test.go new file mode 100644 index 00000000..ab0ab5b6 --- /dev/null +++ b/modules/mailu/cmd/automx/automx_test.go @@ -0,0 +1,260 @@ +package main + +import ( + "bytes" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "sort" + "strconv" + "strings" + "testing" +) + +// The environment mailu gives automx on novox when the fixtures were captured (files/mailu.env): the mail domain, +// the route name for both servers, the ports. +var capturedEnvironment = map[string]string{ + "AMX_MAIL_DOMAINS": "novox.be", + "AMX_IMAP_ADDRESS": "mail.novox.be", + "AMX_IMAP_PORT": "143", + "AMX_SMTP_ADDRESS": "mail.novox.be", + "AMX_SMTP_PORT": "587", + "AUTOMX_PORT": "4243", +} + +func getenvOf(m map[string]string) func(string) string { + return func(k string) string { return m[k] } +} + +func testServer(t *testing.T) *httptest.Server { + t.Helper() + cfg, err := configFrom(getenvOf(capturedEnvironment)) + if err != nil { + t.Fatal(err) + } + s := httptest.NewServer(newHandler(cfg)) + t.Cleanup(s.Close) + return s +} + +const captured = "testdata/captured" + +// A captured request: its first line is the method and the URL the client used, then the headers sent, then after +// a blank line the body sent (CAPTURED, how the files were written). +type request struct { + method, url string + header [][2]string + body string +} + +func readRequest(t *testing.T, stem string) request { + t.Helper() + raw, err := os.ReadFile(stem + ".request") + if err != nil { + t.Fatal(err) + } + head, body, hasBody := strings.Cut(string(raw), "\n\n") + if !hasBody { + head = strings.TrimSuffix(head, "\n") + } + lines := strings.Split(head, "\n") + method, target, ok := strings.Cut(lines[0], " ") + if !ok { + t.Fatalf("%s: first line %q is not METHOD URL", stem, lines[0]) + } + r := request{method: method, url: target, body: body} + for _, l := range lines[1:] { + k, v, ok := strings.Cut(l, ": ") + if !ok { + t.Fatalf("%s: header line %q", stem, l) + } + r.header = append(r.header, [2]string{k, v}) + } + return r +} + +func readFile(t *testing.T, name string) string { + t.Helper() + b, err := os.ReadFile(name) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// A fixture asks for its body to differ only where DIFFERENCES says so: a stem with a .corrected.body is answered +// with that, and its Content-Length is the corrected body's. +func expectedBody(t *testing.T, stem string) (body string, corrected bool) { + if _, err := os.Stat(stem + ".corrected.body"); err == nil { + return readFile(t, stem+".corrected.body"), true + } + return readFile(t, stem+".body"), false +} + +func TestEveryCapturedRequestIsAnsweredAsAutomxAnswered(t *testing.T) { + stems, _ := filepath.Glob(filepath.Join(captured, "*.request")) + if len(stems) < 17 { + t.Fatalf("%d captured requests found, want the 17 of issue 1630", len(stems)) + } + srv := testServer(t) + client := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} + for _, f := range stems { + stem := strings.TrimSuffix(f, ".request") + t.Run(filepath.Base(stem), func(t *testing.T) { + rq := readRequest(t, stem) + u, err := url.Parse(rq.url) + if err != nil { + t.Fatal(err) + } + // The mesh's route reaches the program with the client's name and scheme; the program is told them in + // the forwarding headers (the Location of fixture 14 holds them). + req, err := http.NewRequest(rq.method, srv.URL+u.RequestURI(), strings.NewReader(rq.body)) + if err != nil { + t.Fatal(err) + } + for _, h := range rq.header { + req.Header.Set(h[0], h[1]) + } + req.Host = u.Host + req.Header.Set("X-Forwarded-Proto", u.Scheme) + resp, err := client.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + got, _ := io.ReadAll(resp.Body) + + if want := strings.TrimRight(readFile(t, stem+".status"), "\n"); resp.Status != want { + t.Errorf("status %q, captured %q", resp.Status, want) + } + wantBody, corrected := expectedBody(t, stem) + if !bytes.Equal(got, []byte(wantBody)) { + t.Errorf("body\n%q\nwant\n%q", got, wantBody) + } + want := map[string]string{} + for _, l := range strings.Split(strings.TrimRight(readFile(t, stem+".headers"), "\n"), "\n") { + if k, v, ok := strings.Cut(l, ": "); ok { + want[k] = v + } + } + if corrected { + want["Content-Length"] = strconv.Itoa(len(wantBody)) + } + // Server names the Python program that answered (DIFFERENCES); Date changes on every request. + delete(want, "Server") + var extra []string + for k := range resp.Header { + if k == "Date" || k == "Server" { + continue + } + if _, ok := want[k]; !ok { + extra = append(extra, k) + } + } + sort.Strings(extra) + if len(extra) > 0 { + t.Errorf("headers the capture does not have: %v", extra) + } + for k, v := range want { + if g := resp.Header.Get(k); g != v { + t.Errorf("header %s: %q, captured %q", k, g, v) + } + } + }) + } +} + +// DIFFERENCES: the one change clause 9 of ADR 0480 allows to an answer is the IMAP server's name, written from +// AMX_IMAP_ADDRESS where automx wrote an empty one. +func TestTheOnlyCorrectedAnswerIsTheImapServersName(t *testing.T) { + corrected, _ := filepath.Glob(filepath.Join(captured, "*.corrected.body")) + if len(corrected) != 3 { + t.Fatalf("%d corrected answers, want the three Thunderbird answers (01, 02, 03): %v", len(corrected), corrected) + } + for _, c := range corrected { + orig := readFile(t, strings.TrimSuffix(c, ".corrected.body")+".body") + want := strings.Replace(orig, "", "mail.novox.be", 1) + if strings.Contains(want, "") || want == orig { + t.Fatalf("%s: the capture has no single empty hostname to correct", c) + } + if got := readFile(t, c); got != want { + t.Errorf("%s differs from the capture by more than the IMAP server's name", c) + } + } +} + +func TestTheImapServerIsNamedFromTheImapAddress(t *testing.T) { + env := map[string]string{} + for k, v := range capturedEnvironment { + env[k] = v + } + env["AMX_IMAP_ADDRESS"] = "imap.example.test" + env["AMX_IMAP_PORT"] = "993" + cfg, err := configFrom(getenvOf(env)) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + newHandler(cfg).ServeHTTP(rec, httptest.NewRequest("GET", "/mail/config-v1.1.xml?emailaddress=a@novox.be", nil)) + body := rec.Body.String() + if !strings.Contains(body, `imap.example.test993`) { + t.Errorf("incoming server not named from AMX_IMAP_ADDRESS and AMX_IMAP_PORT: %s", body) + } + if !strings.Contains(body, `mail.novox.be587`) { + t.Errorf("outgoing server changed: %s", body) + } +} + +func TestEveryDomainOfTheEnvironmentIsServed(t *testing.T) { + env := map[string]string{} + for k, v := range capturedEnvironment { + env[k] = v + } + env["AMX_MAIL_DOMAINS"] = "novox.be, example.test" + cfg, err := configFrom(getenvOf(env)) + if err != nil { + t.Fatal(err) + } + h := newHandler(cfg) + for _, d := range []string{"novox.be", "example.test"} { + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest("GET", "/mail/config-v1.1.xml?emailaddress=a@"+d, nil)) + if rec.Code != 200 || !strings.Contains(rec.Body.String(), ""+d+"") { + t.Errorf("%s: %d %s", d, rec.Code, rec.Body.String()) + } + } +} + +// Failure is loud (issue 1631 AC2): a missing variable stops the program and its name is in the message. +func TestAMissingVariableStopsTheProgramNamingIt(t *testing.T) { + for name := range capturedEnvironment { + t.Run(name, func(t *testing.T) { + env := map[string]string{} + for k, v := range capturedEnvironment { + env[k] = v + } + delete(env, name) + if _, err := configFrom(getenvOf(env)); err == nil || !strings.Contains(err.Error(), name) { + t.Fatalf("configFrom without %s: %v", name, err) + } + var out bytes.Buffer + if code := run(getenvOf(env), &out, func(*http.Server) error { t.Fatal("served without its configuration"); return nil }); code == 0 || !strings.Contains(out.String(), name) { + t.Errorf("run without %s: exit %d, said %q", name, code, out.String()) + } + }) + } +} + +func TestAPortThatIsNotANumberIsRefusedNamingIt(t *testing.T) { + env := map[string]string{} + for k, v := range capturedEnvironment { + env[k] = v + } + env["AMX_SMTP_PORT"] = "submission" + if _, err := configFrom(getenvOf(env)); err == nil || !strings.Contains(err.Error(), "AMX_SMTP_PORT") { + t.Fatalf("got %v", err) + } +} diff --git a/modules/mailu/cmd/automx/main.go b/modules/mailu/cmd/automx/main.go new file mode 100644 index 00000000..b7c1732d --- /dev/null +++ b/modules/mailu/cmd/automx/main.go @@ -0,0 +1,290 @@ +// automx answers mail clients' autoconfiguration requests for the domains mailu serves: Thunderbird's +// /mail/config-v1.1.xml, Outlook's /autodiscover/autodiscover.xml and Apple's /mobileconfig. It replaces the +// Python automx2 image (novox/hq ADR 0480 clauses 1, 9, 10): standard library only, no database, every answer +// made from mailu's environment at the start, so a change of the domains, servers or ports reaches clients when +// the program is started again. +// +// The answers are those automx2 2021.6 gave, captured in testdata/captured (CAPTURED, issue 1630), byte for byte, +// but for the one thing testdata/captured/DIFFERENCES lists: the IMAP server is named, from AMX_IMAP_ADDRESS. +package main + +import ( + "encoding/xml" + "errors" + "fmt" + "html" + "io" + "net/http" + "os" + "strconv" + "strings" + "time" +) + +// The environment, all required: a program that cannot say which servers clients use must not run (failure is +// loud). The names are those of mailu's files/mailu.env. +const ( + envDomains = "AMX_MAIL_DOMAINS" + envImapHost = "AMX_IMAP_ADDRESS" + envImapPort = "AMX_IMAP_PORT" + envSmtpHost = "AMX_SMTP_ADDRESS" + envSmtpPort = "AMX_SMTP_PORT" + envListen = "AUTOMX_PORT" +) + +type config struct { + domains []string + imapHost, smtpHost string + imapPort, smtpPort int + listen int +} + +// configFrom reads the environment; it names every variable that is missing or wrong. +func configFrom(getenv func(string) string) (config, error) { + var c config + var problems []string + text := func(name string) string { + v := strings.TrimSpace(getenv(name)) + if v == "" { + problems = append(problems, name+" is not set") + } + return v + } + port := func(name string) int { + v := text(name) + if v == "" { + return 0 + } + n, err := strconv.Atoi(v) + if err != nil || n < 1 || n > 65535 { + problems = append(problems, fmt.Sprintf("%s is %q, not a port number", name, v)) + return 0 + } + return n + } + // automx2's add-domains split the list at commas and spaces. + for _, d := range strings.FieldsFunc(text(envDomains), func(r rune) bool { return r == ',' || r == ' ' }) { + c.domains = append(c.domains, strings.ToLower(d)) + } + c.imapHost, c.imapPort = text(envImapHost), port(envImapPort) + c.smtpHost, c.smtpPort = text(envSmtpHost), port(envSmtpPort) + c.listen = port(envListen) + if len(problems) > 0 { + return config{}, errors.New(strings.Join(problems, "; ")) + } + return c, nil +} + +// What automx2 sent (Werkzeug), copied from the capture. +const ( + htmlType = "text/html; charset=utf-8" + xmlType = "application/xml" + + notFound = ` + +404 Not Found +

Not Found

+

The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again.

+` + notAllowed = ` + +405 Method Not Allowed +

Method Not Allowed

+

The method is not allowed for the requested URL.

+` + internalError = ` + +500 Internal Server Error +

Internal Server Error

+

The server encountered an internal error and was unable to complete your request. Either the server is overloaded or there is an error in the application.

+` + badRequest = ` + +400 Bad Request +

Bad Request

+

The browser (or proxy) sent a request that this server could not understand.

+` + index = `Show Thunderbird-style XML configuration for abc@example.com.` +) + +func reply(w http.ResponseWriter, status int, contentType, body string) { + w.Header().Set("Content-Type", contentType) + if status != http.StatusNoContent { + w.Header().Set("Content-Length", strconv.Itoa(len(body))) + } + w.WriteHeader(status) + if status != http.StatusNoContent { + io.WriteString(w, body) + } +} + +func missing(w http.ResponseWriter, argument string) { + reply(w, http.StatusBadRequest, htmlType, fmt.Sprintf("Missing request argument %q", argument)) +} + +type handler struct{ config } + +func newHandler(c config) http.Handler { return handler{c} } + +func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + var allow string + switch r.URL.Path { + case "/", "/mail/config-v1.1.xml", "/mobileconfig/": + allow = "GET, HEAD, OPTIONS" + case "/autodiscover/autodiscover.xml": + allow = "POST, OPTIONS" + case "/mobileconfig": + h.redirectToSlash(w, r) + return + default: + reply(w, http.StatusNotFound, htmlType, notFound) + return + } + if r.Method == http.MethodOptions { + w.Header().Set("Allow", allow) + reply(w, http.StatusOK, htmlType, "") + return + } + if !strings.Contains(", "+allow+",", ", "+r.Method+",") { + w.Header().Set("Allow", allow) + reply(w, http.StatusMethodNotAllowed, htmlType, notAllowed) + return + } + switch r.URL.Path { + case "/": + reply(w, http.StatusOK, htmlType, index) + case "/mail/config-v1.1.xml": + h.byAddress(w, r.URL.Query().Get("emailaddress"), r.URL.Query().Has("emailaddress"), "emailaddress", h.thunderbird) + case "/mobileconfig/": + h.byAddress(w, r.URL.Query().Get("emailaddress"), r.URL.Query().Has("emailaddress"), "emailaddress", h.apple) + case "/autodiscover/autodiscover.xml": + h.outlook(w, r) + } +} + +// byAddress is the part the three protocols share: the address is required, and a domain that is not served is +// answered with no content. +func (h handler) byAddress(w http.ResponseWriter, address string, present bool, argument string, answer func(http.ResponseWriter, int, string)) { + if !present { + missing(w, argument) + return + } + domain := strings.ToLower(address[strings.LastIndex(address, "@")+1:]) + for i, d := range h.domains { + if strings.Contains(address, "@") && d == domain { + answer(w, i, d) + return + } + } + reply(w, http.StatusNoContent, htmlType, "") +} + +// thunderbird is Mozilla's autoconfiguration, config-v1.1. The provider id is automx2's: the provider rows were +// numbered 1, 11, 21 ... by add-domains. +func (h handler) thunderbird(w http.ResponseWriter, i int, domain string) { + d := html.EscapeString(domain) + server := func(kind, host string, port int) string { + tag := map[string]string{"imap": "incomingServer", "smtp": "outgoingServer"}[kind] + return fmt.Sprintf(`<%s type="%s">%s%dSTARTTLS`+ + `%%EMAILADDRESS%%password-cleartext`, + tag, kind, html.EscapeString(host), port, tag) + } + body := fmt.Sprintf(`%s`+ + `%s%s%s%s`, + 1+10*i, d, d, d, server("imap", h.imapHost, h.imapPort), server("smtp", h.smtpHost, h.smtpPort)) + reply(w, http.StatusOK, xmlType, body) +} + +// apple and outlook answer a served domain as automx2 did: with an error (DIFFERENCES, issue 1691). They are +// kept as captured here, so the move to a Go program changes the answer only where clause 9 says. +func (h handler) apple(w http.ResponseWriter, _ int, _ string) { + reply(w, http.StatusBadRequest, htmlType, badRequest) +} + +// redirectToSlash is Werkzeug's strict-slash redirect, to the name the client used. +func (h handler) redirectToSlash(w http.ResponseWriter, r *http.Request) { + scheme := "http" + if p := strings.TrimSpace(strings.Split(r.Header.Get("X-Forwarded-Proto"), ",")[0]); p != "" { + scheme = p + } + host := r.Host + if f := strings.TrimSpace(strings.Split(r.Header.Get("X-Forwarded-Host"), ",")[0]); f != "" { + host = f + } + target := scheme + "://" + host + "/mobileconfig/" + if r.URL.RawQuery != "" { + target += "?" + r.URL.RawQuery + } + w.Header().Set("Location", target) + e := html.EscapeString(target) + reply(w, http.StatusPermanentRedirect, htmlType, fmt.Sprintf(` + +Redirecting... +

Redirecting...

+

You should be redirected automatically to the target URL: %s. If not, click the link. +`, e, e)) +} + +// outlook reads the request's EMailAddress anywhere in the document, whatever the namespace. A body that is not +// one XML document is automx2's 500. +func (h handler) outlook(w http.ResponseWriter, r *http.Request) { + dec := xml.NewDecoder(io.LimitReader(r.Body, 1<<20)) + var address string + var found, elements, inAddress = false, 0, false + for { + tok, err := dec.Token() + if err == io.EOF { + break + } + if err != nil { + reply(w, http.StatusInternalServerError, htmlType, internalError) + return + } + switch t := tok.(type) { + case xml.StartElement: + elements++ + if t.Name.Local == "EMailAddress" && !found { + inAddress = true + } + case xml.CharData: + if inAddress { + address += string(t) + } + case xml.EndElement: + if inAddress && t.Name.Local == "EMailAddress" { + inAddress, found = false, true + } + } + } + if elements == 0 { + reply(w, http.StatusInternalServerError, htmlType, internalError) + return + } + h.byAddress(w, strings.TrimSpace(address), found, "EMailAddress", func(w http.ResponseWriter, _ int, _ string) { + reply(w, http.StatusInternalServerError, htmlType, internalError) + }) +} + +// run is the program: configuration from the environment, then serving. A configuration that cannot be read is +// said, with the variables' names, and the exit code is not zero. +func run(getenv func(string) string, out io.Writer, serve func(*http.Server) error) int { + c, err := configFrom(getenv) + if err != nil { + fmt.Fprintf(out, "automx: %v\n", err) + return 1 + } + srv := &http.Server{ + Addr: ":" + strconv.Itoa(c.listen), + Handler: newHandler(c), + ReadHeaderTimeout: 10 * time.Second, + } + if err := serve(srv); err != nil { + fmt.Fprintf(out, "automx: %v\n", err) + return 1 + } + return 0 +} + +func main() { + os.Exit(run(os.Getenv, os.Stderr, func(s *http.Server) error { return s.ListenAndServe() })) +} diff --git a/modules/mailu/cmd/automx/manifest_test.go b/modules/mailu/cmd/automx/manifest_test.go new file mode 100644 index 00000000..cf88b181 --- /dev/null +++ b/modules/mailu/cmd/automx/manifest_test.go @@ -0,0 +1,202 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// The digest route-proxy's `trust` artifact runs: one alpine image is held for both (ADR 0480 clause 3). It was +// read from mesh-controller's modules/route-proxy/module.json and is the one hello-web runs. +const alpineDigest = "alpine@sha256:28bd5fe8b56d1bd048e5babf5b10710ebe0bae67db86916198a6eec434943f8b" + +type manifest struct { + Tools []string `json:"tools"` + Resources []map[string]any `json:"resources"` + Listens []struct { + Name string `json:"name"` + Port int `json:"port"` + } `json:"listens"` + Data struct { + Own []struct { + ID string `json:"id"` + Path string `json:"path"` + } `json:"own"` + } `json:"data"` + Build map[string]any `json:"build"` +} + +func readManifest(t *testing.T) manifest { + t.Helper() + var m manifest + if err := json.Unmarshal([]byte(readFile(t, "../../module.json")), &m); err != nil { + t.Fatal(err) + } + return m +} + +func (m manifest) resource(id string) map[string]any { + for _, r := range m.Resources { + if r["id"] == id { + return r + } + } + return nil +} + +func (m manifest) artifact(name string) map[string]any { + arts, _ := m.Build["artifacts"].([]any) + for _, a := range arts { + if a, _ := a.(map[string]any); a["name"] == name { + return a + } + } + return nil +} + +func strs(v any) []string { + var out []string + l, _ := v.([]any) + for _, e := range l { + out = append(out, e.(string)) + } + return out +} + +// Clause 2: a Go bundle from a package of the module's own go.mod, stamped for alpine; an archive resource unpacks +// it; the container mounts that read-only at /opt/mesh, calls the program there and names the archive in restart-on. +func TestAutomxIsAGoBundleForAlpineMountedAtOptMeshAndRestartedWithIt(t *testing.T) { + m := readManifest(t) + b := m.artifact("automx") + if b == nil || b["kind"] != "bundle" || b["language"] != "go" || b["system"] != "alpine" || b["from"] != "cmd/automx" || b["binary"] != "automx" { + t.Fatalf("artifact automx is %v, want a Go bundle, system alpine, from cmd/automx, binary automx", b) + } + if _, loads := b["loads"]; loads { + t.Errorf("a program run in a container is not loaded by the tool runner: %v", b["loads"]) + } + goMod := readFile(t, "../../go.mod") + if !strings.HasPrefix(goMod, "module mailu\n") { + t.Errorf("the module's go.mod is %q; the bundle is a package of it", goMod) + } + if _, err := os.Stat(filepath.Join("..", "..", filepath.FromSlash(b["from"].(string)), "main.go")); err != nil { + t.Errorf("the bundle's package is not in the module's directory: %v", err) + } + + unpack := m.resource("program-automx") + if unpack == nil || unpack["type"] != "archive" || unpack["artifact"] != "automx" || unpack["unknown"] != "program" { + t.Fatalf("resource program-automx is %v, want an archive of the bundle", unpack) + } + where, _ := unpack["path"].(string) + c := m.resource("automx") + if c == nil || c["type"] != "container" { + t.Fatalf("no container automx: %v", c) + } + if got := strs(c["volumes"]); len(got) != 1 || got[0] != where+":/opt/mesh:ro" { + t.Errorf("volumes %v, want the archive's place %q mounted read-only at /opt/mesh", got, where) + } + if got := strs(c["args"]); len(got) != 1 || got[0] != "/opt/mesh/automx" { + t.Errorf("args %v, want the program at /opt/mesh/automx", got) + } + if !contains(strs(c["restart-on"]), "program-automx") { + t.Errorf("restart-on %v does not name the archive: a new program would leave the old one running", c["restart-on"]) + } + if !contains(strs(c["restart-on"]), "config-env") { + t.Errorf("restart-on %v does not name the environment file the program reads at its start", c["restart-on"]) + } +} + +// Clause 3: an upstream artifact pinned by digest, the one route-proxy's trust artifact runs; never an image +// built from a Dockerfile (clause 11), and no Dockerfile or base to build one on. +func TestAutomxRunsTheMirroredAlpineImageAndBuildsNoImage(t *testing.T) { + m := readManifest(t) + c := m.resource("automx") + base := m.artifact(c["artifact"].(string)) + if base == nil || base["kind"] != "upstream" || base["from"] != alpineDigest { + t.Errorf("the container runs artifact %v, want an upstream artifact %s", base, alpineDigest) + } + if _, has := c["image"]; has { + t.Errorf("the container names an image %v, fetched from a registry on the machine", c["image"]) + } + for _, a := range m.Build["artifacts"].([]any) { + if a.(map[string]any)["kind"] == "image" { + t.Errorf("artifact %v is an image built from a Dockerfile (ADR 0452)", a) + } + } + if _, has := m.Build["on"]; has { + t.Errorf("build.on %v: nothing is built on a base image any more", m.Build["on"]) + } + for _, gone := range []string{"../../automx/Dockerfile", "../../automx"} { + if _, err := os.Stat(gone); err == nil { + t.Errorf("%s is still there", gone) + } + } +} + +// Clause 1: the container stays as it was for what watches it. +func TestAutomxKeepsItsContainerNameNetworkPortAndEnvironment(t *testing.T) { + m := readManifest(t) + c := m.resource("automx") + if c["name"] != "mailu-automx" || c["network"] != "mailu" { + t.Errorf("container %v on network %v, want mailu-automx on mailu", c["name"], c["network"]) + } + if got := strs(c["ports"]); len(got) != 1 || got[0] != "4243" { + t.Errorf("ports %v, want 4243", got) + } + if got := strs(c["env-file"]); len(got) != 1 || got[0] != "${dir:state}/mailu.env" { + t.Errorf("env-file %v, want mailu's environment only", got) + } + if !strings.Contains("\n"+readFile(t, "../../files/mailu.env"), "\nAUTOMX_PORT=4243\n") { + t.Errorf("mailu.env does not say AUTOMX_PORT=4243") + } + for _, l := range m.Listens { + if l.Name == "autoconfig" && l.Port == 4243 { + return + } + } + t.Errorf("the autoconfig endpoint is not on 4243: %v", m.Listens) +} + +// Clause 10: a database nobody reads is gone from the declaration (the file stays on disk), and the container says +// whether it answers with one captured request and its captured answer. +func TestAutomxKeepsNoDatabaseAndSaysWhetherItAnswers(t *testing.T) { + m := readManifest(t) + if m.resource("data-automx") != nil { + t.Errorf("the data-automx directory is still declared") + } + for _, it := range m.Data.Own { + if strings.Contains(it.ID, "automx") || strings.Contains(it.Path, "data-automx") { + t.Errorf("data item %v is still declared", it) + } + } + for _, tool := range m.Tools { + if tool == "automx_rows" { + t.Errorf("the tool automx_rows reads a database that no longer exists (issue 1700)") + } + } + h, _ := m.resource("automx")["health"].(map[string]any) + if h["kind"] != "http" || h["endpoint"] != "autoconfig" { + t.Fatalf("health %v, want an http check of the autoconfig endpoint", h) + } + // The request is captured fixture 06 and the answer its status and body. + rq := readRequest(t, filepath.Join(captured, "06-autoconfig-root")) + if u := strings.SplitN(rq.url, "/", 4); "/"+u[len(u)-1] != h["path"] || rq.method != "GET" { + t.Errorf("health path %v is not captured request %s %s", h["path"], rq.method, rq.url) + } + if st := readFile(t, filepath.Join(captured, "06-autoconfig-root.status")); !strings.HasPrefix(st, "200 ") || h["status"] != float64(200) { + t.Errorf("health status %v, captured %q", h["status"], st) + } + if body, _ := h["body"].(string); body == "" || !strings.Contains(readFile(t, filepath.Join(captured, "06-autoconfig-root.body")), body) { + t.Errorf("health body %q is not in the captured answer", h["body"]) + } +} + +func contains(l []string, s string) bool { + for _, e := range l { + if e == s { + return true + } + } + return false +} diff --git a/modules/mailu/cmd/automx/testdata/captured/01-thunderbird-autoconfig-ok.corrected.body b/modules/mailu/cmd/automx/testdata/captured/01-thunderbird-autoconfig-ok.corrected.body new file mode 100644 index 00000000..0adc5c4c --- /dev/null +++ b/modules/mailu/cmd/automx/testdata/captured/01-thunderbird-autoconfig-ok.corrected.body @@ -0,0 +1 @@ +novox.benovox.benovox.bemail.novox.be143STARTTLS%EMAILADDRESS%password-cleartextmail.novox.be587STARTTLS%EMAILADDRESS%password-cleartext \ No newline at end of file diff --git a/modules/mailu/cmd/automx/testdata/captured/02-thunderbird-on-automx-name.corrected.body b/modules/mailu/cmd/automx/testdata/captured/02-thunderbird-on-automx-name.corrected.body new file mode 100644 index 00000000..0adc5c4c --- /dev/null +++ b/modules/mailu/cmd/automx/testdata/captured/02-thunderbird-on-automx-name.corrected.body @@ -0,0 +1 @@ +novox.benovox.benovox.bemail.novox.be143STARTTLS%EMAILADDRESS%password-cleartextmail.novox.be587STARTTLS%EMAILADDRESS%password-cleartext \ No newline at end of file diff --git a/modules/mailu/cmd/automx/testdata/captured/03-thunderbird-on-autodiscover-name.corrected.body b/modules/mailu/cmd/automx/testdata/captured/03-thunderbird-on-autodiscover-name.corrected.body new file mode 100644 index 00000000..0adc5c4c --- /dev/null +++ b/modules/mailu/cmd/automx/testdata/captured/03-thunderbird-on-autodiscover-name.corrected.body @@ -0,0 +1 @@ +novox.benovox.benovox.bemail.novox.be143STARTTLS%EMAILADDRESS%password-cleartextmail.novox.be587STARTTLS%EMAILADDRESS%password-cleartext \ No newline at end of file diff --git a/modules/mailu/cmd/automx/testdata/captured/CAPTURED b/modules/mailu/cmd/automx/testdata/captured/CAPTURED index 88967524..01a9be83 100644 --- a/modules/mailu/cmd/automx/testdata/captured/CAPTURED +++ b/modules/mailu/cmd/automx/testdata/captured/CAPTURED @@ -14,6 +14,13 @@ Each capture is four files with the same stem: The files were written from the tool's answers by a script (not by retyping each by hand); the tool returns JSON, so a later capture that wants exact bytes needs a tool that writes the answer to a file. +A fixture is not the bytes on the wire in four ways (issue 1701): + - Date is left out of .headers (it changes on every request). + - .headers are sorted by name. + - .status holds the tool's status line without the protocol (the tool reported "HTTP/1.1" apart). + - Header names are as the tool's Go HTTP client gives them (canonical case, after the mesh's route), not as + automx wrote them. + What the answers show (read, not guessed): - GET /mail/config-v1.1.xml?emailaddress=... answers 200 application/xml on all three names; same body on each. - The incoming (IMAP) server has an EMPTY hostname (), port 143, STARTTLS. The outgoing server is @@ -27,4 +34,8 @@ What the answers show (read, not guessed): - GET /mobileconfig?emailaddress=... redirects (308) to /mobileconfig/?emailaddress=...; for the served domain that answers 400 with no profile; unknown domain 204; no emailaddress 400. - GET / on autoconfig answers 200 with a small HTML page linking to an example address. - - Not captured: the database rows (no mesh tool reads /data/db.sqlite; issue 1630 AC4). + - The database rows behind the answers were read once with the tool automx_rows (mailu, issue 1630 AC4), which + ran sqlite3 read-only inside mailu-automx. The tool is retired with the database (ADR 0480 clause 10, issue + 1700), so nothing reads the rows any more; the answers above are what they gave. + - The Go program that replaced automx2 answers these requests as they are captured here, but for what + DIFFERENCES lists. diff --git a/modules/mailu/cmd/automx/testdata/captured/DIFFERENCES b/modules/mailu/cmd/automx/testdata/captured/DIFFERENCES new file mode 100644 index 00000000..8274fdd9 --- /dev/null +++ b/modules/mailu/cmd/automx/testdata/captured/DIFFERENCES @@ -0,0 +1,25 @@ +How the Go program (modules/mailu/cmd/automx) answers differently from the automx2 2021.6 answers captured here +(issue 1631, ADR 0480 clause 9). automx_test.go answers every .request and compares with the capture; this is the +whole list of where the two differ. + +1. The IMAP server's name (the one correction clause 9 allows). automx2 answered for the incoming + server because its setup script read AMX_IMAP_SERVER, which mailu's environment does not set (it sets + AMX_IMAP_ADDRESS). The program names it from AMX_IMAP_ADDRESS (and the port from AMX_IMAP_PORT). + Fixtures 01, 02 and 03 each have a .corrected.body: the capture with replaced by + mail.novox.be and nothing else (a test checks that). Their Content-Length is that file's + size, 634, not the captured 612. + +2. The Server header. The capture has "Server: Werkzeug/3.1.9 Python/3.9.5". The program sends no Server header, and + Date, which the captured .headers leave out, is sent as net/http sends it. + +Not changed, though issue 1691 calls them faults; they are kept as captured because clause 9 allows only the first +correction, and their cause is not known from the capture: + - Outlook's POST for a served domain (fixture 08) answers 500. + - Apple's /mobileconfig/ for a served domain (fixture 15) answers 400 with no profile. + Whether automx2 failed there because of the empty IMAP name is a guess; it can be tested only against the + running program after the delivery. Issue 1691 stays open for them. + +Assumptions the fixtures do not settle (answers to requests nobody captured): a domain is compared in lower case; +an address without "@" is an unknown domain (204); HEAD is answered as GET and OPTIONS with the Allow header and +no body; the redirect of /mobileconfig takes its scheme and host from X-Forwarded-Proto and X-Forwarded-Host when +present (the route sends the client's, as fixture 14's Location shows) and from the request otherwise. diff --git a/modules/mailu/go.mod b/modules/mailu/go.mod new file mode 100644 index 00000000..9ea8c75b --- /dev/null +++ b/modules/mailu/go.mod @@ -0,0 +1,3 @@ +module mailu + +go 1.22 diff --git a/modules/mailu/module.json b/modules/mailu/module.json index 98c3cb23..93d72adb 100644 --- a/modules/mailu/module.json +++ b/modules/mailu/module.json @@ -71,8 +71,7 @@ "mailu_list_domains", "mailu_read_mail", "mailu_search_mail", - "mailu_health", - "automx_rows" + "mailu_health" ], "emits": [ "user.created", @@ -216,12 +215,6 @@ "class": "rebuildable", "why": "certificates, issued again" }, - { - "id": "automx", - "path": "${dir:data-automx}", - "class": "rebuildable", - "why": "client autoconfiguration, written again" - }, { "id": "fetchmail", "path": "${dir:data-fetchmail}", @@ -308,12 +301,6 @@ "mode": "0700", "unknown": "program" }, - { - "id": "data-automx", - "type": "directory", - "mode": "0700", - "unknown": "program" - }, { "id": "config-env", "type": "file", @@ -681,11 +668,25 @@ "takes-config": ["domain", "proxy-address", "sitename", "website", "url", "container"], "merge": "json" }, + { + "id": "program-automx", + "type": "archive", + "path": "${dir:state}/automx", + "artifact": "automx", + "unknown": "program" + }, { "id": "automx", "type": "container", "name": "mailu-automx", - "artifact": "automx", + "artifact": "alpine", + "health": { + "kind": "http", + "endpoint": "autoconfig", + "path": "/", + "status": 200, + "body": "Show Thunderbird-style" + }, "network": "mailu", "env-file": [ "${dir:state}/mailu.env" @@ -694,17 +695,18 @@ "4243" ], "volumes": [ - "${dir:data-automx}:/data" + "${dir:state}/automx:/opt/mesh:ro" + ], + "args": [ + "/opt/mesh/automx" + ], + "restart-on": [ + "program-automx", + "config-env" ] } ], "build": { - "on": [ - { - "arg": "PYTHON_BASE", - "image": "python@sha256:25f3cfeaceca14921366af4d1240b56457ef46273bdb508c7b0e8f469f6fd228" - } - ], "artifacts": [ { "name": "code", @@ -731,8 +733,16 @@ }, { "name": "automx", - "kind": "image", - "from": "automx/Dockerfile" + "kind": "bundle", + "language": "go", + "system": "alpine", + "from": "cmd/automx", + "binary": "automx" + }, + { + "name": "alpine", + "kind": "upstream", + "from": "alpine@sha256:28bd5fe8b56d1bd048e5babf5b10710ebe0bae67db86916198a6eec434943f8b" }, { "name": "files", diff --git a/modules/mailu/test/automx-rows.test.ts b/modules/mailu/test/automx-rows.test.ts deleted file mode 100644 index 8ef35222..00000000 --- a/modules/mailu/test/automx-rows.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -// What automx_rows reads (novox/hq issue 1630): the rows of automx2's database as stored, through a read-only -// sqlite3 inside its container. The database here is a real SQLite file made by the test with the tables -// automx/files/setup-db creates and the rows add-domains writes — with the IMAP server's name empty, the case -// the live instance is in. The fake container runs each command's statement against that database and prints -// it the way the image's sqlite3 does in list mode (one line per row, cells apart by "|", nothing for no rows); that sqlite3 -// refuses `-json` ("sqlite3: Error: unknown option: -json", captured from novox/mailu.automx_rows, issue 1630). Run against the compiled module. - -import { test } from "node:test"; -import assert from "node:assert/strict"; -import { DatabaseSync } from "node:sqlite"; - -import { readAutomx, readCommand, selectOf, parseRows, AUTOMX_DATABASE } from "../dist/automx.js"; - -function database(imapName: string): DatabaseSync { - const db = new DatabaseSync(":memory:"); - db.exec(` - CREATE TABLE provider(id INT PRIMARY KEY NOT NULL, name TEXT NOT NULL, short_name TEXT NOT NULL); - CREATE TABLE server(id INT PRIMARY KEY NOT NULL, prio INT NOT NULL DEFAULT 10, name TEXT NOT NULL, port INT NOT NULL, - type TEXT NOT NULL, socket_type TEXT NOT NULL, user_name TEXT NOT NULL, authentication TEXT NOT NULL); - CREATE TABLE domain(id INT PRIMARY KEY NOT NULL, name TEXT NOT NULL, provider_id INT NOT NULL, ldapserver_id INT NULL); - CREATE TABLE server_domain(server_id INT NOT NULL, domain_id INT NOT NULL); - INSERT INTO provider VALUES (1, 'example.org', 'example.org'); - INSERT INTO server(id, port, type, name, socket_type, user_name, authentication) - VALUES (2, 143, 'imap', '${imapName}', 'STARTTLS', '%EMAILADDRESS%', 'password-cleartext'); - INSERT INTO server(id, port, type, name, socket_type, user_name, authentication) - VALUES (3, 587, 'smtp', 'mail.example.org', 'STARTTLS', '%EMAILADDRESS%', 'password-cleartext'); - INSERT INTO domain VALUES (5, 'example.org', 1, NULL); - INSERT INTO server_domain VALUES (2, 5), (3, 5); - `); - return db; -} - -function container(db: DatabaseSync, asked: string[][] = []) { - return async (_name: string, command: string[]) => { - asked.push(command); - if (command.includes("-json")) throw Object.assign(new Error("Command failed: docker exec"), { stderr: "sqlite3: Error: unknown option: -json\n" }); - const rows = db.prepare(command[command.length - 1]).all() as unknown as Record[]; - return rows.length === 0 ? "" : rows.map((r) => Object.values(r).join("|")).join("\n") + "\n"; - }; -} - -test("the rows come back as stored, and an empty server name is said", async () => { - const answer = await readAutomx(container(database("")), "mailu-automx"); - assert.equal(answer.database, AUTOMX_DATABASE); - assert.deepEqual(answer.unreadable, {}); - assert.deepEqual(answer.tables.provider, [{ id: 1, name: "example.org", short_name: "example.org" }]); - assert.deepEqual(answer.tables.domain, [{ id: 5, name: "example.org", provider_id: 1, ldapserver_id: null }]); - assert.deepEqual(answer.tables.server_domain, [{ server_id: 2, domain_id: 5 }, { server_id: 3, domain_id: 5 }]); - assert.deepEqual( - answer.tables.server.map((s) => [s.id, s.type, s.name, s.port]), - [[2, "imap", "", 143], [3, "smtp", "mail.example.org", 587]], - ); - assert.deepEqual(answer.empty_names, [{ id: 2, type: "imap" }]); -}); - -test("a named IMAP server is not reported empty", async () => { - const answer = await readAutomx(container(database("mail.example.org")), "mailu-automx"); - assert.deepEqual(answer.empty_names, []); -}); - -test("every command opens the file read-only and runs one fixed SELECT", async () => { - const asked: string[][] = []; - await readAutomx(container(database("x"), asked), "mailu-automx"); - assert.equal(asked.length, 4); - for (const c of asked) { - assert.deepEqual(c.slice(0, 3), ["sqlite3", "-readonly", AUTOMX_DATABASE]); - assert.match(c[3], /^SELECT /); - assert.equal(c.length, 4); - assert.ok(!c.includes("-json")); - } - assert.deepEqual(readCommand("provider"), ["sqlite3", "-readonly", AUTOMX_DATABASE, selectOf("provider")]); -}); - -test("a table that cannot be read is said and does not hide the others", async () => { - const db = database("x"); - const exec = async (name: string, command: string[]) => { - if (command[3].includes("FROM domain ")) throw Object.assign(new Error("Command failed: docker exec"), { stderr: "Error: unable to open database file\nmore" }); - return container(db)(name, command); - }; - const answer = await readAutomx(exec, "mailu-automx"); - assert.deepEqual(answer.unreadable, { domain: "Error: unable to open database file" }); - assert.equal(answer.tables.provider.length, 1); - assert.equal(answer.tables.domain, undefined); -}); - -test("a NULL, an empty text, a number and a text holding a bar or a newline are read apart", () => { - const hex = (t: string) => Buffer.from(t).toString("hex").toUpperCase(); - const line = [`integer:${hex("5")}`, `text:${hex("a|b\nc")}`, "text:", "null:"].join("|"); - const rows = parseRows("domain", line + "\n"); - assert.deepEqual(rows, [{ id: 5, name: "a|b\nc", provider_id: "", ldapserver_id: null }]); -}); - -test("output that is not cells is an unreadable table, not a guess", async () => { - const exec = async () => "Error: something else\n"; - const answer = await readAutomx(exec, "mailu-automx"); - assert.equal(Object.keys(answer.unreadable).length, 4); -}); - -test("the image's refusal of -json is said as the table's reason", async () => { - const exec = async (_n: string, command: string[]) => container(database("x"))("", ["sqlite3", "-json", ...command.slice(1)]); - const answer = await readAutomx(exec, "mailu-automx"); - assert.deepEqual(answer.unreadable.domain, "sqlite3: Error: unknown option: -json"); -}); diff --git a/modules/mailu/test/mail-flows.test.ts b/modules/mailu/test/mail-flows.test.ts index 3950148c..ddea9791 100644 --- a/modules/mailu/test/mail-flows.test.ts +++ b/modules/mailu/test/mail-flows.test.ts @@ -135,7 +135,6 @@ test("the health tool is served without the admin API, on the smtp container it await import("../dist/tools/index.js"); const mailu = collectTools({ MESH_MAILU_SMTP_CONTAINER: "mailu-smtp" }).find((m) => m.module === "mailu"); const names = (mailu?.tools ?? []).map((t) => t.name); - assert.deepEqual(names, ["mailu_health", "automx_rows"]); + assert.deepEqual(names, ["mailu_health"]); assert.match(mailu!.tools[0].description, /inside mailu-smtp/); - assert.match(mailu!.tools[1].description, /read-only, inside mailu-automx/); }); diff --git a/modules/mailu/tools/index.ts b/modules/mailu/tools/index.ts index c6624f7e..6b8442e9 100644 --- a/modules/mailu/tools/index.ts +++ b/modules/mailu/tools/index.ts @@ -4,7 +4,6 @@ import { registerModuleTools, type ToolDefinition } from "@novox/mesh-sdk/tools"; import { MailuClient } from "../client.js"; -import { readAutomx } from "../automx.js"; import { dockerExec, mailFlows, DEFERRED_BOUND_SECONDS, PUBLIC_NAME, SPREAD_DOMAINS } from "../health.js"; /** @@ -27,23 +26,6 @@ export function getHealthTool(env: NodeJS.ProcessEnv): ToolDefinition { }; } -/** - * What automx holds, as stored (novox/hq issue 1630): the rows behind the answers it gives mail clients. It needs - * no admin API, only the automx container, so like the health tool it is served without the API. - */ -export function getAutomxTool(env: NodeJS.ProcessEnv): ToolDefinition { - const container = env.MESH_MAILU_AUTOMX_CONTAINER || "mailu-automx"; - return { - name: "automx_rows", - description: - `Read the autoconfiguration module's database, as stored, read-only, inside ${container}: its provider, ` + - "server, domain and server_domain rows, and the servers whose name is empty (a mail client is then told " + - "an empty host name). Writes nothing.", - input: {}, - run: async () => readAutomx(dockerExec(), container), - }; -} - export function getMailuTools(mailu: MailuClient): ToolDefinition[] { return [ { @@ -191,5 +173,5 @@ registerModuleTools("mailu", (env) => { } catch { tools = []; } - return [...tools, getHealthTool(env), getAutomxTool(env)]; + return [...tools, getHealthTool(env)]; }); diff --git a/modules/mailu/tsconfig.json b/modules/mailu/tsconfig.json index ad97ec61..a55a92ee 100644 --- a/modules/mailu/tsconfig.json +++ b/modules/mailu/tsconfig.json @@ -8,5 +8,5 @@ "skipLibCheck": true, "noEmit": true }, - "include": ["client.ts", "health.ts", "automx.ts", "index.ts", "tools/index.ts"] + "include": ["client.ts", "health.ts", "index.ts", "tools/index.ts"] } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go index bddf11ac..98462762 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go @@ -17,6 +17,8 @@ import ( "git.novox.be/novox/mesh-sdk/go/buildstep" ) +var commitOK = regexp.MustCompile(`^[0-9a-f]{40}$`) + // ForgeAlias is the ssh name the forge-account provision sets up in the account's ssh files (ADR 0409 §1); the // remote of a fetch is ssh:////.git. const ForgeAlias = "mesh-forge" @@ -37,6 +39,18 @@ type FetchResult struct { Workspace string `json:"workspace"` Commit string `json:"commit"` Modules []string `json:"modules"` + // The merge preview (ADR 0484 clause 3), only when the call gave a base: the trunk commit, the merge commit of the + // head onto it, the paths the merge changes, and the tree of each module directory (a directory holding a + // module.json) in the merge. A head that does not merge cleanly is an answer: Merge is empty, Conflicts names the + // files, and the workspace is left at the trunk commit. + Base string `json:"base,omitempty"` + Merge string `json:"merge,omitempty"` + Conflicts []string `json:"conflicts,omitempty"` + Changed []string `json:"changed,omitempty"` + Trees map[string]buildstep.ModuleTree `json:"trees,omitempty"` + // Beside are the sibling repositories placed as plain directories under the workspace's beside directory, which + // build_go_test hands to the tests as MESH_CHECK_BESIDE (ADR 0484 clause 4). + Beside []Sibling `json:"beside,omitempty"` } // fetchTool is the fetch step tool. @@ -52,6 +66,12 @@ func (m *Module) fetchTool() StepTool { "repository": map[string]any{"type": "string", "description": "/ on the forge"}, "commit": map[string]any{"type": "string", "description": "the full commit, 40 hex digits"}, "ref": map[string]any{"type": "string", "description": "refs/pull//head when only a pull request's head carries the commit"}, + "base": map[string]any{"type": "string", "description": "optional trunk commit, 40 hex digits: the head is merged onto it as the forge's merge would, and the answer carries the merge commit, the files in conflict (an answer, the workspace is then left at the trunk commit), the changed paths and the tree of each module directory"}, + "beside": map[string]any{"type": "array", "description": "optional sibling repositories the tests read, each placed as a plain directory (no symlink) under the workspace's beside/, which build_go_test gives the tests as MESH_CHECK_BESIDE", + "items": map[string]any{"type": "object", "properties": map[string]any{ + "repository": map[string]any{"type": "string", "description": "/ on the forge"}, + "commit": map[string]any{"type": "string", "description": "the full commit, 40 hex digits"}, + }, "required": []string{"repository", "commit"}}}, }, "required": []string{"repository", "commit"}, }, @@ -64,6 +84,14 @@ func (m *Module) fetch(ctx context.Context, args map[string]any) (any, error) { repository, _ := args["repository"].(string) commit, _ := args["commit"].(string) ref, _ := args["ref"].(string) + base, _ := args["base"].(string) + if base != "" && !commitOK.MatchString(base) { + return nil, fmt.Errorf("%q is not a trunk commit (40 hex digits)", base) + } + siblings, err := siblingsArg(args["beside"]) + if err != nil { + return nil, err + } if !repositoryOK.MatchString(repository) || strings.Contains(repository, "..") { return nil, fmt.Errorf("%q is not a repository, /", repository) } @@ -74,14 +102,27 @@ func (m *Module) fetch(ctx context.Context, args map[string]any) (any, error) { if err := os.MkdirAll(tree, 0o700); err != nil { return nil, err } - remote := "ssh://" + ForgeAlias + "/" + repository + ".git" - if err := buildstep.FetchCommit(ctx, m.Run, tree, remote, commit, ref); err != nil { + remote := m.remote(repository) + if err := buildstep.FetchCommit(ctx, m.Run, tree, remote, commit, ref, m.FetchOptions...); err != nil { + return nil, err + } + res := FetchResult{Workspace: id, Commit: commit} + if base != "" { + if err := m.merge(ctx, &res, tree, remote, base, commit); err != nil { + return nil, err + } + } + if res.Beside, err = m.placeSiblings(ctx, root, siblings); err != nil { return nil, err } dirs, err := m.goModDirs(ctx, tree) if err != nil { return nil, err } + res.Modules = dirs + if base != "" && res.Merge == "" { + return res, nil // a conflict: the tree is at the trunk, nothing of the head is built or tested + } for _, d := range dirs { if _, err := os.Stat(filepath.Join(tree, d, "vendor", "modules.txt")); err == nil { continue // a module that vendors resolves nothing @@ -93,7 +134,7 @@ func (m *Module) fetch(ctx context.Context, args map[string]any) (any, error) { } } } - return FetchResult{Workspace: id, Commit: commit, Modules: dirs}, nil + return res, nil } // goModDirs are the directories of the tree that hold a go.mod, relative to it, in order. diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go index d223cdaa..56657e06 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go @@ -30,7 +30,8 @@ type fakeStep struct { gomods string gate chan struct{} // when set, the first git command waits for it goRule func(line string) forgekey.Ran - vendors []string // directories (relative to the tree) that get a vendor/modules.txt at checkout + vendors []string // directories (relative to the tree) that get a vendor/modules.txt at checkout + rule func(line string) (forgekey.Ran, bool) // answers a command before the defaults do } func (f *fakeStep) run(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { @@ -41,6 +42,11 @@ func (f *fakeStep) run(ctx context.Context, env []string, name string, args ...s if f.gate != nil && strings.Contains(line, " init ") { <-f.gate } + if f.rule != nil { + if r, ok := f.rule(line); ok { + return r + } + } switch { case name == "git" && strings.Contains(line, " rev-parse HEAD"): return forgekey.Ran{Stdout: commitSHA + "\n"} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/integration.go b/modules/mesh-build-go/cmd/mesh-build-go/integration.go index d96c4bb5..1f112ed8 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/integration.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/integration.go @@ -60,7 +60,7 @@ func (m *Module) integration(ctx context.Context, workspace, tree string, packag return nil, fmt.Errorf("the integration level's bus: %w", err) } defer stopBus() - env := []string{"MESH_TEST_POSTGRES=" + postgres, "MESH_TEST_NATS=" + nats} + env := append([]string{"MESH_TEST_POSTGRES=" + postgres, "MESH_TEST_NATS=" + nats}, m.besideEnv(workspace)...) // Compiling counts as setup: the tests are built once, here, with the addresses a TestMain may read, and the run // below finds them built. Whether they compile is for the run to say; its failure is not read here. for _, d := range dirs { diff --git a/modules/mesh-build-go/cmd/mesh-build-go/module.go b/modules/mesh-build-go/cmd/mesh-build-go/module.go index 8851a8ff..d0c4d603 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/module.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/module.go @@ -37,6 +37,10 @@ type Module struct { // both outside the account's home (ADR 0466 §1); Home is the account's home. Run forgekey.Runner Home, Work, Cache string + // RemoteBase is where repositories are fetched from, "ssh://mesh-forge/" when empty; FetchOptions are the options of + // the SDK's fetch and merge. A test points them at a local repository (buildstep.AllowFileRemote). + RemoteBase string + FetchOptions []buildstep.Option // HTTP fetches the files an archive artifact is pinned to (url and sha256); a client of its own with a fifteen-minute // timeout when nil. A test serves its own. HTTP *http.Client diff --git a/modules/mesh-build-go/cmd/mesh-build-go/preview.go b/modules/mesh-build-go/cmd/mesh-build-go/preview.go new file mode 100644 index 00000000..14ab2b81 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/preview.go @@ -0,0 +1,119 @@ +package main + +// The merge preview and the siblings of a fetch (novox/hq ADR 0484 clauses 3 and 4; issues 1675 and 1677). The merge +// itself is the SDK's (buildstep.MergeInto), the one every build module's fetch step makes; what is here is the answer +// built from it and the placing of the sibling repositories the tests read. + +import ( + "context" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + + "git.novox.be/novox/mesh-sdk/go/buildstep" +) + +// Sibling is a repository placed beside the fetched tree: its name (the directory under beside/) and its commit. +type Sibling struct { + Repository string `json:"repository"` + Commit string `json:"commit"` + Dir string `json:"dir"` +} + +// remote is the address a repository is fetched from: the forge over ssh as the account, or RemoteBase when a test set it. +func (m *Module) remote(repository string) string { + base := m.RemoteBase + if base == "" { + base = "ssh://" + ForgeAlias + "/" + } + return base + repository + ".git" +} + +// merge makes the merge preview of the head onto base and writes the answer into res. +func (m *Module) merge(ctx context.Context, res *FetchResult, tree, remote, base, commit string) error { + mr, err := buildstep.MergeInto(ctx, m.Run, tree, remote, base, commit, m.FetchOptions...) + if err != nil { + return err + } + res.Base = mr.Base + res.Conflicts, res.Merge, res.Changed = mr.Conflicts, mr.Merge, mr.Changed + if res.Merge == "" { + return nil + } + res.Trees, err = buildstep.ModuleTrees(ctx, m.Run, nil, tree, res.Merge) + return err +} + +type siblingArg struct{ repository, commit string } + +func siblingsArg(v any) ([]siblingArg, error) { + if v == nil { + return nil, nil + } + list, ok := v.([]any) + if !ok { + return nil, fmt.Errorf("beside is a list of {repository, commit}") + } + var out []siblingArg + seen := map[string]bool{} + for _, e := range list { + o, _ := e.(map[string]any) + repository, _ := o["repository"].(string) + commit, _ := o["commit"].(string) + if !repositoryOK.MatchString(repository) || strings.Contains(repository, "..") { + return nil, fmt.Errorf("beside: %q is not a repository, /", repository) + } + if !commitOK.MatchString(commit) { + return nil, fmt.Errorf("beside: %q is not a full commit (40 hex digits)", commit) + } + name := repository[strings.Index(repository, "/")+1:] + if seen[name] { + return nil, fmt.Errorf("beside: two repositories are named %s", name) + } + seen[name] = true + out = append(out, siblingArg{repository, commit}) + } + return out, nil +} + +// BesideDir is where a workspace's siblings are placed, and what MESH_CHECK_BESIDE names. +func (m *Module) besideDir(workspace string) string { + return filepath.Join(m.Work, workspace, "beside") +} + +// placeSiblings fetches each sibling into beside/ of the workspace root, as a plain directory. A symlink anywhere +// in a placed tree fails the step naming it: the mesh creates none, and a test must not be led out of its sibling. +func (m *Module) placeSiblings(ctx context.Context, root string, siblings []siblingArg) ([]Sibling, error) { + var placed []Sibling + for _, s := range siblings { + name := s.repository[strings.Index(s.repository, "/")+1:] + dir := filepath.Join(root, "beside", name) + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, err + } + if err := buildstep.FetchCommit(ctx, m.Run, dir, m.remote(s.repository), s.commit, "", m.FetchOptions...); err != nil { + return nil, fmt.Errorf("beside %s: %w", s.repository, err) + } + if link := firstSymlink(dir); link != "" { + return nil, fmt.Errorf("beside %s holds a symlink, %s", s.repository, link) + } + placed = append(placed, Sibling{Repository: s.repository, Commit: s.commit, Dir: name}) + } + sort.Slice(placed, func(i, j int) bool { return placed[i].Dir < placed[j].Dir }) + return placed, nil +} + +func firstSymlink(dir string) string { + var found string + filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { + if err == nil && d.Type()&fs.ModeSymlink != 0 && found == "" { + found, _ = filepath.Rel(dir, p) + return fs.SkipAll + } + return nil + }) + return found +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/preview_integration_test.go b/modules/mesh-build-go/cmd/mesh-build-go/preview_integration_test.go new file mode 100644 index 00000000..a800d22d --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/preview_integration_test.go @@ -0,0 +1,228 @@ +//go:build integration + +package main + +// The fetch step's merge and siblings on real git repositories and the real go (issues 1675 and 1677): they start +// processes, so they carry the tag; preview_test.go proves the commands with a fake runner. + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/buildstep" + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +func git(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir, "-c", "user.name=t", "-c", "user.email=t@example.invalid", "-c", "commit.gpgsign=false"}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +func put(t *testing.T, dir, name, body string) { + t.Helper() + p := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +// forge is a directory of bare repositories /.git standing in for the forge, and a module fetching from it. +type forge struct { + t *testing.T + root string +} + +func newForge(t *testing.T) *forge { return &forge{t: t, root: t.TempDir()} } + +// repo makes the repository owner/name with the files on main and answers its work directory and the commit. +func (f *forge) repo(name string, files map[string]string) (work, commit string) { + bare := filepath.Join(f.root, "forge", name+".git") + work = filepath.Join(f.root, "work", name) + os.MkdirAll(bare, 0o755) + os.MkdirAll(work, 0o755) + git(f.t, bare, "init", "-q", "--bare") + git(f.t, work, "init", "-q", "-b", "main") + for n, c := range files { + put(f.t, work, n, c) + } + git(f.t, work, "add", ".") + git(f.t, work, "commit", "-q", "-m", "one") + git(f.t, work, "push", "-q", bare, "HEAD:refs/heads/main") + return work, git(f.t, work, "rev-parse", "HEAD") +} + +// commit makes a commit off `from` in the repository's work directory and publishes it as the pull request's head. +func (f *forge) commit(name, work, from string, files map[string]string, ref string) string { + git(f.t, work, "checkout", "-q", "-B", "b"+ref[len(ref)-3:], from) + for n, c := range files { + put(f.t, work, n, c) + } + git(f.t, work, "add", ".") + git(f.t, work, "commit", "-q", "-m", "change") + sha := git(f.t, work, "rev-parse", "HEAD") + git(f.t, work, "push", "-q", "-f", filepath.Join(f.root, "forge", name+".git"), "HEAD:"+ref) + return sha +} + +func (f *forge) module(t *testing.T) (*Module, *keptStates) { + m, states := fetchModule(t, &fakeStep{}) + m.Run = StepRunner(m.Home) + m.RemoteBase = "file://" + filepath.Join(f.root, "forge") + "/" + m.FetchOptions = []buildstep.Option{buildstep.AllowFileRemote()} + return m, states +} + +// waitLong is waitEnd for a step that runs the real go, whose first build in a cold cache takes longer than 3 s. +func waitLong(t *testing.T, s *keptStates) delivery.Step { + t.Helper() + select { + case key := <-s.ended: + s.mu.Lock() + defer s.mu.Unlock() + v := s.steps[key] + return v[len(v)-1] + case <-time.After(2 * time.Minute): + t.Fatal("the step never ended on its key") + return delivery.Step{} + } +} + +func fetched(t *testing.T, m *Module, states *keptStates, args map[string]any) (FetchResult, delivery.Step) { + t.Helper() + if _, err := fetchTool(t, m)(args); err != nil { + t.Fatal(err) + } + st := states.waitEnd(t) + var res FetchResult + if st.State == delivery.StepPassed { + if err := json.Unmarshal(st.Result, &res); err != nil { + t.Fatal(err) + } + } + return res, st +} + +var baseFiles = map[string]string{ + "go.mod": "module fixture/m\n\ngo 1.26\n", + "a.txt": "one\ntwo\nthree\n", + "modules/m/module.json": "{\"name\":\"m\"}\n", + "modules/m/go.sum": "", + "modules/m/m.txt": "m\n", + "modules/other/other.txt": "o\n", +} + +// 1675/AC1 and AC2 on real git: a clean merge answers the merge commit, the changed paths (the trunk's change is not +// among them) and the tree of each module directory, which equals git's own; a conflict is a passed step naming the files, +// leaving the workspace at the trunk commit; and no base is today's fetch. +func TestRealFetchMergesConflictsAndPlainlyFetches(t *testing.T) { + f := newForge(t) + work, fork := f.repo("o/r", baseFiles) + trunk := f.commit("o/r", work, fork, map[string]string{"modules/other/other.txt": "trunk\n", "a.txt": "one\ntwo\nTRUNK\n"}, "refs/heads/main") + clean := f.commit("o/r", work, fork, map[string]string{"modules/m/m.txt": "head\n"}, "refs/pull/1/head") + clash := f.commit("o/r", work, fork, map[string]string{"a.txt": "one\ntwo\nHEAD\n"}, "refs/pull/2/head") + m, states := f.module(t) + + res, st := fetched(t, m, states, map[string]any{"repository": "o/r", "commit": clean, "ref": "refs/pull/1/head", "base": trunk}) + if st.State != delivery.StepPassed || res.Merge == "" || res.Base != trunk || res.Commit != clean || len(res.Conflicts) != 0 { + t.Fatalf("clean: %s %s", st.State, st.Result) + } + if strings.Join(res.Changed, ",") != "modules/m/m.txt" { + t.Errorf("changed = %v, want the head's own change alone", res.Changed) + } + tree := res.Trees["modules/m"] + if want := git(t, filepath.Join(m.Work, res.Workspace, "tree"), "rev-parse", res.Merge+":modules/m"); tree.Tree != want { + t.Errorf("tree = %s, want %s", tree.Tree, want) + } + if tree.Manifest != sha256Hex([]byte("{\"name\":\"m\"}\n")) || tree.Locks["go.sum"] != sha256Hex(nil) { + t.Errorf("manifest and locks = %+v", tree) + } + if got := git(t, filepath.Join(m.Work, res.Workspace, "tree"), "rev-parse", "HEAD"); got != res.Merge { + t.Errorf("the workspace is at %s, want the merge %s", got, res.Merge) + } + + res, st = fetched(t, m, states, map[string]any{"repository": "o/r", "commit": clash, "ref": "refs/pull/2/head", "base": trunk}) + if st.State != delivery.StepPassed || res.Merge != "" || strings.Join(res.Conflicts, ",") != "a.txt" { + t.Fatalf("conflict: %s %s", st.State, st.Result) + } + if got := git(t, filepath.Join(m.Work, res.Workspace, "tree"), "rev-parse", "HEAD"); got != trunk { + t.Errorf("after a conflict the workspace is at %s, want the trunk %s", got, trunk) + } + + res, st = fetched(t, m, states, map[string]any{"repository": "o/r", "commit": clean, "ref": "refs/pull/1/head"}) + if st.State != delivery.StepPassed || res.Merge != "" || res.Base != "" || res.Trees != nil || len(res.Modules) != 1 { + t.Errorf("no base: %s %s", st.State, st.Result) + } +} + +// 1677/AC1 on the real go: the unit level of a repository whose test reads MESH_CHECK_BESIDE fails loudly without the +// sibling and passes with it; the sibling is a plain directory with no symlink in it; and a sibling that holds a +// symlink fails the fetch naming it. +func TestRealTestsReadTheSiblingBesideTheTree(t *testing.T) { + f := newForge(t) + files := map[string]string{} + for _, n := range []string{"go.mod", "beside.go", "beside_test.go"} { + raw, err := os.ReadFile(filepath.Join("testdata", "repos", "beside", n)) + if err != nil { + t.Fatal(err) + } + files[n] = string(raw) + } + _, head := f.repo("o/r", files) + _, sibling := f.repo("o/sibling", map[string]string{"data.txt": "from the sibling\n"}) + m, states := f.module(t) + + without, _ := fetched(t, m, states, map[string]any{"repository": "o/r", "commit": head}) + if _, err := m.testToolRun(t, without.Workspace); err != nil { + t.Fatal(err) + } + var res TestResult + st := waitLong(t, states) + json.Unmarshal(st.Result, &res) + if st.State != delivery.StepPassed || res.Passed || len(res.Failed) != 1 || !strings.Contains(res.Failed[0].Output, "MESH_CHECK_BESIDE is not set") { + t.Fatalf("without the sibling: %s %s", st.State, st.Result) + } + + with, st2 := fetched(t, m, states, map[string]any{"repository": "o/r", "commit": head, + "beside": []any{map[string]any{"repository": "o/sibling", "commit": sibling}}}) + if st2.State != delivery.StepPassed || len(with.Beside) != 1 { + t.Fatalf("with the sibling: %s %s", st2.State, st2.Result) + } + m.testToolRun(t, with.Workspace) + res = TestResult{} + st = waitLong(t, states) + json.Unmarshal(st.Result, &res) + if !res.Passed { + t.Errorf("with the sibling the level failed: %s", st.Result) + } + if link := firstSymlink(filepath.Join(m.Work, with.Workspace, "beside")); link != "" { + t.Errorf("the placed tree holds a symlink, %s", link) + } + + _, bad := f.repo("o/linked", map[string]string{"data.txt": "x\n"}) + work := filepath.Join(f.root, "work", "o/linked") + if err := os.Symlink("/etc", filepath.Join(work, "etc-link")); err != nil { + t.Fatal(err) + } + git(t, work, "add", ".") + git(t, work, "commit", "-q", "-m", "link") + bad = git(t, work, "rev-parse", "HEAD") + git(t, work, "push", "-q", "-f", filepath.Join(f.root, "forge", "o/linked.git"), "HEAD:refs/heads/main") + _, st3 := fetched(t, m, states, map[string]any{"repository": "o/r", "commit": head, + "beside": []any{map[string]any{"repository": "o/linked", "commit": bad}}}) + if st3.State != delivery.StepFailed || !strings.Contains(st3.Reason, "etc-link") { + t.Errorf("a sibling with a symlink: %s %s", st3.State, st3.Reason) + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/preview_test.go b/modules/mesh-build-go/cmd/mesh-build-go/preview_test.go new file mode 100644 index 00000000..2fcee7ea --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/preview_test.go @@ -0,0 +1,317 @@ +package main + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +const ( + baseSHA = "abcdef0123456789abcdef0123456789abcdef01" + mergeSHA = "fedcba9876543210fedcba9876543210fedcba98" + treeSHA = "1111111111111111111111111111111111111111" +) + +// previewRules answers the merge's commands as git does: the merge commit, the changed paths, the manifests, the trees. +func previewRules(conflict bool) func(string) (forgekey.Ran, bool) { + merged := false + return func(line string) (forgekey.Ran, bool) { + switch { + case strings.Contains(line, "--is-shallow-repository"): + return forgekey.Ran{Stdout: "true\n"}, true + case strings.Contains(line, " merge --no-ff"): + if conflict { + return forgekey.Ran{Status: 1, Stderr: "CONFLICT (content): Merge conflict in a.go\n"}, true + } + merged = true + return forgekey.Ran{}, true + case strings.Contains(line, "diff --name-only --diff-filter=U"): + return forgekey.Ran{Stdout: "a.go\nsub/b.go\n"}, true + case strings.Contains(line, "diff --name-only --no-renames"): + return forgekey.Ran{Stdout: "a.go\nmodules/m/main.go\n"}, true + case strings.Contains(line, " rev-parse HEAD") && merged: + return forgekey.Ran{Stdout: mergeSHA + "\n"}, true + case strings.Contains(line, " ls-files ") && strings.Contains(line, "module.json"): + return forgekey.Ran{Stdout: "modules/m/module.json\n"}, true + case strings.Contains(line, " rev-parse "+mergeSHA+":modules/m"): + return forgekey.Ran{Stdout: treeSHA + "\n"}, true + } + return forgekey.Ran{}, false + } +} + +// 1675/AC1: with a base the fetch merges the head onto it with the SDK's merge (the identity and flags of the check +// merge, hooks off), and answers base, merge commit, changed paths and, per module directory, tree, manifest hash and lock hashes. +func TestFetchWithABaseMergesAndAnswersThePreview(t *testing.T) { + f := &fakeStep{gomods: "go.mod\n", rule: previewRules(false)} + m, states := fetchModule(t, f) + m.Run = func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran { + // the workspace's module.json and go.sum are on disk once the fake checks the head out + for i, a := range args { + if a == "-C" && len(args) > i+1 && strings.Contains(strings.Join(args, " "), " checkout ") { + os.MkdirAll(filepath.Join(args[i+1], "modules", "m"), 0o755) + os.WriteFile(filepath.Join(args[i+1], "modules", "m", "module.json"), []byte("{}\n"), 0o644) + os.WriteFile(filepath.Join(args[i+1], "modules", "m", "go.sum"), []byte("sum\n"), 0o644) + } + } + return f.run(ctx, env, name, args...) + } + args := map[string]any{"repository": "novox/mesh-sdk", "commit": commitSHA, "base": baseSHA} + if _, err := fetchTool(t, m)(args); err != nil { + t.Fatal(err) + } + st := states.waitEnd(t) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + var res FetchResult + if err := json.Unmarshal(st.Result, &res); err != nil { + t.Fatal(err) + } + if res.Base != baseSHA || res.Merge != mergeSHA || res.Commit != commitSHA || len(res.Conflicts) != 0 || + strings.Join(res.Changed, ",") != "a.go,modules/m/main.go" { + t.Errorf("result = %s", st.Result) + } + mt, ok := res.Trees["modules/m"] + if !ok || mt.Tree != treeSHA || mt.Manifest != sha256Hex([]byte("{}\n")) || mt.Locks["go.sum"] != sha256Hex([]byte("sum\n")) { + t.Errorf("trees = %+v", res.Trees) + } + lines := strings.Join(f.lines(), "\n") + for _, want := range []string{"fetch --no-tags --unshallow origin " + baseSHA, "checkout -q --detach " + baseSHA, + "core.hooksPath=/dev/null", "merge --no-ff --no-edit --no-verify"} { + if !strings.Contains(lines, want) { + t.Errorf("git was not asked %q:\n%s", want, lines) + } + } +} + +// 1675/AC2: a conflict is an answer: the step ends passed, the files are named, no merge commit, nothing downloaded. +func TestFetchWithAConflictEndsPassedAndNamesTheFiles(t *testing.T) { + f := &fakeStep{gomods: "go.mod\n", rule: previewRules(true)} + m, states := fetchModule(t, f) + args := map[string]any{"repository": "novox/mesh-sdk", "commit": commitSHA, "base": baseSHA} + if _, err := fetchTool(t, m)(args); err != nil { + t.Fatal(err) + } + st := states.waitEnd(t) + if st.State != delivery.StepPassed { + t.Fatalf("a conflict ended the step %s: %s", st.State, st.Reason) + } + var res FetchResult + json.Unmarshal(st.Result, &res) + if res.Merge != "" || res.Base != baseSHA || strings.Join(res.Conflicts, ",") != "a.go,sub/b.go" || len(res.Changed) != 0 { + t.Errorf("result = %s", st.Result) + } + lines := strings.Join(f.lines(), "\n") + if !strings.Contains(lines, "merge --abort") || strings.Contains(lines, "mod download") { + t.Errorf("the merge was not undone, or modules were downloaded:\n%s", lines) + } +} + +// 1675/AC3: no base is today's fetch: no merge, no extra command, and the answer carries none of the preview. +func TestFetchWithoutABaseIsTodaysFetch(t *testing.T) { + f := &fakeStep{gomods: "go.mod\n"} + m, states := fetchModule(t, f) + fetchTool(t, m)(fetchArgs) + st := states.waitEnd(t) + lines := strings.Join(f.lines(), "\n") + if strings.Contains(lines, "merge") || strings.Contains(lines, "unshallow") { + t.Errorf("a fetch without a base merged:\n%s", lines) + } + for _, key := range []string{"base", "merge", "conflicts", "changed", "trees", "beside"} { + if strings.Contains(string(st.Result), `"`+key+`"`) { + t.Errorf("the answer carries %q without a base: %s", key, st.Result) + } + } +} + +func TestABaseThatIsNotACommitIsRefusedBeforeAnythingRuns(t *testing.T) { + f := &fakeStep{} + m, states := fetchModule(t, f) + fetchTool(t, m)(map[string]any{"repository": "novox/mesh-sdk", "commit": commitSHA, "base": "main"}) + if st := states.waitEnd(t); st.State != delivery.StepFailed || len(f.lines()) != 0 { + t.Errorf("the step ended %s after %d commands", st.State, len(f.lines())) + } +} + +// 1677/AC1: each sibling is fetched into beside/ of the workspace, and the tests of the workspace are given +// MESH_CHECK_BESIDE naming that directory, at the unit level; a workspace without siblings is given none. +func TestSiblingsArePlacedBesideAndTheUnitLevelIsGivenTheirDirectory(t *testing.T) { + f := &fakeStep{gomods: "go.mod\n"} + m, states := fetchModule(t, f) + args := map[string]any{"repository": "novox/mesh-controller", "commit": commitSHA, "beside": []any{ + map[string]any{"repository": "novox/mesh-catalog", "commit": commitSHA}, + map[string]any{"repository": "novox/mesh-host", "commit": commitSHA}}} + if _, err := fetchTool(t, m)(args); err != nil { + t.Fatal(err) + } + st := states.waitEnd(t) + var res FetchResult + json.Unmarshal(st.Result, &res) + if st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + if len(res.Beside) != 2 || res.Beside[0].Dir != "mesh-catalog" || res.Beside[1].Dir != "mesh-host" { + t.Fatalf("result = %s", st.Result) + } + lines := strings.Join(f.lines(), "\n") + for _, d := range []string{"beside/mesh-catalog", "beside/mesh-host"} { + if !strings.Contains(lines, filepath.Join(m.Work, res.Workspace, d)+" init") && !strings.Contains(lines, "-C "+filepath.Join(m.Work, res.Workspace, d)) { + t.Errorf("nothing was fetched into %s:\n%s", d, lines) + } + } + if !strings.Contains(lines, "remote add origin ssh://mesh-forge/novox/mesh-catalog.git") { + t.Errorf("the sibling was not fetched over ssh:\n%s", lines) + } + + calls := &[]cmdRecord{} + m.Run = capturedRunner(t, "failing", calls) + if err := os.MkdirAll(filepath.Join(m.Work, res.Workspace, "tree"), 0o755); err != nil { + t.Fatal(err) + } + if _, err := m.testToolRun(t, res.Workspace); err != nil { + t.Fatal(err) + } + states.waitEnd(t) + want := "MESH_CHECK_BESIDE=" + filepath.Join(m.Work, res.Workspace, "beside") + if !envHas(*calls, "go", want) { + t.Errorf("no go command was given %s", want) + } +} + +func (m *Module) testToolRun(t *testing.T, ws string) (any, error) { + t.Helper() + for _, tool := range m.Tools(m.StepTools()) { + if tool.Name == "build_go_test" { + return tool.Run(map[string]any{"workspace": ws, "level": "unit"}) + } + } + t.Fatal("no tool build_go_test") + return nil, nil +} + +func envHas(calls []cmdRecord, name, want string) bool { + for _, c := range calls { + if strings.HasPrefix(c.line, name+" ") { + for _, e := range c.env { + if e == want { + return true + } + } + } + } + return false +} + +func TestAWorkspaceWithoutSiblingsIsGivenNoBesideVariable(t *testing.T) { + m, states, ws, calls := workspaceFor(t, "failing") + runUnit(t, m, states, ws) + for _, c := range *calls { + for _, e := range c.env { + if strings.HasPrefix(e, "MESH_CHECK_BESIDE=") { + t.Errorf("%s was given %s", c.line, e) + } + } + } +} + +func TestSiblingsThatAreNotRepositoriesOrCommitsAreRefused(t *testing.T) { + for name, beside := range map[string]any{ + "not a list": "x", + "bad repository": []any{map[string]any{"repository": "../x/y", "commit": baseSHA}}, + "bad commit": []any{map[string]any{"repository": "novox/a", "commit": "main"}}, + "twice": []any{map[string]any{"repository": "novox/a", "commit": baseSHA}, map[string]any{"repository": "other/a", "commit": baseSHA}}, + } { + f := &fakeStep{} + m, states := fetchModule(t, f) + fetchTool(t, m)(map[string]any{"repository": "novox/mesh-sdk", "commit": commitSHA, "beside": beside}) + if st := states.waitEnd(t); st.State != delivery.StepFailed || len(f.lines()) != 0 { + t.Errorf("%s: the step ended %s after %d commands", name, st.State, len(f.lines())) + } + } +} + +// 1288/AC5 (the checker's contract): an import path runs only in the go.mod directory whose module holds it; a +// relative pattern runs in every directory; no pattern is ./... everywhere. +func TestPackagesRunOnlyInTheModuleThatHoldsThem(t *testing.T) { + tree := t.TempDir() + for dir, mod := range map[string]string{".": "example.org/root", "tools": "example.org/root/tools", "other": "example.org/other"} { + os.MkdirAll(filepath.Join(tree, dir), 0o755) + os.WriteFile(filepath.Join(tree, dir, "go.mod"), []byte("module "+mod+"\n\ngo 1.26\n"), 0o644) + } + dirs := []string{".", "other", "tools"} + got, err := packagesByDir(tree, dirs, []string{"example.org/root/a", "example.org/root/tools/b", "example.org/other"}) + if err != nil { + t.Fatal(err) + } + want := map[string]string{".": "example.org/root/a", "tools": "example.org/root/tools/b", "other": "example.org/other"} + for d, w := range want { + if strings.Join(got[d], ",") != w { + t.Errorf("%s runs %v, want %s", d, got[d], w) + } + } + got, _ = packagesByDir(tree, dirs, []string{"./x/..."}) + if len(got["."]) != 1 || len(got["tools"]) != 1 || len(got["other"]) != 1 { + t.Errorf("a relative pattern = %v", got) + } + got, _ = packagesByDir(tree, dirs, nil) + if got["tools"][0] != "./..." { + t.Errorf("no pattern = %v", got) + } + if _, err := packagesByDir(tree, dirs, []string{"example.org/nowhere/x"}); err == nil || !strings.Contains(err.Error(), "example.org/nowhere/x") { + t.Errorf("a package no module holds: %v", err) + } +} + +func sha256Hex(b []byte) string { h := sha256.Sum256(b); return hex.EncodeToString(h[:]) } + +// 1677/AC (integration level): the integration level's go test is given MESH_CHECK_BESIDE when the workspace has siblings +// beside it, with the store's and the bus's addresses, and is given none when it has not. +func TestTheIntegrationLevelIsGivenTheBesideDirectoryOnlyWhenSiblingsWerePlaced(t *testing.T) { + for _, siblings := range []bool{true, false} { + f := newIntegFake(t, "slow") + m, states := integModule(t, f) + beside := filepath.Join(m.Work, "ws1", "beside") + if siblings { + if err := os.MkdirAll(filepath.Join(beside, "mesh-catalog"), 0o755); err != nil { + t.Fatal(err) + } + } + if _, st := runLevelTool(t, m, states, "integration"); st.State != delivery.StepPassed { + t.Fatalf("the step ended %s: %s", st.State, st.Reason) + } + tested := 0 + for _, c := range f.calls { + if !strings.HasPrefix(c.line, "go ") || !strings.Contains(c.line, "-tags integration") { + continue + } + tested++ + got := "" + hasNATS := false + for _, e := range c.env { + if strings.HasPrefix(e, "MESH_CHECK_BESIDE=") { + got = strings.TrimPrefix(e, "MESH_CHECK_BESIDE=") + } + hasNATS = hasNATS || strings.HasPrefix(e, "MESH_TEST_NATS=") + } + want := "" + if siblings { + want = beside + } + if got != want || !hasNATS { + t.Errorf("siblings %v: %s was given MESH_CHECK_BESIDE=%q (want %q), MESH_TEST_NATS present %v", siblings, c.line, got, want, hasNATS) + } + } + if tested == 0 { + t.Errorf("siblings %v: no integration go test ran", siblings) + } + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/test.go b/modules/mesh-build-go/cmd/mesh-build-go/test.go index 426cdd44..3448fbd1 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/test.go @@ -65,13 +65,23 @@ func (m *Module) test(ctx context.Context, args map[string]any) (any, error) { } switch level { case LevelUnit: - return m.runLevel(ctx, tree, levelOpts{level: LevelUnit, packages: packages, bounds: Bounds{Run: m.unitBound()}}) + return m.runLevel(ctx, tree, levelOpts{level: LevelUnit, packages: packages, bounds: Bounds{Run: m.unitBound()}, env: m.besideEnv(workspace)}) case LevelIntegration: return m.integration(ctx, workspace, tree, packages) } return nil, fmt.Errorf("the level %q is neither unit nor integration", level) } +// besideEnv is MESH_CHECK_BESIDE, naming the directory build_go_fetch placed the workspace's siblings in (ADR 0484 +// clause 4), when it placed any; the tests that read other repositories look there (mesh-sdk go/internal/beside). +func (m *Module) besideEnv(workspace string) []string { + dir := m.besideDir(workspace) + if st, err := os.Stat(dir); err != nil || !st.IsDir() { + return nil + } + return []string{"MESH_CHECK_BESIDE=" + dir} +} + func packagesArg(v any) ([]string, error) { if v == nil { return nil, nil @@ -115,12 +125,16 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR return nil, err } res := &TestResult{Level: o.level, Packages: []PackageResult{}, Setup: o.setup.Seconds(), Store: o.store, Bus: o.bus} - packages := o.packages - if len(packages) == 0 { - packages = []string{"./..."} + byDir, err := packagesByDir(tree, dirs, o.packages) + if err != nil { + return nil, err } began := m.clock() for _, d := range dirs { + packages := byDir[d] + if len(packages) == 0 { + continue // no package asked for lies in this module + } args := []string{"-C", filepath.Join(tree, d), "test", "-json", "-count=1", "-timeout", testTimeout} if len(o.tags) > 0 { args = append(args, "-tags", strings.Join(o.tags, ",")) @@ -141,3 +155,50 @@ func (m *Module) runLevel(ctx context.Context, tree string, o levelOpts) (*TestR res.judge(o.bounds) return res, nil } + +// packagesByDir says which patterns run in which go.mod directory. A relative pattern (./...) runs in every directory, as +// it always did; an import path, as build_go_digest keys its packages, runs only in the directory whose module holds it +// (the longest module path that is the path or its prefix), as another module's path would fail the step. No pattern +// means ./... everywhere. An import path no module of the tree holds is an error that names it. +func packagesByDir(tree string, dirs, patterns []string) (map[string][]string, error) { + out := map[string][]string{} + if len(patterns) == 0 { + patterns = []string{"./..."} + } + modules := map[string]string{} + for _, d := range dirs { + if raw, err := os.ReadFile(filepath.Join(tree, d, "go.mod")); err == nil { + modules[d] = modulePath(string(raw)) + } + } + for _, p := range patterns { + if strings.HasPrefix(p, "./") || p == "." { + for _, d := range dirs { + out[d] = append(out[d], p) + } + continue + } + owner, best := "", -1 + for _, d := range dirs { + mp := modules[d] + if mp != "" && (p == mp || strings.HasPrefix(p, mp+"/")) && len(mp) > best { + owner, best = d, len(mp) + } + } + if best < 0 { + return nil, fmt.Errorf("no module of the workspace holds the package %s", p) + } + out[owner] = append(out[owner], p) + } + return out, nil +} + +// modulePath is the module path a go.mod declares, "" when it declares none. +func modulePath(gomod string) string { + for _, l := range strings.Split(gomod, "\n") { + if f := strings.Fields(l); len(f) == 2 && f[0] == "module" { + return strings.Trim(f[1], `"`) + } + } + return "" +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/beside.go b/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/beside.go new file mode 100644 index 00000000..4b839d02 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/beside.go @@ -0,0 +1 @@ +package beside diff --git a/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/beside_test.go b/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/beside_test.go new file mode 100644 index 00000000..e98b6b71 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/beside_test.go @@ -0,0 +1,22 @@ +package beside + +import ( + "os" + "path/filepath" + "testing" +) + +// A test of a repository that reads another one, as the controller's, the node-engine's, the SDK's and the catalogue's do. +func TestReadsTheSiblingBesideIt(t *testing.T) { + root := os.Getenv("MESH_CHECK_BESIDE") + if root == "" { + t.Fatal("the sibling is not beside this check: MESH_CHECK_BESIDE is not set") + } + raw, err := os.ReadFile(filepath.Join(root, "sibling", "data.txt")) + if err != nil { + t.Fatal(err) + } + if string(raw) != "from the sibling\n" { + t.Fatalf("data.txt = %q", raw) + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/go.mod b/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/go.mod new file mode 100644 index 00000000..206a6e1f --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/testdata/repos/beside/go.mod @@ -0,0 +1,3 @@ +module fixture/beside + +go 1.26 diff --git a/modules/mesh-build-go/go.mod b/modules/mesh-build-go/go.mod index 896d6b95..15a8c845 100644 --- a/modules/mesh-build-go/go.mod +++ b/modules/mesh-build-go/go.mod @@ -3,7 +3,7 @@ module meshbuildgo go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021093645-465beb4ea5e3 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021192148-78364e318bdc github.com/nats-io/nats-server/v2 v2.11.17 ) diff --git a/modules/mesh-build-go/go.sum b/modules/mesh-build-go/go.sum index 08b42fa5..e35e25ab 100644 --- a/modules/mesh-build-go/go.sum +++ b/modules/mesh-build-go/go.sum @@ -1,5 +1,9 @@ git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021093645-465beb4ea5e3 h1:OKxU64kHN1yQ/GOa6LwoUwVmiQ74d83WMSZJRFPVm9c= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021093645-465beb4ea5e3/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021185947-afbe371003a5 h1:1BiPKRPTYpgex8rSHWT9az0BteOafGWRDzd/Ba6GAcA= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021185947-afbe371003a5/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021192148-78364e318bdc h1:ZJScJSnF6LgFl55/VJ9RT/lNroVWun469sXtx0nlVYw= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021192148-78364e318bdc/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-build-node/cmd/mesh-build-node/fetch.go b/modules/mesh-build-node/cmd/mesh-build-node/fetch.go index 687dc6b5..058a80d8 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/fetch.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/fetch.go @@ -36,6 +36,8 @@ const sdkPackage = "@novox/mesh-sdk" // WorkspaceAge is how long a workspace is kept. const WorkspaceAge = 24 * time.Hour +var commitOK = regexp.MustCompile(`^[0-9a-f]{40}$`) + var repositoryOK = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*/[A-Za-z0-9][A-Za-z0-9._-]*$`) // FetchResult is what a passed fetch step says. @@ -48,6 +50,15 @@ type FetchResult struct { // Refused are the bundles whose packages were not installed, each with why (a module naming packages beyond the SDK // with no package-lock.json). The fetch passes; a build of such a bundle fails naming it (issue 1459). Refused []Refusal `json:"refused"` + // The merge preview (ADR 0484 clause 3), only when the call gave a base, as build_go_fetch answers it: the trunk + // commit, the merge commit of the head onto it, the paths the merge changes and the tree of each module directory. + // A head that does not merge cleanly is an answer: Merge is empty, Conflicts names the files, and the workspace is + // left at the trunk commit with nothing installed. + Base string `json:"base,omitempty"` + Merge string `json:"merge,omitempty"` + Conflicts []string `json:"conflicts,omitempty"` + Changed []string `json:"changed,omitempty"` + Trees map[string]buildstep.ModuleTree `json:"trees,omitempty"` } // Refusal is a module a fetch did not install the packages of. @@ -68,6 +79,7 @@ func (m *Module) fetchTool() StepTool { "repository": map[string]any{"type": "string", "description": "/ on the forge"}, "commit": map[string]any{"type": "string", "description": "the full commit, 40 hex digits"}, "ref": map[string]any{"type": "string", "description": "refs/pull//head when only a pull request's head carries the commit"}, + "base": map[string]any{"type": "string", "description": "optional trunk commit, 40 hex digits: the head is merged onto it as the forge's merge would, and the answer carries the merge commit, the files in conflict (an answer, the workspace is then left at the trunk commit), the changed paths and the tree of each module directory"}, }, "required": []string{"repository", "commit"}, }, @@ -80,6 +92,10 @@ func (m *Module) fetch(ctx context.Context, args map[string]any) (any, error) { repository, _ := args["repository"].(string) commit, _ := args["commit"].(string) ref, _ := args["ref"].(string) + base, _ := args["base"].(string) + if base != "" && !commitOK.MatchString(base) { + return nil, fmt.Errorf("%q is not a trunk commit (40 hex digits)", base) + } if !repositoryOK.MatchString(repository) || strings.Contains(repository, "..") { return nil, fmt.Errorf("%q is not a repository, /", repository) } @@ -94,15 +110,23 @@ func (m *Module) fetch(ctx context.Context, args map[string]any) (any, error) { gitRun := func(ctx context.Context, env []string, name string, a ...string) forgekey.Ran { return m.Run(ctx, append(m.baseEnv(), env...), name, a...) } - remote := "ssh://" + ForgeAlias + "/" + repository + ".git" - if err := buildstep.FetchCommit(ctx, gitRun, tree, remote, commit, ref); err != nil { + remote := m.remote(repository) + if err := buildstep.FetchCommit(ctx, gitRun, tree, remote, commit, ref, m.FetchOptions...); err != nil { return nil, err } + res := FetchResult{Workspace: id, Commit: commit, Modules: []string{}, Installed: []string{}, Refused: []Refusal{}} + if base != "" { + if err := m.merge(ctx, gitRun, &res, tree, remote, base, commit); err != nil { + return nil, err + } + if res.Merge == "" { + return res, nil // a conflict: the tree is at the trunk, nothing of the head is installed + } + } dirs, err := m.typeScriptModules(ctx, tree) if err != nil { return nil, err } - res := FetchResult{Workspace: id, Commit: commit, Modules: []string{}, Installed: []string{}, Refused: []Refusal{}} for _, d := range dirs { res.Modules = append(res.Modules, d) did, refused, err := m.installOwn(ctx, root, filepath.Join(tree, filepath.FromSlash(d)), d) diff --git a/modules/mesh-build-node/cmd/mesh-build-node/harness_test.go b/modules/mesh-build-node/cmd/mesh-build-node/harness_test.go index 07fa1e5b..da417017 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/harness_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/harness_test.go @@ -48,7 +48,7 @@ func (f *fakeMachine) run(_ context.Context, env []string, name string, args ... } } else if name == "git" { for _, a := range args { - if slices.Contains([]string{"init", "remote", "fetch", "checkout", "rev-parse", "ls-files"}, a) { + if slices.Contains([]string{"init", "remote", "fetch", "checkout", "rev-parse", "ls-files", "merge", "diff"}, a) { key = "git " + a break } diff --git a/modules/mesh-build-node/cmd/mesh-build-node/module.go b/modules/mesh-build-node/cmd/mesh-build-node/module.go index 1cc1460c..1f9e60d0 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/module.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/module.go @@ -31,9 +31,13 @@ type Module struct { Toolchain *Toolchain // Work is the work directory, outside the home: a workspace is //tree. Run runs the commands of a // step (a fake in a test). Clock is the time the steps are timed by. - Work string - Run forgekey.Runner - Clock func() time.Time + Work string + Run forgekey.Runner + // RemoteBase is where repositories are fetched from, "ssh://mesh-forge/" when empty; FetchOptions are the options of + // the SDK's fetch and merge. A test points them at a local repository (buildstep.AllowFileRemote). + RemoteBase string + FetchOptions []buildstep.Option + Clock func() time.Time // Toolchains and Steps are the Put of the states of those names. Toolchains, Steps buildstep.Put // ReadSteps and ReadToolchainsState read those two states back for the read tools (issue 1562). diff --git a/modules/mesh-build-node/cmd/mesh-build-node/preview.go b/modules/mesh-build-node/cmd/mesh-build-node/preview.go new file mode 100644 index 00000000..6d641b9b --- /dev/null +++ b/modules/mesh-build-node/cmd/mesh-build-node/preview.go @@ -0,0 +1,35 @@ +package main + +// The merge preview of a fetch (novox/hq ADR 0484 clause 3; issue 1676), as build_go_fetch answers it. The merge itself +// is the SDK's (buildstep.MergeInto), the one every build module's fetch step makes. + +import ( + "context" + + "git.novox.be/novox/mesh-sdk/go/buildstep" + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +// remote is the address a repository is fetched from: the forge over ssh as the account, or RemoteBase when a test set it. +func (m *Module) remote(repository string) string { + base := m.RemoteBase + if base == "" { + base = "ssh://" + ForgeAlias + "/" + } + return base + repository + ".git" +} + +// merge makes the merge preview of the head onto base and writes the answer into res. +func (m *Module) merge(ctx context.Context, run forgekey.Runner, res *FetchResult, tree, remote, base, commit string) error { + mr, err := buildstep.MergeInto(ctx, run, tree, remote, base, commit, m.FetchOptions...) + if err != nil { + return err + } + res.Base = mr.Base + res.Conflicts, res.Merge, res.Changed = mr.Conflicts, mr.Merge, mr.Changed + if res.Merge == "" { + return nil + } + res.Trees, err = buildstep.ModuleTrees(ctx, m.Run, m.baseEnv(), tree, res.Merge) + return err +} diff --git a/modules/mesh-build-node/cmd/mesh-build-node/preview_integration_test.go b/modules/mesh-build-node/cmd/mesh-build-node/preview_integration_test.go new file mode 100644 index 00000000..487eebfe --- /dev/null +++ b/modules/mesh-build-node/cmd/mesh-build-node/preview_integration_test.go @@ -0,0 +1,95 @@ +//go:build integration + +package main + +// The fetch step's merge on real git repositories (issue 1676); preview_test.go proves the commands with a fake runner. + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/buildstep" + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +func git(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir, "-c", "user.name=t", "-c", "user.email=t@example.invalid", "-c", "commit.gpgsign=false"}, args...)...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +func TestRealFetchMergesAConflictAndAPlainFetch(t *testing.T) { + root := t.TempDir() + bare, work := filepath.Join(root, "forge", "o", "r.git"), filepath.Join(root, "work") + os.MkdirAll(bare, 0o755) + os.MkdirAll(work, 0o755) + git(t, bare, "init", "-q", "--bare") + git(t, work, "init", "-q", "-b", "main") + files := typeScriptModule(`{"dependencies":{"@novox/mesh-sdk":"^0.1.6"}}`) + files["a.txt"] = "one\ntwo\nthree\n" + for n, c := range files { + write(t, filepath.Join(work, n), c, 0o644) + } + git(t, work, "add", ".") + git(t, work, "commit", "-q", "-m", "fork") + fork := git(t, work, "rev-parse", "HEAD") + change := func(from, ref string, f map[string]string) string { + git(t, work, "checkout", "-q", "-B", "tmp", from) + for n, c := range f { + write(t, filepath.Join(work, n), c, 0o644) + } + git(t, work, "add", ".") + git(t, work, "commit", "-q", "-m", "c") + git(t, work, "push", "-q", "-f", bare, "HEAD:"+ref) + return git(t, work, "rev-parse", "HEAD") + } + trunk := change(fork, "refs/heads/main", map[string]string{"a.txt": "one\ntwo\nTRUNK\n"}) + clean := change(fork, "refs/pull/1/head", map[string]string{"modules/m/extra.ts": "export {}\n"}) + clash := change(fork, "refs/pull/2/head", map[string]string{"a.txt": "one\ntwo\nHEAD\n"}) + + m, _, _ := newMachine(t) + m.Run = forgekey.ExecRunner + m.RemoteBase = "file://" + filepath.Join(root, "forge") + "/" + m.FetchOptions = []buildstep.Option{buildstep.AllowFileRemote()} + + out, err := m.fetch(context.Background(), map[string]any{"repository": "o/r", "commit": clean, "ref": "refs/pull/1/head", "base": trunk}) + if err != nil { + t.Fatal(err) + } + r := out.(FetchResult) + tree := filepath.Join(m.Work, r.Workspace, "tree") + if r.Merge == "" || strings.Join(r.Changed, ",") != "modules/m/extra.ts" || len(r.Conflicts) != 0 || len(r.Modules) != 1 { + t.Fatalf("clean: %+v", r) + } + if want := git(t, tree, "rev-parse", r.Merge+":modules/m"); r.Trees["modules/m"].Tree != want { + t.Errorf("tree = %s, want %s", r.Trees["modules/m"].Tree, want) + } + if got := git(t, tree, "rev-parse", "HEAD"); got != r.Merge { + t.Errorf("the workspace is at %s, want the merge %s", got, r.Merge) + } + + out, err = m.fetch(context.Background(), map[string]any{"repository": "o/r", "commit": clash, "ref": "refs/pull/2/head", "base": trunk}) + if err != nil { + t.Fatalf("a conflict failed the step: %v", err) + } + r = out.(FetchResult) + if r.Merge != "" || strings.Join(r.Conflicts, ",") != "a.txt" { + t.Fatalf("conflict: %+v", r) + } + if got := git(t, filepath.Join(m.Work, r.Workspace, "tree"), "rev-parse", "HEAD"); got != trunk { + t.Errorf("after a conflict the workspace is at %s, want the trunk %s", got, trunk) + } + + out, err = m.fetch(context.Background(), map[string]any{"repository": "o/r", "commit": clean, "ref": "refs/pull/1/head"}) + if err != nil || out.(FetchResult).Merge != "" || out.(FetchResult).Trees != nil { + t.Errorf("no base: %+v %v", out, err) + } +} diff --git a/modules/mesh-build-node/cmd/mesh-build-node/preview_test.go b/modules/mesh-build-node/cmd/mesh-build-node/preview_test.go new file mode 100644 index 00000000..8547d35e --- /dev/null +++ b/modules/mesh-build-node/cmd/mesh-build-node/preview_test.go @@ -0,0 +1,87 @@ +package main + +import ( + "context" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/forgekey" +) + +const baseSHA = "abcdef0123456789abcdef0123456789abcdef01" + +func commandLines(f *fakeMachine) string { + var out []string + for _, c := range f.calls { + out = append(out, joined(c)) + } + return strings.Join(out, "\n") +} + +// 1676/AC1: with a base the fetch merges the head onto it through the SDK's merge (the check merge's identity and flags, +// hooks off), and answers the base, the merge commit, the changed paths and the tree, manifest and locks of each module +// directory; the TypeScript modules are installed from the merged tree. +func TestFetchWithABaseMergesAndAnswersThePreview(t *testing.T) { + m, f := fetchMachine(t, typeScriptModule(`{"dependencies":{"@novox/mesh-sdk":"^0.1.6"}}`), nil) + f.answer["git diff"] = forgekey.Ran{Stdout: "modules/m/package.json\n"} + args := map[string]any{"repository": "novox/mesh-catalog", "commit": commitSHA, "base": baseSHA} + out, err := m.fetch(context.Background(), args) + if err != nil { + t.Fatal(err) + } + r := out.(FetchResult) + if r.Base != baseSHA || r.Merge != commitSHA || strings.Join(r.Changed, ",") != "modules/m/package.json" || len(r.Conflicts) != 0 { + t.Errorf("result = %+v", r) + } + if mt, ok := r.Trees["modules/m"]; !ok || mt.Tree != commitSHA || mt.Manifest == "" { + t.Errorf("trees = %+v", r.Trees) + } + if len(r.Modules) != 1 || r.Modules[0] != "modules/m" { + t.Errorf("modules = %v", r.Modules) + } + for _, want := range []string{"fetch --no-tags origin " + baseSHA, "core.hooksPath=/dev/null", "merge --no-ff --no-edit --no-verify"} { + if !strings.Contains(commandLines(f), want) { + t.Errorf("git was not asked %q:\n%s", want, commandLines(f)) + } + } +} + +// 1676/AC2: a conflict is an answer: the step passes with the files named, no merge commit, and nothing installed. +func TestFetchWithAConflictAnswersTheFilesAndInstallsNothing(t *testing.T) { + files := typeScriptModule(`{"dependencies":{"@novox/mesh-sdk":"^0.1.6","lodash":"^4"}}`) + files["modules/m/package-lock.json"] = `{"lockfileVersion":3}` + m, f := fetchMachine(t, files, nil) + f.answer["git merge"] = forgekey.Ran{Status: 1, Stderr: "CONFLICT\n"} + f.answer["git diff"] = forgekey.Ran{Stdout: "a.ts\nsub/b.ts\n"} + out, err := m.fetch(context.Background(), map[string]any{"repository": "novox/mesh-catalog", "commit": commitSHA, "base": baseSHA}) + if err != nil { + t.Fatalf("a conflict failed the step: %v", err) + } + r := out.(FetchResult) + if r.Merge != "" || strings.Join(r.Conflicts, ",") != "a.ts,sub/b.ts" || len(r.Changed) != 0 || len(npmCalls(f)) != 0 { + t.Errorf("result = %+v, npm calls %d", r, len(npmCalls(f))) + } + if !strings.Contains(commandLines(f), "merge --abort") { + t.Errorf("the merge was not undone:\n%s", commandLines(f)) + } +} + +// 1676/AC3: no base is today's fetch: no merge command, and none of the preview in the answer. +func TestFetchWithoutABaseMakesNoMerge(t *testing.T) { + m, f := fetchMachine(t, typeScriptModule(`{"dependencies":{"@novox/mesh-sdk":"^0.1.6"}}`), nil) + out, err := m.fetch(context.Background(), fetchArgs) + if err != nil { + t.Fatal(err) + } + r := out.(FetchResult) + if strings.Contains(commandLines(f), "merge") || strings.Contains(commandLines(f), "unshallow") || r.Base != "" || r.Merge != "" || r.Trees != nil { + t.Errorf("a fetch without a base merged: %+v\n%s", r, commandLines(f)) + } +} + +func TestABaseThatIsNotACommitIsRefusedBeforeAnythingRuns(t *testing.T) { + m, f := fetchMachine(t, nil, nil) + if _, err := m.fetch(context.Background(), map[string]any{"repository": "novox/mesh-catalog", "commit": commitSHA, "base": "main"}); err == nil || len(f.calls) != 0 { + t.Errorf("err = %v after %d commands", err, len(f.calls)) + } +} diff --git a/modules/mesh-build-node/go.mod b/modules/mesh-build-node/go.mod index 322bf6f2..4b453f9b 100644 --- a/modules/mesh-build-node/go.mod +++ b/modules/mesh-build-node/go.mod @@ -2,4 +2,4 @@ module meshbuildnode go 1.26.0 -require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021093645-465beb4ea5e3 +require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021192148-78364e318bdc diff --git a/modules/mesh-build-node/go.sum b/modules/mesh-build-node/go.sum index 5d8e396c..8097dd56 100644 --- a/modules/mesh-build-node/go.sum +++ b/modules/mesh-build-node/go.sum @@ -1,2 +1,6 @@ git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021093645-465beb4ea5e3 h1:OKxU64kHN1yQ/GOa6LwoUwVmiQ74d83WMSZJRFPVm9c= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021093645-465beb4ea5e3/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021185947-afbe371003a5 h1:1BiPKRPTYpgex8rSHWT9az0BteOafGWRDzd/Ba6GAcA= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021185947-afbe371003a5/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021192148-78364e318bdc h1:ZJScJSnF6LgFl55/VJ9RT/lNroVWun469sXtx0nlVYw= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021192148-78364e318bdc/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go index 71c3fc5b..da3befe5 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go @@ -634,6 +634,7 @@ func (h *Holder) answered(key string, ans sdk.Answer) []func() { for _, d := range covered { r := d.Ready r.Warrant = ans.Words + r.Repeats = 0 // the operator chose: whatever the tool refused before is asked afresh (issue 1106) h.mergeAgainNow(d) // the operator chose: the merge is asked at once switch ans.Option { case OptionApprove: diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go index 07bb1c26..4555603b 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go @@ -173,3 +173,43 @@ func TestAGroupMergedWholeMovesSourceMainOnceAndAnnouncesWithTheTeller(t *testin t.Fatalf("source.main moved again: %d", len(w.source.moves)) } } + +// 1106: the same refusal again and again with the trunk not moved is not asked on without end. Three refusals in a row with +// the same cause end the asking by a verdict, a status or a composed check, and say it urgent; a trunk's move, or the +// operator's Approve, starts it again. +func TestARefusalThatRepeatsWithNoTrunkMoveIsNotAskedAgainWithoutEnd(t *testing.T) { + w := answeredFirst(t) + d := w.h.deliveries[IDOf(gCat, gHk)] + refuse := func(took string) { + w.h.ForgeRefused(sdk.Forge{Kind: sdk.ForgeRefused, Repository: gCat, Number: 2, Head: gHk, Act: "merge", + Words: "novox/mesh-catalog#2 is not merged: its verdict names no trunk commit it was judged against, and main is now f70a99dbf485. " + + "The check of head d2d2d2d2d2d2 merged into trunk f70a99dbf485 was asked; the last check took " + took + ". Ask again once both statuses pass"}) + } + tried := w.now + for i, took := range []string{"4m1s", "4m9s"} { + refuse(took) + d.Ready.LastTry = tried + w.h.mergeAgainNow(d) + if !d.Ready.LastTry.IsZero() { + t.Fatalf("refusal %d: the merge was not asked again at once on a verdict, as after a first refusal", i+1) + } + } + refuse("4m5s") // the third in a row, with only the check's duration changed + if d.Ready.Repeats != 3 { + t.Fatalf("%d refusals counted for the same cause, wanted 3", d.Ready.Repeats) + } + d.Ready.LastTry = tried + w.h.mergeAgainNow(d) // the composed check or a verdict answers: nothing is asked + if d.Ready.LastTry.IsZero() { + t.Fatal("the merge was asked again after the same refusal three times with the trunk not moved") + } + if lines := urgentLines(w); len(lines) != 1 || !strings.Contains(lines[0].Says, "names no trunk commit") { + t.Fatalf("the repeated refusal is not said urgent: %+v", lines) + } + // The trunk moves: the merge is asked again. + d.TrunkNow = "a1a1a1a1a1a1a1a1" + w.h.mergeAgainNow(d) + if !d.Ready.LastTry.IsZero() { + t.Fatal("a trunk's move did not let the merge be asked again") + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go index 0ec30658..4070027b 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -287,9 +287,20 @@ func (h *Holder) ForgeRefused(f sdk.Forge) { if r.RefusedSince == nil { r.RefusedSince = &now } + r.noteRefusal(d, answer) if !outOfDateRefusal(answer) { r.Urgent = true // never a warning again for this head } + if r.Repeats >= maxRepeatedRefusals { + // Loud, not endless (novox/hq issue 1106): the same refusal again with the trunk not moved is no trunk's move, so + // the merge is not asked again by a verdict or a status; the operator is told, urgent, and decides. + r.Urgent = true + if r.Repeats == maxRepeatedRefusals { + h.Logf("[mesh-delivery] %s: the merge tool refused it %d times in a row with the same words and the trunk not moved, so "+ + "the merge is asked no more until the trunk moves, a new head is announced or the operator approves again: %s", + d.ID, r.Repeats, answer) + } + } if answer != r.Answer { h.Logf("[mesh-delivery] %s is approved, and the merge tool did not merge it (asked again at the next verdict, Approve or composed check): %s", d.ID, answer) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go index ffccdb18..f2809277 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -85,6 +85,11 @@ type ReadyRun struct { // Urgent is set once a refusal of this head was one that is urgent at once: its severity never steps down to a // warning after (issue 1275, AC6). Urgent bool `json:"urgent,omitempty"` + // Repeats is how many times in a row the merge tool refused with the same words while the trunk stayed where RepeatTrunk + // says: past maxRepeatedRefusals the merge is asked no more by a verdict, a status or a composed check, and the refusal + // is urgent (novox/hq issue 1106). A trunk's move, a new head or the operator's Approve starts the count again. + Repeats int `json:"repeats,omitempty"` + RepeatTrunk string `json:"repeat_trunk,omitempty"` // RefusedSince is when the merge tool began refusing it without a break. RefusedSince *time.Time `json:"refused_since,omitempty"` // Merged is when the merge tool merged it. @@ -322,10 +327,43 @@ func (h *Holder) askFresh(d *Delivery, now time.Time) func() { // verdict, an Approve, a group's composed check), so the wait after a refusal does not hold it. Under the lock. func (h *Holder) mergeAgainNow(d *Delivery) { if d != nil && d.Ready != nil && d.Ready.Merged == nil { + if d.Ready.repeatedRefusal(d) { + return // the same refusal again and again with the trunk where it was: asking more changes nothing (issue 1106) + } d.Ready.LastTry = time.Time{} } } +// maxRepeatedRefusals is how many refusals in a row, with the same words and the trunk not moved, end the asking again: a +// refusal that repeats with no trunk move is not a trunk's move, and the check asked after each only brings the same words. +const maxRepeatedRefusals = 3 + +// repeatedRefusal is whether the merge was refused maxRepeatedRefusals times with these words and the trunk where it is +// now: it is then asked no more until something changes (a trunk's move, a new head, the operator's Approve). +func (r *ReadyRun) repeatedRefusal(d *Delivery) bool { + return r.Repeats >= maxRepeatedRefusals && r.RepeatTrunk == d.TrunkNow +} + +// refusalKey is a refusal's first sentence, up to the trunk it names ("... and main is now "): what follows says how long the +// last check took and the like, which differs each time and is no change of cause. +func refusalKey(answer string) string { + if i := strings.Index(answer, " is now "); i >= 0 { + if j := strings.Index(answer[i:], ". "); j >= 0 { + return answer[:i+j] + } + } + return answer +} + +// noteRefusal counts a refusal: the same words with the trunk where it was count on, anything else starts the count at one. +func (r *ReadyRun) noteRefusal(d *Delivery, answer string) { + if refusalKey(answer) == refusalKey(r.Answer) && r.RepeatTrunk == d.TrunkNow && r.Repeats > 0 { + r.Repeats++ + return + } + r.Repeats, r.RepeatTrunk = 1, d.TrunkNow +} + // readyWaits is what a delivery's merge waits on; empty when the Approve may be asked and the merge asked. A ready // delivery is asked. So is a proposed one whose verdict went out of date, when an Approve was asked for it already (it // stays through a trunk move, ADR 0421 §3) and its check against the trunk's head is not on its way; one without an diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go index 52ea5bd4..7bd28b50 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client.go @@ -119,6 +119,9 @@ type Client struct { Templates *templateStore // Now dates the reports; nil is time.Now. Now func() time.Time + // SocketUp wakes the client when the node-engine's write socket is made again (watchSocket); nil never does. + // A module whose write failed for want of the socket is filled again then (issue 1165), and nothing else is. + SocketUp <-chan struct{} // watchBound watches a provider's bucket of entries for this machine's keys, once per run (consumers.go); set by // Run. @@ -147,6 +150,8 @@ type state struct { // of the consumers left out of one of its files. providing map[string]bool leftOut map[string]string + // unplaced is a module whose last fill failed for want of the node-engine's write socket (issue 1165). + unplaced map[string]bool // bound is each provider's bucket of entries this client watches, by key: this machine's consumers' entries alone; // boundReady is a bucket whose current entries are all read, and boundUsers the modules filled from each. bound map[string]map[string][]byte @@ -161,6 +166,8 @@ type update struct { changed []byte // asked is the tool runner asking for a module's value (asks.go). Set, entry is nil. asked *nats.Msg + // socketUp is the node-engine's write socket made again (issue 1165). Set, entry is nil. + socketUp bool } // Run serves until ctx ends or a watch stops, which it returns: run again, it starts from the current values. @@ -248,6 +255,23 @@ func (c *Client) Run(ctx context.Context) error { return fmt.Errorf("listening for %s: %w", SecretChangedSubject, err) } defer func() { _ = said.Unsubscribe() }() + // The node-engine's write socket made again: what failed for want of it is filled again, in the loop below. + if c.SocketUp != nil { + go func() { + for { + select { + case <-ctx.Done(): + return + case <-c.SocketUp: + select { + case updates <- update{socketUp: true}: + case <-ctx.Done(): + return + } + } + } + }() + } // This machine's tool runner asking for a module's config value (asks.go): answered from this run's state, in // its loop, so an answer is never read from half an update. asks, err := c.Conn.Subscribe(AskSubject(c.Node), func(m *nats.Msg) { @@ -266,7 +290,7 @@ func (c *Client) Run(ctx context.Context) error { reported: map[string][]byte{}, secrets: map[string]heldSecret{}, items: map[string]map[string]itemValue{}, recorded: map[string][]byte{}, assigned: map[string]bool{}, binds: map[string]binding{}, providing: map[string]bool{}, leftOut: map[string]string{}, bound: map[string]map[string][]byte{}, - boundReady: map[string]bool{}, boundUsers: map[string]map[string]bool{}} + boundReady: map[string]bool{}, unplaced: map[string]bool{}, boundUsers: map[string]map[string]bool{}} // Every current value first: nothing is filled from half of them. current := map[string]bool{} var early []*nats.Msg @@ -282,6 +306,9 @@ func (c *Client) Run(ctx context.Context) error { early = append(early, u.asked) continue } + if u.socketUp { + continue // the first fill below tries every file + } if u.changed != nil { // Nothing is held yet: every secret is asked for when it is first filled. continue @@ -326,6 +353,10 @@ func (c *Client) Run(ctx context.Context) error { switch { case u.asked != nil: c.respond(s, u.asked) + case u.socketUp: + for m := range s.unplaced { + dirty[m] = true + } case u.changed != nil: for _, m := range c.secretChanged(s, u.changed) { dirty[m] = true @@ -550,6 +581,7 @@ func (c *Client) leftModules(s *state) []string { // leave removes a module's needs and its report: it is no longer the provisioner's on this node. func (c *Client) leave(ctx context.Context, kvs map[string]jetstream.KeyValue, s *state, module string) { prefix := c.Node + "." + module + "." + delete(s.unplaced, module) removed := 0 for _, key := range sortedKeys(s.needs) { if strings.HasPrefix(key, prefix) { @@ -660,6 +692,7 @@ func (c *Client) reconcile(ctx context.Context, kvs map[string]jetstream.KeyValu c.forgetItems(ctx, kvs, s, module, keep) } moved := false + delete(s.unplaced, module) for _, f := range targets { // File by file (issue 1553, ADR 0349 clause 4): a target with a value that waits keeps the file there is. if !allKnown(f.text, values) { @@ -672,6 +705,9 @@ func (c *Client) reconcile(ctx context.Context, kvs map[string]jetstream.KeyValu err = errFill(a) } if err != nil { + if socketAbsent(err) { + s.unplaced[module] = true + } failed = append(failed, fmt.Sprintf("%s: %v", f.Path, err)) continue } diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client_test.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client_test.go index 342ecb2d..cc089ba4 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client_test.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/client_test.go @@ -200,6 +200,14 @@ type engine struct { } func startEngine(t *testing.T, owners map[string]string) *engine { + t.Helper() + e := newEngine(t, owners) + e.listen(t) + return e +} + +// newEngine is an engine whose socket is not made yet: a node-engine not started, or replacing itself. +func newEngine(t *testing.T, owners map[string]string) *engine { t.Helper() // A unix socket's path is short: under /tmp, not the test's own directory. dir, err := os.MkdirTemp("/tmp", "p5-") @@ -209,6 +217,12 @@ func startEngine(t *testing.T, owners map[string]string) *engine { t.Cleanup(func() { os.RemoveAll(dir) }) e := &engine{root: t.TempDir(), socket: filepath.Join(dir, "fill.sock"), owner: owners, moved: map[string]int{}, asked: map[string]int{}, wrote: map[string]string{}} + return e +} + +// listen makes the socket and answers on it until stop, which removes it as a node-engine stopping does. +func (e *engine) listen(t *testing.T) (stop func()) { + t.Helper() l, err := net.Listen("unix", e.socket) if err != nil { t.Fatal(err) @@ -233,7 +247,7 @@ func startEngine(t *testing.T, owners map[string]string) *engine { }() } }() - return e + return func() { l.Close(); os.Remove(e.socket) } } func (e *engine) at(path string) string { return filepath.Join(e.root, path) } @@ -349,10 +363,14 @@ func aClient(t *testing.T, b *aBus, e *engine, state string) (*Client, func()) { t.Fatal(err) } secrets.Within = 2 * time.Second - c := &Client{Node: node, JS: js, Conn: nc, Secrets: secrets, + ctx, cancel := context.WithCancel(context.Background()) + up, err := watchSocket(ctx, e.socket) + if err != nil { + t.Fatal(err) + } + c := &Client{Node: node, JS: js, Conn: nc, Secrets: secrets, SocketUp: up, Fill: SocketFiller{Socket: e.socket, Busy: 10 * time.Millisecond, Tries: 3}, Templates: &templateStore{dir: state, fetch: HTTPFetch}} - ctx, cancel := context.WithCancel(context.Background()) done := make(chan error, 1) go func() { done <- c.Run(ctx) }() stopped := false @@ -703,3 +721,23 @@ func TestNatsResticsFilesFillFromTheBackupPlacement(t *testing.T) { b.assign(t, "restic", plan("# a\n/srv/a\n", "# a\nstate\n")) eventually(t, "a module taken away refills backups.conf", func() bool { return e.read(t, backups) == head+"# a\n/srv/a\n" }) } + +// defaults writes the defaults a module's manifest declares as mesh-config resolves them (source default), for a +// moved module whose templates read config values (issue 1780). +func (b *aBus) defaults(t *testing.T, module string, manifest []byte) { + t.Helper() + var m struct { + Config map[string]struct { + Default json.RawMessage `json:"default"` + } `json:"config"` + } + if err := json.Unmarshal(manifest, &m); err != nil { + t.Fatal(err) + } + for key, d := range m.Config { + if len(d.Default) > 0 && string(d.Default) != "null" { + b.put(t, ConfigBucket, node+"."+module+"."+key, fmt.Sprintf(`{"node":%q,"module":%q,"key":%q,"value":%s,`+ + `"source":"default"}`, node, module, key, d.Default)) + } + } +} diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/environment_test.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/environment_test.go index 25b4e88e..ae455c35 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/environment_test.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/environment_test.go @@ -158,6 +158,7 @@ func TestNatsTheEnvironmentOfTheCataloguesModulesIsWhatTheOldFormComposed(t *tes if !manifest.Moved || len(manifest.Items) == 0 { t.Fatalf("%s is not a moved contributor of items", m) } + b.defaults(t, m, read(m+"/module.json")) texts := map[string]string{} var planned []PlannedItem for _, it := range manifest.Items { diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/main.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/main.go index 72919bc8..3d9dc49c 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/main.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/main.go @@ -133,7 +133,11 @@ func serve(ctx context.Context, credential, socket, stateDir string) error { if err != nil { return err } - client := &Client{Node: c.Node, JS: js, Conn: nc, Secrets: secrets, + up, err := watchSocket(ctx, socket) + if err != nil { + return err + } + client := &Client{Node: c.Node, SocketUp: up, JS: js, Conn: nc, Secrets: secrets, Fill: SocketFiller{Socket: socket, Busy: time.Second, Tries: 10}, Templates: &templateStore{dir: filepath.Join(stateDir, "templates"), fetch: HTTPFetch}} wait := time.Second diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/socket_integration_test.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/socket_integration_test.go new file mode 100644 index 00000000..094a7e29 --- /dev/null +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/socket_integration_test.go @@ -0,0 +1,108 @@ +//go:build integration + +package main + +// Issue 1165: these tests make a real unix socket and watch its directory, so they run in the integration level. + +import ( + "context" + "net" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// Issue 1165: a file whose write failed because the node-engine's socket was gone is written when the socket is made +// again, with no timer; a file that was placed is not asked again by it. +func TestNatsAFillThatFailedForWantOfTheSocketIsTriedWhenTheSocketReturns(t *testing.T) { + b := startBus(t) + heard := b.listen(t) + e := newEngine(t, map[string]string{dunstrc: "owned"}) + b.config(t, "dunst", "font-size", "13") + b.assign(t, "dunst", dunst(t)) + aClient(t, b, e, t.TempDir()) + + eventually(t, "the failure is said", func() bool { + r, _ := heard.last("dunst") + return r.Status == StatusFailed && strings.Contains(r.Reason, "fill.sock") + }) + if got := e.read(t, dunstrc); got != "" { + t.Fatalf("written without a socket: %q", got) + } + e.listen(t) + eventually(t, "dunstrc is written once the socket is back", func() bool { + return strings.Contains(e.read(t, dunstrc), "Monospace 13") + }) + eventually(t, "and the report says applied", func() bool { + r, _ := heard.last("dunst") + return r.Status == StatusApplied + }) +} + +func TestNatsAReturningSocketAsksNothingOfAModuleThatWasPlaced(t *testing.T) { + b := startBus(t) + e := newEngine(t, map[string]string{dunstrc: "owned"}) + stop := e.listen(t) + b.config(t, "dunst", "font-size", "13") + b.assign(t, "dunst", dunst(t)) + aClient(t, b, e, t.TempDir()) + eventually(t, "dunstrc is filled", func() bool { return strings.Contains(e.read(t, dunstrc), "Monospace 13") }) + _, before := e.counts(dunstrc) + + // The node-engine replaces itself: the socket goes and returns, and a later change proves the loop ran past it. + stop() + e.listen(t) + b.config(t, "dunst", "font-size", "15") + eventually(t, "the change is written", func() bool { return strings.Contains(e.read(t, dunstrc), "Monospace 15") }) + _, after := e.counts(dunstrc) + if after-before != 1 { + t.Errorf("dunstrc was asked %d times for one change", after-before) + } +} + +func TestWatchSocketWakesWhenTheSocketIsMadeAndWhenItsDirectoryReturns(t *testing.T) { + base, err := os.MkdirTemp("/tmp", "ws-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(base) }) + dir := filepath.Join(base, "engine") + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatal(err) + } + socket := filepath.Join(dir, "fill.sock") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + up, err := watchSocket(ctx, socket) + if err != nil { + t.Fatal(err) + } + heard := func(what string) { + t.Helper() + select { + case <-up: + case <-time.After(5 * time.Second): + t.Fatalf("no wake for %s", what) + } + } + l, err := net.Listen("unix", socket) + if err != nil { + t.Fatal(err) + } + heard("the socket made") + l.Close() + // The whole directory goes, as with a runtime directory removed on stop, and returns with the socket. + if err := os.RemoveAll(dir); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatal(err) + } + if l, err = net.Listen("unix", socket); err != nil { + t.Fatal(err) + } + defer l.Close() + heard("the directory and socket back") +} diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/socketwatch.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/socketwatch.go new file mode 100644 index 00000000..78df0b6f --- /dev/null +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/socketwatch.go @@ -0,0 +1,103 @@ +package main + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "syscall" + "unsafe" + + "golang.org/x/sys/unix" +) + +// socketAbsent says whether a write failed because nothing answers at the node-engine's write socket: its file is +// gone, or it is there with no one listening. That is the node-engine replacing itself (issue 1165); the write is +// tried again when the socket is back (watchSocket). +func socketAbsent(err error) bool { + return errors.Is(err, syscall.ENOENT) || errors.Is(err, syscall.ECONNREFUSED) +} + +// watchSocket tells, on the channel it returns, each time the file at socket is made, moved in or changed in its +// attributes — a node-engine that starts makes its write socket and sets its mode — until ctx ends. It is the +// operating system's own event (inotify), not a timer: the client is on the node-engine's machine. Events that +// come while one is unread are one. When the socket's directory is gone it watches the directory above for its +// return. The channel only wakes: what is owed a write is the client's to read. +func watchSocket(ctx context.Context, socket string) (<-chan struct{}, error) { + fd, err := unix.InotifyInit1(unix.IN_CLOEXEC | unix.IN_NONBLOCK) + if err != nil { + return nil, fmt.Errorf("watching %s: %w", socket, err) + } + f := os.NewFile(uintptr(fd), "inotify") + out := make(chan struct{}, 1) + dir := filepath.Dir(socket) + // The directory's watch and, while the directory is absent, its parent's. + if err := addWatches(fd, dir); err != nil { + _ = f.Close() + return nil, fmt.Errorf("watching %s: %w", socket, err) + } + go func() { <-ctx.Done(); _ = f.Close() }() + go func() { + buf := make([]byte, 16*(unix.SizeofInotifyEvent+unix.NAME_MAX+1)) + for { + n, err := f.Read(buf) + if err != nil { + return + } + wake, rewatch := false, false + for off := 0; off+unix.SizeofInotifyEvent <= n; { + ev := (*unix.InotifyEvent)(unsafe.Pointer(&buf[off])) + name := "" + if ev.Len > 0 { + raw := buf[off+unix.SizeofInotifyEvent : off+unix.SizeofInotifyEvent+int(ev.Len)] + for i, b := range raw { + if b == 0 { + raw = raw[:i] + break + } + } + name = string(raw) + } + off += unix.SizeofInotifyEvent + int(ev.Len) + switch { + case ev.Mask&(unix.IN_DELETE_SELF|unix.IN_MOVE_SELF|unix.IN_IGNORED) != 0: + rewatch = true + case name == filepath.Base(socket): + wake = true + case name == filepath.Base(dir): + // The directory came back: watched again, and the socket may already be in it. + rewatch, wake = true, true + } + } + if rewatch { + if err := addWatches(fd, dir); err != nil { + logf("[provisioner-client] watching %s again: %v", socket, err) + } + if _, err := os.Lstat(socket); err == nil { + wake = true + } + } + if wake { + select { + case out <- struct{}{}: + default: + } + } + } + }() + return out, nil +} + +// addWatches watches dir for what is made in it, and its parent for dir's own return; a watch already there is kept. +func addWatches(fd int, dir string) error { + if _, err := unix.InotifyAddWatch(fd, filepath.Dir(dir), unix.IN_CREATE|unix.IN_MOVED_TO); err != nil { + return err + } + _, err := unix.InotifyAddWatch(fd, dir, unix.IN_CREATE|unix.IN_MOVED_TO|unix.IN_ATTRIB|unix.IN_DELETE_SELF| + unix.IN_MOVE_SELF) + if errors.Is(err, syscall.ENOENT) { + return nil // absent: its parent's watch tells when it is made + } + return err +} diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/xorg_golden_test.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/xorg_golden_test.go index 2f99d55b..fabc35ac 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/xorg_golden_test.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/xorg_golden_test.go @@ -63,6 +63,7 @@ func TestNatsTheSessionFilesFilledFromTheirContributorsItemsAreWhatTheOldFormCom if !c.Moved || len(c.Shell) != 0 { t.Fatalf("%s is not moved or still holds shell entries", m) } + b.defaults(t, m, []byte(read(m+"/module.json"))) texts := map[string]string{} var planned []PlannedItem for _, it := range c.Items { diff --git a/modules/mesh-provisioner-client/go.mod b/modules/mesh-provisioner-client/go.mod index 1d3e1806..c444be2e 100644 --- a/modules/mesh-provisioner-client/go.mod +++ b/modules/mesh-provisioner-client/go.mod @@ -6,6 +6,7 @@ require ( git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261021114014-66e8fdaa62e2 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 + golang.org/x/sys v0.48.0 ) require ( @@ -17,6 +18,5 @@ require ( github.com/nats-io/nkeys v0.4.16 // indirect github.com/nats-io/nuid v1.0.1 // indirect golang.org/x/crypto v0.57.0 // indirect - golang.org/x/sys v0.48.0 // indirect golang.org/x/time v0.15.0 // indirect )