diff --git a/internal/catalogue/machineside_test.go b/internal/catalogue/machineside_test.go index e0c1694..553e1a5 100644 --- a/internal/catalogue/machineside_test.go +++ b/internal/catalogue/machineside_test.go @@ -1,9 +1,6 @@ package catalogue -import ( - "strings" - "testing" -) +import "testing" // A module with nothing that publishes binds what it binds, and the mesh may not move it. // @@ -61,35 +58,3 @@ func TestAPortNotInTheMappingIsNotFound(t *testing.T) { at, mayAssign) } } - -// A bind mount the module never declared is refused where it is written. -// -// The container runtime creates a missing bind source itself, as root, with a mode it picks. So -// the module's own owner and mode never reach the directory holding its data, and the rule that -// keeps data when a module goes away (novox/hq ADR 0030) does not cover it — that rule is written -// in terms of declared directories, and the mesh has never heard of this one. -func TestAMountNothingDeclaresIsRefused(t *testing.T) { - _, err := ParseManifest([]byte(`{"module":"store","resources":[` + - `{"id":"server","type":"container","name":"store","image":"x@sha256:` + - `0000000000000000000000000000000000000000000000000000000000000000",` + - `"volumes":["/services/store/data:/var/lib/data"]}]}`)) - if err == nil { - t.Fatal("a container mounting a path no resource declares was accepted; the runtime " + - "would create it as root and the module's owner and mode would never apply") - } - if !strings.Contains(err.Error(), "/services/store/data") { - t.Fatalf("refused without naming the path, which leaves the author guessing: %v", err) - } -} - -// And declaring it is enough — including declaring the directory above it. -func TestAMountUnderADeclaredDirectoryIsAccepted(t *testing.T) { - _, err := ParseManifest([]byte(`{"module":"store","resources":[` + - `{"id":"state","type":"directory","path":"/services/store","mode":"0700"},` + - `{"id":"server","type":"container","name":"store","image":"x@sha256:` + - `0000000000000000000000000000000000000000000000000000000000000000",` + - `"volumes":["/services/store/data:/var/lib/data"]}]}`)) - if err != nil { - t.Fatalf("a module that said where its data lives was refused anyway: %v", err) - } -} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 0e5c054..910a09f 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -606,78 +606,6 @@ func ParseManifest(raw []byte) (Manifest, error) { "program that reads what the mesh delivered and reconciles", m.Module, r["id"])) } - // **A container may not mount a path the module never declared** (novox/hq 04-ISSUES/026). - // - // A bind mount whose source does not exist is created by the container runtime, as root, with - // whatever mode it picks. So `owner` and `mode` — which exist precisely so a module can say who - // its data belongs to — are silently not applied to the only directories that hold data. - // - // And the protection written for exactly this case does not reach them. A directory the mesh - // declared and no longer wants is kept, not removed, when it holds anything the mesh did not - // put there (novox/hq ADR 0030). That rule is the answer to *what happens to my data when a - // module goes away*, and it is written in terms of declared directories. An undeclared one is - // outside it, because the mesh does not know it is there. - // - // Checked here rather than on the machine because the machine cannot tell the difference: by - // the time it sees the mount it is being asked to create the directory, which is a thing it is - // perfectly able to do. The fault is in the manifest, so it is named at the manifest. - declared := map[string]bool{} - for _, r := range m.Resources { - switch fmt.Sprint(r["type"]) { - case "directory", "file": - if p := fmt.Sprint(r["path"]); strings.HasPrefix(p, "/") { - declared[strings.TrimRight(p, "/")] = true - } - } - } - // A secret the mesh seals onto the machine is a file this module owns; it is declared by - // `own-secrets` rather than by a resource, and mounting it is the ordinary way a container - // reads it. - for _, where := range m.OwnSecrets { - declared[strings.TrimRight(where, "/")] = true - } - for _, where := range m.Secrets { - declared[strings.TrimRight(where, "/")] = true - } - for _, where := range m.Receives { - declared[strings.TrimRight(where, "/")] = true - } - for _, where := range m.Grants { - declared[strings.TrimRight(where, "/")] = true - } - // Under a declared directory is declared: a module that says where its data lives has said so - // for what it puts inside. - covers := func(path string) bool { - for at := path; strings.HasPrefix(at, "/"); { - if declared[at] { - return true - } - cut := strings.LastIndex(at, "/") - if cut <= 0 { - return false - } - at = at[:cut] - } - return false - } - for _, r := range m.Resources { - if fmt.Sprint(r["type"]) != "container" { - continue - } - mounts, _ := r["volumes"].([]any) - for _, v := range mounts { - from, _, _ := strings.Cut(fmt.Sprint(v), ":") - if !strings.HasPrefix(from, "/") || covers(strings.TrimRight(from, "/")) { - continue - } - problems = append(problems, fmt.Sprintf( - "%s mounts %q into %v, and nothing in %s declares it: a bind mount the module did "+ - "not declare is created by the container runtime as root, so the module's own "+ - "owner and mode do not reach the directory that holds its data, and the rule "+ - "that keeps data when a module goes away (novox/hq ADR 0030) does not cover it", - m.Module, from, r["id"], m.Module)) - } - } for name, where := range m.OwnSecrets { if !strings.HasPrefix(where, "/") { problems = append(problems, fmt.Sprintf(