From aef4993d101ad89e7c52e70e8352612174273644 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 22 Sep 2026 18:32:20 +0200 Subject: [PATCH] Hold an archive, a process's unit and a user found on an adopted node for an untaken module (hq ADR 0103) --- internal/apply/hold.go | 57 ++++++++++++++++++++++++++++ internal/apply/hold_test.go | 74 +++++++++++++++++++++++++++++++++++++ internal/apply/process.go | 5 ++- 3 files changed, 134 insertions(+), 2 deletions(-) diff --git a/internal/apply/hold.go b/internal/apply/hold.go index fce255d..74ca6c7 100644 --- a/internal/apply/hold.go +++ b/internal/apply/hold.go @@ -72,6 +72,25 @@ func lookBefore(ctx context.Context, sys system.System, d *declaration.Declarati if present(res.Path) && !recordedPath(known, res.Path) { seen["path:"+res.Path] = true } + case *declaration.Archive: + // Unpacking over it, and re-owning it recursively, would change the predecessor's + // files. + if present(res.Path) && !recordedPath(known, res.Path) { + seen["path:"+res.Path] = true + } + case *declaration.Process: + // Its unit would be written over and restarted. + if !known.Recorded(string(declaration.TypeProcess), res.Name) && + present(filepath.Join(unitDir, res.Name+".service")) { + seen["unit-file:"+res.Name] = true + } + case *declaration.User: + // Its shell and groups would be changed. + if !known.Recorded(string(declaration.TypeUser), res.Name) { + if _, exists, err := system.LookUpUser(ctx, run, res.Name); err == nil && exists { + seen["user:"+res.Name] = true + } + } case *declaration.Service: if known.Recorded(string(declaration.TypeService), res.Unit) { continue @@ -239,6 +258,12 @@ func holdOnAdopted(ctx context.Context, sys system.System, r declaration.Resourc } case *declaration.Directory: isFound = before["path:"+res.Path] + case *declaration.Archive: + isFound = before["path:"+res.Path] + case *declaration.Process: + isFound = before["unit-file:"+res.Name] + case *declaration.User: + isFound = before["user:"+res.Name] case *declaration.Service: isFound = before["unit:"+res.Unit] } @@ -396,6 +421,38 @@ func hold(ctx context.Context, sys system.System, r declaration.Resource, module } } detail = "found on the machine; its mode, owner and contents kept until " + module + " is taken" + case *declaration.Archive: + if _, err := os.Lstat(res.Path); errors.Is(err, os.ErrNotExist) { + if !already { + return out, h, fmt.Errorf("%s was found and is gone before it could be held", res.Path) + } + changed = "gone" + } else if err != nil { + return out, h, err + } + detail = "something is already at " + res.Path + "; nothing unpacked over it or re-owned until " + + module + " is taken" + case *declaration.Process: + unit := filepath.Join(unitDir, res.Name+".service") + if !present(unit) { + if !already { + return out, h, fmt.Errorf("%s was found and is gone before it could be held", unit) + } + changed = "gone" + } + detail = "its unit " + unit + " was found on the machine; not written over or restarted until " + + module + " is taken" + case *declaration.User: + _, exists, err := system.LookUpUser(ctx, run, res.Name) + switch { + case err != nil: + return out, h, err + case !exists && !already: + return out, h, fmt.Errorf("the user %s was found and is gone before it could be held", res.Name) + case !exists: + changed = "gone" + } + detail = "the user was found on the machine; its shell and groups are kept until " + module + " is taken" case *declaration.Service: state, err := sys.ServiceState(ctx, run, res.Unit) switch { diff --git a/internal/apply/hold_test.go b/internal/apply/hold_test.go index 98813b9..66148e5 100644 --- a/internal/apply/hold_test.go +++ b/internal/apply/hold_test.go @@ -24,6 +24,7 @@ type machine struct { // named volumes. units map[string]*fakeUnit volumes map[string]bool + users map[string]bool } type fakeUnit struct { @@ -80,6 +81,12 @@ func (m *machine) run(_ context.Context, name string, args ...string) (string, e if name == "systemctl" { return m.systemctl(args) } + if name == "getent" { + if m.users[args[len(args)-1]] { + return args[len(args)-1] + ":x:1500:1500::/home/" + args[len(args)-1] + ":/bin/bash\n", nil + } + return "", errors.New("exit status 2") + } if name != "docker" { return "", nil } @@ -680,3 +687,70 @@ func TestAnActionInAHeldContainerIsHeldUntilItsModuleIsTaken(t *testing.T) { t.Errorf("holds outlived the take: %+v", state.Held) } } + +const sixtyFourZeros = "0000000000000000000000000000000000000000000000000000000000000000" + +func TestAnArchiveOverSomethingFoundIsNotUnpacked(t *testing.T) { + dir := t.TempDir() + at := filepath.Join(dir, "site") + if err := os.Mkdir(at, 0o750); err != nil { + t.Fatal(err) + } + theirs := filepath.Join(at, "index.html") + _ = os.WriteFile(theirs, []byte("the predecessor's site\n"), 0o640) + m := &machine{containers: map[string]*fakeContainer{}} + // The source is unreachable: fetching it would fail the apply, so a pass means it was not tried. + report, state := applyAdopted(t, adopted(t, untaken("hello-web.site"), + `{"id":"hello-web.site","type":"archive","source":"http://192.0.2.1/site.tar.gz", + "digest":"sha256:`+sixtyFourZeros+`","path":"`+at+`","owner":"root"}`), store.State{}, m, dir) + if o := outcomeOf(report, "hello-web.site"); o.Action != "held" || !strings.Contains(o.Detail, "nothing unpacked") { + t.Errorf("an archive over found files was not held: %+v", o) + } + if got, _ := os.ReadFile(theirs); string(got) != "the predecessor's site\n" { + t.Errorf("the found files were touched: %q", got) + } + if _, ok := state.HeldAt("hello-web.site"); !ok { + t.Error("the hold was not recorded") + } +} + +func TestAProcessWhoseUnitIsFoundIsNotWrittenOverOrRestarted(t *testing.T) { + dir := t.TempDir() + was := unitDir + unitDir = dir + t.Cleanup(func() { unitDir = was }) + unit := filepath.Join(dir, "hello-daemon.service") + _ = os.WriteFile(unit, []byte("[Service]\nExecStart=/opt/predecessor/hello\n"), 0o644) + m := &machine{containers: map[string]*fakeContainer{}} + report, state := applyAdopted(t, adopted(t, untaken("hello-web.daemon"), + `{"id":"hello-web.daemon","type":"process","name":"hello-daemon","source":"http://192.0.2.1/d.tar.gz", + "digest":"sha256:`+sixtyFourZeros+`","run":["hello"]}`), store.State{}, m, dir) + if o := outcomeOf(report, "hello-web.daemon"); o.Action != "held" || !strings.Contains(o.Detail, "not written over or restarted") { + t.Errorf("a process whose unit was found was not held: %+v", o) + } + if got, _ := os.ReadFile(unit); string(got) != "[Service]\nExecStart=/opt/predecessor/hello\n" { + t.Errorf("the found unit was written over: %q", got) + } + if m.did("systemctl") { + t.Errorf("the found unit was touched: %v", m.asked) + } + if _, ok := state.HeldAt("hello-web.daemon"); !ok { + t.Error("the hold was not recorded") + } +} + +func TestAUserFoundOnTheMachineKeepsItsShellAndGroups(t *testing.T) { + dir := t.TempDir() + m := &machine{containers: map[string]*fakeContainer{}, users: map[string]bool{"hello": true}} + report, _ := applyAdopted(t, adopted(t, untaken("hello-web.user"), + `{"id":"hello-web.user","type":"user","name":"hello","shell":"/bin/zsh","groups":["docker"]}`), + store.State{}, m, dir) + if o := outcomeOf(report, "hello-web.user"); o.Action != "held" || !strings.Contains(o.Detail, "shell and groups") { + t.Errorf("a found user was not held: %+v", o) + } + for _, a := range m.asked { + if strings.HasPrefix(a, "usermod") || strings.HasPrefix(a, "useradd") { + t.Errorf("a found user was changed: %s", a) + } + } +} diff --git a/internal/apply/process.go b/internal/apply/process.go index 686b7f1..d9956e7 100644 --- a/internal/apply/process.go +++ b/internal/apply/process.go @@ -32,8 +32,9 @@ import ( // owners end up disagreeing about one path. const daemonRoot = "/var/lib/mesh/daemons" -// unitDir is where the mesh writes the units it owns. -const unitDir = "/etc/systemd/system" +// unitDir is where the mesh writes the units it owns. A variable only so a test can point it at a +// directory of its own. +var unitDir = "/etc/systemd/system" func applyProcess(ctx context.Context, r *declaration.Process, run Runner, changed map[string]bool, previous store.Applied) (Outcome, error) {