diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 80ffbda..0afa4aa 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -751,7 +751,8 @@ func (m *Manager) MigrateSpeaker(deviceIP, targetURL, proxyURL string, options m // rw pre-flight, both of which would fail on devices that haven't been // rooted via remote_services. if method == MigrationMethodTelnet { - return m.migrateViaTelnet(deviceIP, targetURL) + urls := telnetURLsFromOptions(targetURL, options) + return m.migrateViaTelnet(deviceIP, targetURL, urls) } var logs string diff --git a/pkg/service/setup/telnet_migration.go b/pkg/service/setup/telnet_migration.go index 8a0fa0e..9ef4c4d 100644 --- a/pkg/service/setup/telnet_migration.go +++ b/pkg/service/setup/telnet_migration.go @@ -6,20 +6,77 @@ import ( "strings" ) -// telnetURLConfigCommands returns the canonical sequence of telnet commands -// that point a SoundTouch device at the given local-service base URL. +// telnetURLs holds the four URLs the migration writes via telnet. Most +// users keep all four pointing at the same service base; per-field +// overrides exist mainly so soundcork users can append /marge to the +// marge URL. +type telnetURLs struct { + Marge string + Stats string + SwUpdate string + BmxRegistry string +} + +// defaultTelnetURLs returns the canonical URL set derived from the +// soundtouch-service base targetURL. +func defaultTelnetURLs(targetURL string) telnetURLs { + return telnetURLs{ + Marge: targetURL, + Stats: targetURL, + SwUpdate: targetURL + "/updates/soundtouch", + BmxRegistry: targetURL + "/bmx/registry/v1/services", + } +} + +// telnetURLsFromOptions resolves the four URLs from targetURL plus +// per-field overrides supplied via the migration options map. Recognised +// keys are marge_url, stats_url, sw_update_url, bmx_url; missing or empty +// entries fall back to the canonical default. // -// Order matters: `sys configuration …` writes the runtime URL, while -// `envswitch boseurls set …` writes a parallel persistence layer that -// otherwise wins on the next reboot. See docs/analysis/TELNET-MIGRATION-METHOD.md -// §2.1 for the discussion this is derived from. -func telnetURLConfigCommands(targetURL string) []string { +// We deliberately do not expose a "proxied"/"original" semantic here +// (unlike the XML method's applyProxyOptions): per the discussion that +// motivated this iteration, the goal is to keep the user model simple — +// one base URL plus optional path suffixes — and let the service layer +// hold any non-trivial logic. +func telnetURLsFromOptions(targetURL string, options map[string]string) telnetURLs { + u := defaultTelnetURLs(targetURL) + + if v := options["marge_url"]; v != "" { + u.Marge = v + } + + if v := options["stats_url"]; v != "" { + u.Stats = v + } + + if v := options["sw_update_url"]; v != "" { + u.SwUpdate = v + } + + if v := options["bmx_url"]; v != "" { + u.BmxRegistry = v + } + + return u +} + +// Commands returns the canonical sequence of telnet commands. Order +// matters: `sys configuration …` writes the runtime layer; the closing +// `envswitch boseurls set …` writes the parallel persistence layer that +// otherwise wins on the next reboot. +// +// Envswitch derivation rule: arg1 mirrors u.Marge verbatim, arg2 mirrors +// u.SwUpdate verbatim. Soundcork users who set Marge to "/marge" +// therefore get "envswitch boseurls set /marge /updates/soundtouch" +// without any extra plumbing — the parallel layer stays consistent with +// the runtime layer. +func (u telnetURLs) Commands() []string { return []string{ - "sys configuration bmxRegistryUrl " + targetURL + "/bmx/registry/v1/services", - "sys configuration statsServerUrl " + targetURL, - "sys configuration margeServerUrl " + targetURL, - "sys configuration swUpdateUrl " + targetURL + "/updates/soundtouch", - "envswitch boseurls set " + targetURL + " " + targetURL + "/updates/soundtouch", + "sys configuration bmxRegistryUrl " + u.BmxRegistry, + "sys configuration statsServerUrl " + u.Stats, + "sys configuration margeServerUrl " + u.Marge, + "sys configuration swUpdateUrl " + u.SwUpdate, + "envswitch boseurls set " + u.Marge + " " + u.SwUpdate, } } @@ -31,7 +88,12 @@ func telnetURLConfigCommands(targetURL string) []string { // The sequence aborts on the first non-OK response so we never half-write the // configuration; the caller can retry safely after fixing the underlying // issue (closed port, hardened firmware, etc.). -func (m *Manager) migrateViaTelnet(deviceIP, targetURL string) (string, error) { +// +// targetURL is kept as a separate verification anchor: most users have +// every URL share that base, so substring-matching it against the +// device's `getpdo` reply is the simplest "did the writes stick?" check +// that still works for the soundcork "/marge on one field" case. +func (m *Manager) migrateViaTelnet(deviceIP, targetURL string, urls telnetURLs) (string, error) { if m.NewTelnet == nil { return "", errors.New("telnet migration not configured: Manager.NewTelnet is nil") } @@ -50,7 +112,7 @@ func (m *Manager) migrateViaTelnet(deviceIP, targetURL string) (string, error) { fmt.Fprintf(&logs, "Telnet banner: %q\n", strings.TrimSpace(banner)) } - for _, cmd := range telnetURLConfigCommands(targetURL) { + for _, cmd := range urls.Commands() { resp, err := t.SendCommand(cmd) if err != nil { return logs.String(), fmt.Errorf("telnet command %q failed: %w", cmd, err) diff --git a/pkg/service/setup/telnet_migration_test.go b/pkg/service/setup/telnet_migration_test.go index e854482..12920d4 100644 --- a/pkg/service/setup/telnet_migration_test.go +++ b/pkg/service/setup/telnet_migration_test.go @@ -66,7 +66,7 @@ func TestMigrateViaTelnet_HappyPath(t *testing.T) { } m := newFakeTelnetManager(f) - logs, err := m.migrateViaTelnet("192.0.2.1", target) + logs, err := m.migrateViaTelnet("192.0.2.1", target, defaultTelnetURLs(target)) if err != nil { t.Fatalf("migrateViaTelnet: %v", err) } @@ -103,7 +103,7 @@ func TestMigrateViaTelnet_DialFailureReturnsError(t *testing.T) { f := &fakeTelnet{dialErr: errors.New("connection refused")} m := newFakeTelnetManager(f) - _, err := m.migrateViaTelnet("192.0.2.1", "http://example:8000") + _, err := m.migrateViaTelnet("192.0.2.1", "http://example:8000", defaultTelnetURLs("http://example:8000")) if err == nil { t.Fatal("expected dial error, got nil") } @@ -126,7 +126,7 @@ func TestMigrateViaTelnet_CommandNotFoundAborts(t *testing.T) { f := &fakeTelnet{responses: resp} m := newFakeTelnetManager(f) - _, err := m.migrateViaTelnet("192.0.2.1", target) + _, err := m.migrateViaTelnet("192.0.2.1", target, defaultTelnetURLs(target)) if err == nil { t.Fatal("expected error when envswitch is rejected, got nil") } @@ -153,7 +153,7 @@ func TestMigrateViaTelnet_VerifyMismatchFails(t *testing.T) { f := &fakeTelnet{responses: resp} m := newFakeTelnetManager(f) - _, err := m.migrateViaTelnet("192.0.2.1", target) + _, err := m.migrateViaTelnet("192.0.2.1", target, defaultTelnetURLs(target)) if err == nil { t.Fatal("expected verification mismatch error, got nil") } @@ -173,7 +173,7 @@ func TestMigrateViaTelnet_TransportErrorAborts(t *testing.T) { } m := newFakeTelnetManager(f) - _, err := m.migrateViaTelnet("192.0.2.1", target) + _, err := m.migrateViaTelnet("192.0.2.1", target, defaultTelnetURLs(target)) if err == nil { t.Fatal("expected transport error, got nil") } @@ -186,7 +186,7 @@ func TestMigrateViaTelnet_TransportErrorAborts(t *testing.T) { func TestMigrateViaTelnet_MissingNewTelnetIsClearError(t *testing.T) { m := &Manager{ServerURL: "http://example:8000"} // NewTelnet deliberately nil - _, err := m.migrateViaTelnet("192.0.2.1", "http://example:8000") + _, err := m.migrateViaTelnet("192.0.2.1", "http://example:8000", defaultTelnetURLs("http://example:8000")) if err == nil { t.Fatal("expected error when NewTelnet is nil") } diff --git a/pkg/service/setup/telnet_urls_test.go b/pkg/service/setup/telnet_urls_test.go new file mode 100644 index 0000000..ac4227c --- /dev/null +++ b/pkg/service/setup/telnet_urls_test.go @@ -0,0 +1,147 @@ +package setup + +import ( + "reflect" + "strings" + "testing" +) + +func TestDefaultTelnetURLs_DerivesAllFourFromBase(t *testing.T) { + got := defaultTelnetURLs("http://example:8000") + + want := telnetURLs{ + Marge: "http://example:8000", + Stats: "http://example:8000", + SwUpdate: "http://example:8000/updates/soundtouch", + BmxRegistry: "http://example:8000/bmx/registry/v1/services", + } + + if !reflect.DeepEqual(got, want) { + t.Errorf("defaultTelnetURLs = %+v, want %+v", got, want) + } +} + +func TestTelnetURLsFromOptions_NilOptionsReturnsDefaults(t *testing.T) { + got := telnetURLsFromOptions("http://example:8000", nil) + want := defaultTelnetURLs("http://example:8000") + + if !reflect.DeepEqual(got, want) { + t.Errorf("telnetURLsFromOptions(nil) = %+v, want defaults %+v", got, want) + } +} + +func TestTelnetURLsFromOptions_EmptyValueFallsBackToDefault(t *testing.T) { + options := map[string]string{ + "marge_url": "", // empty override should be ignored + } + + got := telnetURLsFromOptions("http://example:8000", options) + + if got.Marge != "http://example:8000" { + t.Errorf("Marge with empty override = %q, want default", got.Marge) + } +} + +func TestTelnetURLsFromOptions_PerFieldOverrides(t *testing.T) { + options := map[string]string{ + "marge_url": "http://example:8000/marge", // soundcork-style + "stats_url": "", // ignored + "sw_update_url": "http://example:8000/custom/updates", + "bmx_url": "http://example:8000/custom/bmx", + } + + got := telnetURLsFromOptions("http://example:8000", options) + + if got.Marge != "http://example:8000/marge" { + t.Errorf("Marge = %q, want override", got.Marge) + } + + if got.Stats != "http://example:8000" { + t.Errorf("Stats = %q, want default (empty override)", got.Stats) + } + + if got.SwUpdate != "http://example:8000/custom/updates" { + t.Errorf("SwUpdate = %q, want override", got.SwUpdate) + } + + if got.BmxRegistry != "http://example:8000/custom/bmx" { + t.Errorf("BmxRegistry = %q, want override", got.BmxRegistry) + } +} + +// TestTelnetURLs_Commands_EnvswitchTracksMargeAndSwUpdate is the load-bearing +// test for the soundcork case: if the user added /marge to Marge, the +// envswitch arg1 must follow the same suffix verbatim, otherwise the +// parallel persistence layer will revert margeServerUrl on next reboot +// (the very failure mode the user described as "envswitch silently +// restores my typo"). +func TestTelnetURLs_Commands_EnvswitchTracksMargeAndSwUpdate(t *testing.T) { + urls := telnetURLs{ + Marge: "http://example:8000/marge", + Stats: "http://example:8000", + SwUpdate: "http://example:8000/updates/soundtouch", + BmxRegistry: "http://example:8000/bmx/registry/v1/services", + } + + cmds := urls.Commands() + + var envswitch string + + for _, c := range cmds { + if strings.HasPrefix(c, "envswitch boseurls set ") { + envswitch = c + break + } + } + + if envswitch == "" { + t.Fatalf("Commands missing envswitch boseurls set:\n%v", cmds) + } + + wantEnv := "envswitch boseurls set http://example:8000/marge http://example:8000/updates/soundtouch" + if envswitch != wantEnv { + t.Errorf("envswitch =\n %q\nwant\n %q", envswitch, wantEnv) + } +} + +func TestMigrateViaTelnet_SoundcorkMargeSuffixPropagatesToEnvswitch(t *testing.T) { + target := "http://example:8000" + urls := telnetURLs{ + Marge: "http://example:8000/marge", + Stats: "http://example:8000", + SwUpdate: "http://example:8000/updates/soundtouch", + BmxRegistry: "http://example:8000/bmx/registry/v1/services", + } + + // Build a happy-path responder that matches the *new* command set. + resp := map[string]string{ + "sys configuration bmxRegistryUrl " + urls.BmxRegistry: "OK\n", + "sys configuration statsServerUrl " + urls.Stats: "OK\n", + "sys configuration margeServerUrl " + urls.Marge: "OK\n", + "sys configuration swUpdateUrl " + urls.SwUpdate: "OK\n", + "envswitch boseurls set " + urls.Marge + " " + urls.SwUpdate: "OK\n", + "getpdo CurrentSystemConfiguration": "margeServerUrl=" + urls.Marge + "\n", + } + + f := &fakeTelnet{responses: resp} + m := newFakeTelnetManager(f) + + if _, err := m.migrateViaTelnet("192.0.2.1", target, urls); err != nil { + t.Fatalf("migrateViaTelnet: %v", err) + } + + wantEnvCmd := "envswitch boseurls set http://example:8000/marge http://example:8000/updates/soundtouch" + + var saw bool + + for _, c := range f.commands { + if c == wantEnvCmd { + saw = true + break + } + } + + if !saw { + t.Errorf("never sent expected envswitch command %q\nactual commands:\n%v", wantEnvCmd, f.commands) + } +}