From e0bf87c2e3da778fe7998dceb75044a0a4f3d32b Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 21:24:12 +0200 Subject: [PATCH] Give /var/lib/systemd and /var/lib/postgres back to their packages, and keep the modules' files in their own directories (issue 1873) The systemd and postgres modules' state directories said place ".", which is /var/lib/systemd and /var/lib/postgres: directories the systemd and postgresql packages own (root 0755; postgres 0700). The node-engine took both before issue 339's rule and gave them to the operator's account, 0700, on every machine; postgres's grants directory, placed by default, lay inside /var/lib/postgres too. What each module keeps moves to a directory of its own, "data", placed "mesh" (/var/lib/mesh/systemd, /var/lib/mesh/postgres): the unlink record, and postgres's own secrets (the same values: the vault keys them by name), consumers.json and grants. The store's data stays /var/lib/mesh-store, untouched. The node-engine keeps no found owner or mode for a directory, and forgets one no longer declared as it stands; it sets the declared owner and mode on a directory it recorded at the same path under the same id. So for this delivery "state" names the package's directory with the package's owner and mode, and the engine gives it back; the next change drops "state". The postgres container is recreated, its secret's volume moving. checks/owned/system-known keeps systemd state and postgres state, declared for this delivery only; postgres's consumers and grants leave the list. --- checks/owned/system-known | 2 - modules/postgres/README.md | 3 +- .../cmd/postgres-provider/manifest_test.go | 94 +++++++++++++++++++ modules/postgres/module.json | 27 ++++-- modules/systemd/README.md | 3 +- .../cmd/systemd-tools/manifest_test.go | 52 ++++++++++ modules/systemd/module.json | 14 ++- 7 files changed, 179 insertions(+), 16 deletions(-) create mode 100644 modules/postgres/cmd/postgres-provider/manifest_test.go create mode 100644 modules/systemd/cmd/systemd-tools/manifest_test.go diff --git a/checks/owned/system-known b/checks/owned/system-known index dcb8b120..465a9ba8 100644 --- a/checks/owned/system-known +++ b/checks/owned/system-known @@ -1,7 +1,5 @@ # Modules whose directory lies in a system package's directory today (rule 5 of checks/owned, issue 1841), each with # the issue that moves it out: " issue ". This list only shrinks: a line whose module # no longer collides is refused, and so is a line the trunk's copy does not hold. -postgres consumers issue 1873 -postgres grants issue 1873 postgres state issue 1873 systemd state issue 1873 diff --git a/modules/postgres/README.md b/modules/postgres/README.md index be777275..60f0468f 100644 --- a/modules/postgres/README.md +++ b/modules/postgres/README.md @@ -119,7 +119,8 @@ what is declared, refusing secrets, taking back what is no longer declared, and Each directory or archive this module declares says how it treats a file it did not write (ADR 0422, ADR 0416). -- `${dir:state}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- `${dir:data}` (`/var/lib/mesh/`): everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- `${dir:state}`: the package's own directory, declared for one delivery with the package's owner and mode so the node-engine gives it back (issue 1873); the module keeps nothing there. - `${dir:grants}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `/var/lib/mesh-store`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). - `${dir:store-data}/dumps`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). diff --git a/modules/postgres/cmd/postgres-provider/manifest_test.go b/modules/postgres/cmd/postgres-provider/manifest_test.go new file mode 100644 index 00000000..c4a29d58 --- /dev/null +++ b/modules/postgres/cmd/postgres-provider/manifest_test.go @@ -0,0 +1,94 @@ +package main + +// The module's directories (novox/hq issue 1873): its state directory said place ".", which is /var/lib/postgres, +// the postgresql package's own (tmpfiles.d/postgresql.conf: postgres, 0700), and its grants directory, placed by +// default, lay inside it; the node-engine gave /var/lib/postgres to the operator's account. What the module keeps +// moves to the mesh's directory for it, and for one delivery the record "state" states the package's owner and mode, +// so the node-engine, which takes a directory it recorded at the same path under the same id as its own, gives it +// back. The store's data is not touched: it is /var/lib/mesh-store, as before. + +import ( + "encoding/json" + "os" + "reflect" + "strings" + "testing" +) + +func postgresManifest(t *testing.T) (map[string]map[string]any, map[string]any) { + t.Helper() + raw, err := os.ReadFile("../../module.json") + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(raw), "${dir:state}") { + t.Errorf("the manifest still keeps something in ${dir:state}, which is /var/lib/postgres") + } + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + out := map[string]map[string]any{} + list, _ := m["resources"].([]any) + for _, x := range list { + r, _ := x.(map[string]any) + out[r["id"].(string)] = r + } + return out, m +} + +// 1873/AC2: secrets, consumers and grants are kept in /mesh/postgres; "state" gives /var/lib/postgres back +// to postgres, 0700; the store's data and dumps are as they were. +func TestTheModuleKeepsItsFilesOutsideThePackagesDirectory(t *testing.T) { + rs, m := postgresManifest(t) + data := rs["data"] + if data == nil || data["type"] != "directory" || data["place"] != "mesh" || data["path"] != nil || data["mode"] != "0700" { + t.Fatalf("the module's directory is placed \"mesh\" with no path, 0700 (/var/lib/mesh/postgres): %v", data) + } + if g := rs["grants"]; g == nil || g["path"] != "${dir:data}/grants" || g["place"] != nil { + t.Errorf("grants lies in the module's directory: %v", g) + } + if c := rs["consumers"]; c == nil || c["path"] != "${dir:data}/consumers.json" { + t.Errorf("consumers.json lies in the module's directory: %v", c) + } + own, _ := m["own-secrets"].(map[string]any) + if own["superuser"] != "${dir:data}/superuser.secret" || own["reader"] != "${dir:data}/reader.secret" { + t.Errorf("the own secrets lie in the module's directory: %v", own) + } + state := rs["state"] + if state == nil || state["type"] != "directory" || state["path"] != "/var/lib/postgres" || state["owner"] != "postgres" || + state["mode"] != "0700" || state["place"] != nil { + t.Errorf("state gives /var/lib/postgres back as postgresql declares it, postgres 0700: %v", state) + } + + // The store's data, its dumps and how the container reads them: exactly as before. + if s := rs["store-data"]; !reflect.DeepEqual(s, map[string]any{"id": "store-data", "type": "directory", + "path": "/var/lib/mesh-store", "mode": "0700", "owner": "999:70", "unknown": "program"}) { + t.Errorf("store-data changed: %v", s) + } + if d := rs["dumps"]; !reflect.DeepEqual(d, map[string]any{"id": "dumps", "type": "directory", + "path": "${dir:store-data}/dumps", "mode": "0700", "owner": "999:70", "unknown": "program"}) { + t.Errorf("dumps changed: %v", d) + } + server := rs["server"] + if !reflect.DeepEqual(server["volumes"], []any{"/var/lib/mesh-store:/var/lib/postgresql/data", + "${dir:data}/superuser.secret:/run/secrets/superuser:ro"}) { + t.Errorf("the container's volumes: %v", server["volumes"]) + } + if env, _ := server["env"].(map[string]any); env["PGDATA"] != "/var/lib/postgresql/data/pgdata" { + t.Errorf("PGDATA changed: %v", env) + } + build, _ := m["build"].(map[string]any) + artifacts, _ := build["artifacts"].([]any) + env := artifacts[0].(map[string]any)["env"].(map[string]any) + for key, want := range map[string]string{ + "MESH_PROVISION_PASSWORD_FILE": "${dir:data}/superuser.secret", + "MESH_POSTGRES_READER_PASSWORD_FILE": "${dir:data}/reader.secret", + "MESH_CONSUMERS_FILE": "${dir:data}/consumers.json", + "MESH_RECEIVES": "${dir:grants}/mesh.json", + } { + if env[key] != want { + t.Errorf("%s is %v, not %s", key, env[key], want) + } + } +} diff --git a/modules/postgres/module.json b/modules/postgres/module.json index 919df421..e8a59069 100644 --- a/modules/postgres/module.json +++ b/modules/postgres/module.json @@ -61,8 +61,8 @@ "postgres-database": "${dir:grants}" }, "own-secrets": { - "superuser": "${dir:state}/superuser.secret", - "reader": "${dir:state}/reader.secret" + "superuser": "${dir:data}/superuser.secret", + "reader": "${dir:data}/reader.secret" }, "data": { "own": [ @@ -93,22 +93,31 @@ }, "resources": [ { - "id": "state", + "id": "data", "type": "directory", "mode": "0700", - "place": ".", + "place": "mesh", + "unknown": "program" + }, + { + "id": "state", + "type": "directory", + "path": "/var/lib/postgres", + "owner": "postgres", + "mode": "0700", "unknown": "program" }, { "id": "grants", "type": "directory", + "path": "${dir:data}/grants", "mode": "0700", "unknown": "program" }, { "id": "consumers", "type": "file", - "path": "${dir:state}/consumers.json", + "path": "${dir:data}/consumers.json", "mode": "0600", "template": "files/consumers.json" }, @@ -146,7 +155,7 @@ ], "volumes": [ "/var/lib/mesh-store:/var/lib/postgresql/data", - "${dir:state}/superuser.secret:/run/secrets/superuser:ro" + "${dir:data}/superuser.secret:/run/secrets/superuser:ro" ] }, { @@ -169,10 +178,10 @@ ], "env": { "MESH_PROVISION_POSTGRES": "postgres://postgres@127.0.0.1:${port:5432}/postgres?sslmode=disable", - "MESH_PROVISION_PASSWORD_FILE": "${dir:state}/superuser.secret", + "MESH_PROVISION_PASSWORD_FILE": "${dir:data}/superuser.secret", "MESH_RECEIVES": "${dir:grants}/mesh.json", - "MESH_POSTGRES_READER_PASSWORD_FILE": "${dir:state}/reader.secret", - "MESH_CONSUMERS_FILE": "${dir:state}/consumers.json" + "MESH_POSTGRES_READER_PASSWORD_FILE": "${dir:data}/reader.secret", + "MESH_CONSUMERS_FILE": "${dir:data}/consumers.json" } }, { diff --git a/modules/systemd/README.md b/modules/systemd/README.md index cf68a0c1..1ee8bfe2 100644 --- a/modules/systemd/README.md +++ b/modules/systemd/README.md @@ -6,4 +6,5 @@ The `systemd` module of the mesh catalogue: installs the package `systemd`; clai Each directory or archive this module declares says how it treats a file it did not write (ADR 0422, ADR 0416). -- `${dir:state}`: everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- `${dir:data}` (`/var/lib/mesh/`): everything in it is the program's; the mesh moves nothing out (ADR 0422 §2). +- `${dir:state}`: the package's own directory, declared for one delivery with the package's owner and mode so the node-engine gives it back (issue 1873); the module keeps nothing there. diff --git a/modules/systemd/cmd/systemd-tools/manifest_test.go b/modules/systemd/cmd/systemd-tools/manifest_test.go new file mode 100644 index 00000000..be356c78 --- /dev/null +++ b/modules/systemd/cmd/systemd-tools/manifest_test.go @@ -0,0 +1,52 @@ +package main + +// The module's directories (novox/hq issue 1873): its state directory said place ".", which is /var/lib/systemd, +// systemd's own directory (tmpfiles.d/systemd.conf: d /var/lib/systemd 0755 root root); the node-engine gave it to +// the operator's account, mode 0700, on every machine. What the tools keep moves to the mesh's directory for the +// module, and for one delivery the record "state" states the package's owner and mode, so the node-engine, which +// takes a directory it recorded at the same path under the same id as its own, gives it back. + +import ( + "encoding/json" + "os" + "testing" +) + +func manifestResources(t *testing.T) (map[string]map[string]any, map[string]any) { + t.Helper() + raw, err := os.ReadFile("../../module.json") + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + out := map[string]map[string]any{} + list, _ := m["resources"].([]any) + for _, x := range list { + r, _ := x.(map[string]any) + out[r["id"].(string)] = r + } + return out, m +} + +// 1873/AC1: the tools' record is kept in /mesh/systemd; "state" gives /var/lib/systemd back to root, 0755. +func TestTheToolsKeepTheirRecordOutsideSystemdsOwnDirectory(t *testing.T) { + rs, m := manifestResources(t) + data := rs["data"] + if data == nil || data["type"] != "directory" || data["place"] != "mesh" || data["path"] != nil { + t.Fatalf("the tools' directory is placed \"mesh\" with no path (/var/lib/mesh/systemd): %v", data) + } + build, _ := m["build"].(map[string]any) + artifacts, _ := build["artifacts"].([]any) + env := artifacts[0].(map[string]any)["env"].(map[string]any) + if env["MESH_SYSTEMD_STATE"] != "${dir:data}" { + t.Errorf("MESH_SYSTEMD_STATE is %v, not ${dir:data}", env["MESH_SYSTEMD_STATE"]) + } + state := rs["state"] + if state == nil || state["type"] != "directory" || state["path"] != "/var/lib/systemd" || state["owner"] != "root" || + state["mode"] != "0755" || state["place"] != nil { + t.Errorf("state gives /var/lib/systemd back as systemd declares it, root 0755: %v", state) + } +} diff --git a/modules/systemd/module.json b/modules/systemd/module.json index 984ccf3a..8c9403d2 100644 --- a/modules/systemd/module.json +++ b/modules/systemd/module.json @@ -41,7 +41,7 @@ "systemd-tools" ], "env": { - "MESH_SYSTEMD_STATE": "${dir:state}" + "MESH_SYSTEMD_STATE": "${dir:data}" } } ] @@ -53,10 +53,18 @@ "package": "systemd" }, { - "id": "state", + "id": "data", "type": "directory", "mode": "0700", - "place": ".", + "place": "mesh", + "unknown": "program" + }, + { + "id": "state", + "type": "directory", + "path": "/var/lib/systemd", + "owner": "root", + "mode": "0755", "unknown": "program" } ]