Withdraw the mount check: it refuses the builder
The rule was right about data and wrong about everything else. The builder mounts the container runtime's socket, which is not its data, does not belong to it, and must not be declared as one of its directories — and the check refused the builder's own manifest. Caught by the lab, though not honestly: the run was already going when this went in, so the builder binary was rebuilt mid-run with the check compiled into it and the failure was mine, not the mesh's. Confirmed against the manifest directly rather than inferred from the log. What it was protecting is real and stands — the fourteen mounts are all declared. But enforcing it needs a way to tell "the directory my data lives in" from "a machine facility I was granted", and the mesh has no vocabulary for the second. `capabilities` is the closest thing and does not name paths. That is a design decision, so it goes back to 04-ISSUES/026 rather than being invented here to make a check pass. The check that every real manifest still parses is kept. It costs nothing and it is how the next attempt at this finds out sooner.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user