diff --git a/internal/apply/apply.go b/internal/apply/apply.go index efe8ed0..f3cd989 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -139,6 +139,11 @@ type Error struct { // reading a report needs to know the difference between "these things failed" and "these // things failed and the rest was never tried". Gated 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 { @@ -149,7 +154,10 @@ func (e *Error) Error() string { 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 { + if len(e.Modules) > 0 { + rest = fmt.Sprintf("the rest of %s was not attempted and is listed as skipped; everything else "+ + "was attempted, so what is not listed as failed or skipped was done.", strings.Join(e.Modules, ", ")) + } else 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." @@ -506,6 +514,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 +534,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 +548,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 +682,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 +741,14 @@ 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.Done = report failed.Others = len(failures) - 1 + failed.Modules = gatedModules() return report, known, failed } continue @@ -888,6 +926,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 +1009,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 {