chick
9949cfe5b0
我在真内核上把安装序列完整走了一遍（容器里补了 useradd/systemctl 的替身，其余全是真的）， 暴露出一个只有真跑才看得见的缺陷： 装完 → `install` 启动服务 → 但那时**还没有配对的手机**，而守护进程**故意**拒绝空的设备列表 （一个什么都不能授权的 agent 不该坐在那里显得健康）。于是 `Restart=always` + `RestartSec=5` 变成每 5 秒重启一次，永远，journal 里刷同一行。实测：exit 1，"devices is empty, so no phone could ever be authorised"。 配对发生在安装**之后**（运维在节点上跑 `--pair`，然后在手机上配对），所以这不是异常状态， 而是流程的必经一步。修法两条： - 单元加 `StartLimitIntervalSec=300` / `StartLimitBurst=10`：十次之后 systemd 放弃，单元进入 `failed`——一个运维看得见、能处理的状态，而不是无限重启。 - `install` 的结束语改成说清这一步：服务会退出并停住，下一步是 `--pair`，用 `systemctl status` 看状态。此前它说"installed and running"，而那是假的。 同一轮演练确认了此前所有修复在真内核上成立：服务用户建了、配置与日志目录交给它了、配置对 该用户可读、单元来自二进制且内容一致、卸载按序拆干净（用户/目录/单元全无残留）。 Go 全包通过（新增一项守着"单元必须有启动上限，且 install 必须说出下一步"）。
477 lines
18 KiB
Go
477 lines
18 KiB
Go
package install
|
|
|
|
import (
|
|
"errors"
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"regexp"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
// recorder stands in for systemctl and userdel, so the sequence can be tested without systemd.
|
|
type recorder struct {
|
|
calls []string
|
|
// failOn makes a named command fail, for the cases where a missing unit or an absent user must not
|
|
// abort the uninstall.
|
|
failOn string
|
|
}
|
|
|
|
func (r *recorder) Run(name string, args ...string) error {
|
|
call := name + " " + strings.Join(args, " ")
|
|
r.calls = append(r.calls, call)
|
|
if r.failOn != "" && strings.HasPrefix(call, r.failOn) {
|
|
return fmt.Errorf("simulated failure")
|
|
}
|
|
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()
|
|
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.UnitPath, paths.ConfigPath, filepath.Join(paths.LogDir, "audit.log"), paths.BinaryPath} {
|
|
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)
|
|
}
|
|
}
|
|
return paths
|
|
}
|
|
|
|
func TestUninstallRemovesEverythingTheInstallPlaced(t *testing.T) {
|
|
paths := installed(t)
|
|
runner := &recorder{}
|
|
var lines []string
|
|
err := Uninstall(Options{
|
|
Paths: paths,
|
|
Runner: runner,
|
|
Log: func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) },
|
|
NotifyRelay: func() error {
|
|
lines = append(lines, "relay notified")
|
|
return nil
|
|
},
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("Uninstall: %v", err)
|
|
}
|
|
for _, path := range []string{paths.UnitPath, paths.ConfigPath, paths.LogDir, paths.BinaryPath} {
|
|
if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
|
|
t.Fatalf("%s survived the uninstall", path)
|
|
}
|
|
}
|
|
// The config directory goes with the config: it holds relay-ca.pem and anything else the operator
|
|
// put beside it.
|
|
if _, err := os.Stat(filepath.Dir(paths.ConfigPath)); !errors.Is(err, os.ErrNotExist) {
|
|
t.Fatal("the config directory survived; it may still hold pinned CA material")
|
|
}
|
|
joined := strings.Join(runner.calls, "\n")
|
|
for _, want := range []string{"systemctl disable --now msa-agent", "systemctl daemon-reload", "userdel msa-agent"} {
|
|
if !strings.Contains(joined, want) {
|
|
t.Fatalf("expected %q among the commands, got:\n%s", want, joined)
|
|
}
|
|
}
|
|
}
|
|
|
|
// The order is the point: the relay has to be told before the node stops existing, or it keeps routing
|
|
// to a node that is gone and a phone that believes it is paired points at nothing.
|
|
func TestUninstallTellsTheRelayBeforeItRemovesAnything(t *testing.T) {
|
|
paths := installed(t)
|
|
configStillThere := false
|
|
err := Uninstall(Options{
|
|
Paths: paths,
|
|
Runner: &recorder{},
|
|
Log: func(string, ...any) {},
|
|
NotifyRelay: func() error {
|
|
// At the moment the relay is told, the config must still exist: the notification reads it.
|
|
_, statErr := os.Stat(paths.ConfigPath)
|
|
configStillThere = statErr == nil
|
|
return nil
|
|
},
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("Uninstall: %v", err)
|
|
}
|
|
if !configStillThere {
|
|
t.Fatal("the config was removed before the relay was told; the device list could not be read")
|
|
}
|
|
}
|
|
|
|
// A relay that cannot be reached must not stop an uninstall. The operator is removing the node either
|
|
// way, and the alternative — aborting half way — leaves the machine in a state nobody described.
|
|
func TestUninstallCompletesWhenTheRelayIsUnreachable(t *testing.T) {
|
|
paths := installed(t)
|
|
var lines []string
|
|
err := Uninstall(Options{
|
|
Paths: paths,
|
|
Runner: &recorder{},
|
|
Log: func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) },
|
|
NotifyRelay: func() error { return fmt.Errorf("relay unreachable") },
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("Uninstall: %v", err)
|
|
}
|
|
if _, err := os.Stat(paths.ConfigPath); !errors.Is(err, os.ErrNotExist) {
|
|
t.Fatal("the uninstall did not finish")
|
|
}
|
|
// And the operator is told what is left to do by hand, because the relay still lists this node.
|
|
output := strings.Join(lines, "\n")
|
|
if !strings.Contains(output, "WARNING") || !strings.Contains(output, "agents.json") {
|
|
t.Fatalf("a failed relay notification was not explained:\n%s", output)
|
|
}
|
|
}
|
|
|
|
// A dry run removes nothing, and that includes not stopping the service: an operator asking what an
|
|
// uninstall would do must not lose the node while they read the answer.
|
|
func TestDryRunRemovesNothingAndRunsNothing(t *testing.T) {
|
|
paths := installed(t)
|
|
runner := &recorder{}
|
|
var lines []string
|
|
err := Uninstall(Options{
|
|
Paths: paths,
|
|
DryRun: true,
|
|
Runner: runner,
|
|
Log: func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) },
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("Uninstall: %v", err)
|
|
}
|
|
if len(runner.calls) != 0 {
|
|
t.Fatalf("a dry run ran commands: %v", runner.calls)
|
|
}
|
|
for _, path := range []string{paths.UnitPath, paths.ConfigPath, paths.BinaryPath} {
|
|
if _, err := os.Stat(path); err != nil {
|
|
t.Fatalf("a dry run removed %s", path)
|
|
}
|
|
}
|
|
if !strings.Contains(strings.Join(lines, "\n"), "dry run: nothing was removed") {
|
|
t.Fatal("a dry run did not say so")
|
|
}
|
|
}
|
|
|
|
func TestKeepLogsLeavesTheAuditLog(t *testing.T) {
|
|
paths := installed(t)
|
|
audit := filepath.Join(paths.LogDir, "audit.log")
|
|
err := Uninstall(Options{
|
|
Paths: paths,
|
|
KeepLogs: true,
|
|
Runner: &recorder{},
|
|
Log: func(string, ...any) {},
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("Uninstall: %v", err)
|
|
}
|
|
if _, err := os.Stat(audit); err != nil {
|
|
t.Fatalf("--keep-logs removed the audit log: %v", err)
|
|
}
|
|
}
|
|
|
|
// A missing unit or an absent user is the normal case on a partially installed node, and it must not
|
|
// abort: reporting it as an error would train the operator to ignore the errors that matter.
|
|
func TestMissingPiecesDoNotAbortTheUninstall(t *testing.T) {
|
|
paths := installed(t)
|
|
// Remove the unit first, then uninstall.
|
|
if err := os.Remove(paths.UnitPath); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
runner := &recorder{failOn: "systemctl disable"}
|
|
err := Uninstall(Options{
|
|
Paths: paths,
|
|
Runner: runner,
|
|
Log: func(string, ...any) {},
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("a missing unit aborted the uninstall: %v", err)
|
|
}
|
|
if _, err := os.Stat(paths.ConfigPath); !errors.Is(err, os.ErrNotExist) {
|
|
t.Fatal("the config survived")
|
|
}
|
|
}
|
|
|
|
// The plan names every path, so an operator can check it before running anything.
|
|
func TestThePlanNamesEveryPath(t *testing.T) {
|
|
plan := DefaultPaths().String()
|
|
for _, want := range []string{"/etc/systemd/system/msa-agent.service", "/etc/msa-agent/config.json",
|
|
"/var/log/msa-agent", "/opt/msa-agent/msa-agent", "msa-agent"} {
|
|
if !strings.Contains(plan, want) {
|
|
t.Fatalf("the plan does not mention %s:\n%s", want, plan)
|
|
}
|
|
}
|
|
}
|
|
|
|
// 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)
|
|
}
|
|
}
|
|
|
|
// The hint at the end has to name the right place, because the two records live in different places and
|
|
// are removed by different people. Telling the operator the wrong one sends them to the wrong place.
|
|
func TestTheRemovalHintIsTheOneItWasGiven(t *testing.T) {
|
|
paths := installed(t)
|
|
var lines []string
|
|
err := Uninstall(Options{
|
|
Paths: paths,
|
|
Runner: &recorder{},
|
|
Log: func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) },
|
|
RemovalHint: "the relay still lists this node in its enrollment state; remove it in the app",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("Uninstall: %v", err)
|
|
}
|
|
output := strings.Join(lines, "\n")
|
|
if !strings.Contains(output, "remove it in the app") {
|
|
t.Fatalf("the hint was not printed:\n%s", output)
|
|
}
|
|
// And the old hard-coded sentence is gone: it named agents.json for every node, including the
|
|
// self-enrolled ones that are not there.
|
|
if strings.Contains(output, "agents.json still lists this node") {
|
|
t.Fatalf("the uninstall still claims every node is in agents.json:\n%s", output)
|
|
}
|
|
}
|
|
|
|
// The install ends by starting a service that cannot run yet, and the unit has to say what to do about
|
|
// that. A freshly installed node has no paired devices — pairing happens after install — and the daemon
|
|
// refuses to run in that state on purpose, because an agent that could authorise nothing must not look
|
|
// healthy. With `Restart=always` and `RestartSec=5` and no start limit, that is a restart every five
|
|
// seconds until a phone pairs.
|
|
//
|
|
// Found by rehearsing the install on a real kernel: exit 1, "devices is empty, so no phone could ever be
|
|
// authorised", and the journal filling with the same line.
|
|
func TestTheUnitGivesUpInsteadOfRestartingForever(t *testing.T) {
|
|
unit := string(UnitFile())
|
|
if !strings.Contains(unit, "Restart=always") {
|
|
t.Skip("the unit no longer restarts, so there is nothing to bound")
|
|
}
|
|
if !strings.Contains(unit, "StartLimitIntervalSec=") || !strings.Contains(unit, "StartLimitBurst=") {
|
|
t.Fatal("the unit restarts without a start limit: an unpaired node would restart every RestartSec forever")
|
|
}
|
|
// And the install tells the operator what to do next, so the failed state is expected rather than
|
|
// surprising.
|
|
var lines []string
|
|
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)
|
|
}
|
|
}
|
|
if err := Install(InstallOptions{
|
|
Paths: paths,
|
|
Runner: &recorder{failOn: "getent"},
|
|
Log: func(format string, args ...any) { lines = append(lines, fmt.Sprintf(format, args...)) },
|
|
}); err != nil {
|
|
t.Fatalf("Install: %v", err)
|
|
}
|
|
if !strings.Contains(strings.Join(lines, "\n"), "--pair") {
|
|
t.Fatalf("the install does not say to pair next:\n%s", strings.Join(lines, "\n"))
|
|
}
|
|
}
|