A container may not mount a path the module never declared
Closes the half of 04-ISSUES/026 that would otherwise come back. The fourteen mounts across the forge, the mail system, the store and the object store are all declared now — but nothing said they had to be, so they were right by coincidence and the next volume added would not be. A bind mount whose source does not exist is created by the container runtime, as root, with a mode it picks. So `owner` and `mode` — which exist precisely so a module can say who its data belongs to — were silently not applied to the only directories holding data. And the rule written for exactly this case did 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 (ADR 0030). That 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 sits outside it, because the mesh does not know it is there. Refused where it is written rather than on the machine, which cannot tell the difference — by the time the host sees the mount it is being asked to make a directory, which it is perfectly able to do. The fault is in the manifest, so it is named at the manifest. Same argument as the action refusal directly above it. A path under a declared directory counts as declared, as do the files a module already names: its own secrets, its grants, what it receives. Every real manifest is checked to still parse, and the refusal bites.
This commit is contained in:
@@ -1,6 +1,9 @@
|
|||||||
package catalogue
|
package catalogue
|
||||||
|
|
||||||
import "testing"
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
// A module with nothing that publishes binds what it binds, and the mesh may not move it.
|
// A module with nothing that publishes binds what it binds, and the mesh may not move it.
|
||||||
//
|
//
|
||||||
@@ -58,3 +61,35 @@ func TestAPortNotInTheMappingIsNotFound(t *testing.T) {
|
|||||||
at, mayAssign)
|
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,6 +606,78 @@ func ParseManifest(raw []byte) (Manifest, error) {
|
|||||||
"program that reads what the mesh delivered and reconciles",
|
"program that reads what the mesh delivered and reconciles",
|
||||||
m.Module, r["id"]))
|
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 {
|
for name, where := range m.OwnSecrets {
|
||||||
if !strings.HasPrefix(where, "/") {
|
if !strings.HasPrefix(where, "/") {
|
||||||
problems = append(problems, fmt.Sprintf(
|
problems = append(problems, fmt.Sprintf(
|
||||||
|
|||||||
@@ -0,0 +1,23 @@
|
|||||||
|
package catalogue
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestEveryExampleManifestStillParses(t *testing.T) {
|
||||||
|
found, err := filepath.Glob("../../examples/modules/*.json")
|
||||||
|
if err != nil || len(found) == 0 {
|
||||||
|
t.Fatalf("no example manifests found: %v", err)
|
||||||
|
}
|
||||||
|
for _, f := range found {
|
||||||
|
raw, err := os.ReadFile(f)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if _, err := ParseManifest(raw); err != nil {
|
||||||
|
t.Errorf("%s no longer parses: %v", filepath.Base(f), err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user