Merge pull request 'Give /var/lib/systemd and /var/lib/postgres back to their packages and keep the two modules' own files under /var/lib/mesh (issue 1873, delivery A)' (#453) from fix/1873-system-state-dirs into main

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: "<module> <resource id> issue <number>". 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

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/<module>`): 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).

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 <data root>/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)
+		}
+	}
+}

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"
         }
       },
       {

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/<module>`): 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.

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 <data root>/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)
+	}
+}

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