From 0697f54724671f7cc56fe1ad3d2c75454061408b Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 5 Sep 2026 22:24:14 +0200 Subject: [PATCH] fix(setup): say when a URL was rejected rather than blaming telnet Adding urls.validate() to setAllBoseURLsViaTelnet made a rejected URL come back through resyncBoseURLsAfterXML as "could not re-sync boseurls over telnet", which reads as the device being unreachable and sends the user to look at port 17000. applyURLOverrides lets the XML migration accept URLs the telnet validator now refuses, a query string for instance, so this is reachable: the XML write succeeds with that value while the runtime re-sync is silently skipped. A reboot does reconcile from the XML, so the outcome is fine; the diagnosis was not. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/service/setup/setup.go | 8 ++++++++ pkg/service/setup/telnet_migration_test.go | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 61035647..cfbf8d0e 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -1178,6 +1178,14 @@ func (m *Manager) resyncBoseURLsAfterXML(deviceIP string, urls telnetURLs) strin rlogs, rerr := m.setAllBoseURLsViaTelnet(deviceIP, urls) if rerr != nil { + // A rejected URL is not the device being unreachable, and saying so + // would send the user looking at telnet. The XML write has already + // happened with this value, so the URL itself is what needs attention. + if errors.Is(rerr, ErrInvalidTelnetURL) { + return fmt.Sprintf("Note: skipped the telnet boseurls re-sync because a URL was rejected (%v); "+ + "the XML configuration was still written, and a device reboot will reconcile the runtime layer.\n", rerr) + } + return fmt.Sprintf("Note: could not re-sync boseurls over telnet (%v); a device reboot will reconcile the runtime layer.\n", rerr) } diff --git a/pkg/service/setup/telnet_migration_test.go b/pkg/service/setup/telnet_migration_test.go index 9dafe7e1..91ddd6bb 100644 --- a/pkg/service/setup/telnet_migration_test.go +++ b/pkg/service/setup/telnet_migration_test.go @@ -543,3 +543,25 @@ func TestMigrateViaTelnet_FailureReportsWhatTheDeviceHolds(t *testing.T) { t.Errorf("logs did not record the read-back:\n%s", logs) } } + +// TestResyncBoseURLsAfterXML_DistinguishesRejectedURL: applyURLOverrides lets +// the XML path accept URLs the telnet validator refuses, so the XML write +// succeeds while the re-sync is skipped. Reporting that as "could not re-sync +// over telnet" points the user at the wrong thing. +func TestResyncBoseURLsAfterXML_DistinguishesRejectedURL(t *testing.T) { + m := newFakeTelnetManager(&fakeTelnet{responses: map[string]string{}}) + + rejected := m.resyncBoseURLsAfterXML("192.0.2.1", telnetURLs{ + Marge: "http://example:8000?probe=1", + Stats: "http://example:8000", + SwUpdate: "http://example:8000/updates/soundtouch", + BmxRegistry: "http://example:8000/bmx/registry/v1/services", + }) + + if !strings.Contains(rejected, "a URL was rejected") { + t.Errorf("note = %q, want it to name the rejected URL as the cause", rejected) + } + if strings.Contains(rejected, "could not re-sync boseurls over telnet") { + t.Errorf("note = %q, want it not to blame telnet availability", rejected) + } +}