fix(agent): 卸载时指出该去哪删节点,而不是一律说 agents.json
agent 不持有运维令牌——节点不是运维,一个能删掉邻居的被控节点比它解决的问题更糟——所以 它不能自己从中继上摘掉自己。此前卸载的结束语对所有节点都写"the relay's agents.json still lists this node",而自注册的节点根本不在那里:它在中继的状态文件里,要从手机上删。运维照着 那句话去翻一个没有这条记录的文件。 现在节点在登记时把来历写进配置(`selfEnrolled`,带 `--code` 为真),卸载结束语据此给出 正确的那一句: - 自注册的 → "在中继的登记状态里,去 App 的「中继节点」移除" - 手工登记的 → "在 agents.json 里,删掉那条并重启中继" - 配置读不出来 → 两句都提,因为读不到不是让卸载失败的理由 测试两项:卸载打印的是它被告知的那一句(且不再对自注册节点说 agents.json)、登记时正确 记录来历(带码为真、不带码不写这个字段)。 Go 全包、Node 141 项、Swift 121 项通过。
This commit is contained in:
@@ -51,6 +51,9 @@ func runEnroll(ctx context.Context, opts EnrollOptions) error {
|
||||
}
|
||||
|
||||
result, err := enroll.Generate(enroll.Options{
|
||||
// The flag is what records the node's own provenance: with a code the relay holds a
|
||||
// self-enrolled entry for it, without one an operator wrote it into agents.json.
|
||||
SelfEnrolled: opts.Code != "",
|
||||
RelayURL: opts.RelayURL,
|
||||
LocalPort: opts.LocalPort,
|
||||
AuditPath: opts.AuditPath,
|
||||
@@ -132,6 +135,9 @@ func runUninstall(ctx context.Context, opts install.Options, configPath string)
|
||||
if opts.NotifyRelay == nil && !opts.DryRun {
|
||||
opts.NotifyRelay = func() error { return retireAtRelay(ctx, configPath) }
|
||||
}
|
||||
if opts.RemovalHint == "" {
|
||||
opts.RemovalHint = removalHint(configPath)
|
||||
}
|
||||
return install.Uninstall(opts)
|
||||
}
|
||||
|
||||
@@ -195,3 +201,22 @@ func retireAtRelay(ctx context.Context, configPath string) error {
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// removalHint says what is left at the relay, which depends on how this node got there.
|
||||
//
|
||||
// The distinction matters because the two records live in different places and are removed by different
|
||||
// people. A self-enrolled node is in the relay's own state file and comes off from the app; a node an
|
||||
// operator wrote into agents.json is in a file the relay only reads, and has to be edited and the relay
|
||||
// restarted. Telling the operator the wrong one sends them to the wrong place.
|
||||
//
|
||||
// An unreadable config is not a reason to fail an uninstall, so it falls back to naming both.
|
||||
func removalHint(configPath string) string {
|
||||
cfg, err := config.LoadForPairing(configPath)
|
||||
if err != nil {
|
||||
return "the relay may still list this node; check its agents.json and, if it accepts self-enrollment, the app"
|
||||
}
|
||||
if cfg.SelfEnrolled {
|
||||
return "the relay still lists this node in its enrollment state; remove it in the app (中继节点 → 移除)"
|
||||
}
|
||||
return "the relay's agents.json still lists this node; remove that entry and restart the relay"
|
||||
}
|
||||
|
||||
@@ -55,6 +55,14 @@ type File struct {
|
||||
// certificate. It exists for a relay behind a private CA; the default is the system store, which
|
||||
// is what a public certificate needs. Setting it pins trust rather than disabling verification.
|
||||
RelayCAFile string `json:"relayCAFile,omitempty"`
|
||||
// SelfEnrolled records that this node registered itself with the relay using a one-time code,
|
||||
// rather than being written into the relay's agents.json by an operator.
|
||||
//
|
||||
// It exists so `uninstall` can tell the operator which of two removals applies, because the agent
|
||||
// cannot do either: it holds no admin token, by design — a node is not an operator, and giving it
|
||||
// one would let a compromised node remove its neighbours. So the node records which half of the
|
||||
// relay's state mentions it, and the operator is told exactly what to delete.
|
||||
SelfEnrolled bool `json:"selfEnrolled,omitempty"`
|
||||
// AuditPath is where the audit log is appended.
|
||||
AuditPath string `json:"auditPath"`
|
||||
// Devices may be empty only in pair mode (--pair). In normal operation an agent with no
|
||||
|
||||
@@ -40,6 +40,9 @@ type Options struct {
|
||||
RelayCAFile string
|
||||
// NodePassword is written into the config when non-empty. Never generated.
|
||||
NodePassword string
|
||||
// SelfEnrolled records that this node registered itself with a one-time code, so `uninstall` can
|
||||
// tell the operator which of the relay's two records mentions it.
|
||||
SelfEnrolled bool
|
||||
}
|
||||
|
||||
// Result is everything the caller has to place or show.
|
||||
@@ -129,6 +132,9 @@ func Generate(opts Options) (Result, error) {
|
||||
if opts.RelayCAFile != "" {
|
||||
config["relayCAFile"] = opts.RelayCAFile
|
||||
}
|
||||
if opts.SelfEnrolled {
|
||||
config["selfEnrolled"] = true
|
||||
}
|
||||
|
||||
encoded, err := json.MarshalIndent(config, "", " ")
|
||||
if err != nil {
|
||||
|
||||
@@ -142,3 +142,27 @@ func TestMissingInputIsRefused(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The node records how it got into the relay, because `uninstall` cannot work it out any other way: the
|
||||
// agent holds no admin token, so it cannot remove its own entry, and the operator has to be told which
|
||||
// of the relay's two records mentions this node.
|
||||
func TestEnrollmentRecordsItsOwnProvenance(t *testing.T) {
|
||||
withCode, err := Generate(Options{
|
||||
RelayURL: "https://relay.example", LocalPort: 3000, AuditPath: "/a", SelfEnrolled: true,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.Contains(string(withCode.Config), `"selfEnrolled": true`) {
|
||||
t.Fatal("a self-enrolled node does not say so")
|
||||
}
|
||||
// And a node an operator wrote into agents.json does not claim to have enrolled itself, or the
|
||||
// uninstall would send the operator to the app to remove an entry that is not there.
|
||||
manual, err := Generate(Options{RelayURL: "https://relay.example", LocalPort: 3000, AuditPath: "/a"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if strings.Contains(string(manual.Config), "selfEnrolled") {
|
||||
t.Fatal("a manually registered node claims to be self-enrolled")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -81,6 +81,10 @@ type Options struct {
|
||||
// NotifyRelay retires this node's device bearers at the relay, and is called before anything is
|
||||
// removed. Nil skips it, which is right for a node that was never paired.
|
||||
NotifyRelay func() error
|
||||
// RemovalHint is the sentence printed at the end telling the operator what is left at the relay.
|
||||
// It depends on how the node got there — self-enrolled or written into agents.json — and the caller
|
||||
// is what knows that, from the config the node wrote at enrollment time.
|
||||
RemovalHint string
|
||||
Runner Runner
|
||||
Log func(format string, args ...any)
|
||||
}
|
||||
@@ -164,7 +168,10 @@ func Uninstall(opts Options) error {
|
||||
opts.Log("dry run: nothing was removed")
|
||||
} else {
|
||||
opts.Log("msa-agent removed")
|
||||
opts.Log("the relay's agents.json still lists this node; remove that entry unless you are reinstalling")
|
||||
// The one thing this command cannot do for the operator, and it cannot by design: the agent holds
|
||||
// no admin token, because a node is not an operator, and a compromised node that could remove its
|
||||
// neighbours would be a worse problem than the one this hint solves.
|
||||
opts.Log("%s", opts.RemovalHint)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -401,3 +401,28 @@ func TestInstallDryRunReportsPreconditionsInsteadOfRefusing(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user