novox/mesh-host
6
0
Fork 0
Code Issues Pull Requests 2 Releases Wiki Activity

Merge pull request 'A refused directory gates its module, and a file meets the placement guard (issue 495)' (#83) from fix/495-a-refused-directory-gates-its-module into main

Browse Source
This commit was merged in pull request #83.
This commit is contained in:
mesh-admin
2026-10-11 17:00:22 +00:00
parent 6defec8f68 4695dfc0a2
commit 3333a05a74
3 changed files with 368 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/apply/apply.go
+80 -11
View File
@@ -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
}
internal/apply/placement_guard.go
+66 -3
View File
@@ -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 {
internal/apply/refused_directory_test.go
+222
View File
@@ -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)
}
}