Merge pull request 'Issue 339 follow-up to #64: ask the running runtimes where they keep their data, and read every form a unit says it in' (#65) from fix/339-runtime-roots-asked into main

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)

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 {

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
+// unit and the unit's drop-ins (an ExecStart with --data-root, or the older -g/--graph, continued over lines,
-// storage.conf (graphroot). containerd keeps its own under /var/lib/containerd, which forbiddenBelow names; a
+// 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,78 @@ var runtimeConfigs = runtimeFiles{
 }
 
 var (
-	dataRootFlag = regexp.MustCompile(`(?:--data-root|--graph|-g)(?:=|\s+)("?)(/[^\s"]+)`)
+	dataRootFlag = regexp.MustCompile(`(?:--data-root|--graph|-g)(?:=|\s+)(\S+)`)
-	graphRoot    = regexp.MustCompile(`(?m)^\s*graphroot\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
+// runtimeRoots is where the container runtimes keep their data, as the last apply read it (readRuntimeRoots);
-// trees forbiddenBelow names: every container's filesystem is there.
+// 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) {
+		p = strings.Trim(strings.TrimSpace(p), `"'`)
-			out = append(out, filepath.Clean(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 {
+		// 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()
+		}
+	}
+	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 +176,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") {
+		e, x := unitLines(string(raw))
-			if !strings.HasPrefix(strings.TrimSpace(line), "ExecStart") {
+		for k, v := range e {
-				continue
+			env[k] = v
-			}
+		}
-			for _, m := range dataRootFlag.FindAllStringSubmatch(line, -1) {
+		execs = append(execs, x...)
-				add(m[2])
+	}
+	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"
 

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)
+	}
+}

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

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")
+	}
+}