From fa158bc49872b917ca2997e0e02b50be3847b70f Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Mon, 8 Jun 2026 21:13:09 +0200 Subject: [PATCH] feat(cli/setup): friendlier enable-ssh timeout + re-sync boseurls after XML migration (refs #471) Two follow-ups from the #471 field reports on the BETA `setup enable-ssh`: 1. enable-ssh: when sshd (:22) does not come up within the wait window, this is no longer treated as a hard error. On some devices (e.g. the Wireless Link Adapter) the envswitch injection is accepted but sshd only starts after the speaker restarts. The command now prints a warning with power-cycle + retry guidance (and the exact ssh command), deliberately leaves the injected boseurls in place so a restart re-triggers the unlock, and exits cleanly instead of failing. 2. XML migration: re-apply the boseurls over telnet at the end of migrateViaXML so the runtime layer reported by `getpdo CurrentSystemConfiguration` matches the persisted SoundTouchSdkPrivateCfg.xml. After enable-ssh bootstraps SSH, that runtime layer still points at the placeholder (https://aftertouch.invalid), so the preflight cross-check keeps warning that margeServerUrl/swUpdateUrl differ between transports until a reboot. The re-apply reconciles it now. Best-effort: if telnet is unavailable (e.g. port 17000 was closed via --close-17000), a reboot still reconciles the layers, so it only logs a note and never fails the migration. Tests cover the re-apply command and its best-effort (non-fatal) behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/soundtouch-cli/cmd_setup.go | 21 ++++++++++- pkg/service/setup/setup.go | 30 +++++++++++++++ pkg/service/setup/setup_test.go | 66 +++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/cmd/soundtouch-cli/cmd_setup.go b/cmd/soundtouch-cli/cmd_setup.go index f6a6993..edb7f19 100644 --- a/cmd/soundtouch-cli/cmd_setup.go +++ b/cmd/soundtouch-cli/cmd_setup.go @@ -604,8 +604,25 @@ func setupEnableSSHCmd() *cli.Command { fmt.Printf("Waiting up to %s for sshd (:22) to come up...\n", c.Duration("wait")) if err := setup.WaitForSSHPort(cfg.Host, c.Duration("wait")); err != nil { - PrintError(err.Error()) - return err + // Not a hard failure: on some devices (e.g. the Wireless Link + // Adapter, see #471) the envswitch injection is accepted but + // sshd only actually starts after the speaker restarts. We + // deliberately leave the injected boseurls in place (no reset) + // so a power-cycle re-triggers the unlock, and guide the user + // to reboot and retry rather than exiting with an error. + fmt.Println() + PrintWarning(fmt.Sprintf("sshd (:22) did not come up within %s, but the speaker accepted the SSH-enable command.", c.Duration("wait"))) + fmt.Println("On some devices sshd only starts after a restart. Next steps:") + fmt.Println(" 1. Power-cycle the speaker (unplug it, wait a few seconds, plug it back in).") + fmt.Println(" 2. Once it is back online, run this same command again, or just connect with:") + fmt.Printf(" ssh -o HostKeyAlgorithms=+ssh-rsa,ssh-dss root@%s\n", cfg.Host) + fmt.Println("The temporary boseurls were left in place on purpose, so the restart re-triggers the unlock.") + + if placeholder { + fmt.Println("(No --service-url was given; you'll set the real service URLs later during migration.)") + } + + return nil } PrintSuccess("SSH is up on " + cfg.Host) diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index ab5eb0f..140267d 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -1077,9 +1077,39 @@ func (m *Manager) migrateViaXML(deviceIP, targetURL, proxyURL string, options ma } } + logs += m.resyncBoseURLsAfterXML(deviceIP, cfg.MargeServerUrl, cfg.SwUpdateUrl) + return logs, nil } +// resyncBoseURLsAfterXML re-applies the boseurls over telnet so the runtime +// URL layer matches the XML just written by migrateViaXML. +// +// The XML migration only updates the persisted SoundTouchSdkPrivateCfg.xml; it +// does not touch the runtime/persistence layer that `getpdo +// CurrentSystemConfiguration` reports. When SSH was bootstrapped via #471 +// (`enable-ssh`), that layer still points at the placeholder boseurls +// (https://aftertouch.invalid), so the preflight cross-check keeps warning that +// margeServerUrl/swUpdateUrl differ between transports until a reboot. +// Re-applying the real boseurls over telnet :17000 reconciles it immediately. +// +// Best-effort: telnet may be unavailable (no port 17000, or it was closed via +// --close-17000), in which case a reboot still reconciles the layers, so this +// only returns a note and never fails the migration. Returns the log lines to +// append. +func (m *Manager) resyncBoseURLsAfterXML(deviceIP, marge, swUpdate string) string { + if m.NewTelnet == nil { + return "" + } + + rlogs, rerr := m.setBoseURLsViaTelnet(deviceIP, marge, swUpdate) + if rerr != nil { + return fmt.Sprintf("Note: could not re-sync boseurls over telnet (%v); a device reboot will reconcile the runtime layer.\n", rerr) + } + + return "Re-applied boseurls over telnet so the runtime layer matches the new configuration:\n" + rlogs +} + // BackupConfigOffDevice creates a local backup of the speaker's configuration files in the DataStore. func (m *Manager) BackupConfigOffDevice(deviceIP string) error { if m.DataStore == nil { diff --git a/pkg/service/setup/setup_test.go b/pkg/service/setup/setup_test.go index c519075..054fdab 100644 --- a/pkg/service/setup/setup_test.go +++ b/pkg/service/setup/setup_test.go @@ -2018,3 +2018,69 @@ func TestMigrateSpeaker_ResolvBlocking(t *testing.T) { t.Errorf("Did not expect pre-flight DNS errors, got %v", err) } } + +// TestMigrateViaXML_ReappliesBoseURLsOverTelnet verifies the #471 follow-up: +// after an XML migration, the runtime boseurls layer is re-applied over telnet +// so it matches the persisted config (otherwise the preflight cross-check keeps +// warning about the enable-ssh placeholder until a reboot). +func TestMigrateViaXML_ReappliesBoseURLsOverTelnet(t *testing.T) { + cm := certmanager.NewCertificateManager(filepath.Join(t.TempDir(), "certs")) + if err := cm.EnsureCA(); err != nil { + t.Fatalf("EnsureCA: %v", err) + } + + target := "https://192.0.2.10:8443" + m := NewManager(target, nil, cm) + m.NewSSH = func(string) SSHClient { + return &mockSSH{runFunc: func(string) (string, error) { return "", nil }} + } + + ft := &fakeTelnet{banner: "->", responses: map[string]string{}} + m.NewTelnet = func(string) TelnetClient { return ft } + + if _, err := m.MigrateSpeaker("192.0.2.10", target, "", nil, MigrationMethodXML); err != nil { + t.Fatalf("MigrateSpeaker: %v", err) + } + + want := `envswitch boseurls set "` + target + `" "` + target + `/updates/soundtouch"` + + var found bool + for _, c := range ft.commands { + if c == want { + found = true + break + } + } + + if !found { + t.Errorf("expected boseurls re-apply %q after XML migration; sent: %v", want, ft.commands) + } +} + +// TestMigrateViaXML_BoseURLsResyncIsBestEffort verifies that a telnet failure +// during the post-migration boseurls re-apply does not fail the migration (a +// device reboot still reconciles the runtime layer). +func TestMigrateViaXML_BoseURLsResyncIsBestEffort(t *testing.T) { + cm := certmanager.NewCertificateManager(filepath.Join(t.TempDir(), "certs")) + if err := cm.EnsureCA(); err != nil { + t.Fatalf("EnsureCA: %v", err) + } + + target := "https://192.0.2.10:8443" + m := NewManager(target, nil, cm) + m.NewSSH = func(string) SSHClient { + return &mockSSH{runFunc: func(string) (string, error) { return "", nil }} + } + m.NewTelnet = func(string) TelnetClient { + return &fakeTelnet{dialErr: errors.New("connection refused")} + } + + logs, err := m.MigrateSpeaker("192.0.2.10", target, "", nil, MigrationMethodXML) + if err != nil { + t.Fatalf("MigrateSpeaker should not fail when the telnet re-sync fails: %v", err) + } + + if !strings.Contains(logs, "could not re-sync boseurls over telnet") { + t.Errorf("expected a best-effort note about the failed telnet re-sync; logs:\n%s", logs) + } +}