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" } ]