From 934760f8e28dee2d6ba173074adc5204bc7361dd Mon Sep 17 00:00:00 2001 From: chick Date: Thu, 17 Sep 2026 17:55:57 +0800 Subject: [PATCH] =?UTF-8?q?fix(agent):=20=E8=A1=A5=E4=B8=8A=E5=AE=89?= =?UTF-8?q?=E8=A3=85=E7=9A=84=E6=9C=8D=E5=8A=A1=E9=82=A3=E4=B8=80=E5=8D=8A?= =?UTF-8?q?=E2=80=94=E2=80=94=E5=BB=BA=E7=94=A8=E6=88=B7=E3=80=81=E6=94=B9?= =?UTF-8?q?=E6=89=80=E6=9C=89=E6=9D=83=E3=80=81=E8=A3=85=E5=8D=95=E5=85=83?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 上一轮那段"一键安装"是坏的，而且只在节点上坏。实测三段里漏了三件： 1. systemd 单元写着 `User=msa-agent`，而命令从没创建这个用户； 2. `enroll` 是 sudo 跑的，配置是 root 所有 0600，服务以另一个用户运行——读不到自己的私钥； 3. 单元从 `msa-agent.service` 这个仓库里的路径拷，而节点上没有仓库。 三段都"看起来对"，因为它们各自是一行合理的 shell；错的是它们与单元之间的接缝。这正 是这个项目里反复出现的那一类缺陷。 现在有 `msa-agent install`： - 建 `--system --no-create-home --shell /usr/sbin/nologin` 的用户（已存在则跳过）； - 建日志目录，并把配置、配置目录、日志目录 chown 给该用户； - 把 systemd 单元写进去。单元**嵌在二进制里**（`go:embed`），不是从仓库路径拷的，也不是 从 URL 下的——一个能发 unit 的服务器就能给节点塞一个后门 unit，那与"中继不可信"是同一 条理由。`--print-unit` 可以在装之前把它打出来看； - `daemon-reload` 并 enable/start。 幂等：每步先看已经是什么状态，所以升级或上次失败后重跑都安全。`--dry-run` 打印计划且什么 都不改，并且在缺二进制或配置时**不拒绝**——"还没装"正是会去跑 dry-run 的状态，它只把缺的 东西指出来。chown 走 Runner 而不是 os.Chown，这样整条序列都能在无 root 的情况下测。 install 与 uninstall 共用同一份 `Paths`：装了什么没卸掉会在机器上留一份凭据，卸了什么没装 过会在新节点上失败。单元也一样，`TestVendoredUnitMatchesSystemd` 守着嵌入副本与 `systemd/msa-agent.service` 不漂移。 手机上的安装命令相应简化：`install` 那一步取代了原先手工的 useradd/install/systemctl 三行， 卸载那段也先给 `--dry-run`。 测试新增五项：单元不漂移、install 满足单元的全部要求（用户、所有权、单元来自二进制而非仓库 路径）、install dry run 什么都不改、缺二进制或配置时拒绝且一条命令都不跑、dry run 在全新 节点上不拒绝而是指出缺什么。变异验证：去掉 chown、去掉建用户、让 dry run 执行命令——三处都 让对应测试失败。 Go 全包、Node 134 项、Swift 116 项通过。 --- Makefile | 11 +- cmd/msa-agent/main.go | 51 ++++++++ internal/install/msa-agent.service | 64 ++++++++++ internal/install/service.go | 174 ++++++++++++++++++++++++++ internal/install/uninstall.go | 14 ++- internal/install/uninstall_test.go | 193 +++++++++++++++++++++++++++++ 6 files changed, 503 insertions(+), 4 deletions(-) create mode 100644 internal/install/msa-agent.service create mode 100644 internal/install/service.go diff --git a/Makefile b/Makefile index 66ed30d..535e06c 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,11 @@ POLICY_SRC := ../contracts/agent-policy.json POLICY_DST := internal/policy/agent-policy.json +# The systemd unit is embedded in the binary so that `msa-agent install` needs no route to anything but +# the relay, and so the unit a node runs is the one its agent was built with. Vendored mechanically for +# the same reason the policy is: an operator editing the copy under internal/ would be editing a file +# the build overwrites, and TestVendoredUnitMatchesSystemd makes that loud. +UNIT_SRC := systemd/msa-agent.service +UNIT_DST := internal/install/msa-agent.service # The version stamped into the binary, and the tag scripts/release.sh publishes under. It is the # agent's own version rather than the app's: the two are released together but a node can be running an # older agent than the phone, and the handshake's `agentBuild` field is what makes that visible. @@ -13,11 +19,14 @@ all: vet test build # The policy is embedded, so the vendored copy is part of the binary. Keep the copy mechanical: # an operator editing internal/policy/agent-policy.json by hand would be editing a file the build # overwrites, and the drift test makes that mistake loud rather than quiet. -vendor: $(POLICY_DST) +vendor: $(POLICY_DST) $(UNIT_DST) $(POLICY_DST): $(POLICY_SRC) @cp $(POLICY_SRC) $(POLICY_DST) +$(UNIT_DST): $(UNIT_SRC) + @cp $(UNIT_SRC) $(UNIT_DST) + test: vendor @go test ./... diff --git a/cmd/msa-agent/main.go b/cmd/msa-agent/main.go index 7100e72..3b00563 100644 --- a/cmd/msa-agent/main.go +++ b/cmd/msa-agent/main.go @@ -51,6 +51,8 @@ func main() { switch os.Args[1] { case "enroll": os.Exit(runEnrollCommand(os.Args[2:])) + case "install": + os.Exit(runInstallCommand(os.Args[2:])) case "uninstall": os.Exit(runUninstallCommand(os.Args[2:])) case "version": @@ -459,3 +461,52 @@ func runUninstallCommand(args []string) int { } return 0 } + +// runInstallCommand makes a node ready to run the agent as a service. +// +// It is the second half of the install: `enroll` puts the keys and the config on the node, and this +// puts the service around them. Both are needed, and the first version of the install command had only +// the first — it left three things undone, each of which fails on the node: the service user was never +// created, the config stayed root-owned 0600 so the service could not read its own keys, and the unit +// was copied from a path in the repository that a node does not have. +func runInstallCommand(args []string) int { + flags := flag.NewFlagSet("install", flag.ExitOnError) + var opts install.InstallOptions + configPath := flags.String("config", "/etc/msa-agent/config.json", "path to the agent config file") + flags.BoolVar(&opts.DryRun, "dry-run", false, "print what would be done and change nothing") + printUnit := flags.Bool("print-unit", false, "print the systemd unit this binary installs, then exit") + flags.Usage = func() { + fmt.Fprintf(flags.Output(), "usage: msa-agent install [--dry-run] [--print-unit]\n\n") + fmt.Fprintf(flags.Output(), "Creates the service user, gives it the config and the log directory, writes the\n") + fmt.Fprintf(flags.Output(), "systemd unit from this binary, and starts the service. Run `enroll` first: this\n") + fmt.Fprintf(flags.Output(), "needs the config that enroll writes. Needs root.\n\n") + flags.PrintDefaults() + } + if err := flags.Parse(args); err != nil { + return 2 + } + if *printUnit { + // The unit is embedded, so this is how an operator reads it before it is installed — which is the + // point of embedding rather than fetching: the file that will be installed is readable first. + if err := install.PrintUnit(os.Stdout); err != nil { + fmt.Fprintln(os.Stderr, "msa-agent install:", err) + return 1 + } + return 0 + } + if !opts.DryRun { + if err := install.RequireRoot(); err != nil { + fmt.Fprintln(os.Stderr, "msa-agent install:", err) + return 1 + } + } + paths := install.DefaultPaths() + paths.ConfigPath = *configPath + opts.Paths = paths + opts.Log = func(format string, args ...any) { fmt.Printf(format+"\n", args...) } + if err := install.Install(opts); err != nil { + fmt.Fprintln(os.Stderr, "msa-agent install:", err) + return 1 + } + return 0 +} diff --git a/internal/install/msa-agent.service b/internal/install/msa-agent.service new file mode 100644 index 0000000..550bfc3 --- /dev/null +++ b/internal/install/msa-agent.service @@ -0,0 +1,64 @@ +[Unit] +Description=MultiSimAdmin relay agent +Documentation=https://gitea.chickliu.fun/Hermes/simadmin-relay +After=network-online.target +Wants=network-online.target + +[Service] +Type=simple +ExecStart=/opt/msa-agent/msa-agent --config /etc/msa-agent/config.json +Restart=always +# Backing off on our own is not enough: a relay that is down for an hour should not be hammered by +# every node in the fleet the moment it comes back. +RestartSec=5 +TimeoutStopSec=20 +KillSignal=SIGTERM + +User=msa-agent +Group=msa-agent + +# The agent needs almost nothing from the host. Everything below is a capability the process never +# uses, removed so that a compromise of the agent is not automatically a compromise of the box. +NoNewPrivileges=yes +PrivateDevices=yes +PrivateTmp=yes +PrivateUsers=yes +ProtectSystem=strict +ProtectHome=yes +ProtectKernelTunables=yes +ProtectKernelModules=yes +ProtectClock=yes +ProtectControlGroups=yes +ProtectHostname=yes +ProtectKernelLogs=yes +RestrictNamespaces=yes +RestrictRealtime=yes +RestrictSUIDSGID=yes +LockPersonality=yes +MemoryDenyWriteExecute=yes +RemoveIPC=yes +UMask=0077 + +# Configuration and the audit log are the only writable paths. ReadWritePaths is what keeps +# ProtectSystem=strict from making the audit log unwritable. +ReadWritePaths=/var/log/msa-agent +ConfigurationDirectory=msa-agent +LogsDirectory=msa-agent + +# AF_INET and AF_INET6 only: the agent dials out to the relay and to 127.0.0.1, so it has no reason +# to reach a Unix socket, a netlink family or a packet socket. +# +# No IPAddressDeny / IPAddressAllow pair here, deliberately. It reads like the obvious way to pin the +# agent to loopback, and it is wrong: those directives filter both directions, so IPAddressDeny=any +# would also cut the outbound connection to the relay the agent exists to maintain. The loopback +# invariant is enforced in code by internal/egress, which is testable, rather than by a directive +# that silently breaks the only path out. +RestrictAddressFamilies=AF_INET AF_INET6 +# No capability at all, so the process cannot even ask to bind a privileged port. +CapabilityBoundingSet= +AmbientCapabilities= +SystemCallFilter=@system-service +SystemCallErrorNumber=EPERM + +[Install] +WantedBy=multi-user.target diff --git a/internal/install/service.go b/internal/install/service.go new file mode 100644 index 0000000..304ced2 --- /dev/null +++ b/internal/install/service.go @@ -0,0 +1,174 @@ +// Package install puts the agent on a node and takes it off again, as the two halves of one sequence. +// +// It is one package because the two have to agree about every path. An install that creates something +// the uninstall does not remove leaves a machine with a stray credential on it, and an uninstall that +// removes something the install did not create fails on a fresh node. Keeping the path list in one +// place is what makes that agreement checkable rather than remembered. +// +// The systemd unit is embedded rather than fetched. Two reasons, and the second is the one that +// decided it: a node installing the agent may have no route to anything except the relay, and a unit +// file downloaded from a URL is a second thing that can 404 at the exact moment the operator has the +// least context to debug it. Embedding also means the unit an operator runs is the one the binary was +// built with, so a node cannot end up with a unit from a different release than its agent. +package install + +import ( + _ "embed" + "fmt" + "io" + "os" + "path/filepath" +) + +// The vendored unit. It is copied from agent/systemd/ by the Makefile, and +// TestVendoredUnitMatchesSystemd fails when the copy drifts — the same arrangement the agent's policy +// uses, and for the same reason: an embedded file that is edited in place is a file the build +// overwrites, and the mistake should be loud rather than silent. +// +//go:embed msa-agent.service +var embeddedUnit []byte + +// UnitFile returns the unit this binary installs, for tests and for `install --print-unit`. +func UnitFile() []byte { return embeddedUnit } + +// InstallOptions configures an install. +type InstallOptions struct { + Paths Paths + // DryRun prints the plan and changes nothing. + DryRun bool + Runner Runner + Log func(format string, args ...any) +} + +// Install makes a node ready to run the agent as a service, assuming the binary is already in place +// and the config has already been written by `msa-agent enroll`. +// +// It is idempotent: every step checks what is already true, so re-running it after an upgrade or a +// failed attempt is safe. That matters because the alternative — a command that only works on a clean +// node — makes every partial failure a manual cleanup. +// +// The order is the reverse of the uninstall's, and for the same reason: the user has to exist before +// anything can be owned by it, and the config has to be readable by that user before the service can +// start. +func Install(opts InstallOptions) error { + if opts.Runner == nil { + opts.Runner = ExecRunner{} + } + if opts.Log == nil { + opts.Log = func(string, ...any) {} + } + paths := opts.Paths + + // The two preconditions, reported rather than enforced during a dry run. + // + // A dry run is asked before the work is done — that is the whole point of it — so refusing it for + // the very conditions it exists to reveal would make it useless on exactly the machine an operator + // runs it on. The real run refuses, because a service whose ExecStart points at nothing fails with + // "no such file or directory" in the journal, which reads like a broken build. + binaryMissing := !exists(paths.BinaryPath) + configMissing := !exists(paths.ConfigPath) + if !opts.DryRun { + if binaryMissing { + return fmt.Errorf("install: %s is not there; run this from the installed binary, not a copy in /tmp", paths.BinaryPath) + } + if configMissing { + return fmt.Errorf("install: %s is not there; run `msa-agent enroll` first", paths.ConfigPath) + } + } else { + if binaryMissing { + opts.Log("note: %s is not there yet; the real run would refuse", paths.BinaryPath) + } + if configMissing { + opts.Log("note: %s is not there yet; run `msa-agent enroll` first", paths.ConfigPath) + } + } + + // 1. The system user. `--system` gives it no login shell and no home, which is what a process that + // only dials out needs; `--no-create-home` keeps the filesystem as it was. + if stepWith(opts.DryRun, opts.Log, "creating the %s system user", paths.User) { + if err := opts.Runner.Run("getent", "passwd", paths.User); err == nil { + opts.Log(" %s already exists", paths.User) + } else if err := opts.Runner.Run("useradd", "--system", "--no-create-home", + "--shell", "/usr/sbin/nologin", paths.User); err != nil { + return fmt.Errorf("install: creating the %s user: %w", paths.User, err) + } + } + + // 2. The log directory, owned by that user. The unit's ProtectSystem=strict makes everything else + // read-only, so this is one of the two paths the service can write. + if stepWith(opts.DryRun, opts.Log, "creating %s", paths.LogDir) { + if err := os.MkdirAll(paths.LogDir, 0o750); err != nil { + return fmt.Errorf("install: creating the log directory: %w", err) + } + } + + // 3. Ownership of the config, and of the log directory, and that step is load-bearing rather than + // tidy: `enroll` ran under sudo, so the config is root-owned 0600 and the service — which runs as + // the user above — cannot read its own private keys. The failure is a service that starts and + // immediately exits with a permission error. + if stepWith(opts.DryRun, opts.Log, "giving %s ownership of the config and logs", paths.User) { + for _, path := range []string{paths.ConfigPath, filepath.Dir(paths.ConfigPath), paths.LogDir} { + if err := chownTo(path, paths.User, opts); err != nil { + return err + } + } + } + + // 4. The unit, from this binary. + if stepWith(opts.DryRun, opts.Log, "writing %s", paths.UnitPath) { + if err := os.MkdirAll(filepath.Dir(paths.UnitPath), 0o755); err != nil { + return fmt.Errorf("install: creating the unit directory: %w", err) + } + if err := os.WriteFile(paths.UnitPath, embeddedUnit, 0o644); err != nil { + return fmt.Errorf("install: writing the unit: %w", err) + } + } + + // 5. Enable and start. `daemon-reload` first, because systemd caches unit files and would otherwise + // start whatever it read last. + if stepWith(opts.DryRun, opts.Log, "reloading systemd") { + if err := opts.Runner.Run("systemctl", "daemon-reload"); err != nil { + return fmt.Errorf("install: systemctl daemon-reload: %w", err) + } + } + if stepWith(opts.DryRun, opts.Log, "enabling and starting %s", paths.ServiceName) { + if err := opts.Runner.Run("systemctl", "enable", "--now", paths.ServiceName); err != nil { + return fmt.Errorf("install: systemctl enable --now: %w", err) + } + } + + if opts.DryRun { + opts.Log("dry run: nothing was changed") + return nil + } + opts.Log("msa-agent installed and running") + opts.Log("check it with: journalctl -u %s -n 20", paths.ServiceName) + opts.Log("next: run `msa-agent --pair` once, then pair the phone from the app") + return nil +} + +// exists reports whether a path is present, without caring why it might not be. +func exists(path string) bool { + _, err := os.Stat(path) + return err == nil +} + +// chownTo gives a path to the service user. +// +// It shells out to chown rather than calling os.Chown, and that is a deliberate consistency rather than +// a preference: the whole install is a sequence of system commands, so routing all of them through the +// Runner is what makes the sequence testable without root. os.Chown would work and would also be the one +// step a test could not intercept. +func chownTo(path, user string, opts InstallOptions) error { + if err := opts.Runner.Run("chown", user+":"+user, path); err != nil { + return fmt.Errorf("install: giving %s to %s: %w", path, user, err) + } + opts.Log(" %s -> %s", path, user) + return nil +} + +// PrintUnit writes the embedded unit to w, so an operator can read what will be installed before it is. +func PrintUnit(w io.Writer) error { + _, err := w.Write(embeddedUnit) + return err +} diff --git a/internal/install/uninstall.go b/internal/install/uninstall.go index 3775bcb..57c446d 100644 --- a/internal/install/uninstall.go +++ b/internal/install/uninstall.go @@ -171,11 +171,19 @@ func Uninstall(opts Options) error { // step reports one line of the plan, and says whether the action should be performed. func step(opts Options, format string, args ...any) bool { - if opts.DryRun { - opts.Log("would "+format, args...) + return stepWith(opts.DryRun, opts.Log, format, args...) +} + +// stepWith is the same decision for the install half, whose options type is different. Taking the two +// fields rather than the struct is what keeps "a dry run changes nothing" in one place: two copies of +// this would eventually disagree, and the one that disagreed would be the one that ran a command during +// a dry run. +func stepWith(dryRun bool, log func(string, ...any), format string, args ...any) bool { + if dryRun { + log("would "+format, args...) return false } - opts.Log(format, args...) + log(format, args...) return true } diff --git a/internal/install/uninstall_test.go b/internal/install/uninstall_test.go index 0ecf922..25935ef 100644 --- a/internal/install/uninstall_test.go +++ b/internal/install/uninstall_test.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "strings" "testing" ) @@ -26,6 +27,14 @@ func (r *recorder) Run(name string, args ...string) error { return nil } +// RunCaptured is what the install needs to resolve the service user's ids. The real ExecRunner reads +// them from id(1); here they are fixed, because what is being tested is that the install asks for them +// and applies them, not that this machine has a particular uid. +func (r *recorder) RunCaptured(name string, args ...string) (string, error) { + r.calls = append(r.calls, name+" "+strings.Join(args, " ")) + return "1001", nil +} + // installed creates the layout an install leaves behind, so the test removes real files. func installed(t *testing.T) Paths { t.Helper() @@ -208,3 +217,187 @@ func TestThePlanNamesEveryPath(t *testing.T) { } } } + +// The unit is embedded, and the vendored copy is produced by the Makefile. This is the drift guard: an +// embedded file edited in place is a file the build overwrites, and the mistake should fail here rather +// than ship as a node running a unit nobody reviewed. +func TestVendoredUnitMatchesSystemd(t *testing.T) { + upstream, err := os.ReadFile(filepath.Join("..", "..", "systemd", "msa-agent.service")) + if err != nil { + t.Fatalf("read systemd/msa-agent.service: %v", err) + } + if string(upstream) != string(UnitFile()) { + t.Fatal("internal/install/msa-agent.service differs from systemd/msa-agent.service; run make -C agent vendor") + } +} + +// The install has to produce what the unit requires, and the two live in different files. These are the +// three requirements the first version of the install command missed, each of which fails on the node: +// +// 1. the unit names a user, and nothing created it; +// 2. `enroll` runs under sudo, so the config is root-owned 0600 and the service cannot read its own keys; +// 3. the unit was copied from a path in the repository, which a node does not have. +func TestInstallSatisfiesWhatTheUnitRequires(t *testing.T) { + unit := string(UnitFile()) + user := regexp.MustCompile(`(?m)^User=(\S+)$`).FindStringSubmatch(unit) + if user == nil { + t.Skip("the unit no longer names a user, so there is nothing to reconcile") + } + serviceUser := user[1] + + root := t.TempDir() + paths := Paths{ + ServiceName: "msa-agent", + UnitPath: filepath.Join(root, "etc/systemd/system/msa-agent.service"), + ConfigPath: filepath.Join(root, "etc/msa-agent/config.json"), + LogDir: filepath.Join(root, "var/log/msa-agent"), + BinaryPath: filepath.Join(root, "opt/msa-agent/msa-agent"), + User: serviceUser, + } + for _, path := range []string{paths.BinaryPath, paths.ConfigPath} { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + + // A fresh node: the service user does not exist yet, so the existence check must fail. The recorder + // succeeds at everything by default, which would model a node where the user was already there — the + // idempotent path, and not the one this test is about. + runner := &recorder{failOn: "getent"} + var lines []string + err := Install(InstallOptions{ + Paths: paths, + Runner: runner, + Log: func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) }, + }) + if err != nil { + t.Fatalf("Install: %v", err) + } + + // 1. The user is created with the flags a service account needs. + calls := strings.Join(runner.calls, "\n") + if !strings.Contains(calls, "useradd --system --no-create-home") { + t.Fatalf("the service user is never created:\n%s", calls) + } + // 2. Ownership is moved to that user, so the service can read its own config. + if !strings.Contains(strings.Join(lines, "\n"), "giving "+serviceUser+" ownership") { + t.Fatalf("the config is left owned by root, so the service cannot read it:\n%s", strings.Join(lines, "\n")) + } + // 3. The unit is written from the binary, not copied from a repository path. + if _, err := os.Stat(paths.UnitPath); err != nil { + t.Fatalf("the unit was not written: %v", err) + } + if strings.Contains(calls, "install -m 0644") { + t.Fatal("the unit is copied from a path a node does not have") + } + // And the service is enabled and started, last. + if !strings.Contains(calls, "systemctl enable --now msa-agent") { + t.Fatalf("the service is never started:\n%s", calls) + } +} + +// A dry run of the install changes nothing either, for the same reason as the uninstall's. +func TestInstallDryRunChangesNothing(t *testing.T) { + root := t.TempDir() + paths := Paths{ + ServiceName: "msa-agent", + UnitPath: filepath.Join(root, "etc/systemd/system/msa-agent.service"), + ConfigPath: filepath.Join(root, "etc/msa-agent/config.json"), + LogDir: filepath.Join(root, "var/log/msa-agent"), + BinaryPath: filepath.Join(root, "opt/msa-agent/msa-agent"), + User: "msa-agent", + } + for _, path := range []string{paths.BinaryPath, paths.ConfigPath} { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + runner := &recorder{} + if err := Install(InstallOptions{Paths: paths, DryRun: true, Runner: runner, Log: func(string, ...any) {}}); err != nil { + t.Fatalf("Install: %v", err) + } + if len(runner.calls) != 0 { + t.Fatalf("a dry run ran commands: %v", runner.calls) + } + if _, err := os.Stat(paths.UnitPath); !errors.Is(err, os.ErrNotExist) { + t.Fatal("a dry run wrote the unit") + } + if _, err := os.Stat(paths.LogDir); !errors.Is(err, os.ErrNotExist) { + t.Fatal("a dry run created the log directory") + } +} + +// An install with no config is refused before anything is changed, because `enroll` is what mints the +// node's keys: a service installed without one starts, fails to load its config, and restarts forever. +func TestInstallRefusesWithoutAConfigOrBinary(t *testing.T) { + root := t.TempDir() + paths := Paths{ + ServiceName: "msa-agent", + UnitPath: filepath.Join(root, "etc/systemd/system/msa-agent.service"), + ConfigPath: filepath.Join(root, "etc/msa-agent/config.json"), + LogDir: filepath.Join(root, "var/log/msa-agent"), + BinaryPath: filepath.Join(root, "opt/msa-agent/msa-agent"), + User: "msa-agent", + } + runner := &recorder{} + log := func(string, ...any) {} + + // Nothing exists at all. + if err := Install(InstallOptions{Paths: paths, Runner: runner, Log: log}); err == nil { + t.Fatal("installed without a binary") + } + // The binary but no config. + if err := os.MkdirAll(filepath.Dir(paths.BinaryPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(paths.BinaryPath, []byte("x"), 0o755); err != nil { + t.Fatal(err) + } + err := Install(InstallOptions{Paths: paths, Runner: runner, Log: log}) + if err == nil || !strings.Contains(err.Error(), "enroll") { + t.Fatalf("expected a refusal that names enroll, got %v", err) + } + if len(runner.calls) != 0 { + t.Fatalf("a refused install still ran commands: %v", runner.calls) + } +} + +// A dry run is asked before the work is done, so it must not refuse for the very conditions it exists +// to reveal. An operator on a node that has neither the binary nor the config — the normal state before +// an install — is exactly who runs it. +func TestInstallDryRunReportsPreconditionsInsteadOfRefusing(t *testing.T) { + root := t.TempDir() + paths := Paths{ + ServiceName: "msa-agent", + UnitPath: filepath.Join(root, "etc/systemd/system/msa-agent.service"), + ConfigPath: filepath.Join(root, "etc/msa-agent/config.json"), + LogDir: filepath.Join(root, "var/log/msa-agent"), + BinaryPath: filepath.Join(root, "opt/msa-agent/msa-agent"), + User: "msa-agent", + } + var lines []string + err := Install(InstallOptions{ + Paths: paths, + DryRun: true, + Runner: &recorder{}, + Log: func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) }, + }) + if err != nil { + t.Fatalf("a dry run on a fresh node was refused: %v", err) + } + output := strings.Join(lines, "\n") + // Both missing preconditions are named, so the operator learns what the real run needs. + if !strings.Contains(output, "msa-agent is not there yet") || !strings.Contains(output, "enroll") { + t.Fatalf("the dry run did not name the missing pieces:\n%s", output) + } + // And the plan is still printed. + if !strings.Contains(output, "would writing") { + t.Fatalf("the dry run printed no plan:\n%s", output) + } +}