diff --git a/internal/apply/apply.go b/internal/apply/apply.go index efe8ed0..2e64757 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -135,10 +135,20 @@ type Error struct { // line is what somebody reads. Others int - // Gated is true when what failed was an action, so nothing after it was attempted. A person - // reading a report needs to know the difference between "these things failed" and "these + // Gated is true when what failed was a step (an action, or a run-once container or process). A + // person reading a report needs to know the difference between "these things failed" and "these // things failed and the rest was never tried". Gated bool + + // Stopped is true when that step belongs to no module, so it gated the whole machine and + // nothing after it was attempted. A module's step gates only its module (novox/hq ADR 0136), + // which Modules names. + Stopped bool + + // Modules are the modules whose rest was not attempted, behind a step that did not complete + // (novox/hq ADR 0136) or a directory the engine refused (issue 495). Named, because "everything + // was attempted" is then untrue, and a report that says so after skipping a module lies. + Modules []string } func (e *Error) Error() string { @@ -148,14 +158,40 @@ func (e *Error) Error() string { } else if e.Others > 1 { also = fmt.Sprintf(", and %d other resources also failed", e.Others) } - rest := "everything was attempted, so what is not listed as failed was done." - if e.Gated { - // An action is a gate: it exists to make something true before the next thing needs it. - rest = "this is an action, so nothing after it was attempted — the machine is in " + - "whatever state that left it." + // Gated with no module named is an Error made before Stopped existed: a step that stopped the machine. + stopped := e.Stopped || (e.Gated && len(e.Modules) == 0) + // What the apply did not attempt, said whole (novox/hq issue 495): a module gated earlier in an apply + // that a machine-wide step then stopped is both, and saying either alone is untrue. + gatedRest := "" + if len(e.Modules) > 0 { + gatedRest = fmt.Sprintf("the rest of %s was not attempted and is listed as skipped", strings.Join(e.Modules, ", ")) } - return fmt.Sprintf("applying %q: %v%s\n\n%d resource(s) were applied and remain; %s", - e.Resource, e.Err, also, len(e.Done.Outcomes), rest) + rest := "everything was attempted, so what is not listed as failed was done." + switch { + case stopped && gatedRest != "": + rest = gatedRest + "; and this step gates the whole machine, so nothing after it was attempted — " + + "the machine is in whatever state that left it." + case stopped: + // An action is a gate: it exists to make something true before the next thing needs it. + rest = "this step gates the whole machine, so nothing after it was attempted — the machine is in " + + "whatever state that left it." + case gatedRest != "": + rest = gatedRest + "; everything else was attempted, so what is not listed as failed or skipped was done." + } + // Skipped is not applied: counted apart, so the count says what is on the machine. + applied, skipped := 0, 0 + for _, o := range e.Done.Outcomes { + if o.Action == "skipped" { + skipped++ + } else { + applied++ + } + } + count := fmt.Sprintf("%d resource(s) were applied and remain", applied) + if skipped > 0 { + count += fmt.Sprintf(", %d were skipped", skipped) + } + return fmt.Sprintf("applying %q: %v%s\n\n%s; %s", e.Resource, e.Err, also, count, rest) } func (e *Error) Unwrap() error { return e.Err } @@ -506,6 +542,16 @@ func ApplyMindingWindows( // Modules whose own step did not complete. What follows *within such a module* is not attempted; // the rest of the machine is (novox/hq ADR 0136). gated := map[string]bool{} + // Why each gated module was gated, said on every resource of it that was skipped. + gatedWhy := map[string]string{} + gatedModules := func() []string { + out := make([]string, 0, len(gated)) + for m := range gated { + out = append(out, m) + } + sort.Strings(out) + return out + } for i, resource := range ordered { if !orphansRemoved && i == guardFirst { // **Only a guard that is up may let the filter go.** Removing the derived filter's @@ -516,6 +562,7 @@ func ApplyMindingWindows( first := failures[0] first.Done = report first.Others = len(failures) - 1 + first.Modules = gatedModules() log(" kept " + guardPrefix + "*: the guard is not up, so what it replaces was left in force") return report, known, first } @@ -529,7 +576,7 @@ func ApplyMindingWindows( if module, ours := moduleOf(resource.Identity()); ours && gated[module] { report.Outcomes = append(report.Outcomes, Outcome{ ID: resource.Identity(), Type: string(resource.Kind()), Target: resource.Target(), - Action: "skipped", Detail: "a step this module declares did not complete", + Action: "skipped", Detail: gatedWhy[module], }) continue } @@ -663,6 +710,23 @@ func ApplyMindingWindows( // dynsec store was not seeded must not be started, so a run-once step that did not // complete gates what follows exactly as a failed action does — that is the whole of // how "before that container starts" is enforced, since the step is declared before it. + // **A directory the engine refused gates the rest of its module** (novox/hq issue 495), as a + // step that did not complete does. What a module declares after its directory — a file in it, + // a step that reads it, a container that mounts it — was written for that directory, and the + // engine refused Docker's data root and then wrote a file into it and ran the module's step. + // The file guard (refusedPlaces) refuses what is below the directory whatever its module; this + // keeps the module from going on as if its directory were there. Only the guard's refusals: a + // directory that failed to be made is a failure like any other. + if dir, ok := resource.(*declaration.Directory); ok && in.refused[filepath.Clean(dir.Path)] != nil { + if module, ours := moduleOf(resource.Identity()); ours { + gated[module] = true + gatedWhy[module] = fmt.Sprintf("its directory %s was refused (novox/hq issue 495), so "+ + "nothing more of this module was attempted", filepath.Clean(dir.Path)) + log(fmt.Sprintf(" gated %s.*: its directory %s was refused, so the rest of it was not "+ + "attempted", module, filepath.Clean(dir.Path))) + } + continue + } gates := resource.Kind() == declaration.TypeAction if c, ok := resource.(*declaration.Container); ok && c.RunOnce { gates = true @@ -705,12 +769,15 @@ func ApplyMindingWindows( // next possible, and they belong to no module. if module, ours := moduleOf(resource.Identity()); ours { gated[module] = true + gatedWhy[module] = "a step this module declares did not complete" log(fmt.Sprintf(" gated %s.*: a step it declares did not complete, so the rest "+ "of it was not attempted", module)) continue } + failed.Stopped = true failed.Done = report failed.Others = len(failures) - 1 + failed.Modules = gatedModules() return report, known, failed } continue @@ -888,6 +955,7 @@ func ApplyMindingWindows( first := failures[0] first.Done = report first.Others = len(failures) - 1 + first.Modules = gatedModules() return report, known, first } return report, known, nil @@ -970,7 +1038,8 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru // Nothing is placed in or mounted from the machine's own trees, whoever asked (novox/hq issue 339): a // refusal is this resource failing, with its reason, in the node's report. switch res := r.(type) { - case *declaration.Directory, *declaration.Access: + case *declaration.Directory, *declaration.Access, *declaration.File, *declaration.Archive: + // A file and an archive too: below a refused directory, or in a tree nothing is placed in (issue 495). if err := in.refused[filepath.Clean(r.Target())]; err != nil { return begin(r), err } diff --git a/internal/apply/placement_guard.go b/internal/apply/placement_guard.go index 49c1d83..01112d5 100644 --- a/internal/apply/placement_guard.go +++ b/internal/apply/placement_guard.go @@ -31,6 +31,13 @@ package apply // 5. **A mount source that is a refused directory or a refused access is refused with it**: the container // would otherwise bind the very path the engine would not place, and the runtime creates a missing one // as root. +// 6. **A file is judged as a directory is** (novox/hq issue 495): never one of the machine's own roots, nor +// in a tree nothing is placed in (rule 2), and never below a directory or an access refused here. A file +// or an archive below a refused path is refused with it, whichever module declares it and in whatever +// order. Rules 3 and 4 are a directory's owner and do not reach a file: a file below /etc is the mesh's +// ordinary configuration, and one below a home is judged by home_links.go and agent_home_guard.go. +// A refused directory also gates the rest of its module, as a step that did not complete does +// (apply.go's `gated`): what a module declares after its directory was written for that directory. // // Each is a failed resource with its reason in the node's report; nothing is touched. Paths are judged as // declared and again with every link in them resolved, so a link at /srv/x pointing at /etc places nothing. @@ -400,6 +407,7 @@ const ( placingDirectory placing = iota placingAccess placingMount + placingFile ) // refusePath says why a path is not placed or mounted; nil when it may be. module is the resource's module, @@ -466,7 +474,7 @@ func refuseOne(path string, kind placing, module, owner string) *PlacementRefuse for _, part := range strings.Split(path, "/") { ssh = ssh || part == ".ssh" } - if ssh && kind != placingDirectory { + if ssh && (kind == placingAccess || kind == placingMount) { return &PlacementRefusedError{Path: path, Why: "it is an account's .ssh, which holds its keys and who may " + "log in as it, and is never an access or a mount"} } @@ -502,8 +510,12 @@ func moduleOfID(id string) string { return module } -// refusedPlaces judges every directory and access of a declaration before anything is applied, and answers -// each refusal by path: what is refused is refused again as a container's mount source. +// refusedPlaces judges every directory, access, file and archive of a declaration before anything is applied, +// and answers each refusal by path: what is refused is refused again as a container's mount source. +// +// Directories and accesses first, so a file or an archive below one of them is refused with it whatever order +// they are declared in (novox/hq issue 495): the engine refused Docker's data root as a directory and then wrote +// a file into it, because nothing judged the file's path. func refusedPlaces(resources []declaration.Resource) map[string]error { out := map[string]error{} for _, r := range resources { @@ -520,9 +532,60 @@ func refusedPlaces(resources []declaration.Resource) map[string]error { out[filepath.Clean(r.Target())] = err } } + trees := make(map[string]error, len(out)) + for path, err := range out { + trees[path] = err + } + for _, r := range resources { + var err error + switch res := r.(type) { + case *declaration.File: + if err = belowRefused(res.Path, trees); err == nil { + err = refusePath(res.Path, placingFile, moduleOfID(res.ID), "") + } + case *declaration.Archive: + // Only below a refused path: where an archive may unpack is otherwise the archive's own judgement + // (archive.go), and the engine's own module unpacks its versions into its own tree. + err = belowRefused(res.Path, trees) + default: + continue + } + if err != nil { + out[filepath.Clean(r.Target())] = err + } + } return out } +// belowRefused says why a path below a refused directory or access is refused with it; nil when it is below +// none. Judged as declared and with its links resolved, as refusePath judges. +func belowRefused(path string, refused map[string]error) error { + clean := filepath.Clean(path) + if !filepath.IsAbs(clean) { + return nil + } + for _, p := range []string{clean, resolved(clean)} { + // The deepest refused tree it is below, so the same declaration always gives the same reason. + var tree string + for t := range refused { + if below(p, t) && len(t) > len(tree) { + tree = t + } + } + if tree == "" { + continue + } + why := refused[tree] + reason := why.Error() + var refusal *PlacementRefusedError + if errors.As(why, &refusal) { + reason = refusal.Why + } + return &PlacementRefusedError{Path: clean, Why: "it is below " + tree + ", which is refused: " + reason} + } + return nil +} + // refuseMounts says why a container's mounts are not made; nil when they may be. func refuseMounts(c *declaration.Container, refused map[string]error) error { for _, v := range c.Volumes { diff --git a/internal/apply/refused_directory_test.go b/internal/apply/refused_directory_test.go new file mode 100644 index 0000000..997cf28 --- /dev/null +++ b/internal/apply/refused_directory_test.go @@ -0,0 +1,222 @@ +package apply + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// A refused directory gates its module, and a file is judged by the same guard as a directory (novox/hq +// issue 495). +// +// The shape it was found in, on a server during mesh-catalog #205's walk (issue 452): the docker module +// declared a directory at Docker's data root, a file below it, and a run-once step after both. The engine +// refused the directory as not placed and said nothing was touched, then wrote the file into the very tree +// it had refused and ran the step. + +// aRuntimeRoot makes a temporary directory the container runtime's data root, as its daemon.json says, so +// the guard refuses it as it refused /var/lib/docker. +func aRuntimeRoot(t *testing.T) string { + t.Helper() + dir := t.TempDir() + root := filepath.Join(dir, "docker") + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatal(err) + } + daemon := filepath.Join(dir, "daemon.json") + if err := os.WriteFile(daemon, []byte(`{"data-root":"`+root+`"}`), 0o644); err != nil { + t.Fatal(err) + } + before := runtimeConfigs + runtimeConfigs = runtimeFiles{daemonJSON: daemon, storageConf: filepath.Join(dir, "none.conf")} + t.Cleanup(func() { + runtimeConfigs = before + runtimeRootsMu.Lock() + runtimeRoots = nil + runtimeRootsMu.Unlock() + }) + return root +} + +func TestARefusedDirectoryGatesItsModule(t *testing.T) { + root := aRuntimeRoot(t) + b := newStepBed(t) + elsewhere := filepath.Join(b.dir, "other.conf") + report, _, err := Apply(context.Background(), archHost(t), declare(t, + `{"id":"mod.state","type":"directory","path":"`+root+`"},`+ + `{"id":"mod.rotation","type":"file","path":"`+root+`/log-rotation.conf","content":"max-size=10m\n"},`+ + `{"id":"mod.check","type":"process","name":"mod-check","source":"`+b.body+`","digest":"`+b.digest+`",`+ + `"run":["./check"],"run-once":true,"restart-on":["mod.rotation"]},`+ + `{"id":"other.conf","type":"file","path":"`+elsewhere+`","content":"kept apart\n"}`), + store.State{}, store.OriginDeclared, b.run, nil, nil) + + var refusal *PlacementRefusedError + if err == nil || !errors.As(err, &refusal) { + t.Fatalf("the refused directory was not reported as refused: %v", err) + } + if _, statErr := os.Stat(filepath.Join(root, "log-rotation.conf")); statErr == nil { + t.Error("a file was written below the directory the engine had just refused") + } + if b.starts != 0 { + t.Errorf("the module's step ran %d time(s) after its directory was refused", b.starts) + } + for _, id := range []string{"mod.rotation", "mod.check"} { + o := outcomeOf(report, id) + if o.Action != "skipped" || !strings.Contains(o.Detail, root) { + t.Errorf("%s is not reported as skipped behind the refused directory: %+v", id, o) + } + } + // The gate reaches the module, not the machine. + if got, readErr := os.ReadFile(elsewhere); readErr != nil || string(got) != "kept apart\n" { + t.Errorf("another module's file was not applied: %q, %v", got, readErr) + } + // And the report says truthfully what was not attempted. + msg := err.Error() + if strings.Contains(msg, "everything was attempted") || !strings.Contains(msg, "the rest of mod") { + t.Errorf("the report does not say the rest of the module was not attempted: %s", msg) + } +} + +// A file is judged as a directory is: refused in a tree nothing is placed in, and below a directory the +// engine refused, whichever module declares it and in whatever order. +func TestAFileIsJudgedByTheSameGuardAsADirectory(t *testing.T) { + root := aRuntimeRoot(t) + _, _, err := applyOnce(t, `{"id":"notes.conf","type":"file","path":"`+root+`/notes.conf","content":"x\n"}`, + nil, store.State{}) + refusedFor339(t, "a file in the container runtime's data", err) + if _, statErr := os.Stat(filepath.Join(root, "notes.conf")); statErr == nil { + t.Error("a file was written into the container runtime's data") + } + + // Below a directory refused for another reason (a home's directory declared root's), declared before it. + dir := t.TempDir() + home := filepath.Join(dir, "home", "alice") + if err := os.MkdirAll(home, 0o755); err != nil { + t.Fatal(err) + } + passwd := filepath.Join(dir, "passwd") + if err := os.WriteFile(passwd, []byte("alice:x:4242:4242::"+home+":/bin/sh\n"), 0o644); err != nil { + t.Fatal(err) + } + before := passwdFile + passwdFile = passwd + t.Cleanup(func() { passwdFile = before }) + if err := os.MkdirAll(filepath.Join(home, "data"), 0o755); err != nil { + t.Fatal(err) + } + report, _, err := applyOnce(t, + `{"id":"notes.conf","type":"file","path":"`+home+`/data/notes.conf","content":"x\n"},`+ + `{"id":"notes.data","type":"directory","path":"`+home+`/data"}`, nil, store.State{}) + refusedFor339(t, "a file below a refused directory", err) + if _, statErr := os.Stat(filepath.Join(home, "data", "notes.conf")); statErr == nil { + t.Error("a file was written below a directory the engine refused") + } + // A refused file has no outcome: it is a failure. Declared first, it is the failure the apply reports, + // and the directory the other. + var applyErr *Error + if !errors.As(err, &applyErr) || applyErr.Resource != "notes.conf" || applyErr.Others != 1 { + t.Errorf("the file below a refused directory is not among the failures: %v", err) + } + if o := outcomeOf(report, "notes.conf"); o.ID != "" { + t.Errorf("a refused file has an outcome as if applied: %+v", o) + } +} + +// An archive below a refused path is refused with it, whichever module declares it and in whatever order. +func TestAnArchiveBelowARefusedPathIsRefused(t *testing.T) { + dir := t.TempDir() + home := filepath.Join(dir, "home", "alice") + if err := os.MkdirAll(home, 0o755); err != nil { + t.Fatal(err) + } + passwd := filepath.Join(dir, "passwd") + if err := os.WriteFile(passwd, []byte("alice:x:4242:4242::"+home+":/bin/sh\n"), 0o644); err != nil { + t.Fatal(err) + } + before := passwdFile + passwdFile = passwd + t.Cleanup(func() { passwdFile = before }) + body, digest := anArchive(t, map[string]string{"theme.css": "x\n"}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write(body) })) + t.Cleanup(server.Close) + + _, _, err := applyOnce(t, + `{"id":"theme.files","type":"archive","path":"`+home+`/data/theme","source":"`+server.URL+`/t.tar.gz",`+ + `"digest":"`+digest+`"},`+ + `{"id":"notes.data","type":"directory","path":"`+home+`/data"}`, nil, store.State{}) + refusedFor339(t, "an archive below a refused directory", err) + var applyErr *Error + if !errors.As(err, &applyErr) || applyErr.Resource != "theme.files" || applyErr.Others != 1 { + t.Errorf("the archive below a refused directory is not among the failures: %v", err) + } + if _, statErr := os.Stat(filepath.Join(home, "data")); statErr == nil { + t.Error("an archive was unpacked below a directory the engine refused") + } +} + +// Gated is per apply: a module gated behind its refused directory is tried again by the next apply, gated +// again while the directory is refused, and applied whole once it is not. +func TestAGatedModuleIsTriedAgainByTheNextApply(t *testing.T) { + root := aRuntimeRoot(t) + b := newStepBed(t) + declared := func() string { + return `{"id":"mod.state","type":"directory","path":"` + root + `"},` + + `{"id":"mod.rotation","type":"file","path":"` + root + `/log-rotation.conf","content":"max-size=10m\n"},` + + `{"id":"mod.check","type":"process","name":"mod-check","source":"` + b.body + `","digest":"` + b.digest + `",` + + `"run":["./check"],"run-once":true,"restart-on":["mod.rotation"]}` + } + apply := func(known store.State) (Report, store.State, error) { + return Apply(context.Background(), archHost(t), declare(t, declared()), known, store.OriginDeclared, b.run, nil, nil) + } + report, known, err := apply(store.State{}) + if err == nil || !skippedIn(report, "mod.check") || b.starts != 0 { + t.Fatalf("apply 1: err %v, skipped %v, starts %d", err, skippedIn(report, "mod.check"), b.starts) + } + report, known, err = apply(known) + if err == nil || !skippedIn(report, "mod.check") || b.starts != 0 { + t.Fatalf("apply 2, still refused: err %v, skipped %v, starts %d", err, skippedIn(report, "mod.check"), b.starts) + } + // The runtime moves its data elsewhere: the directory is no longer refused, and the module is applied. + elsewhere := filepath.Join(t.TempDir(), "docker") + if err := os.WriteFile(runtimeConfigs.daemonJSON, []byte(`{"data-root":"`+elsewhere+`"}`), 0o644); err != nil { + t.Fatal(err) + } + report, _, err = apply(known) + if err != nil || skippedIn(report, "mod.check") || b.starts != 1 { + t.Fatalf("apply 3, no longer refused: err %v, skipped %v, starts %d", err, skippedIn(report, "mod.check"), b.starts) + } + if _, statErr := os.Stat(filepath.Join(root, "log-rotation.conf")); statErr != nil { + t.Errorf("the module's file was not written once its directory was placed: %v", statErr) + } +} + +// A module gated behind its refused directory, then a step of no module that stops the machine: the report +// names both, and says nothing after the step was attempted (novox/hq issue 495). +func TestAMachineStopAfterAGatedModuleSaysBoth(t *testing.T) { + root := aRuntimeRoot(t) + after := filepath.Join(t.TempDir(), "after.conf") + _, _, err := Apply(context.Background(), archHost(t), parseTrusted(t, `{"declaration":1,"resources":[ + {"id":"mod.state","type":"directory","path":"`+root+`"}, + {"id":"gate","type":"action","command":["false"],"verify":["false"]}, + {"id":"other.after","type":"file","path":"`+after+`","content":"b\n"} + ]}`), store.State{}, store.OriginCarried, ExecRunner, nil, nil) + var applyErr *Error + if !errors.As(err, &applyErr) || !applyErr.Stopped { + t.Fatalf("the failed action did not stop the apply: %v", err) + } + if _, statErr := os.Stat(after); statErr == nil { + t.Error("the apply continued past a failed action") + } + msg := err.Error() + if !strings.Contains(msg, "the rest of mod") || !strings.Contains(msg, "nothing after it was attempted") || + strings.Contains(msg, "everything else was attempted") { + t.Errorf("the report does not say both what was gated and that the machine stopped: %s", msg) + } +}