From 9116067e11ca7e4788e4dca513d18b3d5a8ed5f6 Mon Sep 17 00:00:00 2001 From: jochens Date: Fri, 13 Nov 2026 01:01:35 +0100 Subject: [PATCH] Z18 zfs-callsite: the call-site test pins every form of ADR 0679 whole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renamed TestTheBundleRunsOnlyTheFormsOfADR0679: each root form is held to its whole argument list, no verb the decision never allows, zgenhostid only with the parsed hostid, a device only as cache or log. Builds: design 42 zfs-owns-zfs §19 --- modules/zfs/cmd/zfs-tools/manifest_test.go | 78 ++++++++++++++++++++-- 1 file changed, 72 insertions(+), 6 deletions(-) diff --git a/modules/zfs/cmd/zfs-tools/manifest_test.go b/modules/zfs/cmd/zfs-tools/manifest_test.go index c8e5f7a45..3c028f2a0 100644 --- a/modules/zfs/cmd/zfs-tools/manifest_test.go +++ b/modules/zfs/cmd/zfs-tools/manifest_test.go @@ -298,11 +298,10 @@ func TestTheHealthChecksAndWhoRunsWhat(t *testing.T) { } } -// Nothing but reading, a snapshot and a scrub's start, pause or stop. Every command the bundle runs goes -// through ask, AsRoot, Escalated, Run, exec.Command or exec.CommandContext with an argument list from one -// of the functions below, each held to its program and verb; a call site that builds its own list fails -// this test. A new verb needs a decision first, and then a line here. -func TestTheBundleRunsOnlyReadingVerbsSnapshotAndScrub(t *testing.T) { +// §19, ADR 0679 §7, §17: every command the bundle runs goes through ask, AsRoot, Escalated, Run, exec.Command or +// exec.CommandContext with an argument list from one of the functions below, each held to its program, its verb and its whole +// form; a call site that builds its own list fails this test. A new verb needs a decision first, and then a line here. +func TestTheBundleRunsOnlyTheFormsOfADR0679(t *testing.T) { allowed := map[string][2]string{ "DatasetsArgv": {"zfs", "list"}, "SnapshotsArgv": {"zfs", "list"}, "PropertiesArgv": {"zfs", "get"}, "SnapshotArgv": {"zfs", "snapshot"}, @@ -339,6 +338,73 @@ func TestTheBundleRunsOnlyReadingVerbsSnapshotAndScrub(t *testing.T) { t.Errorf("%s runs %q, not %q", fn, argv[0], allowed[fn][1]) } } + // The whole form of each root form §19 pins, and never a forbidden verb or flag. + pinned := map[string][]string{ + "AddArgv cache": AddArgv("tank", "cache", false, "/dev/a", "/dev/b"), + "AddArgv log": AddArgv("tank", "log", true, "/dev/a", "/dev/b"), + "RemoveArgv": RemoveArgv("tank", "/dev/a"), + "PoolSetArgv": PoolSetArgv("tank", "autotrim", "on"), + "DatasetSetArgv": DatasetSetArgv("tank/a", "atime", "off"), + "DatasetInherit": DatasetInheritArgv("tank/a", "atime"), + "SnapshotArgv": SnapshotArgv("tank/a", "x"), + "SnapshotSetArgv": SnapshotSetArgv("tank/a@mesh-hourly-x", "hourly"), + "DestroyArgv": DestroyArgv("tank/a", "mesh-hourly-x"), + "TrimArgv": TrimArgv("tank"), + "WipefsArgv": WipefsArgv("/dev/a"), + "ZdbLabelArgv": ZdbLabelArgv("/dev/a"), + "ZdbConfigArgv": ZdbConfigArgv("tank"), + "ZgenhostidArgv": ZgenhostidArgv("0a0b0c0d"), + "UdevArgv": UdevArgv(), + } + forms := map[string]string{ + "AddArgv cache": "add -- tank cache /dev/a /dev/b", "AddArgv log": "add -- tank log mirror /dev/a /dev/b", "RemoveArgv": "remove -- tank /dev/a", + "PoolSetArgv": "set autotrim=on -- tank", "DatasetSetArgv": "set atime=off -- tank/a", "DatasetInherit": "inherit atime -- tank/a", + "SnapshotArgv": "snapshot -- tank/a@x", "SnapshotSetArgv": "set mesh:schedule=hourly -- tank/a@mesh-hourly-x", + "DestroyArgv": "destroy -- tank/a@mesh-hourly-x", "TrimArgv": "trim -- tank", "WipefsArgv": "--no-act /dev/a", "ZdbLabelArgv": "-l /dev/a", + "ZdbConfigArgv": "-C tank", "ZgenhostidArgv": "-f 0a0b0c0d", "UdevArgv": "monitor --udev --subsystem-match=block", + } + for name, argv := range pinned { + if got := strings.Join(argv, " "); got != forms[name] { + t.Errorf("%s is %q, the pinned form is %q", name, got, forms[name]) + } + for _, a := range argv { + if (a == "-f" && name != "ZgenhostidArgv") || a == "-r" || a == "-R" || a == "-d" { + t.Errorf("%s holds %s", name, a) + } + } + } + // No argument function of a zpool or zfs program runs a verb ADR 0679 does not allow. + never := map[string]bool{"upgrade": true, "rollback": true, "rename": true, "clear": true, "send": true, "receive": true, "create": true, + "import": true, "export": true, "attach": true, "detach": true, "offline": true, "online": true, "replace": true, "labelclear": true, + "split": true, "reguid": true, "promote": true, "clone": true, "destroy-pool": true, "reopen": true, "sync": true, "checkpoint": true} + for fn, pv := range allowed { + if (pv[0] == "zpool" || pv[0] == "zfs") && never[pv[1]] { + t.Errorf("%s runs %s %s", fn, pv[0], pv[1]) + } + if pv[1] == "destroy" && fn != "DestroyArgv" || pv[0] == "zdb" && fn != "ZdbLabelArgv" && fn != "ZdbConfigArgv" || pv[0] == "zgenhostid" && fn != "ZgenhostidArgv" { + t.Errorf("%s runs %s %s", fn, pv[0], pv[1]) + } + } + // zgenhostid only with the value the parse validated; a device only as cache or log; a destroy only of a snapshot. + files, _ := filepath.Glob("*.go") + for _, file := range files { + if strings.HasSuffix(file, "_test.go") { + continue + } + b, _ := os.ReadFile(file) + for i, l := range strings.Split(string(b), "\n") { + trimmed := strings.TrimSpace(l) + if strings.HasPrefix(trimmed, "//") || strings.HasPrefix(trimmed, "func ") { + continue + } + if strings.Contains(l, "ZgenhostidArgv(") && !strings.Contains(l, "ZgenhostidArgv(c.HostID)") { + t.Errorf("%s:%d runs zgenhostid with a value the parse did not validate: %s", file, i+1, trimmed) + } + if m := regexp.MustCompile(`AddArgv\(\w+(?:\.\w+)?, ("[a-z]+"|\w+),`).FindStringSubmatch(l); m != nil && m[1] != `"cache"` && m[1] != `"log"` { + t.Errorf("%s:%d adds a device that is neither cache nor log: %s", file, i+1, trimmed) + } + } + } for _, act := range []string{"start", "pause", "stop", "anything"} { argv := ScrubArgv(act, "tank") if argv[0] != "scrub" || argv[len(argv)-2] != "--" || argv[len(argv)-1] != "tank" || len(argv) > 4 { @@ -360,7 +426,7 @@ func TestTheBundleRunsOnlyReadingVerbsSnapshotAndScrub(t *testing.T) { "follow.go: if err := cmd.Start(); err != nil {": true, "follow_wake.go: if err := cmd.Start(); err != nil {": true, } - files, _ := filepath.Glob("*.go") + files, _ = filepath.Glob("*.go") sites := 0 for _, file := range files { if strings.HasSuffix(file, "_test.go") {