diff --git a/modules/docker/README.md b/modules/docker/README.md index 416c72e0..47149c89 100644 --- a/modules/docker/README.md +++ b/modules/docker/README.md @@ -14,7 +14,9 @@ tools below are the module's own. | `socket` | `docker.socket` running, enabled at boot | given back as found when the module goes (ADR 0118) | | `prune-service`, `prune-timer` | `/etc/systemd/system/docker-prune.{service,timer}`, written whole | removed with the module | | `prune` | `docker-prune.timer` running, enabled at boot; restarted when either file changes | stopped and disabled with the module (the mesh made the unit) | -| `daemon` | `live-restore` and the mesh's registry in `insecure-registries` written into `/etc/docker/daemon.json`, beside other keys | each key given back as found when the module goes; only the member this module added leaves the list | +| `state`, `log-rotation` | the module's own directory, and in it `log-rotation.conf`: the settings `log-max-size` and `log-max-file` as the step judges them (0600) | removed with the module | +| `log-rotation-check` | a step, `docker-tools check-log-rotation`, run as root when `log-rotation` changes; it refuses a bad value, and the rest of the module (`daemon` with it) is then skipped | removed with the module | +| `daemon` | `live-restore`, the mesh's registry in `insecure-registries`, `log-driver` and `log-opts` written into `/etc/docker/daemon.json`, beside other keys | each key given back as found when the module goes; only the member this module added leaves the list | | `runtime` | `docker.service` running, enabled at boot; reloaded, never restarted, when `daemon` changes | given back as found (ADR 0118) | The weekly prune takes **dangling images and build cache unused for a week, and nothing else**. It @@ -64,8 +66,27 @@ once, for `daemon`. The address is the same one, so the runtime's trust does not Still elsewhere: -- **Nobody** writes log rotation. One machine has `log-driver` and `log-opts` by hand; they are left - as they are until a size is chosen for every machine. + +## Log rotation (novox/hq issue 452) + +`daemon` writes `"log-driver": "json-file"` and `"log-opts": {"max-size": …, "max-file": …}` from two +preferences: `log-max-size` (default `100m`) and `log-max-file` (default `10`), about 1 GB of log per +container. The defaults are what the one machine with rotation set by hand already had, so that machine +does not change; the others gain rotation. Both settings are terminal-only (`"trusted": true`). + +**A bad value never reaches `daemon.json`.** The controller pastes a setting into a file as text and judges +only that it is one line (novox/hq issue 346), and the runtime is the only reader of `daemon.json`: a bad +size would be found as a runtime that does not start, and a quote could add any key. So the same two values +are written into `log-rotation.conf` first, and the step `log-rotation-check` judges them before `daemon` +is applied: a size is a whole number from 1 with `k`, `m` or `g` after it, a count is 1 to 99. Refused, the +step fails, the host skips the rest of this module (ADR 0136), `daemon.json` keeps what it had, and the +apply's report names the setting. + +**Nothing restarts.** The runtime reads `log-driver` and `log-opts` only when it starts. A change to +`daemon` reloads it, as before; the reload takes neither key, and stops no container. The new rotation +holds from the runtime's next start (the machine's next boot, for one), and then only for +containers created after it: a container keeps the log options it was created with until it is created +again. `docker_daemon_config` lists the keys the daemon has not taken yet. ## What it does not declare yet, and why @@ -242,6 +263,8 @@ The tests run against a fake runner and cover: - a printed secret found by name and never answered by value, in the scan and in `docker_logs`; - size parsing; - what the daemon has not yet taken; +- the log rotation: the defaults `daemon.json` takes, the step declared before `daemon`, a bad size or + count refused, a good one read, and the runtime never restarted for it; - event filtering; - volume ownership; - that the tools served are exactly the manifest's `tools`. diff --git a/modules/docker/cmd/docker-tools/logrotation.go b/modules/docker/cmd/docker-tools/logrotation.go new file mode 100644 index 00000000..ead74c79 --- /dev/null +++ b/modules/docker/cmd/docker-tools/logrotation.go @@ -0,0 +1,80 @@ +package main + +// The container runtime's log rotation, checked before it reaches daemon.json (novox/hq issue 452). +// +// **Why a step and not the file alone.** daemon.json's log-driver and log-opts are filled by the controller +// from this module's settings log-max-size and log-max-file, pasted as text (novox/hq issue 346): the +// controller judges nothing but that a value is one line. The runtime is the only reader of daemon.json and +// reads log-opts only when it starts, so a bad value would be found at the runtime's next start, as a runtime +// that does not start, and a value with a quote in it could add any key to the file. So the module's +// manifest writes the same two values into log-rotation.conf first, and runs this as a step before daemon.json: +// a value refused here fails the step, and the host skips the rest of the module (novox/hq ADR 0136), so +// daemon.json keeps what it had, and the refusal is in the apply's report. + +import ( + "fmt" + "os" + "regexp" + "strings" +) + +// logRotation is what log-rotation.conf says, both values as the runtime takes them: text. +type logRotation struct{ Size, Files string } + +var ( + // A size as the json-file driver reads it, with the unit required and in lower case, as decided on issue + // 452: a whole number from 1 with k, m or g, at most nine digits, so no value overflows the runtime's count. + logSize = regexp.MustCompile(`^[1-9][0-9]{0,8}[kmg]$`) + // A count of files from 1 to 99, without a leading zero. + logFiles = regexp.MustCompile(`^[1-9][0-9]?$`) +) + +// parseLogRotation reads `size=` and `files=` once each; a comment or an empty line is skipped, and any other +// line is refused, so a value that brought a line of its own is never read past. +func parseLogRotation(text string) (logRotation, error) { + var got logRotation + seen := map[string]bool{} + for _, line := range strings.Split(text, "\n") { + if line == "" || strings.HasPrefix(line, "#") { + continue + } + key, value, ok := strings.Cut(line, "=") + if !ok || (key != "size" && key != "files") { + return logRotation{}, fmt.Errorf("log-rotation.conf has a line that is not size= or files=: %q", line) + } + if seen[key] { + return logRotation{}, fmt.Errorf("log-rotation.conf says %s twice", key) + } + seen[key] = true + switch key { + case "size": + if !logSize.MatchString(value) { + return logRotation{}, fmt.Errorf("log-max-size %q is not a size: a whole number from 1 with k, m or g "+ + "after it, such as 100m; daemon.json was left as it is", value) + } + got.Size = value + case "files": + if !logFiles.MatchString(value) { + return logRotation{}, fmt.Errorf("log-max-file %q is not a count from 1 to 99; daemon.json was left "+ + "as it is", value) + } + got.Files = value + } + } + for _, key := range []string{"size", "files"} { + if !seen[key] { + return logRotation{}, fmt.Errorf("log-rotation.conf does not say %s", key) + } + } + return got, nil +} + +// checkLogRotationFile is the step: the file the manifest wrote, judged. +func checkLogRotationFile(path string) error { + raw, err := os.ReadFile(path) + if err != nil { + return err + } + _, err = parseLogRotation(string(raw)) + return err +} diff --git a/modules/docker/cmd/docker-tools/logrotation_test.go b/modules/docker/cmd/docker-tools/logrotation_test.go new file mode 100644 index 00000000..e969730f --- /dev/null +++ b/modules/docker/cmd/docker-tools/logrotation_test.go @@ -0,0 +1,214 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "reflect" + "regexp" + "strings" + "testing" +) + +// The container runtime's log rotation is a preference (novox/hq issue 452). + +type manifestForLogs struct { + Settings map[string]struct { + Kind string `json:"kind"` + Default any `json:"default"` + Why string `json:"why"` + } `json:"settings"` + Resources []map[string]any `json:"resources"` +} + +func readManifestForLogs(t *testing.T) manifestForLogs { + t.Helper() + raw, err := os.ReadFile("../../module.json") + if err != nil { + t.Fatal(err) + } + var m manifestForLogs + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + return m +} + +func resourceByID(m manifestForLogs, id string) (map[string]any, int) { + for i, r := range m.Resources { + if r["id"] == id { + return r, i + } + } + return nil, -1 +} + +// filled is a file's content as the controller fills it from the settings' defaults alone: each +// ${setting:} replaced by the default as text, the way the controller writes a value into a file. +func filled(t *testing.T, m manifestForLogs, content string) string { + t.Helper() + ref := regexp.MustCompile(`\$\{setting:([a-z0-9][a-z0-9_.-]*)\}`) + var missing []string + out := ref.ReplaceAllStringFunc(content, func(s string) string { + key := ref.FindStringSubmatch(s)[1] + d, ok := m.Settings[key] + if !ok || d.Kind != "preference" { + missing = append(missing, key) + return s + } + switch v := d.Default.(type) { + case float64: + return fmt.Sprintf("%d", int64(v)) + default: + return fmt.Sprint(v) + } + }) + if len(missing) > 0 { + t.Fatalf("the content asks for %v, which the manifest gives no preference default", missing) + } + return strings.ReplaceAll(out, "${seat:mesh-artifact-store:reach}", "registry.example:5100") +} + +func TestTheDefaultLogRotationIsTheOneTheHandSetMachineHas(t *testing.T) { + m := readManifestForLogs(t) + daemon, _ := resourceByID(m, "daemon") + if daemon == nil { + t.Fatal("no daemon resource") + } + var keys map[string]any + if err := json.Unmarshal([]byte(filled(t, m, daemon["content"].(string))), &keys); err != nil { + t.Fatalf("daemon.json's keys with the defaults are not JSON: %v", err) + } + if keys["log-driver"] != "json-file" { + t.Errorf("log-driver is %v, want json-file (the driver every machine runs today)", keys["log-driver"]) + } + want := map[string]any{"max-size": "100m", "max-file": "10"} + if !reflect.DeepEqual(keys["log-opts"], want) { + t.Errorf("log-opts is %v, want %v", keys["log-opts"], want) + } + if keys["live-restore"] != true { + t.Errorf("live-restore is %v", keys["live-restore"]) + } + // The values a person set are the terminal's only (novox/hq issue 346): the file says so. + if daemon["trusted"] != true { + t.Errorf("daemon asks for settings and says trusted %v, want true", daemon["trusted"]) + } +} + +func TestTheLogRotationIsCheckedBeforeDaemonJSONIsWritten(t *testing.T) { + m := readManifestForLogs(t) + conf, confAt := resourceByID(m, "log-rotation") + check, checkAt := resourceByID(m, "log-rotation-check") + _, daemonAt := resourceByID(m, "daemon") + if conf == nil || check == nil { + t.Fatal("no log-rotation file or log-rotation-check step") + } + // The host applies a module's resources in declared order, and a step that fails skips the rest + // of its module (novox/hq ADR 0136): declared before daemon, a refused value never reaches it. + if !(confAt < checkAt && checkAt < daemonAt) { + t.Errorf("order: log-rotation %d, log-rotation-check %d, daemon %d", confAt, checkAt, daemonAt) + } + if check["type"] != "process" || check["run-once"] != true || check["artifact"] != "tools" { + t.Errorf("the check is not a step of the tools bundle: %v", check) + } + if !reflect.DeepEqual(check["run"], []any{"./docker-tools", "check-log-rotation"}) { + t.Errorf("run %v", check["run"]) + } + if !reflect.DeepEqual(check["restart-on"], []any{"log-rotation"}) { + t.Errorf("restart-on %v", check["restart-on"]) + } + if _, asRoot := check["user"]; asRoot { + t.Errorf("the check reads a 0600 file and must run as root") + } + if conf["trusted"] != true { + t.Errorf("log-rotation says trusted %v, want true", conf["trusted"]) + } + // The step reads the very values daemon.json takes: the same keys, filled the same way. + values, err := parseLogRotation(filled(t, m, conf["content"].(string))) + if err != nil { + t.Fatalf("the defaults are refused: %v", err) + } + if values != (logRotation{Size: "100m", Files: "10"}) { + t.Errorf("defaults read as %+v", values) + } + daemon, _ := resourceByID(m, "daemon") + for _, key := range []string{"${setting:log-max-size}", "${setting:log-max-file}"} { + if !strings.Contains(daemon["content"].(string), key) || !strings.Contains(conf["content"].(string), key) { + t.Errorf("%s is not in both daemon and log-rotation", key) + } + } +} + +func TestTheRuntimeIsNeverRestartedForTheLogRotation(t *testing.T) { + m := readManifestForLogs(t) + runtime, _ := resourceByID(m, "runtime") + if _, restarts := runtime["restart-on"]; restarts { + t.Errorf("docker.service restarts on %v: a restart stops every container", runtime["restart-on"]) + } + if !reflect.DeepEqual(runtime["reload-on"], []any{"daemon"}) { + t.Errorf("reload-on %v", runtime["reload-on"]) + } +} + +func TestABadLogRotationIsRefused(t *testing.T) { + good := map[string]string{"size": "100m", "files": "10"} + conf := func(over map[string]string) string { + lines := []string{"# a comment"} + for _, k := range []string{"size", "files"} { + v := good[k] + if o, ok := over[k]; ok { + v = o + } + lines = append(lines, k+"="+v) + } + return strings.Join(lines, "\n") + "\n" + } + for _, bad := range []map[string]string{ + {"size": "100"}, {"size": "10x"}, {"size": "0m"}, {"size": "-1"}, {"size": ""}, {"size": "m"}, + {"size": "1.5m"}, {"size": "10M"}, {"size": "100m\""}, {"size": `100m", "data-root": "/tmp`}, + {"size": "100m\nfiles=3"}, {"size": " 100m"}, {"size": "99999999999m"}, + {"files": "0"}, {"files": "100"}, {"files": "-1"}, {"files": ""}, {"files": "01"}, {"files": "1\""}, + {"files": "1\nfiles=2"}, {"files": "3.0"}, {"files": "ten"}, + } { + if v, err := parseLogRotation(conf(bad)); err == nil { + t.Errorf("%q accepted as %+v", bad, v) + } + } + for _, broken := range []string{"size=100m\n", "files=10\n", "size=100m\nfiles=10\nfiles=3\n", + "size=100m\nfiles=10\nmode=x\n", "size=100m\nfiles\n"} { + if v, err := parseLogRotation(broken); err == nil { + t.Errorf("%q accepted as %+v", broken, v) + } + } +} + +func TestAGoodLogRotationIsAccepted(t *testing.T) { + for _, c := range []struct{ size, files string }{{"100m", "10"}, {"512k", "1"}, {"2g", "99"}, {"50m", "3"}} { + got, err := parseLogRotation("# comment\nsize=" + c.size + "\nfiles=" + c.files + "\n") + if err != nil { + t.Errorf("%v refused: %v", c, err) + continue + } + if got != (logRotation{Size: c.size, Files: c.files}) { + t.Errorf("%v read as %+v", c, got) + } + } + // And from the file the step is handed. + path := filepath.Join(t.TempDir(), "log-rotation.conf") + if err := os.WriteFile(path, []byte("size=50m\nfiles=3\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := checkLogRotationFile(path); err != nil { + t.Errorf("a good file refused: %v", err) + } + if err := os.WriteFile(path, []byte("size=50\nfiles=3\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := checkLogRotationFile(path); err == nil || !strings.Contains(err.Error(), "log-max-size") { + t.Errorf("a bad size was not refused by its setting's name: %v", err) + } + if err := checkLogRotationFile(filepath.Join(t.TempDir(), "absent")); err == nil { + t.Error("an absent file was accepted") + } +} diff --git a/modules/docker/cmd/docker-tools/main.go b/modules/docker/cmd/docker-tools/main.go index 66abc0bf..f89c0edd 100644 --- a/modules/docker/cmd/docker-tools/main.go +++ b/modules/docker/cmd/docker-tools/main.go @@ -16,6 +16,20 @@ import ( ) func main() { + // Started as `docker-tools check-log-rotation` it is the module's step that judges the log rotation before + // daemon.json takes it (logrotation.go, novox/hq issue 452): run as root by the host, reading the file + // DOCKER_LOG_ROTATION names. With no argument, the runtime's tools bundle. + if len(os.Args) > 1 { + if os.Args[1] != "check-log-rotation" || len(os.Args) != 2 { + fmt.Fprintln(os.Stderr, "usage: docker-tools [check-log-rotation]") + os.Exit(2) + } + if err := checkLogRotationFile(os.Getenv("DOCKER_LOG_ROTATION")); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + return + } // An empty name serves as the module the runtime names (MESH_SERVED_MODULE): docker. if err := stdio.Serve("", tools(NewClient())); err != nil { fmt.Fprintln(os.Stderr, err) diff --git a/modules/docker/module.json b/modules/docker/module.json index 73c8f74c..2a0dde99 100644 --- a/modules/docker/module.json +++ b/modules/docker/module.json @@ -44,6 +44,18 @@ "docker exec cat /etc/resolv.conf" ] }, + "settings": { + "log-max-size": { + "kind": "preference", + "default": "100m", + "why": "each container's log is cut at this size and the runtime keeps log-max-file of them, so 100m and 10 hold about 1 GB per container: what the one machine with rotation set by hand has (novox/hq issue 452). A whole number from 1 with k, m or g after it, refused before daemon.json is written. The runtime reads it only at its next start, which the mesh never forces, and a container created before that keeps the options it was created with until it is created again" + }, + "log-max-file": { + "kind": "preference", + "default": 10, + "why": "how many of a container's log files of log-max-size the runtime keeps, the oldest dropped first; a count from 1 to 99, refused before daemon.json is written, taken at the runtime's next start and by containers created after it, like log-max-size (novox/hq issue 452)" + } + }, "resources": [ { "id": "package", @@ -62,13 +74,45 @@ "state": "running", "boot": "enabled" }, + { + "id": "state", + "type": "directory", + "mode": "0700", + "place": "." + }, + { + "id": "log-rotation", + "type": "file", + "path": "${dir:state}/log-rotation.conf", + "mode": "0600", + "trusted": true, + "content": "# The log rotation daemon.json takes, written by the mesh from this machine's settings log-max-size and\n# log-max-file, and judged by the step log-rotation-check before daemon.json is written (novox/hq issue 452).\n# Replaced on every push; change the settings, never this file.\nsize=${setting:log-max-size}\nfiles=${setting:log-max-file}\n" + }, + { + "id": "log-rotation-check", + "type": "process", + "name": "docker-log-rotation-check", + "artifact": "tools", + "run": [ + "./docker-tools", + "check-log-rotation" + ], + "run-once": true, + "env": { + "DOCKER_LOG_ROTATION": "${dir:state}/log-rotation.conf" + }, + "restart-on": [ + "log-rotation" + ] + }, { "id": "daemon", "type": "file", "path": "/etc/docker/daemon.json", "mode": "0644", "into": "json", - "content": "{\"live-restore\": true, \"insecure-registries\": [\"${seat:mesh-artifact-store:reach}\"]}\n" + "trusted": true, + "content": "{\"live-restore\": true, \"insecure-registries\": [\"${seat:mesh-artifact-store:reach}\"], \"log-driver\": \"json-file\", \"log-opts\": {\"max-size\": \"${setting:log-max-size}\", \"max-file\": \"${setting:log-max-file}\"}}\n" }, { "id": "runtime",