From ff2553f4db19abf5f2d1abf0aa943815dd363acf Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 01:22:11 +0200 Subject: [PATCH 1/2] Ask the running runtimes where they keep their data, and read every form a unit says it in The third review of #64 (hq issue 339): an ExecStart continued over lines, a quoted path, a variable from Environment= or EnvironmentFile= across the unit and its drop-ins, dockerd's --config-file and podman's TOML literal strings each hid a data root from the guard. The running runtimes are now asked first (docker info, podman info) on a runner of the guard's own, and the files are read beside them in all those forms. --- cmd/mesh-host/main.go | 2 + internal/apply/apply.go | 1 + internal/apply/placement_guard.go | 186 ++++++++++++++++++++-- internal/apply/runtime_roots_more_test.go | 62 ++++++++ 4 files changed, 235 insertions(+), 16 deletions(-) create mode 100644 internal/apply/runtime_roots_more_test.go diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 3cda687..f5b47ec 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -232,6 +232,8 @@ func run(ctx context.Context, command string, opts options) error { // found (novox/hq issue 339). handedOverFrom(opts.state) foundState.Store(opts.state) + // And the guard asks the running container runtimes where they keep their data (novox/hq issue 339). + apply.AskRuntimes = apply.ExecRunner switch command { case "hand-over": return handOverCommand(opts) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 3266ccb..22359ef 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -467,6 +467,7 @@ func ApplyMindingWindows( groups: wanted} // Where nothing is placed or mounted from, judged before anything is applied (novox/hq issue 339), and // which declared directories were already here: one this apply makes is the mesh's, one found may not be. + refreshRuntimeRoots(ctx) in.refused = refusedPlaces(d.Resources) in.dirsBefore = map[string]bool{} for _, resource := range d.Resources { diff --git a/internal/apply/placement_guard.go b/internal/apply/placement_guard.go index 2433980..1f53994 100644 --- a/internal/apply/placement_guard.go +++ b/internal/apply/placement_guard.go @@ -37,6 +37,7 @@ package apply import ( "bufio" + "context" "encoding/json" "errors" "fmt" @@ -45,6 +46,8 @@ import ( "regexp" "strconv" "strings" + "sync" + "time" "github.com/novox/mesh-host/internal/declaration" ) @@ -71,8 +74,9 @@ var ( ) // runtimeFiles are where the container runtimes say where they keep their data: dockerd's daemon.json, its -// unit and the unit's drop-ins (an ExecStart with --data-root, or the older -g/--graph), and podman's -// storage.conf (graphroot). containerd keeps its own under /var/lib/containerd, which forbiddenBelow names; a +// unit and the unit's drop-ins (an ExecStart with --data-root, or the older -g/--graph, continued over lines, +// quoted, through Environment= or EnvironmentFile=, or a --config-file naming another daemon.json), and podman's +// storage.conf (graphroot, a basic or a literal string). The running runtimes are asked first. containerd keeps its own under /var/lib/containerd, which forbiddenBelow names; a // containerd configured elsewhere, and podman's rootless stores under each account's home, are not read: the // first is no runtime this mesh runs, and the second is below a home, which rule 4 already keeps for its // account. A variable so a test names its own. @@ -91,20 +95,77 @@ var runtimeConfigs = runtimeFiles{ } var ( - dataRootFlag = regexp.MustCompile(`(?:--data-root|--graph|-g)(?:=|\s+)("?)(/[^\s"]+)`) - graphRoot = regexp.MustCompile(`(?m)^\s*graphroot\s*=\s*"(/[^"]+)"`) + dataRootFlag = regexp.MustCompile(`(?:--data-root|--graph|-g)(?:=|\s+)(\S+)`) + configFlag = regexp.MustCompile(`--config-file(?:=|\s+)(\S+)`) + graphRoot = regexp.MustCompile(`(?m)^\s*graphroot\s*=\s*(?:"([^"]+)"|'([^']+)')`) + envVar = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)\}|\$([A-Za-z_][A-Za-z0-9_]*)`) ) -// runtimeDataRoots is every place the container runtimes are configured to keep their data, beyond the default -// trees forbiddenBelow names: every container's filesystem is there. +// runtimeRoots is where the container runtimes keep their data, as the last apply read it (readRuntimeRoots); +// nil until an apply has read it, when the guard reads the files itself. +var ( + runtimeRootsMu sync.Mutex + runtimeRoots []string +) + +// AskRuntimes is how the engine asks the running container runtimes where they keep their data: set by the engine +// to run the commands, nil in a test, which then reads the files alone. Its own, never the apply's runner, whose +// commands a test reads back as what the apply did. +var AskRuntimes Runner + +// refreshRuntimeRoots reads where the runtimes keep their data once, at the start of an apply. +func refreshRuntimeRoots(ctx context.Context) { + roots := readRuntimeRoots(ctx, AskRuntimes) + runtimeRootsMu.Lock() + runtimeRoots = roots + runtimeRootsMu.Unlock() +} + +// runtimeDataRoots is every place the container runtimes keep their data, beyond the default trees forbiddenBelow +// names: every container's filesystem is there. func runtimeDataRoots() []string { + runtimeRootsMu.Lock() + roots := runtimeRoots + runtimeRootsMu.Unlock() + if roots != nil { + return roots + } + return readRuntimeRoots(context.Background(), nil) +} + +// readRuntimeRoots asks the running runtimes where they keep their data, when run is given (`docker info`, +// `podman info`), and reads their configuration besides: an answer from a runtime that is running is what it +// really does, and the files say what it will do when it starts again. Both count. A runtime that is not running +// or not installed answers nothing, which is no error. +func readRuntimeRoots(ctx context.Context, run Runner) []string { + seen := map[string]bool{} var out []string add := func(p string) { - if filepath.IsAbs(p) { - out = append(out, filepath.Clean(p)) + p = strings.Trim(strings.TrimSpace(p), `"'`) + if !filepath.IsAbs(p) { + return + } + p = filepath.Clean(p) + if !seen[p] { + seen[p] = true + out = append(out, p) } } - if raw, err := os.ReadFile(runtimeConfigs.daemonJSON); err == nil { + if run != nil { + ask, cancel := context.WithTimeout(ctx, 10*time.Second) + if root, err := run(ask, "docker", "info", "--format", "{{.DockerRootDir}}"); err == nil { + add(root) + } + if root, err := run(ask, "podman", "info", "--format", "{{.Store.GraphRoot}}"); err == nil { + add(root) + } + cancel() + } + daemonJSON := func(path string) { + raw, err := os.ReadFile(path) + if err != nil { + return + } var c struct { DataRoot string `json:"data-root"` Graph string `json:"graph"` @@ -114,33 +175,126 @@ func runtimeDataRoots() []string { add(c.Graph) } } + daemonJSON(runtimeConfigs.daemonJSON) units := append([]string(nil), runtimeConfigs.units...) for _, dir := range runtimeConfigs.dropInDirs { matches, _ := filepath.Glob(filepath.Join(dir, "*.conf")) units = append(units, matches...) } + // The unit and its drop-ins are one unit to systemd: a variable set in one is seen by an ExecStart in another. + env := map[string]string{} + var execs []string for _, u := range units { raw, err := os.ReadFile(u) if err != nil { continue } - for _, line := range strings.Split(string(raw), "\n") { - if !strings.HasPrefix(strings.TrimSpace(line), "ExecStart") { - continue - } - for _, m := range dataRootFlag.FindAllStringSubmatch(line, -1) { - add(m[2]) + e, x := unitLines(string(raw)) + for k, v := range e { + env[k] = v + } + execs = append(execs, x...) + } + for _, line := range execs { + line = envVar.ReplaceAllStringFunc(line, func(ref string) string { + m := envVar.FindStringSubmatch(ref) + if v, ok := env[m[1]+m[2]]; ok { + return v } + return ref + }) + for _, m := range dataRootFlag.FindAllStringSubmatch(line, -1) { + add(m[1]) + } + for _, m := range configFlag.FindAllStringSubmatch(line, -1) { + daemonJSON(strings.Trim(m[1], `"'`)) } } if raw, err := os.ReadFile(runtimeConfigs.storageConf); err == nil { for _, m := range graphRoot.FindAllStringSubmatch(string(raw), -1) { - add(m[1]) + add(m[1] + m[2]) } } return out } +// unitLines reads a unit file as systemd does for what matters here: a line ending in a backslash continues on the +// next, Environment= sets variables (quoted or not, several to a line), EnvironmentFile= (a leading - says it may +// be missing) reads KEY=value lines, and every ExecStart line is returned whole. +func unitLines(text string) (map[string]string, []string) { + var joined []string + var cur strings.Builder + for _, line := range strings.Split(text, "\n") { + trimmed := strings.TrimRight(line, " \t") + if strings.HasSuffix(trimmed, "\\") { + cur.WriteString(strings.TrimSuffix(trimmed, "\\") + " ") + continue + } + cur.WriteString(line) + joined = append(joined, cur.String()) + cur.Reset() + } + if cur.Len() > 0 { + joined = append(joined, cur.String()) + } + env := map[string]string{} + setPairs := func(s string) { + for _, f := range splitQuoted(s) { + if k, v, ok := strings.Cut(f, "="); ok { + env[strings.TrimSpace(k)] = strings.Trim(strings.TrimSpace(v), `"'`) + } + } + } + var execs []string + for _, line := range joined { + l := strings.TrimSpace(line) + switch { + case strings.HasPrefix(l, "Environment="): + setPairs(strings.TrimPrefix(l, "Environment=")) + case strings.HasPrefix(l, "EnvironmentFile="): + path := strings.TrimPrefix(strings.TrimSpace(strings.TrimPrefix(l, "EnvironmentFile=")), "-") + if raw, err := os.ReadFile(path); err == nil { + for _, kv := range strings.Split(string(raw), "\n") { + kv = strings.TrimSpace(kv) + if kv == "" || strings.HasPrefix(kv, "#") { + continue + } + setPairs(kv) + } + } + case strings.HasPrefix(l, "ExecStart"): + execs = append(execs, l) + } + } + return env, execs +} + +// splitQuoted splits on blanks outside double or single quotes, keeping the quotes' contents whole. +func splitQuoted(s string) []string { + var out []string + var cur strings.Builder + var quote rune + for _, r := range s { + switch { + case quote != 0 && r == quote: + quote = 0 + case quote == 0 && (r == '"' || r == '\''): + quote = r + case quote == 0 && (r == ' ' || r == '\t'): + if cur.Len() > 0 { + out = append(out, cur.String()) + cur.Reset() + } + default: + cur.WriteRune(r) + } + } + if cur.Len() > 0 { + out = append(out, cur.String()) + } + return out +} + // engineModule is the module whose resources may place in the engine's own trees. const engineModule = "mesh-host" diff --git a/internal/apply/runtime_roots_more_test.go b/internal/apply/runtime_roots_more_test.go new file mode 100644 index 0000000..a8a396b --- /dev/null +++ b/internal/apply/runtime_roots_more_test.go @@ -0,0 +1,62 @@ +package apply + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +// The forms a runtime's data root is said in (novox/hq issue 339, the third review): an ExecStart continued over +// lines, single quotes, a variable from Environment= or EnvironmentFile=, dockerd's --config-file, and TOML literal +// strings in podman's storage.conf. And the running runtimes are asked first: they say where they really keep it. +func TestEveryFormOfADataRootIsRead(t *testing.T) { + dir := t.TempDir() + write := func(name, content string) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return p + } + envFile := write("docker.env", "DOCKER_ROOT=/data/from-env-file\n") + other := write("other-daemon.json", `{"data-root": "/data/from-config-file"}`) + unit := write("lib/docker.service", "[Service]\nEnvironment=\"ROOT=/data/from-env\"\nEnvironmentFile=-"+envFile+"\n"+ + "ExecStart=/usr/bin/dockerd \\\n -H fd:// \\\n --data-root '/data/continued'\n") + dropIn := write("etc/docker.service.d/root.conf", "[Service]\nExecStart=\nExecStart=/usr/bin/dockerd --data-root=$ROOT "+ + "--graph=${DOCKER_ROOT} --config-file "+other+"\n") + storage := write("containers/storage.conf", "[storage]\ngraphroot = '/data/podman-literal'\n") + before := runtimeConfigs + runtimeConfigs = runtimeFiles{daemonJSON: filepath.Join(dir, "none.json"), units: []string{unit}, + dropInDirs: []string{filepath.Dir(dropIn)}, storageConf: storage} + t.Cleanup(func() { runtimeConfigs = before }) + + roots := strings.Join(readRuntimeRoots(context.Background(), nil), " ") + for _, want := range []string{"/data/continued", "/data/from-env", "/data/from-env-file", "/data/from-config-file", + "/data/podman-literal"} { + if !strings.Contains(roots+" ", want+" ") { + t.Errorf("%s was not read: %s", want, roots) + } + } + + // Asked of the running runtimes first; what they answer counts beside the files. + asked := func(_ context.Context, name string, args ...string) (string, error) { + switch name { + case "docker": + return "/srv/docker-live\n", nil + case "podman": + return "", errors.New("podman: not installed") + } + return "", errors.New("unexpected") + } + roots = strings.Join(readRuntimeRoots(context.Background(), asked), " ") + if !strings.Contains(roots, "/srv/docker-live") || !strings.Contains(roots, "/data/continued") { + t.Errorf("the running runtime's answer, or the files beside it, are missing: %s", roots) + } +} From 7c046a8a1d8427ba9605fbbe424c003f32802ac8 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 01:42:09 +0200 Subject: [PATCH 2/2] Accept and ignore a file's `trusted`, and give each runtime its own time to answer The fourth review (hq issue 339): `trusted` is the catalogue's word and the controller takes it out, but a controller that passes it on, an older one or one rolled back to, must not cost a node its whole declaration. A file now accepts it and nothing reads it; any other resource still refuses it. And a podman that hangs no longer eats docker's ten seconds. --- internal/apply/placement_guard.go | 15 ++++++++------- internal/declaration/declaration.go | 6 ++++++ internal/declaration/trusted_test.go | 23 +++++++++++++++++++++++ 3 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 internal/declaration/trusted_test.go diff --git a/internal/apply/placement_guard.go b/internal/apply/placement_guard.go index 1f53994..49c1d83 100644 --- a/internal/apply/placement_guard.go +++ b/internal/apply/placement_guard.go @@ -152,14 +152,15 @@ func readRuntimeRoots(ctx context.Context, run Runner) []string { } } if run != nil { - ask, cancel := context.WithTimeout(ctx, 10*time.Second) - if root, err := run(ask, "docker", "info", "--format", "{{.DockerRootDir}}"); err == nil { - add(root) + // Each runtime has its own ten seconds: one that hangs costs the other nothing. + for _, q := range [][]string{{"docker", "info", "--format", "{{.DockerRootDir}}"}, + {"podman", "info", "--format", "{{.Store.GraphRoot}}"}} { + ask, cancel := context.WithTimeout(ctx, 10*time.Second) + if root, err := run(ask, q[0], q[1:]...); err == nil { + add(root) + } + cancel() } - if root, err := run(ask, "podman", "info", "--format", "{{.Store.GraphRoot}}"); err == nil { - add(root) - } - cancel() } daemonJSON := func(path string) { raw, err := os.ReadFile(path) diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 2472adf..a66d8ba 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -198,6 +198,12 @@ type File struct { // by looking rather than by knowing which field won. Sealed string `json:"sealed,omitempty"` + // Trusted is the catalogue's word on whether the settings this file's content took are trusted (novox/hq + // issue 339). It is the controller's to act on, and a controller takes it out before it sends a declaration. + // Accepted here, and nothing is done with it, so that a controller that passes it on — an older one, or one + // rolled back to — never costs a node its whole declaration. Not part of the file's digest. + Trusted *bool `json:"trusted,omitempty"` + // Secrets are sealed values put into Content where it says `${secret:name}`. // // **The one place a secret and a configuration meet, and it happens on the machine.** A diff --git a/internal/declaration/trusted_test.go b/internal/declaration/trusted_test.go new file mode 100644 index 0000000..c533334 --- /dev/null +++ b/internal/declaration/trusted_test.go @@ -0,0 +1,23 @@ +package declaration + +import "testing" + +// `trusted` is the catalogue's word on a file (novox/hq issue 339): a controller takes it out before it sends a +// declaration, and one that does not — an older one, or one rolled back to — must not cost the node its whole +// declaration. So a file accepts it and the engine does nothing with it; any other resource still refuses it. +func TestAFileAcceptsAndIgnoresTrusted(t *testing.T) { + for _, v := range []string{"true", "false"} { + d, err := Parse([]byte(`{"declaration":1,"resources":[{"id":"m.f","type":"file","path":"/etc/m.conf",` + + `"content":"x","trusted":` + v + `}]}`)) + if err != nil { + t.Fatalf("trusted %s: %v", v, err) + } + if f, ok := d.Resources[0].(*File); !ok || f.Content != "x" { + t.Fatalf("trusted %s: %+v", v, d.Resources[0]) + } + } + if _, err := Parse([]byte(`{"declaration":1,"resources":[{"id":"m.d","type":"directory","path":"/srv/m",` + + `"trusted":true}]}`)); err == nil { + t.Fatal("a directory took trusted") + } +}