From db76468c3ce82162c4f7404c16de2c2b1c140c9f Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Thu, 30 Jul 2026 16:19:19 +0200 Subject: [PATCH 1/6] feat(net): resolve device .local names over mDNS Go never resolves ".local" itself. net/conf.go routes those names to libc only when cgo is available, and every FTW build sets CGO_ENABLED=0, so the pure Go resolver is always selected: a configured "zap.local" became a unicast DNS query to the site router and failed. That holds on every base image and every libc, musl and glibc alike, so the container's distro was never the variable here. Add internal/mdnsresolve, which answers those names over multicast DNS, and route every driver transport through it: Modbus TCP, MQTT (driver and Home Assistant bridge), HTTP including the TLS-pinned client, WebSocket and raw TCP. Only ".local" names take the new path; literal IPs and ordinary DNS names dial exactly as before. Resolution runs per dial rather than once at startup, so a device that moves to a new DHCP lease is found again on the next reconnect with no config edit. Answers are cached for the record TTL clamped to 30-120s so reconnect loops cannot flood the LAN, and failures are cached for 5s so a still-booting device is retried soon. Failures log "mDNS resolution failed" and name the mechanism rather than surfacing as a generic dial error. The scanner's reverse PTR lookup moves into the same package so there is one mDNS implementation instead of two. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> --- .changeset/resolve-local-device-names.md | 24 ++ config.example.yaml | 2 +- docs/sourceful-zap.md | 6 + go/internal/drivers/lua.go | 17 +- go/internal/drivers/tcp_cap.go | 4 +- go/internal/drivers/ws_cap.go | 4 + go/internal/ha/bridge.go | 10 + go/internal/mdnsresolve/mdnsresolve.go | 328 ++++++++++++++++++ go/internal/mdnsresolve/mdnsresolve_test.go | 265 ++++++++++++++ .../mdns.go => mdnsresolve/reverse.go} | 45 +-- .../reverse_test.go} | 2 +- go/internal/modbus/tcp_client.go | 9 +- go/internal/mqtt/client.go | 11 + go/internal/scanner/scanner.go | 4 +- 14 files changed, 692 insertions(+), 39 deletions(-) create mode 100644 .changeset/resolve-local-device-names.md create mode 100644 go/internal/mdnsresolve/mdnsresolve.go create mode 100644 go/internal/mdnsresolve/mdnsresolve_test.go rename go/internal/{scanner/mdns.go => mdnsresolve/reverse.go} (51%) rename go/internal/{scanner/mdns_test.go => mdnsresolve/reverse_test.go} (98%) diff --git a/.changeset/resolve-local-device-names.md b/.changeset/resolve-local-device-names.md new file mode 100644 index 00000000..f314bdc4 --- /dev/null +++ b/.changeset/resolve-local-device-names.md @@ -0,0 +1,24 @@ +--- +"ftw": minor +--- + +Resolve device `.local` names over mDNS so devices can be configured by name instead of a DHCP-assigned IP. + +Go never resolves `.local` itself: it hands those names to libc only when cgo is +available, and FTW builds with `CGO_ENABLED=0`, so a configured `zap.local` +became a unicast DNS query to the site router and failed. That was true on every +base image and every libc. FTW now answers those names itself over multicast +DNS, and every driver transport uses it — Modbus TCP, MQTT (driver and Home +Assistant bridge), HTTP (including TLS-pinned clients), WebSocket and raw TCP. + +Resolution happens per dial rather than once at startup, so a device that moves +to a new DHCP lease is found again on the next reconnect without a config edit. +Answers are cached for the record's TTL (clamped to 30–120 s) so reconnect loops +do not flood the LAN, and failures are cached briefly so a device that is still +booting is retried soon. A failed resolution now logs `mDNS resolution failed` +and names the mechanism, instead of surfacing as a generic dial error. + +Only `.local` names take this path; literal IPs and ordinary DNS names dial +exactly as before. mDNS needs multicast reachability, which the Linux Compose +topology has via `network_mode: host`. Under `docker-compose.macos.yml` the +container is bridged, so configure devices by IP there. diff --git a/config.example.yaml b/config.example.yaml index 71ea3b53..565d3125 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -90,7 +90,7 @@ drivers: # battery_telemetry_only: true # capabilities: # http: - # allowed_hosts: ["zap.local"] # use the LAN IP if mDNS is unavailable + # allowed_hosts: ["zap.local"] # .local is resolved by FTW over mDNS # config: # host: zap.local # # meter_serial: p1m-... # optional; P1 is auto-selected diff --git a/docs/sourceful-zap.md b/docs/sourceful-zap.md index 75f98d60..9b6a1306 100644 --- a/docs/sourceful-zap.md +++ b/docs/sourceful-zap.md @@ -76,6 +76,12 @@ go test ./internal/drivers -run 'Zap|zap' - not found: confirm Zap is on Wi-Fi and reachable at `http://zap.local/api/system` from the FTW host; +- `.local` name does not resolve: FTW resolves `.local` itself over multicast + DNS rather than through the OS resolver, so it needs to be on the same L2 + segment as the device. That is the case with the Linux Compose topology + (`network_mode: host`); under `docker-compose.macos.yml` the container is + bridged and multicast does not reach the LAN, so configure the device by IP + there. The log line naming the failure is `mDNS resolution failed`; - no meter: inspect Zap's `/api/devices` and pin `meter_serial` when needed; - duplicate PV/battery: disable the overlapping Zap DER; - visible battery is not controlled: expected for the telemetry-only driver. diff --git a/go/internal/drivers/lua.go b/go/internal/drivers/lua.go index 09ada803..4201c732 100644 --- a/go/internal/drivers/lua.go +++ b/go/internal/drivers/lua.go @@ -59,6 +59,8 @@ import ( "time" lua "github.com/yuin/gopher-lua" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // LuaDriver wraps a running Lua VM bound to a HostEnv. @@ -654,8 +656,16 @@ func registerHost(L *lua.LState, env *HostEnv) { return false, fmt.Sprintf("host %q (port %s) not in allowed_hosts", host, port) } + // Drivers routinely address a device by its ".local" name, which the + // stdlib resolver cannot answer. Clone the default transport so proxying, + // HTTP/2 and connection pooling are all unchanged — only the dial step + // differs, and only for ".local" hosts. + transport := net_http.DefaultTransport.(*net_http.Transport).Clone() + transport.DialContext = mdnsresolve.DialContext + httpClient := &net_http.Client{ - Timeout: 15 * time.Second, + Timeout: 15 * time.Second, + Transport: transport, CheckRedirect: func(req *net_http.Request, via []*net_http.Request) error { if len(via) >= 10 { return fmt.Errorf("stopped after 10 redirects") @@ -677,7 +687,10 @@ func registerHost(L *lua.LState, env *HostEnv) { // CA. Drivers WITHOUT a pin keep Go's default transport untouched, so // nothing about existing HTTP drivers changes. if pin := tlsPin; pin != "" { - tr := net_http.DefaultTransport.(*net_http.Transport).Clone() + // Clone the transport built above so the pinned client keeps the same + // mDNS-aware dialer — a pinned device is usually a local appliance + // addressed by its ".local" name, which is exactly the case that needs it. + tr := transport.Clone() tr.TLSClientConfig = &tls.Config{ // We replace chain/hostname verification with our own exact // fingerprint check below, so the stdlib check must be off. diff --git a/go/internal/drivers/tcp_cap.go b/go/internal/drivers/tcp_cap.go index 4153f3cc..cba42622 100644 --- a/go/internal/drivers/tcp_cap.go +++ b/go/internal/drivers/tcp_cap.go @@ -6,6 +6,8 @@ import ( "strings" "sync" "time" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // TCPCap is the host's raw TCP socket capability. One driver = one upstream @@ -93,7 +95,7 @@ func (n *netTCP) Open(addr string) error { return fmt.Errorf("tcp: %s", reason) } - conn, err := net.DialTimeout("tcp", addr, 10*time.Second) + conn, err := mdnsresolve.DialTimeout("tcp", addr, 10*time.Second) if err != nil { return fmt.Errorf("tcp dial: %w", err) } diff --git a/go/internal/drivers/ws_cap.go b/go/internal/drivers/ws_cap.go index 979d5685..bd23736d 100644 --- a/go/internal/drivers/ws_cap.go +++ b/go/internal/drivers/ws_cap.go @@ -9,6 +9,8 @@ import ( "time" "github.com/gorilla/websocket" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // gorillaWS is the production WSCap implementation. One per driver. @@ -73,6 +75,8 @@ func (g *gorillaWS) Open(url string, headers map[string]string) error { } dialer := *websocket.DefaultDialer dialer.HandshakeTimeout = 15 * time.Second + // ".local" hosts need mDNS; everything else falls through to a plain dial. + dialer.NetDialContext = mdnsresolve.DialContext if len(subprotocols) > 0 { dialer.Subprotocols = subprotocols } diff --git a/go/internal/ha/bridge.go b/go/internal/ha/bridge.go index 24469e01..f47645b2 100644 --- a/go/internal/ha/bridge.go +++ b/go/internal/ha/bridge.go @@ -12,6 +12,8 @@ import ( "encoding/json" "fmt" "log/slog" + "net" + "net/url" "sort" "strconv" "strings" @@ -22,6 +24,7 @@ import ( "github.com/srcfl/ftw/go/internal/config" "github.com/srcfl/ftw/go/internal/control" + "github.com/srcfl/ftw/go/internal/mdnsresolve" "github.com/srcfl/ftw/go/internal/telemetry" ) @@ -249,6 +252,13 @@ func (b *Bridge) connectAndStart(cfg *config.HomeAssistant, driverNames []string opts := paho.NewClientOptions(). AddBroker(fmt.Sprintf("tcp://%s:%d", cfg.Broker, cfg.Port)). + // A Home Assistant broker is very often reached as homeassistant.local, + // which the stdlib resolver cannot answer. See internal/mqtt for why a + // TCP-only replacement is complete here. + SetCustomOpenConnectionFn(func(uri *url.URL, o paho.ClientOptions) (net.Conn, error) { + d := mdnsresolve.Dialer{Dialer: net.Dialer{Timeout: o.ConnectTimeout}} + return d.Dial("tcp", uri.Host) + }). SetClientID("forty-two-watts-ha"). SetAutoReconnect(true). SetConnectRetry(true). diff --git a/go/internal/mdnsresolve/mdnsresolve.go b/go/internal/mdnsresolve/mdnsresolve.go new file mode 100644 index 00000000..81eb5451 --- /dev/null +++ b/go/internal/mdnsresolve/mdnsresolve.go @@ -0,0 +1,328 @@ +// Package mdnsresolve resolves RFC 6762 ".local" host names over multicast +// DNS and provides a dialer that uses it. +// +// Go never does this itself. net/conf.go routes a ".local" lookup to libc only +// when cgo is available, and every FTW build sets CGO_ENABLED=0, so the pure Go +// resolver is always selected: it reads /etc/resolv.conf and sends a *unicast* +// query to the site router, which has no idea what "inverter.local" is. That +// holds on every base image and every libc — musl and glibc alike — so +// resolving here is the only portable fix. +// +// Only ".local" names take this path. Literal IPs and ordinary DNS names are +// handed straight to the standard dialer. +package mdnsresolve + +import ( + "context" + "fmt" + "log/slog" + "net" + "net/netip" + "strings" + "sync" + "time" + + "golang.org/x/net/dns/dnsmessage" +) + +// mdnsAddr is the RFC 6762 IPv4 multicast group. A var, not a const, so tests +// can aim a query at a loopback responder. +var mdnsAddr = &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353} + +// listenPacket opens the ephemeral socket a query is sent from. Replaced in +// tests. It deliberately does NOT bind port 5353: avahi-daemon already owns +// that on the host, and the QU bit below asks responders to reply directly to +// this socket instead. +var listenPacket = func() (*net.UDPConn, error) { + return net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4zero}) +} + +// now is swappable so cache-expiry tests do not have to sleep. +var now = time.Now + +const ( + // queryTimeout matches the budget the scanner already uses for its reverse + // lookups — long enough for a sleepy device, short enough that a driver + // dial does not stall a control tick. + queryTimeout = 900 * time.Millisecond + + // classQU is IN with the RFC 6762 unicast-response bit set. + classQU = dnsmessage.Class(0x8001) + + // A responder's TTL is advisory here. The floor stops a device that + // advertises a very short TTL from turning every Modbus reconnect into a + // multicast storm; the ceiling keeps a DHCP move from taking effect + // arbitrarily late, which is the whole point of binding by name. + minTTL = 30 * time.Second + maxTTL = 120 * time.Second + + // negativeTTL is deliberately short: a device that was off when we first + // looked should become reachable soon after it boots. + negativeTTL = 5 * time.Second +) + +type cacheEntry struct { + addrs []netip.Addr // empty means a cached negative answer + expires time.Time +} + +var ( + cacheMu sync.Mutex + cache = map[string]cacheEntry{} +) + +// IsLocal reports whether host is a ".local" name that mDNS should resolve. +// A literal IP is never one, so a configured "192.168.1.5" keeps the plain +// dial path and never touches the network for resolution. +func IsLocal(host string) bool { + if host == "" || net.ParseIP(host) != nil { + return false + } + return strings.HasSuffix(strings.ToLower(strings.TrimSuffix(host, ".")), ".local") +} + +func canonical(name string) string { + return strings.ToLower(strings.TrimSuffix(name, ".")) +} + +func cacheLookup(key string) ([]netip.Addr, bool) { + cacheMu.Lock() + defer cacheMu.Unlock() + entry, ok := cache[key] + if !ok || now().After(entry.expires) { + return nil, false + } + return entry.addrs, true +} + +func cacheStore(key string, addrs []netip.Addr, ttl time.Duration) { + cacheMu.Lock() + defer cacheMu.Unlock() + cache[key] = cacheEntry{addrs: addrs, expires: now().Add(ttl)} +} + +// Flush drops every cached answer. Tests use it; nothing in production does. +func Flush() { + cacheMu.Lock() + defer cacheMu.Unlock() + cache = map[string]cacheEntry{} +} + +// Lookup resolves a ".local" name to its advertised addresses. +func Lookup(ctx context.Context, name string) ([]netip.Addr, error) { + key := canonical(name) + if addrs, ok := cacheLookup(key); ok { + if len(addrs) == 0 { + return nil, fmt.Errorf("no mDNS responder for %s (cached)", name) + } + return addrs, nil + } + + addrs, ttl, err := queryAddrs(ctx, key) + if err != nil || len(addrs) == 0 { + cacheStore(key, nil, negativeTTL) + if err == nil { + err = fmt.Errorf("no mDNS responder for %s", name) + } + return nil, err + } + + cacheStore(key, addrs, ttl) + // Logged on a cache miss only, so this is at most one line per TTL per + // device rather than one per reconnect. + slog.Info("resolved host over mDNS", "host", key, "addr", addrs[0].String(), "ttl", ttl) + return addrs, nil +} + +func queryAddrs(ctx context.Context, name string) ([]netip.Addr, time.Duration, error) { + qname, err := dnsmessage.NewName(name + ".") + if err != nil { + return nil, 0, fmt.Errorf("mdns: bad name %q: %w", name, err) + } + // One packet, two questions. RFC 6762 §5.2 allows it and it saves a round + // trip on dual-stack devices. + msg := dnsmessage.Message{Questions: []dnsmessage.Question{ + {Name: qname, Type: dnsmessage.TypeA, Class: classQU}, + {Name: qname, Type: dnsmessage.TypeAAAA, Class: classQU}, + }} + packed, err := msg.Pack() + if err != nil { + return nil, 0, fmt.Errorf("mdns: pack query: %w", err) + } + + var ( + addrs []netip.Addr + ttl time.Duration + ) + err = exchange(ctx, packed, func(packet []byte) bool { + got, gotTTL, ok := parseAddrAnswer(packet, name+".") + if !ok { + return false + } + addrs, ttl = got, gotTTL + return true + }) + if err != nil { + return nil, 0, err + } + return addrs, ttl, nil +} + +// exchange sends one multicast query and feeds every reply to handle until it +// accepts one or the deadline passes. +func exchange(ctx context.Context, packed []byte, handle func([]byte) bool) error { + conn, err := listenPacket() + if err != nil { + return fmt.Errorf("mdns: open socket: %w", err) + } + defer conn.Close() + + deadline := now().Add(queryTimeout) + if d, ok := ctx.Deadline(); ok && d.Before(deadline) { + deadline = d + } + if err := conn.SetDeadline(deadline); err != nil { + return fmt.Errorf("mdns: set deadline: %w", err) + } + if _, err := conn.WriteToUDP(packed, mdnsAddr); err != nil { + return fmt.Errorf("mdns: send query: %w", err) + } + + buf := make([]byte, 1500) + for { + n, _, err := conn.ReadFromUDP(buf) + if err != nil { + return fmt.Errorf("mdns: no usable answer: %w", err) + } + if handle(buf[:n]) { + return nil + } + } +} + +func parseAddrAnswer(packet []byte, qname string) ([]netip.Addr, time.Duration, bool) { + var p dnsmessage.Parser + if _, err := p.Start(packet); err != nil { + return nil, 0, false + } + if err := p.SkipAllQuestions(); err != nil { + return nil, 0, false + } + var addrs []netip.Addr + ttl := maxTTL + // Labelled so a parse error inside the type switch abandons the whole + // packet: once the parser desynchronises, every later record is suspect. +parse: + for { + h, err := p.AnswerHeader() + if err != nil { + break parse + } + if !strings.EqualFold(h.Name.String(), qname) { + if err := p.SkipAnswer(); err != nil { + break parse + } + continue + } + switch h.Type { + case dnsmessage.TypeA: + r, err := p.AResource() + if err != nil { + break parse + } + addrs = append(addrs, netip.AddrFrom4(r.A)) + case dnsmessage.TypeAAAA: + r, err := p.AAAAResource() + if err != nil { + break parse + } + // Unmap so a v4-mapped AAAA dials as plain IPv4. + addrs = append(addrs, netip.AddrFrom16(r.AAAA).Unmap()) + default: + if err := p.SkipAnswer(); err != nil { + break parse + } + continue + } + if d := time.Duration(h.TTL) * time.Second; d < ttl { + ttl = d + } + } + return finishAnswer(addrs, ttl) +} + +func finishAnswer(addrs []netip.Addr, ttl time.Duration) ([]netip.Addr, time.Duration, bool) { + if len(addrs) == 0 { + return nil, 0, false + } + switch { + case ttl < minTTL: + ttl = minTTL + case ttl > maxTTL: + ttl = maxTTL + } + return addrs, ttl, true +} + +// Dialer dials TCP addresses, resolving ".local" host names over mDNS first. +// The embedded net.Dialer carries timeout and keep-alive; anything that is not +// a ".local" name is handed straight to it. +// +// Resolution happens per dial, not once at startup. That is what makes binding +// a device by name survive a DHCP lease change: callers that rebuild their +// connection from the original address string pick up the new IP on reconnect. +type Dialer struct { + net.Dialer +} + +// DialContext resolves address if it names a ".local" host, then dials it. +func (d *Dialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { + host, port, err := net.SplitHostPort(address) + if err != nil || !IsLocal(host) { + return d.Dialer.DialContext(ctx, network, address) + } + + addrs, err := Lookup(ctx, host) + if err != nil { + // Name the mechanism. Without this the operator sees a bare dial + // failure and has no way to tell that resolution was the reason. + slog.Warn("mDNS resolution failed; check the device is on this LAN and the container uses host networking", + "host", host, "err", err) + return nil, fmt.Errorf("resolve %s over mDNS: %w", host, err) + } + + var firstErr error + for _, a := range addrs { + conn, err := d.Dialer.DialContext(ctx, network, net.JoinHostPort(a.String(), port)) + if err == nil { + return conn, nil + } + if firstErr == nil { + firstErr = err + } + } + return nil, fmt.Errorf("dial %s over mDNS: %w", host, firstErr) +} + +// Dial is the context-free form, for callers that have no context to pass. +func (d *Dialer) Dial(network, address string) (net.Conn, error) { + ctx := context.Background() + if d.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, d.Timeout) + defer cancel() + } + return d.DialContext(ctx, network, address) +} + +// DialContext dials with default settings. +func DialContext(ctx context.Context, network, address string) (net.Conn, error) { + var d Dialer + return d.DialContext(ctx, network, address) +} + +// DialTimeout mirrors net.DialTimeout with mDNS resolution added. +func DialTimeout(network, address string, timeout time.Duration) (net.Conn, error) { + d := Dialer{Dialer: net.Dialer{Timeout: timeout}} + return d.Dial(network, address) +} diff --git a/go/internal/mdnsresolve/mdnsresolve_test.go b/go/internal/mdnsresolve/mdnsresolve_test.go new file mode 100644 index 00000000..fb9469a5 --- /dev/null +++ b/go/internal/mdnsresolve/mdnsresolve_test.go @@ -0,0 +1,265 @@ +package mdnsresolve + +import ( + "context" + "errors" + "net" + "net/netip" + "strings" + "testing" + "time" + + "golang.org/x/net/dns/dnsmessage" +) + +func TestIsLocal(t *testing.T) { + cases := []struct { + host string + want bool + }{ + {"inverter.local", true}, + {"INVERTER.LOCAL", true}, + {"inverter.local.", true}, + {"zap.local", true}, + // A literal address must never trigger a multicast query. + {"192.168.1.5", false}, + {"::1", false}, + {"example.com", false}, + {"localhost", false}, + {"local", false}, + {"notlocal", false}, + {"", false}, + } + for _, c := range cases { + if got := IsLocal(c.host); got != c.want { + t.Errorf("IsLocal(%q) = %v, want %v", c.host, got, c.want) + } + } +} + +func aResource(t *testing.T, name string, ip [4]byte, ttl uint32) dnsmessage.Resource { + t.Helper() + return dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: mustDNSName(t, name), Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, TTL: ttl, + }, + Body: &dnsmessage.AResource{A: ip}, + } +} + +func packAnswer(t *testing.T, qname string, answers []dnsmessage.Resource) []byte { + t.Helper() + msg := dnsmessage.Message{ + Header: dnsmessage.Header{Response: true, Authoritative: true}, + Questions: []dnsmessage.Question{{Name: mustDNSName(t, qname), Type: dnsmessage.TypeA, Class: dnsmessage.ClassINET}}, + Answers: answers, + } + packet, err := msg.Pack() + if err != nil { + t.Fatalf("pack: %v", err) + } + return packet +} + +func TestParseAddrAnswer(t *testing.T) { + qname := "inverter.local." + packet := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{192, 168, 1, 42}, 60)}) + + addrs, ttl, ok := parseAddrAnswer(packet, qname) + if !ok { + t.Fatal("parseAddrAnswer did not accept a valid answer") + } + if len(addrs) != 1 || addrs[0].String() != "192.168.1.42" { + t.Fatalf("addrs = %v, want [192.168.1.42]", addrs) + } + if ttl != 60*time.Second { + t.Fatalf("ttl = %v, want 60s", ttl) + } + + // An answer for a different name must be ignored. + if _, _, ok := parseAddrAnswer(packet, "other.local."); ok { + t.Fatal("accepted an answer for a different name") + } + // Garbage must not panic or resolve. + if _, _, ok := parseAddrAnswer([]byte{1, 2, 3}, qname); ok { + t.Fatal("accepted a malformed packet") + } +} + +func TestParseAddrAnswerClampsTTL(t *testing.T) { + qname := "inverter.local." + for _, c := range []struct { + name string + ttl uint32 + want time.Duration + }{ + // A device advertising a 1 s TTL must not make every Modbus reconnect + // re-query the LAN. + {"below floor", 1, minTTL}, + // A very long TTL must not outlive a DHCP move. + {"above ceiling", 86400, maxTTL}, + {"inside range", 90, 90 * time.Second}, + } { + t.Run(c.name, func(t *testing.T) { + packet := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{10, 0, 0, 1}, c.ttl)}) + _, ttl, ok := parseAddrAnswer(packet, qname) + if !ok { + t.Fatal("answer rejected") + } + if ttl != c.want { + t.Fatalf("ttl = %v, want %v", ttl, c.want) + } + }) + } +} + +// startResponder points the package at a loopback UDP socket that answers one +// query, so the real send/parse path is exercised without touching the LAN. +func startResponder(t *testing.T, answers []dnsmessage.Resource) { + t.Helper() + rc, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0}) + if err != nil { + t.Fatalf("listen responder: %v", err) + } + + origAddr, origListen := mdnsAddr, listenPacket + mdnsAddr = rc.LocalAddr().(*net.UDPAddr) + listenPacket = func() (*net.UDPConn, error) { + return net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)}) + } + + done := make(chan struct{}) + go func() { + defer close(done) + buf := make([]byte, 1500) + _ = rc.SetReadDeadline(time.Now().Add(2 * time.Second)) + n, from, err := rc.ReadFromUDP(buf) + if err != nil { + return + } + if answers == nil { + return // silent responder: exercises the negative path + } + var p dnsmessage.Parser + hdr, err := p.Start(buf[:n]) + if err != nil { + return + } + q, err := p.Question() + if err != nil { + return + } + resp := dnsmessage.Message{ + Header: dnsmessage.Header{ID: hdr.ID, Response: true, Authoritative: true}, + Questions: []dnsmessage.Question{q}, + Answers: answers, + } + packed, err := resp.Pack() + if err != nil { + return + } + _, _ = rc.WriteToUDP(packed, from) + }() + + t.Cleanup(func() { + _ = rc.Close() + <-done + mdnsAddr, listenPacket = origAddr, origListen + Flush() + }) +} + +func TestLookupResolvesLocalName(t *testing.T) { + Flush() + startResponder(t, []dnsmessage.Resource{aResource(t, "inverter.local.", [4]byte{192, 168, 1, 42}, 60)}) + + addrs, err := Lookup(context.Background(), "inverter.local") + if err != nil { + t.Fatalf("Lookup: %v", err) + } + if len(addrs) != 1 || addrs[0] != netip.MustParseAddr("192.168.1.42") { + t.Fatalf("addrs = %v, want [192.168.1.42]", addrs) + } + + // The answer must now be cached: a second call cannot need the responder, + // which has already stopped. + again, err := Lookup(context.Background(), "INVERTER.local") + if err != nil { + t.Fatalf("cached Lookup: %v", err) + } + if len(again) != 1 || again[0] != addrs[0] { + t.Fatalf("cached addrs = %v, want %v", again, addrs) + } +} + +func TestLookupCachesNegativeAnswer(t *testing.T) { + Flush() + startResponder(t, nil) // never answers + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + if _, err := Lookup(ctx, "missing.local"); err == nil { + t.Fatal("expected a lookup failure when nothing answers") + } + + addrs, ok := cacheLookup("missing.local") + if !ok { + t.Fatal("a failed lookup should be negatively cached") + } + if len(addrs) != 0 { + t.Fatalf("negative cache holds %v, want no addresses", addrs) + } +} + +func TestCacheExpires(t *testing.T) { + Flush() + base := time.Now() + orig := now + now = func() time.Time { return base } + t.Cleanup(func() { now = orig; Flush() }) + + cacheStore("inverter.local", []netip.Addr{netip.MustParseAddr("192.168.1.9")}, 30*time.Second) + if _, ok := cacheLookup("inverter.local"); !ok { + t.Fatal("entry should be live immediately after store") + } + + now = func() time.Time { return base.Add(31 * time.Second) } + if _, ok := cacheLookup("inverter.local"); ok { + t.Fatal("entry should have expired") + } +} + +func TestDialerSkipsResolutionForPlainHosts(t *testing.T) { + orig := listenPacket + listenPacket = func() (*net.UDPConn, error) { + t.Error("issued an mDNS query for a host that is not a .local name") + return nil, errors.New("should not be called") + } + t.Cleanup(func() { listenPacket = orig }) + + d := Dialer{Dialer: net.Dialer{Timeout: 500 * time.Millisecond}} + // Nothing listens on port 1; the point is that the failure comes from the + // dial, not from resolution. + if _, err := d.Dial("tcp", "127.0.0.1:1"); err == nil { + t.Fatal("expected the dial to fail") + } else if strings.Contains(err.Error(), "mDNS") { + t.Fatalf("plain IP dial went through mDNS: %v", err) + } +} + +func TestDialerReportsResolutionFailure(t *testing.T) { + Flush() + startResponder(t, nil) // never answers + + d := Dialer{Dialer: net.Dialer{Timeout: 100 * time.Millisecond}} + _, err := d.Dial("tcp", "missing.local:502") + if err == nil { + t.Fatal("expected a failure") + } + // The error must name the mechanism — an operator reading the log has to be + // able to tell resolution apart from an unreachable device. + if !strings.Contains(err.Error(), "mDNS") { + t.Fatalf("error %q does not mention mDNS", err) + } +} diff --git a/go/internal/scanner/mdns.go b/go/internal/mdnsresolve/reverse.go similarity index 51% rename from go/internal/scanner/mdns.go rename to go/internal/mdnsresolve/reverse.go index 4963ed52..b1206136 100644 --- a/go/internal/scanner/mdns.go +++ b/go/internal/mdnsresolve/reverse.go @@ -1,20 +1,19 @@ -package scanner +package mdnsresolve import ( "context" "fmt" "net" "strings" - "time" "golang.org/x/net/dns/dnsmessage" ) -var mdnsAddr = &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353} - -// reverseMDNS sends a reverse PTR query with the QU bit set so devices reply -// directly to our ephemeral socket instead of requiring a bind to port 5353. -func reverseMDNS(ctx context.Context, ip string) string { +// ReverseLookup asks the device at ip to name itself, so a discovered device +// can be shown and stored by its self-broadcast ".local" name rather than a +// DHCP-assigned address. Returns "" when nothing answers — callers treat a +// missing name as ordinary, not as an error. +func ReverseLookup(ctx context.Context, ip string) string { v4 := net.ParseIP(ip).To4() if v4 == nil { return "" @@ -25,35 +24,19 @@ func reverseMDNS(ctx context.Context, ip string) string { return "" } msg := dnsmessage.Message{Questions: []dnsmessage.Question{{ - Name: name, Type: dnsmessage.TypePTR, Class: dnsmessage.Class(0x8001), + Name: name, Type: dnsmessage.TypePTR, Class: classQU, }}} packed, err := msg.Pack() if err != nil { return "" } - conn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4zero}) - if err != nil { - return "" - } - defer conn.Close() - deadline := time.Now().Add(900 * time.Millisecond) - if d, ok := ctx.Deadline(); ok && d.Before(deadline) { - deadline = d - } - _ = conn.SetDeadline(deadline) - if _, err := conn.WriteToUDP(packed, mdnsAddr); err != nil { - return "" - } - buf := make([]byte, 1500) - for { - n, _, err := conn.ReadFromUDP(buf) - if err != nil { - return "" - } - if host := parsePTRAnswer(buf[:n], qname); host != "" { - return host - } - } + + var host string + _ = exchange(ctx, packed, func(packet []byte) bool { + host = parsePTRAnswer(packet, qname) + return host != "" + }) + return host } func parsePTRAnswer(packet []byte, qname string) string { diff --git a/go/internal/scanner/mdns_test.go b/go/internal/mdnsresolve/reverse_test.go similarity index 98% rename from go/internal/scanner/mdns_test.go rename to go/internal/mdnsresolve/reverse_test.go index 5a77c8f6..af8801bc 100644 --- a/go/internal/scanner/mdns_test.go +++ b/go/internal/mdnsresolve/reverse_test.go @@ -1,4 +1,4 @@ -package scanner +package mdnsresolve import ( "testing" diff --git a/go/internal/modbus/tcp_client.go b/go/internal/modbus/tcp_client.go index 78b7e7ba..b5b790ab 100644 --- a/go/internal/modbus/tcp_client.go +++ b/go/internal/modbus/tcp_client.go @@ -7,6 +7,8 @@ import ( "io" "net" "time" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) const ( @@ -39,10 +41,13 @@ func newTCPClient(addr string, timeout, keepAlive time.Duration) *tcpClient { } func (c *tcpClient) Open() error { - dialer := net.Dialer{ + // Resolution happens here, on every Open, so a device configured by its + // ".local" name is found again after a DHCP lease moves it — the reconnect + // path rebuilds the client from c.addr and picks up the new address. + dialer := mdnsresolve.Dialer{Dialer: net.Dialer{ Timeout: modbusDialTimeout, KeepAlive: c.keepAlive, - } + }} conn, err := dialer.Dial("tcp", c.addr) if err != nil { return err diff --git a/go/internal/mqtt/client.go b/go/internal/mqtt/client.go index fb1ec167..a711dd96 100644 --- a/go/internal/mqtt/client.go +++ b/go/internal/mqtt/client.go @@ -4,12 +4,15 @@ package mqtt import ( "fmt" "log/slog" + "net" + "net/url" "sync" "time" paho "github.com/eclipse/paho.mqtt.golang" "github.com/srcfl/ftw/go/internal/drivers" + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // Capability wraps a paho client to match drivers.MQTTCap. @@ -51,6 +54,14 @@ func Dial(host string, port int, username, password, clientID string) (*Capabili } opts := paho.NewClientOptions(). AddBroker(fmt.Sprintf("tcp://%s:%d", host, port)). + // paho's built-in dialer goes through the stdlib resolver, which never + // answers a ".local" name. Every broker URL built here is tcp://, so a + // TCP-only replacement is complete; non-".local" hosts fall through to + // a plain dial inside mdnsresolve. + SetCustomOpenConnectionFn(func(uri *url.URL, o paho.ClientOptions) (net.Conn, error) { + d := mdnsresolve.Dialer{Dialer: net.Dialer{Timeout: o.ConnectTimeout}} + return d.Dial("tcp", uri.Host) + }). SetClientID(clientID). SetAutoReconnect(true). SetConnectRetry(true). diff --git a/go/internal/scanner/scanner.go b/go/internal/scanner/scanner.go index 0d8dea31..612d8207 100644 --- a/go/internal/scanner/scanner.go +++ b/go/internal/scanner/scanner.go @@ -15,6 +15,8 @@ import ( "strings" "sync" "time" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // FoundDevice is one open port discovered on the local network. @@ -129,7 +131,7 @@ func resolveHostnames(ctx context.Context, devices []FoundDevice) { } if name == "" && ctx.Err() == nil { mdnsCtx, cancel := context.WithTimeout(ctx, 900*time.Millisecond) - name = reverseMDNS(mdnsCtx, ip) + name = mdnsresolve.ReverseLookup(mdnsCtx, ip) cancel() } if name != "" { From 2193399d5412a41d76853d31582d7e6c94ffedd2 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 09:48:29 +0200 Subject: [PATCH 2/6] build: run one base across the whole stack (debian:bookworm-slim) Core was alpine:3.22 and the updater sidecar was docker:27-cli (also alpine), while the optimizer was already python:3.12-slim-bookworm. That is two libcs and two security streams in one deployment, and two base rootfs blobs pulled per host. Put all three on debian:bookworm-slim so the layer is pulled once and tracked once. glibc additionally lets the image run ordinary prebuilt vendor binaries, which musl cannot, and a full userland makes on-site docker exec debugging practical. libnss-mdns is installed and wired into nsswitch.conf so .local resolves for glibc tools inside the container when an avahi socket is mounted; the FTW process does not depend on that, because a CGO_ENABLED=0 binary bypasses NSS entirely and resolves .local in Go instead. The binary stays fully static and still cross-compiles on the build platform, so only the small runtime layer is emulated for arm64. Measured: 79s for a full arm64 build, image 148MB (amd64 128MB, up from 53MB). wget is now installed explicitly and asserted by the boundary test. It is contractual rather than incidental: ftw-updater docker-execs it inside the core image to decide whether an update commits, and updaters already in the field will keep doing so, so the core image cannot stop shipping it. The boundary test's `^FROM alpine:` line is replaced by the invariant it was actually standing in for -- core must not build on a Python base -- and given a real error message. Also embed tzdata in the binary as a fallback. Production code reads time.Local, which silently degrades to UTC when zoneinfo is missing, and the tzdata tests skip rather than fail, so that regression would ship green. Verified uid 100 / gid 101 have no passwd entry on this base, and corrected the two comments that attributed them to an alpine adduser. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> --- .changeset/one-base-across-the-stack.md | 33 ++++++++++++++ Dockerfile | 45 ++++++++++++++++--- Dockerfile.updater | 22 ++++++--- .../pi-gen/stage-ftw/01-ftw-setup/00-run.sh | 5 ++- docs/operations.md | 23 ++++++++++ go/cmd/ftw/main.go | 8 ++++ scripts/install.sh | 4 +- scripts/test-container-boundaries.sh | 19 +++++++- 8 files changed, 142 insertions(+), 17 deletions(-) create mode 100644 .changeset/one-base-across-the-stack.md diff --git a/.changeset/one-base-across-the-stack.md b/.changeset/one-base-across-the-stack.md new file mode 100644 index 00000000..f010a263 --- /dev/null +++ b/.changeset/one-base-across-the-stack.md @@ -0,0 +1,33 @@ +--- +"ftw": minor +--- + +Move the core and updater images to `debian:bookworm-slim`, so the whole deployment runs one base and one libc. + +The core runtime was `alpine:3.22` and the updater sidecar was `docker:27-cli` +(also alpine), while the optimizer was already `python:3.12-slim-bookworm`. All +three now share the same Debian rootfs, so a host pulls that layer once instead +of pulling two different userlands, and there is a single security stream to +track rather than musl and glibc side by side. + +What the move buys beyond consistency: glibc means the image can run ordinary +prebuilt vendor binaries, which musl cannot; a full userland makes on-site +`docker exec` debugging far easier; and `libnss-mdns` is installed and wired +into `/etc/nsswitch.conf`, so `.local` names resolve for glibc tools inside the +container when an avahi socket is mounted. + +The FTW process itself does not depend on that: it resolves `.local` in Go via +`internal/mdnsresolve`, which works regardless of base or libc. NSS is bypassed +entirely by a `CGO_ENABLED=0` binary, so the OS-level resolver is a debugging +and future-compatibility affordance, not the mechanism. + +The binary stays fully static and cross-compiled on the build platform. `wget` +is now installed explicitly — it is contractual, because `ftw-updater` +`docker exec`s it inside the core image to decide whether an update commits, and +updaters already deployed in the field will keep doing so. The zoneinfo database +is also embedded in the binary as a fallback so a base image without `tzdata` +can never silently push `time.Local` to UTC and mis-time price and plan windows. + +Uncompressed image size grows from about 53 MB to about 128 MB. Data ownership +is unchanged: the process still runs as the bare numeric uid 100 / gid 101, and +gid 101 is still what grants access to the optimizer's socket. diff --git a/Dockerfile b/Dockerfile index 6526349c..f4642ac0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -22,8 +22,10 @@ RUN cd go && go mod download COPY go/ ./go/ -# Cross-compile by mapping TARGETARCH → GOARCH. CGO is off so the -# binary is fully static and runs on alpine without glibc. +# Cross-compile by mapping TARGETARCH → GOARCH. CGO stays off: the binary is +# fully static, so it is the runtime's *userland* we are choosing below, not a +# libc the binary depends on. Keeping CGO off is what lets the toolchain run +# natively on the build platform instead of under emulation. ARG TARGETOS=linux ARG TARGETARCH ARG VERSION=dev @@ -36,11 +38,34 @@ RUN cd go && \ go build -trimpath -ldflags="-s -w -X main.Version=${VERSION}" \ -o /out/ftw-backup ./cmd/ftw-backup # --- Runtime --------------------------------------------------------------- -FROM alpine:3.22 +# Debian bookworm-slim, matching Dockerfile.optimizer's python:3.12-slim-bookworm +# and Dockerfile.updater. One rootfs blob is pulled once and shared by all three +# images, so the extra bytes over alpine are paid a single time per host rather +# than per image — and there is one libc and one security stream to track. +# +# glibc also means the image can run ordinary prebuilt vendor binaries, which +# musl cannot, and ships a full userland for on-site debugging. +FROM debian:bookworm-slim -# HTTPS integrations and timezone-aware price/plan windows need these at -# runtime. BusyBox wget provides the health check without adding Python/curl. -RUN apk add --no-cache ca-certificates tzdata +# ca-certificates — HTTPS integrations. +# tzdata — timezone-aware price/plan windows. Without a zoneinfo tree +# time.Local silently degrades to UTC and mis-times plan +# boundaries with no error, so this is load-bearing. +# wget — the HEALTHCHECK below AND ftw-updater's readiness probe, +# which `docker exec`s wget in THIS image to decide whether +# an update commits. Debian slim ships neither wget nor curl, +# so it must be installed explicitly; dropping it would make +# every self-update fail its health gate and roll back. +# libnss-mdns — resolves ".local" for glibc programs in the image (getent, +# curl, any future cgo build). It needs a reachable +# avahi-daemon socket; see docs/operations.md. The FTW binary +# itself does not rely on this — it resolves ".local" in Go +# via internal/mdnsresolve, which works with CGO_ENABLED=0 +# where NSS by definition cannot. +RUN apt-get update && \ + apt-get install -y --no-install-recommends \ + ca-certificates tzdata wget libnss-mdns && \ + rm -rf /var/lib/apt/lists/* # Image layout: # /app/ftw binary (immutable, replaced on upgrade) @@ -79,7 +104,13 @@ EXPOSE 8080 # with each release. # # UID note: the process runs as uid 100 / gid 101 for compatibility with -# existing bind mounts. Named docker volumes inherit ownership from the image +# existing bind mounts. These are deliberately NUMERIC — no account is created +# and none is needed, which is why ENV HOME above is load-bearing. Verified on +# this base: uid 100 and gid 101 have no passwd/group entry, so ownership simply +# renders numerically. Do not renumber: gid 101 is what grants access to the +# optimizer's 0660 socket, and existing installs (and every flashed SD card) +# already own their data dir as 100:101. +# Named docker volumes inherit ownership from the image # automatically and just work. For HOST BIND MOUNTS, the host # directory must be owned by uid 100 (or world-writable) before the # container starts: diff --git a/Dockerfile.updater b/Dockerfile.updater index e00b5d5c..ed8c6ebb 100644 --- a/Dockerfile.updater +++ b/Dockerfile.updater @@ -28,12 +28,24 @@ RUN cd go && \ -o /out/ftw-updater ./cmd/ftw-updater # --- Runtime --------------------------------------------------------------- -# docker:27-cli bundles the `docker` binary + `docker compose` plugin + -# ca-certificates + tzdata, which is exactly what we need to execute -# compose pulls against the host daemon. -FROM docker:27-cli +# Same debian:bookworm-slim rootfs as Dockerfile and Dockerfile.optimizer, so +# the whole stack pulls one base layer instead of three. docker:27-cli is +# alpine-based and was the last thing keeping a second libc in the deployment. +# +# The docker CLI and the compose plugin are copied straight out of the official +# CLI image rather than installed from Docker's apt repository: it is the same +# upstream artifact, it pins the version explicitly, and it keeps apt out of the +# emulated arm64 layer. +FROM debian:bookworm-slim + +RUN apt-get update && \ + apt-get install -y --no-install-recommends ca-certificates tzdata && \ + rm -rf /var/lib/apt/lists/* + +COPY --from=docker:27-cli /usr/local/bin/docker /usr/local/bin/docker +COPY --from=docker:27-cli /usr/local/libexec/docker/cli-plugins/docker-compose \ + /usr/local/libexec/docker/cli-plugins/docker-compose -# Compose pull hits GHCR over TLS; docker:cli already bundles ca-certs. COPY --from=builder /out/ftw-updater /usr/local/bin/ftw-updater COPY LICENSE NOTICE /usr/share/doc/ftw/ diff --git a/deploy/pi-gen/stage-ftw/01-ftw-setup/00-run.sh b/deploy/pi-gen/stage-ftw/01-ftw-setup/00-run.sh index 9e8aaa23..581a86d8 100755 --- a/deploy/pi-gen/stage-ftw/01-ftw-setup/00-run.sh +++ b/deploy/pi-gen/stage-ftw/01-ftw-setup/00-run.sh @@ -69,8 +69,9 @@ EOF # records /opt/ftw automatically. COMPOSE_PROJECT_NAME # stays the literal `ftw`. # -# data/ is owned 100:101 because the in-container ftw user (alpine -# `adduser -S`) needs to own it before SQLite can create state.db. +# data/ is owned 100:101 because the container process runs as that bare +# numeric uid/gid (no account exists in the image) and needs to own the +# directory before SQLite can create state.db. # Same UID/GID mapping as scripts/install.sh. install -d -m 0755 "${ROOTFS_DIR}/opt/ftw" install -d -m 0755 -o 100 -g 101 "${ROOTFS_DIR}/opt/ftw/data" diff --git a/docs/operations.md b/docs/operations.md index 8e95e98a..5d643a2a 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -180,6 +180,29 @@ Verify the broker address from the same network namespace as core, then inspect broker and driver logs. Device credentials and topic mappings belong to the driver configuration. +### A device `.local` name does not resolve + +FTW resolves `.local` names itself over multicast DNS, so this works on any +base image and any libc. It does need multicast to reach the LAN, which the +Linux Compose topology provides through `network_mode: host`. Under +`docker-compose.macos.yml` the container is bridged and multicast does not +reach the LAN, so configure devices by IP there. The log line to look for is +`mDNS resolution failed`. + +The image also ships `libnss-mdns`, which lets ordinary glibc tools inside the +container (`getent hosts zap.local`, `curl`) resolve `.local`. That path needs a +reachable avahi socket, which is not shared by host networking — mount it +explicitly if you want it for debugging: + +```yaml + volumes: + - /run/avahi-daemon/socket:/run/avahi-daemon/socket:ro +``` + +Only add this on a host that actually runs `avahi-daemon`; without it Docker +creates a directory at that path. The FTW process does not use this path, so +omitting the mount changes nothing about device connectivity. + ### Configuration rejected Read the validation error, compare with diff --git a/go/cmd/ftw/main.go b/go/cmd/ftw/main.go index 509f7946..de9f05ca 100644 --- a/go/cmd/ftw/main.go +++ b/go/cmd/ftw/main.go @@ -25,6 +25,14 @@ import ( "syscall" "time" + // Embed the zoneinfo database as a fallback. The image's system tree still + // wins whenever it is present; this exists so time.Local can never silently + // degrade to UTC and mis-time price and plan windows if a base image ships + // without tzdata. The failure it guards against is invisible — production + // code reads time.Local, and the tzdata tests skip rather than fail — so the + // ~450 KB is worth it. + _ "time/tzdata" + "github.com/srcfl/ftw/go/internal/api" "github.com/srcfl/ftw/go/internal/arp" "github.com/srcfl/ftw/go/internal/battery" diff --git a/scripts/install.sh b/scripts/install.sh index 33179091..942f28a5 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -149,8 +149,8 @@ fi echo "" echo "==[3/5]== Preparing install directory: $INSTALL_DIR" mkdir -p "$INSTALL_DIR/data" -# The image runs as uid 100 / gid 101 (the `ftw` user created in the -# alpine runtime stage — see Dockerfile). A bind-mounted host dir must +# The image runs as uid 100 / gid 101 — a bare numeric USER, no account is +# created in the image at all (see Dockerfile). A bind-mounted host dir must # match those IDs so SQLite can create state.db inside it. $SUDO chown -R 100:101 "$INSTALL_DIR/data" diff --git a/scripts/test-container-boundaries.sh b/scripts/test-container-boundaries.sh index ea827f6b..1b4c40d5 100755 --- a/scripts/test-container-boundaries.sh +++ b/scripts/test-container-boundaries.sh @@ -9,7 +9,24 @@ if grep -Eq 'COPY optimizer/|--from=optimizer|/opt/venv|FTW_OPTIMIZER_(PYTHON|DI exit 1 fi -grep -q '^FROM alpine:' Dockerfile +# The core runtime must stay a small, Python-free userland. This used to be +# asserted as `^FROM alpine:`, which was a proxy for the real rule and gave no +# diagnostic when it failed — Python originally reached core precisely by way of +# the optimizer's base image, so the check existed to catch a base that drags an +# interpreter in, not to mandate one distro. Assert that directly instead. +if grep -Eq '^FROM .*(python|pypy)' Dockerfile; then + echo "Dockerfile must not build on a Python base image; use Dockerfile.optimizer for Python/CVXPY" >&2 + exit 1 +fi +# wget is contractual, not incidental: the HEALTHCHECK uses it, and +# ftw-updater docker-execs it inside this image to decide whether an update +# commits. An updater already deployed in the field will keep doing so, so the +# core image cannot stop shipping wget without breaking self-update on every +# host running an older sidecar. +if ! grep -Eq 'wget' Dockerfile; then + echo "Dockerfile must provide wget: the HEALTHCHECK and ftw-updater's readiness probe both exec it" >&2 + exit 1 +fi grep -q '^COPY optimizer/' Dockerfile.optimizer grep -q '/out/ftw-backup' Dockerfile grep -q '/app/ftw-backup' Dockerfile From 2048a7b00a28555290fffeb4fc002ea029a44bb3 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 10:02:57 +0200 Subject: [PATCH 3/6] build: pin the stack to Debian 13 trixie, the current stable bookworm is oldstable. Move all three images to trixie so the deployment tracks the suite that is actually receiving full security support, and so the base matches the Raspberry Pi OS release the SD image is built from (deploy/pi-gen/config: RELEASE=trixie). This also brings the optimizer along. Leaving it on a bookworm-derived python image would have split the shared base layer, which is the whole reason the images were aligned -- verified after the move that core, updater and optimizer resolve to one identical base layer digest. Verified on trixie: CVXPY 1.9.2 solves and highspy imports under Python 3.12.13; the updater's copied docker CLI 27.5.1 and compose plugin v2.33.0 both run; core has wget, the CA bundle, zoneinfo and an nsswitch.conf wired for mdns, and runs as uid 100 / gid 101. Pinned to the codename rather than a stable alias, so a major-version jump can never arrive silently on a rebuild. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> --- .changeset/one-base-across-the-stack.md | 64 ++++++++++++++----------- Dockerfile | 16 +++++-- Dockerfile.optimizer | 4 +- Dockerfile.updater | 4 +- 4 files changed, 52 insertions(+), 36 deletions(-) diff --git a/.changeset/one-base-across-the-stack.md b/.changeset/one-base-across-the-stack.md index f010a263..6baf7340 100644 --- a/.changeset/one-base-across-the-stack.md +++ b/.changeset/one-base-across-the-stack.md @@ -2,32 +2,42 @@ "ftw": minor --- -Move the core and updater images to `debian:bookworm-slim`, so the whole deployment runs one base and one libc. - -The core runtime was `alpine:3.22` and the updater sidecar was `docker:27-cli` -(also alpine), while the optimizer was already `python:3.12-slim-bookworm`. All -three now share the same Debian rootfs, so a host pulls that layer once instead -of pulling two different userlands, and there is a single security stream to -track rather than musl and glibc side by side. - -What the move buys beyond consistency: glibc means the image can run ordinary -prebuilt vendor binaries, which musl cannot; a full userland makes on-site -`docker exec` debugging far easier; and `libnss-mdns` is installed and wired -into `/etc/nsswitch.conf`, so `.local` names resolve for glibc tools inside the +Move the whole container stack to one base: Debian 13 "trixie" slim. + +Core was `alpine:3.22`, the updater sidecar was `docker:27-cli` (also alpine), +and the optimizer was `python:3.12-slim-bookworm` — two libcs, three unrelated +base images and three security streams in one deployment. Core, updater and +optimizer now all sit on `debian:trixie-slim`, verified to share a single base +layer, so a host pulls that rootfs once and there is one suite to track. It also +matches the Raspberry Pi OS release the SD image is built from. + +What the move buys: glibc, so the image can run ordinary prebuilt vendor +binaries, which musl cannot; a full userland, which makes `docker exec` +debugging on a live site practical; and `libnss-mdns` wired into +`/etc/nsswitch.conf`, so `.local` names resolve for glibc tools inside the container when an avahi socket is mounted. -The FTW process itself does not depend on that: it resolves `.local` in Go via -`internal/mdnsresolve`, which works regardless of base or libc. NSS is bypassed -entirely by a `CGO_ENABLED=0` binary, so the OS-level resolver is a debugging -and future-compatibility affordance, not the mechanism. - -The binary stays fully static and cross-compiled on the build platform. `wget` -is now installed explicitly — it is contractual, because `ftw-updater` -`docker exec`s it inside the core image to decide whether an update commits, and -updaters already deployed in the field will keep doing so. The zoneinfo database -is also embedded in the binary as a fallback so a base image without `tzdata` -can never silently push `time.Local` to UTC and mis-time price and plan windows. - -Uncompressed image size grows from about 53 MB to about 128 MB. Data ownership -is unchanged: the process still runs as the bare numeric uid 100 / gid 101, and -gid 101 is still what grants access to the optimizer's socket. +The FTW process does not depend on that last part — it resolves `.local` in Go +via `internal/mdnsresolve`, which works regardless of base or libc, because a +`CGO_ENABLED=0` binary bypasses NSS entirely. The binary stays fully static and +still cross-compiles on the build platform. + +`wget` is now installed explicitly and asserted by the container boundary test. +It is contractual rather than incidental: `ftw-updater` `docker exec`s it inside +the core image to decide whether an update commits, and updaters already +deployed in the field will keep doing so. The zoneinfo database is also embedded +in the binary as a fallback, so a base image without `tzdata` can never silently +push `time.Local` to UTC and mis-time price and plan windows. + +Size: the core image grows from about 53 MB to about 133 MB uncompressed. The +updater is unchanged at about 203 MB — `docker:27-cli` was never a small image — +and now shares its base with the other two, so the per-host download is well +below the nominal sum. + +Data ownership is unchanged: the process still runs as the bare numeric uid 100 +/ gid 101, and gid 101 is still what grants access to the optimizer's socket. + +The base is pinned to the `trixie` codename rather than a `stable` alias so a +major-version jump can never arrive silently on a rebuild. A new scheduled +`debian base currency` workflow watches for a newer Debian stable and opens a +tracking issue when one appears. diff --git a/Dockerfile b/Dockerfile index f4642ac0..9acd6e8c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -38,14 +38,20 @@ RUN cd go && \ go build -trimpath -ldflags="-s -w -X main.Version=${VERSION}" \ -o /out/ftw-backup ./cmd/ftw-backup # --- Runtime --------------------------------------------------------------- -# Debian bookworm-slim, matching Dockerfile.optimizer's python:3.12-slim-bookworm -# and Dockerfile.updater. One rootfs blob is pulled once and shared by all three -# images, so the extra bytes over alpine are paid a single time per host rather -# than per image — and there is one libc and one security stream to track. +# Debian trixie-slim — current Debian stable (13), and the same suite as +# Dockerfile.updater and Dockerfile.optimizer's python:3.12-slim-trixie. One +# rootfs blob is pulled once and shared by all three images, so the extra bytes +# over alpine are paid a single time per host rather than per image, and there +# is one libc and one security stream to track. It also matches the Raspberry Pi +# OS release the SD image is built from (deploy/pi-gen/config: RELEASE=trixie). +# +# Pinned to the codename, not `stable-slim`: a suite alias would silently jump +# major versions on some future rebuild. The `debian base currency` workflow +# watches for a new stable and files an issue, so the bump stays deliberate. # # glibc also means the image can run ordinary prebuilt vendor binaries, which # musl cannot, and ships a full userland for on-site debugging. -FROM debian:bookworm-slim +FROM debian:trixie-slim # ca-certificates — HTTPS integrations. # tzdata — timezone-aware price/plan windows. Without a zoneinfo tree diff --git a/Dockerfile.optimizer b/Dockerfile.optimizer index f8847321..1afb7f3c 100644 --- a/Dockerfile.optimizer +++ b/Dockerfile.optimizer @@ -1,11 +1,11 @@ # Independently releasable FTW mathematical optimizer. -FROM python:3.12-slim-bookworm AS build +FROM python:3.12-slim-trixie AS build COPY optimizer/ /src/optimizer/ RUN python -m venv /opt/venv && \ /opt/venv/bin/pip install --no-cache-dir /src/optimizer -FROM python:3.12-slim-bookworm +FROM python:3.12-slim-trixie ARG VERSION=dev ARG BUILD_SHA="" diff --git a/Dockerfile.updater b/Dockerfile.updater index ed8c6ebb..b002dbbe 100644 --- a/Dockerfile.updater +++ b/Dockerfile.updater @@ -28,7 +28,7 @@ RUN cd go && \ -o /out/ftw-updater ./cmd/ftw-updater # --- Runtime --------------------------------------------------------------- -# Same debian:bookworm-slim rootfs as Dockerfile and Dockerfile.optimizer, so +# Same debian:trixie-slim rootfs as Dockerfile and Dockerfile.optimizer, so # the whole stack pulls one base layer instead of three. docker:27-cli is # alpine-based and was the last thing keeping a second libc in the deployment. # @@ -36,7 +36,7 @@ RUN cd go && \ # CLI image rather than installed from Docker's apt repository: it is the same # upstream artifact, it pins the version explicitly, and it keeps apt out of the # emulated arm64 layer. -FROM debian:bookworm-slim +FROM debian:trixie-slim RUN apt-get update && \ apt-get install -y --no-install-recommends ca-certificates tzdata && \ From fade690d421f8af1d0a3d613f6f776aa9d3cc139 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 10:04:38 +0200 Subject: [PATCH 4/6] ci: notice when the container base falls behind Debian stable The images pin a Debian codename rather than a suite alias, so a major version jump can never arrive silently on a rebuild. The cost of pinning is that nothing notices when a new stable ships, and a suite quietly stops receiving full security support once it becomes oldstable -- which is exactly the state bookworm was already in. Truth comes from Debian's own Release file for the stable suite, not from registry tag listings: tags are noisy, rate-limited, and say nothing about which suite Debian considers stable. The check reads the pin out of the three Dockerfiles rather than hard- coding it, so it cannot drift from what actually ships, and it fails the run rather than reporting "current" if it could not reach Debian. It also flags the images having drifted apart, since a split pin silently costs the shared base layer they were aligned for. When a new stable is found it reports whether debian:-slim and the matching python:-slim- are actually published yet, because the python image can lag by days and moving core alone would split the pin. Follows the bundled-driver-pin workflow: one issue, rewritten each run and closed automatically once the pin is current, so a suite left alone for a year costs one thread rather than fifty-two. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> --- .github/workflows/debian-base-currency.yml | 95 ++++++++++++++++++++++ Makefile | 2 +- scripts/check-debian-base.sh | 92 +++++++++++++++++++++ 3 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/debian-base-currency.yml create mode 100755 scripts/check-debian-base.sh diff --git a/.github/workflows/debian-base-currency.yml b/.github/workflows/debian-base-currency.yml new file mode 100644 index 00000000..2eaaf8e0 --- /dev/null +++ b/.github/workflows/debian-base-currency.yml @@ -0,0 +1,95 @@ +name: debian base currency + +# The container images pin a Debian codename (debian:trixie-slim) rather than a +# suite alias, so that a major-version jump can never arrive silently on some +# future rebuild. The cost of pinning is that nothing notices when a new stable +# ships, and a base left behind quietly stops receiving security updates once +# its suite leaves LTS. +# +# This notices. It opens a single issue and keeps editing it, so a suite left +# alone for a year costs one thread rather than fifty-two. + +on: + schedule: + # 07:00 UTC on Mondays. Debian stable ships roughly every two years, so a + # daily check would be noise; weekly still surfaces a new release within + # days of it landing, which is far inside the window that matters. + - cron: "0 7 * * 1" + workflow_dispatch: + +permissions: + contents: read + issues: write + +concurrency: + group: debian-base-currency + cancel-in-progress: false + +jobs: + currency: + name: pinned base follows Debian stable + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v5 + + - name: Compare the pinned suite against Debian stable + id: compare + run: | + set +e + output="$(bash scripts/check-debian-base.sh 2>&1)" + status=$? + set -e + echo "$output" + { + echo "status=${status}" + echo "report<> "$GITHUB_OUTPUT" + # A network or parse failure must not read as "the base is current", + # and must not open an issue claiming a new release we never saw. + # 1 means a newer stable (or a split pin), 0 means current. + if [ "$status" != "0" ] && [ "$status" != "1" ]; then + echo "::error::the Debian currency check could not run (exit ${status})" + exit "$status" + fi + + - name: Open or update the tracking issue + if: steps.compare.outputs.status == '1' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPORT: ${{ steps.compare.outputs.report }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + title="Container base is behind Debian stable" + body="$(printf '%s\n\n```\n%s\n```\n\n%s\n\nFrom %s\n\nThis issue is rewritten by each run of the `debian base currency` workflow. Closing it without moving the pin means it comes back next Monday.\n' \ + "The Debian suite pinned by the container images is no longer the current Debian stable, or the three images have drifted apart. Core, updater and optimizer are pinned to one suite on purpose: they share a single base layer, so a host pulls that rootfs once instead of three times." \ + "${REPORT}" \ + "Moving the pin means updating the FROM lines in \`Dockerfile\`, \`Dockerfile.updater\` and \`Dockerfile.optimizer\` together, then rebuilding all three and confirming the optimizer still resolves CVXPY and HiGHS wheels on the new suite. Do not move core alone — that splits the shared layer. If the readiness lines above say a tag is not published yet, wait for it rather than splitting the pin." \ + "${RUN_URL}")" + + existing="$(gh issue list --state open --search "in:title \"${title}\"" \ + --json number,title --jq "[.[] | select(.title == \"${title}\")][0].number")" + + if [ -n "$existing" ] && [ "$existing" != "null" ]; then + gh issue edit "$existing" --body "$body" + echo "updated issue #${existing}" + else + gh issue create --title "$title" --body "$body" + fi + + - name: Close the tracking issue once the pin is current + if: steps.compare.outputs.status == '0' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + title="Container base is behind Debian stable" + existing="$(gh issue list --state open --search "in:title \"${title}\"" \ + --json number,title --jq "[.[] | select(.title == \"${title}\")][0].number")" + if [ -n "$existing" ] && [ "$existing" != "null" ]; then + gh issue close "$existing" --comment "Every image now pins the current Debian stable. Closed by the \`debian base currency\` workflow." + echo "closed issue #${existing}" + else + echo "base is current and no issue is open" + fi diff --git a/Makefile b/Makefile index 8a0e1b5f..1b50629d 100644 --- a/Makefile +++ b/Makefile @@ -87,7 +87,7 @@ optimizer-test: optimizer/.venv/.installed optimizer/.venv/bin/pytest -q optimizer/tests compose-migration-test: - bash -n scripts/enable-modular-stack.sh scripts/migrate-legacy-compose.sh scripts/install-macos.sh scripts/deploy-home-link-web.sh scripts/sync-bundled-drivers.sh scripts/check-driver-versions.sh + bash -n scripts/enable-modular-stack.sh scripts/migrate-legacy-compose.sh scripts/install-macos.sh scripts/deploy-home-link-web.sh scripts/sync-bundled-drivers.sh scripts/check-driver-versions.sh scripts/check-debian-base.sh bash scripts/test-modular-compose.sh container-boundary-test: diff --git a/scripts/check-debian-base.sh b/scripts/check-debian-base.sh new file mode 100755 index 00000000..9fb7b963 --- /dev/null +++ b/scripts/check-debian-base.sh @@ -0,0 +1,92 @@ +#!/usr/bin/env bash +# Reports whether the Debian suite pinned by the container images is still the +# current Debian stable. +# +# The images pin a codename (debian:trixie-slim) rather than a suite alias +# (debian:stable-slim) so a major-version jump can never arrive silently on a +# rebuild. The cost of pinning is that nothing notices when a new stable ships — +# this is what notices. +# +# Truth comes from Debian's own Release file for the `stable` suite, not from +# registry tag listings: tags are noisy, rate-limited and say nothing about +# which suite Debian considers stable. +# +# Exit codes, matching scripts/sync-bundled-drivers.sh --behind: +# 0 pinned suite is current stable +# 1 a newer stable exists, or the Dockerfiles disagree with each other +# 2 the check could not run (network, parse) — never reported as "fine" +set -euo pipefail + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +cd "$ROOT" + +RELEASE_URL="${DEBIAN_RELEASE_URL:-https://deb.debian.org/debian/dists/stable/Release}" + +# Read the pin out of the Dockerfiles instead of hard-coding it here, so this +# check cannot drift away from what actually ships. +pinned_debian() { sed -n 's/^FROM debian:\([a-z][a-z]*\)-slim.*/\1/p' "$1" | head -1; } +pinned_python() { sed -n 's/^FROM python:[0-9.][0-9.]*-slim-\([a-z][a-z]*\).*/\1/p' "$1" | head -1; } + +core=$(pinned_debian Dockerfile) +updater=$(pinned_debian Dockerfile.updater) +optimizer=$(pinned_python Dockerfile.optimizer) + +for pair in "Dockerfile:$core" "Dockerfile.updater:$updater" "Dockerfile.optimizer:$optimizer"; do + if [ -z "${pair#*:}" ]; then + echo "could not read a Debian suite from ${pair%%:*}" >&2 + exit 2 + fi +done + +echo "pinned suite:" +printf ' %-22s %s\n' "Dockerfile" "$core" "Dockerfile.updater" "$updater" "Dockerfile.optimizer" "$optimizer" + +if [ "$core" != "$updater" ] || [ "$core" != "$optimizer" ]; then + echo "" + echo "The three images no longer agree on one Debian suite. Sharing a single" + echo "base layer is the reason they were aligned, and that benefit is lost" + echo "while they differ." + exit 1 +fi + +release=$(curl -fsSL --max-time 20 "$RELEASE_URL" 2>/dev/null) || { + echo "could not fetch $RELEASE_URL" >&2 + exit 2 +} +stable=$(printf '%s\n' "$release" | sed -n 's/^Codename: *//p' | head -1) +version=$(printf '%s\n' "$release" | sed -n 's/^Version: *//p' | head -1) +if [ -z "$stable" ]; then + echo "no Codename field in $RELEASE_URL" >&2 + exit 2 +fi + +echo "" +echo "debian stable: $stable${version:+ (}${version}${version:+)}" + +if [ "$core" = "$stable" ]; then + echo "" + echo "The pinned suite is current." + exit 0 +fi + +echo "" +echo "A newer Debian stable is available: $core -> $stable" + +# Advisory only. A new Debian stable is tagged in the official images promptly, +# but python:-slim- can lag by days, and moving core without the +# optimizer would split the shared base layer. Never fail the check on this — +# it is a readiness note, not the finding. +if command -v docker >/dev/null 2>&1; then + echo "" + echo "image readiness:" + python_tag=$(sed -n 's/^FROM \(python:[0-9.][0-9.]*\)-slim-[a-z][a-z]*.*/\1/p' Dockerfile.optimizer | head -1) + for image in "debian:${stable}-slim" "${python_tag}-slim-${stable}"; do + if docker manifest inspect "$image" >/dev/null 2>&1; then + printf ' %-32s available\n' "$image" + else + printf ' %-32s NOT PUBLISHED YET\n' "$image" + fi + done +fi + +exit 1 From cc08a903d7b2476b3dd12eabdbe16ba1f678a871 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 10:46:11 +0200 Subject: [PATCH 5/6] build: bump the optimizer to 1.4.0 for the new base The optimizer releases independently of core, and optimizer-release.yml skips the build entirely when an image already exists for the requested version, then asserts that the published image's org.opencontainers.image.revision label matches the commit being released. Changing Dockerfile.optimizer without moving the version therefore does not merely produce two different artifacts under one number -- it cannot ship at all. Releasing 1.3.2 from this commit would either reuse the bookworm image or fail metadata verification. Minor rather than patch: the Python surface is untouched, but the runtime underneath it moves a Debian major version, so consumers are getting a materially different artifact. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> --- .changeset/one-base-across-the-stack.md | 5 +++++ optimizer/pyproject.toml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.changeset/one-base-across-the-stack.md b/.changeset/one-base-across-the-stack.md index 6baf7340..a3ac6c7e 100644 --- a/.changeset/one-base-across-the-stack.md +++ b/.changeset/one-base-across-the-stack.md @@ -37,6 +37,11 @@ below the nominal sum. Data ownership is unchanged: the process still runs as the bare numeric uid 100 / gid 101, and gid 101 is still what grants access to the optimizer's socket. +The optimizer is versioned independently and moves to 1.4.0, because its image +is a materially different artifact once the base changes. Its release workflow +verifies that a published image's revision label matches the commit it claims, +so the new base could not have shipped under the old version number at all. + The base is pinned to the `trixie` codename rather than a `stable` alias so a major-version jump can never arrive silently on a rebuild. A new scheduled `debian base currency` workflow watches for a newer Debian stable and opens a diff --git a/optimizer/pyproject.toml b/optimizer/pyproject.toml index 2611fca7..8ef2a8de 100644 --- a/optimizer/pyproject.toml +++ b/optimizer/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "ftw-optimizer" -version = "1.3.2" +version = "1.4.0" description = "CVXPY planning engine for ftw" requires-python = ">=3.11" dependencies = [ From 256f939508e1adca835077264728f85738f3a4bc Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 14:06:54 +0200 Subject: [PATCH 6/6] build: leave .local name resolution to its own change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @ The base move and the resolver were stacked in one branch, which made the resolver look like a consequence of choosing Debian. It is not: the two are independent, and the resolver is the one with the device-facing risk. Take the resolver back out so this branch is only the base move, and restate what libnss-mdns actually buys here — tools inside the image, not the FTW process, which is CGO_ENABLED=0 and never consults NSS. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> @ --- .changeset/one-base-across-the-stack.md | 11 +- .changeset/resolve-local-device-names.md | 24 -- Dockerfile | 12 +- config.example.yaml | 2 +- docs/operations.md | 30 +- docs/sourceful-zap.md | 6 - go/internal/drivers/lua.go | 17 +- go/internal/drivers/tcp_cap.go | 4 +- go/internal/drivers/ws_cap.go | 4 - go/internal/ha/bridge.go | 10 - go/internal/mdnsresolve/mdnsresolve.go | 328 ------------------ go/internal/mdnsresolve/mdnsresolve_test.go | 265 -------------- go/internal/modbus/tcp_client.go | 9 +- go/internal/mqtt/client.go | 11 - .../reverse.go => scanner/mdns.go} | 45 ++- .../reverse_test.go => scanner/mdns_test.go} | 2 +- go/internal/scanner/scanner.go | 4 +- 17 files changed, 68 insertions(+), 716 deletions(-) delete mode 100644 .changeset/resolve-local-device-names.md delete mode 100644 go/internal/mdnsresolve/mdnsresolve.go delete mode 100644 go/internal/mdnsresolve/mdnsresolve_test.go rename go/internal/{mdnsresolve/reverse.go => scanner/mdns.go} (51%) rename go/internal/{mdnsresolve/reverse_test.go => scanner/mdns_test.go} (98%) diff --git a/.changeset/one-base-across-the-stack.md b/.changeset/one-base-across-the-stack.md index a3ac6c7e..dae4bc95 100644 --- a/.changeset/one-base-across-the-stack.md +++ b/.changeset/one-base-across-the-stack.md @@ -15,12 +15,13 @@ What the move buys: glibc, so the image can run ordinary prebuilt vendor binaries, which musl cannot; a full userland, which makes `docker exec` debugging on a live site practical; and `libnss-mdns` wired into `/etc/nsswitch.conf`, so `.local` names resolve for glibc tools inside the -container when an avahi socket is mounted. +container — `getent hosts zap.local`, `curl`, `wget` — once an avahi socket is +mounted. Alpine has no NSS plugin mechanism at all, so none of that was +available before. -The FTW process does not depend on that last part — it resolves `.local` in Go -via `internal/mdnsresolve`, which works regardless of base or libc, because a -`CGO_ENABLED=0` binary bypasses NSS entirely. The binary stays fully static and -still cross-compiles on the build platform. +That covers tools in the image, not the FTW process itself: the binary is built +`CGO_ENABLED=0` and so never consults NSS. It stays fully static and still +cross-compiles on the build platform. `wget` is now installed explicitly and asserted by the container boundary test. It is contractual rather than incidental: `ftw-updater` `docker exec`s it inside diff --git a/.changeset/resolve-local-device-names.md b/.changeset/resolve-local-device-names.md deleted file mode 100644 index f314bdc4..00000000 --- a/.changeset/resolve-local-device-names.md +++ /dev/null @@ -1,24 +0,0 @@ ---- -"ftw": minor ---- - -Resolve device `.local` names over mDNS so devices can be configured by name instead of a DHCP-assigned IP. - -Go never resolves `.local` itself: it hands those names to libc only when cgo is -available, and FTW builds with `CGO_ENABLED=0`, so a configured `zap.local` -became a unicast DNS query to the site router and failed. That was true on every -base image and every libc. FTW now answers those names itself over multicast -DNS, and every driver transport uses it — Modbus TCP, MQTT (driver and Home -Assistant bridge), HTTP (including TLS-pinned clients), WebSocket and raw TCP. - -Resolution happens per dial rather than once at startup, so a device that moves -to a new DHCP lease is found again on the next reconnect without a config edit. -Answers are cached for the record's TTL (clamped to 30–120 s) so reconnect loops -do not flood the LAN, and failures are cached briefly so a device that is still -booting is retried soon. A failed resolution now logs `mDNS resolution failed` -and names the mechanism, instead of surfacing as a generic dial error. - -Only `.local` names take this path; literal IPs and ordinary DNS names dial -exactly as before. mDNS needs multicast reachability, which the Linux Compose -topology has via `network_mode: host`. Under `docker-compose.macos.yml` the -container is bridged, so configure devices by IP there. diff --git a/Dockerfile b/Dockerfile index 9acd6e8c..7cc31194 100644 --- a/Dockerfile +++ b/Dockerfile @@ -63,11 +63,13 @@ FROM debian:trixie-slim # so it must be installed explicitly; dropping it would make # every self-update fail its health gate and roll back. # libnss-mdns — resolves ".local" for glibc programs in the image (getent, -# curl, any future cgo build). It needs a reachable -# avahi-daemon socket; see docs/operations.md. The FTW binary -# itself does not rely on this — it resolves ".local" in Go -# via internal/mdnsresolve, which works with CGO_ENABLED=0 -# where NSS by definition cannot. +# curl, wget), so in-container debugging agrees with the +# host. apt wires mdns4_minimal into /etc/nsswitch.conf on +# install. At run time it forwards to avahi-daemon over +# /run/avahi-daemon/socket, which must be bind-mounted; see +# docs/operations.md. It does nothing for the FTW binary +# itself, which is CGO_ENABLED=0 and therefore never consults +# NSS — see the note on the builder stage above. RUN apt-get update && \ apt-get install -y --no-install-recommends \ ca-certificates tzdata wget libnss-mdns && \ diff --git a/config.example.yaml b/config.example.yaml index ff047809..0999020f 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -91,7 +91,7 @@ drivers: # battery_telemetry_only: true # capabilities: # http: - # allowed_hosts: ["zap.local"] # .local is resolved by FTW over mDNS + # allowed_hosts: ["zap.local"] # use the LAN IP if mDNS is unavailable # config: # host: zap.local # # meter_serial: p1m-... # optional; P1 is auto-selected diff --git a/docs/operations.md b/docs/operations.md index 5d643a2a..b1a97cee 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -180,28 +180,30 @@ Verify the broker address from the same network namespace as core, then inspect broker and driver logs. Device credentials and topic mappings belong to the driver configuration. -### A device `.local` name does not resolve +### `.local` names inside the container -FTW resolves `.local` names itself over multicast DNS, so this works on any -base image and any libc. It does need multicast to reach the LAN, which the -Linux Compose topology provides through `network_mode: host`. Under -`docker-compose.macos.yml` the container is bridged and multicast does not -reach the LAN, so configure devices by IP there. The log line to look for is -`mDNS resolution failed`. +The image ships `libnss-mdns`, so ordinary glibc tools inside the container — +`getent hosts zap.local`, `curl`, `wget` — resolve `.local` the way they do on +the host. `apt` wires `mdns4_minimal [NOTFOUND=return]` into +`/etc/nsswitch.conf` when the package is installed; nothing else is needed at +build time. -The image also ships `libnss-mdns`, which lets ordinary glibc tools inside the -container (`getent hosts zap.local`, `curl`) resolve `.local`. That path needs a -reachable avahi socket, which is not shared by host networking — mount it -explicitly if you want it for debugging: +At run time that path talks to `avahi-daemon` over a Unix socket, and a socket +is not shared by host networking the way a port is. Mount it explicitly: ```yaml volumes: - /run/avahi-daemon/socket:/run/avahi-daemon/socket:ro ``` -Only add this on a host that actually runs `avahi-daemon`; without it Docker -creates a directory at that path. The FTW process does not use this path, so -omitting the mount changes nothing about device connectivity. +Only add this on a host that actually runs `avahi-daemon` — the Raspberry Pi +image does. Without the daemon Docker creates a *directory* at that path, which +resolves nothing and is harmless but confusing; `ls -l` there is the quickest +way to tell the two apart. + +This makes the container's own tooling agree with the host. Whether the FTW +process itself resolves a device's `.local` name is a separate question, +answered by `internal/mdnsresolve`. ### Configuration rejected diff --git a/docs/sourceful-zap.md b/docs/sourceful-zap.md index 9b6a1306..75f98d60 100644 --- a/docs/sourceful-zap.md +++ b/docs/sourceful-zap.md @@ -76,12 +76,6 @@ go test ./internal/drivers -run 'Zap|zap' - not found: confirm Zap is on Wi-Fi and reachable at `http://zap.local/api/system` from the FTW host; -- `.local` name does not resolve: FTW resolves `.local` itself over multicast - DNS rather than through the OS resolver, so it needs to be on the same L2 - segment as the device. That is the case with the Linux Compose topology - (`network_mode: host`); under `docker-compose.macos.yml` the container is - bridged and multicast does not reach the LAN, so configure the device by IP - there. The log line naming the failure is `mDNS resolution failed`; - no meter: inspect Zap's `/api/devices` and pin `meter_serial` when needed; - duplicate PV/battery: disable the overlapping Zap DER; - visible battery is not controlled: expected for the telemetry-only driver. diff --git a/go/internal/drivers/lua.go b/go/internal/drivers/lua.go index f9cdc9d5..bb62185c 100644 --- a/go/internal/drivers/lua.go +++ b/go/internal/drivers/lua.go @@ -69,8 +69,6 @@ import ( "time" lua "github.com/yuin/gopher-lua" - - "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // LuaDriver wraps a running Lua VM bound to a HostEnv. @@ -1152,16 +1150,8 @@ func registerHost(L *lua.LState, env *HostEnv) { return false, fmt.Sprintf("host %q (port %s) not in allowed_hosts", host, port) } - // Drivers routinely address a device by its ".local" name, which the - // stdlib resolver cannot answer. Clone the default transport so proxying, - // HTTP/2 and connection pooling are all unchanged — only the dial step - // differs, and only for ".local" hosts. - transport := net_http.DefaultTransport.(*net_http.Transport).Clone() - transport.DialContext = mdnsresolve.DialContext - httpClient := &net_http.Client{ - Timeout: 15 * time.Second, - Transport: transport, + Timeout: 15 * time.Second, CheckRedirect: func(req *net_http.Request, via []*net_http.Request) error { if len(via) >= 10 { return fmt.Errorf("stopped after 10 redirects") @@ -1190,10 +1180,7 @@ func registerHost(L *lua.LState, env *HostEnv) { // CA. Drivers WITHOUT a pin keep Go's default transport untouched, so // nothing about existing HTTP drivers changes. if pin := tlsPin; pin != "" { - // Clone the transport built above so the pinned client keeps the same - // mDNS-aware dialer — a pinned device is usually a local appliance - // addressed by its ".local" name, which is exactly the case that needs it. - tr := transport.Clone() + tr := net_http.DefaultTransport.(*net_http.Transport).Clone() tr.TLSClientConfig = &tls.Config{ // We replace chain/hostname verification with our own exact // fingerprint check below, so the stdlib check must be off. diff --git a/go/internal/drivers/tcp_cap.go b/go/internal/drivers/tcp_cap.go index cba42622..4153f3cc 100644 --- a/go/internal/drivers/tcp_cap.go +++ b/go/internal/drivers/tcp_cap.go @@ -6,8 +6,6 @@ import ( "strings" "sync" "time" - - "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // TCPCap is the host's raw TCP socket capability. One driver = one upstream @@ -95,7 +93,7 @@ func (n *netTCP) Open(addr string) error { return fmt.Errorf("tcp: %s", reason) } - conn, err := mdnsresolve.DialTimeout("tcp", addr, 10*time.Second) + conn, err := net.DialTimeout("tcp", addr, 10*time.Second) if err != nil { return fmt.Errorf("tcp dial: %w", err) } diff --git a/go/internal/drivers/ws_cap.go b/go/internal/drivers/ws_cap.go index bd23736d..979d5685 100644 --- a/go/internal/drivers/ws_cap.go +++ b/go/internal/drivers/ws_cap.go @@ -9,8 +9,6 @@ import ( "time" "github.com/gorilla/websocket" - - "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // gorillaWS is the production WSCap implementation. One per driver. @@ -75,8 +73,6 @@ func (g *gorillaWS) Open(url string, headers map[string]string) error { } dialer := *websocket.DefaultDialer dialer.HandshakeTimeout = 15 * time.Second - // ".local" hosts need mDNS; everything else falls through to a plain dial. - dialer.NetDialContext = mdnsresolve.DialContext if len(subprotocols) > 0 { dialer.Subprotocols = subprotocols } diff --git a/go/internal/ha/bridge.go b/go/internal/ha/bridge.go index dd05fd77..c39e6f57 100644 --- a/go/internal/ha/bridge.go +++ b/go/internal/ha/bridge.go @@ -12,8 +12,6 @@ import ( "encoding/json" "fmt" "log/slog" - "net" - "net/url" "sort" "strconv" "strings" @@ -24,7 +22,6 @@ import ( "github.com/srcfl/ftw/go/internal/config" "github.com/srcfl/ftw/go/internal/control" - "github.com/srcfl/ftw/go/internal/mdnsresolve" "github.com/srcfl/ftw/go/internal/telemetry" ) @@ -252,13 +249,6 @@ func (b *Bridge) connectAndStart(cfg *config.HomeAssistant, driverNames []string opts := paho.NewClientOptions(). AddBroker(fmt.Sprintf("tcp://%s:%d", cfg.Broker, cfg.Port)). - // A Home Assistant broker is very often reached as homeassistant.local, - // which the stdlib resolver cannot answer. See internal/mqtt for why a - // TCP-only replacement is complete here. - SetCustomOpenConnectionFn(func(uri *url.URL, o paho.ClientOptions) (net.Conn, error) { - d := mdnsresolve.Dialer{Dialer: net.Dialer{Timeout: o.ConnectTimeout}} - return d.Dial("tcp", uri.Host) - }). SetClientID("forty-two-watts-ha"). SetAutoReconnect(true). SetConnectRetry(true). diff --git a/go/internal/mdnsresolve/mdnsresolve.go b/go/internal/mdnsresolve/mdnsresolve.go deleted file mode 100644 index 81eb5451..00000000 --- a/go/internal/mdnsresolve/mdnsresolve.go +++ /dev/null @@ -1,328 +0,0 @@ -// Package mdnsresolve resolves RFC 6762 ".local" host names over multicast -// DNS and provides a dialer that uses it. -// -// Go never does this itself. net/conf.go routes a ".local" lookup to libc only -// when cgo is available, and every FTW build sets CGO_ENABLED=0, so the pure Go -// resolver is always selected: it reads /etc/resolv.conf and sends a *unicast* -// query to the site router, which has no idea what "inverter.local" is. That -// holds on every base image and every libc — musl and glibc alike — so -// resolving here is the only portable fix. -// -// Only ".local" names take this path. Literal IPs and ordinary DNS names are -// handed straight to the standard dialer. -package mdnsresolve - -import ( - "context" - "fmt" - "log/slog" - "net" - "net/netip" - "strings" - "sync" - "time" - - "golang.org/x/net/dns/dnsmessage" -) - -// mdnsAddr is the RFC 6762 IPv4 multicast group. A var, not a const, so tests -// can aim a query at a loopback responder. -var mdnsAddr = &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353} - -// listenPacket opens the ephemeral socket a query is sent from. Replaced in -// tests. It deliberately does NOT bind port 5353: avahi-daemon already owns -// that on the host, and the QU bit below asks responders to reply directly to -// this socket instead. -var listenPacket = func() (*net.UDPConn, error) { - return net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4zero}) -} - -// now is swappable so cache-expiry tests do not have to sleep. -var now = time.Now - -const ( - // queryTimeout matches the budget the scanner already uses for its reverse - // lookups — long enough for a sleepy device, short enough that a driver - // dial does not stall a control tick. - queryTimeout = 900 * time.Millisecond - - // classQU is IN with the RFC 6762 unicast-response bit set. - classQU = dnsmessage.Class(0x8001) - - // A responder's TTL is advisory here. The floor stops a device that - // advertises a very short TTL from turning every Modbus reconnect into a - // multicast storm; the ceiling keeps a DHCP move from taking effect - // arbitrarily late, which is the whole point of binding by name. - minTTL = 30 * time.Second - maxTTL = 120 * time.Second - - // negativeTTL is deliberately short: a device that was off when we first - // looked should become reachable soon after it boots. - negativeTTL = 5 * time.Second -) - -type cacheEntry struct { - addrs []netip.Addr // empty means a cached negative answer - expires time.Time -} - -var ( - cacheMu sync.Mutex - cache = map[string]cacheEntry{} -) - -// IsLocal reports whether host is a ".local" name that mDNS should resolve. -// A literal IP is never one, so a configured "192.168.1.5" keeps the plain -// dial path and never touches the network for resolution. -func IsLocal(host string) bool { - if host == "" || net.ParseIP(host) != nil { - return false - } - return strings.HasSuffix(strings.ToLower(strings.TrimSuffix(host, ".")), ".local") -} - -func canonical(name string) string { - return strings.ToLower(strings.TrimSuffix(name, ".")) -} - -func cacheLookup(key string) ([]netip.Addr, bool) { - cacheMu.Lock() - defer cacheMu.Unlock() - entry, ok := cache[key] - if !ok || now().After(entry.expires) { - return nil, false - } - return entry.addrs, true -} - -func cacheStore(key string, addrs []netip.Addr, ttl time.Duration) { - cacheMu.Lock() - defer cacheMu.Unlock() - cache[key] = cacheEntry{addrs: addrs, expires: now().Add(ttl)} -} - -// Flush drops every cached answer. Tests use it; nothing in production does. -func Flush() { - cacheMu.Lock() - defer cacheMu.Unlock() - cache = map[string]cacheEntry{} -} - -// Lookup resolves a ".local" name to its advertised addresses. -func Lookup(ctx context.Context, name string) ([]netip.Addr, error) { - key := canonical(name) - if addrs, ok := cacheLookup(key); ok { - if len(addrs) == 0 { - return nil, fmt.Errorf("no mDNS responder for %s (cached)", name) - } - return addrs, nil - } - - addrs, ttl, err := queryAddrs(ctx, key) - if err != nil || len(addrs) == 0 { - cacheStore(key, nil, negativeTTL) - if err == nil { - err = fmt.Errorf("no mDNS responder for %s", name) - } - return nil, err - } - - cacheStore(key, addrs, ttl) - // Logged on a cache miss only, so this is at most one line per TTL per - // device rather than one per reconnect. - slog.Info("resolved host over mDNS", "host", key, "addr", addrs[0].String(), "ttl", ttl) - return addrs, nil -} - -func queryAddrs(ctx context.Context, name string) ([]netip.Addr, time.Duration, error) { - qname, err := dnsmessage.NewName(name + ".") - if err != nil { - return nil, 0, fmt.Errorf("mdns: bad name %q: %w", name, err) - } - // One packet, two questions. RFC 6762 §5.2 allows it and it saves a round - // trip on dual-stack devices. - msg := dnsmessage.Message{Questions: []dnsmessage.Question{ - {Name: qname, Type: dnsmessage.TypeA, Class: classQU}, - {Name: qname, Type: dnsmessage.TypeAAAA, Class: classQU}, - }} - packed, err := msg.Pack() - if err != nil { - return nil, 0, fmt.Errorf("mdns: pack query: %w", err) - } - - var ( - addrs []netip.Addr - ttl time.Duration - ) - err = exchange(ctx, packed, func(packet []byte) bool { - got, gotTTL, ok := parseAddrAnswer(packet, name+".") - if !ok { - return false - } - addrs, ttl = got, gotTTL - return true - }) - if err != nil { - return nil, 0, err - } - return addrs, ttl, nil -} - -// exchange sends one multicast query and feeds every reply to handle until it -// accepts one or the deadline passes. -func exchange(ctx context.Context, packed []byte, handle func([]byte) bool) error { - conn, err := listenPacket() - if err != nil { - return fmt.Errorf("mdns: open socket: %w", err) - } - defer conn.Close() - - deadline := now().Add(queryTimeout) - if d, ok := ctx.Deadline(); ok && d.Before(deadline) { - deadline = d - } - if err := conn.SetDeadline(deadline); err != nil { - return fmt.Errorf("mdns: set deadline: %w", err) - } - if _, err := conn.WriteToUDP(packed, mdnsAddr); err != nil { - return fmt.Errorf("mdns: send query: %w", err) - } - - buf := make([]byte, 1500) - for { - n, _, err := conn.ReadFromUDP(buf) - if err != nil { - return fmt.Errorf("mdns: no usable answer: %w", err) - } - if handle(buf[:n]) { - return nil - } - } -} - -func parseAddrAnswer(packet []byte, qname string) ([]netip.Addr, time.Duration, bool) { - var p dnsmessage.Parser - if _, err := p.Start(packet); err != nil { - return nil, 0, false - } - if err := p.SkipAllQuestions(); err != nil { - return nil, 0, false - } - var addrs []netip.Addr - ttl := maxTTL - // Labelled so a parse error inside the type switch abandons the whole - // packet: once the parser desynchronises, every later record is suspect. -parse: - for { - h, err := p.AnswerHeader() - if err != nil { - break parse - } - if !strings.EqualFold(h.Name.String(), qname) { - if err := p.SkipAnswer(); err != nil { - break parse - } - continue - } - switch h.Type { - case dnsmessage.TypeA: - r, err := p.AResource() - if err != nil { - break parse - } - addrs = append(addrs, netip.AddrFrom4(r.A)) - case dnsmessage.TypeAAAA: - r, err := p.AAAAResource() - if err != nil { - break parse - } - // Unmap so a v4-mapped AAAA dials as plain IPv4. - addrs = append(addrs, netip.AddrFrom16(r.AAAA).Unmap()) - default: - if err := p.SkipAnswer(); err != nil { - break parse - } - continue - } - if d := time.Duration(h.TTL) * time.Second; d < ttl { - ttl = d - } - } - return finishAnswer(addrs, ttl) -} - -func finishAnswer(addrs []netip.Addr, ttl time.Duration) ([]netip.Addr, time.Duration, bool) { - if len(addrs) == 0 { - return nil, 0, false - } - switch { - case ttl < minTTL: - ttl = minTTL - case ttl > maxTTL: - ttl = maxTTL - } - return addrs, ttl, true -} - -// Dialer dials TCP addresses, resolving ".local" host names over mDNS first. -// The embedded net.Dialer carries timeout and keep-alive; anything that is not -// a ".local" name is handed straight to it. -// -// Resolution happens per dial, not once at startup. That is what makes binding -// a device by name survive a DHCP lease change: callers that rebuild their -// connection from the original address string pick up the new IP on reconnect. -type Dialer struct { - net.Dialer -} - -// DialContext resolves address if it names a ".local" host, then dials it. -func (d *Dialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - host, port, err := net.SplitHostPort(address) - if err != nil || !IsLocal(host) { - return d.Dialer.DialContext(ctx, network, address) - } - - addrs, err := Lookup(ctx, host) - if err != nil { - // Name the mechanism. Without this the operator sees a bare dial - // failure and has no way to tell that resolution was the reason. - slog.Warn("mDNS resolution failed; check the device is on this LAN and the container uses host networking", - "host", host, "err", err) - return nil, fmt.Errorf("resolve %s over mDNS: %w", host, err) - } - - var firstErr error - for _, a := range addrs { - conn, err := d.Dialer.DialContext(ctx, network, net.JoinHostPort(a.String(), port)) - if err == nil { - return conn, nil - } - if firstErr == nil { - firstErr = err - } - } - return nil, fmt.Errorf("dial %s over mDNS: %w", host, firstErr) -} - -// Dial is the context-free form, for callers that have no context to pass. -func (d *Dialer) Dial(network, address string) (net.Conn, error) { - ctx := context.Background() - if d.Timeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, d.Timeout) - defer cancel() - } - return d.DialContext(ctx, network, address) -} - -// DialContext dials with default settings. -func DialContext(ctx context.Context, network, address string) (net.Conn, error) { - var d Dialer - return d.DialContext(ctx, network, address) -} - -// DialTimeout mirrors net.DialTimeout with mDNS resolution added. -func DialTimeout(network, address string, timeout time.Duration) (net.Conn, error) { - d := Dialer{Dialer: net.Dialer{Timeout: timeout}} - return d.Dial(network, address) -} diff --git a/go/internal/mdnsresolve/mdnsresolve_test.go b/go/internal/mdnsresolve/mdnsresolve_test.go deleted file mode 100644 index fb9469a5..00000000 --- a/go/internal/mdnsresolve/mdnsresolve_test.go +++ /dev/null @@ -1,265 +0,0 @@ -package mdnsresolve - -import ( - "context" - "errors" - "net" - "net/netip" - "strings" - "testing" - "time" - - "golang.org/x/net/dns/dnsmessage" -) - -func TestIsLocal(t *testing.T) { - cases := []struct { - host string - want bool - }{ - {"inverter.local", true}, - {"INVERTER.LOCAL", true}, - {"inverter.local.", true}, - {"zap.local", true}, - // A literal address must never trigger a multicast query. - {"192.168.1.5", false}, - {"::1", false}, - {"example.com", false}, - {"localhost", false}, - {"local", false}, - {"notlocal", false}, - {"", false}, - } - for _, c := range cases { - if got := IsLocal(c.host); got != c.want { - t.Errorf("IsLocal(%q) = %v, want %v", c.host, got, c.want) - } - } -} - -func aResource(t *testing.T, name string, ip [4]byte, ttl uint32) dnsmessage.Resource { - t.Helper() - return dnsmessage.Resource{ - Header: dnsmessage.ResourceHeader{ - Name: mustDNSName(t, name), Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, TTL: ttl, - }, - Body: &dnsmessage.AResource{A: ip}, - } -} - -func packAnswer(t *testing.T, qname string, answers []dnsmessage.Resource) []byte { - t.Helper() - msg := dnsmessage.Message{ - Header: dnsmessage.Header{Response: true, Authoritative: true}, - Questions: []dnsmessage.Question{{Name: mustDNSName(t, qname), Type: dnsmessage.TypeA, Class: dnsmessage.ClassINET}}, - Answers: answers, - } - packet, err := msg.Pack() - if err != nil { - t.Fatalf("pack: %v", err) - } - return packet -} - -func TestParseAddrAnswer(t *testing.T) { - qname := "inverter.local." - packet := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{192, 168, 1, 42}, 60)}) - - addrs, ttl, ok := parseAddrAnswer(packet, qname) - if !ok { - t.Fatal("parseAddrAnswer did not accept a valid answer") - } - if len(addrs) != 1 || addrs[0].String() != "192.168.1.42" { - t.Fatalf("addrs = %v, want [192.168.1.42]", addrs) - } - if ttl != 60*time.Second { - t.Fatalf("ttl = %v, want 60s", ttl) - } - - // An answer for a different name must be ignored. - if _, _, ok := parseAddrAnswer(packet, "other.local."); ok { - t.Fatal("accepted an answer for a different name") - } - // Garbage must not panic or resolve. - if _, _, ok := parseAddrAnswer([]byte{1, 2, 3}, qname); ok { - t.Fatal("accepted a malformed packet") - } -} - -func TestParseAddrAnswerClampsTTL(t *testing.T) { - qname := "inverter.local." - for _, c := range []struct { - name string - ttl uint32 - want time.Duration - }{ - // A device advertising a 1 s TTL must not make every Modbus reconnect - // re-query the LAN. - {"below floor", 1, minTTL}, - // A very long TTL must not outlive a DHCP move. - {"above ceiling", 86400, maxTTL}, - {"inside range", 90, 90 * time.Second}, - } { - t.Run(c.name, func(t *testing.T) { - packet := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{10, 0, 0, 1}, c.ttl)}) - _, ttl, ok := parseAddrAnswer(packet, qname) - if !ok { - t.Fatal("answer rejected") - } - if ttl != c.want { - t.Fatalf("ttl = %v, want %v", ttl, c.want) - } - }) - } -} - -// startResponder points the package at a loopback UDP socket that answers one -// query, so the real send/parse path is exercised without touching the LAN. -func startResponder(t *testing.T, answers []dnsmessage.Resource) { - t.Helper() - rc, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0}) - if err != nil { - t.Fatalf("listen responder: %v", err) - } - - origAddr, origListen := mdnsAddr, listenPacket - mdnsAddr = rc.LocalAddr().(*net.UDPAddr) - listenPacket = func() (*net.UDPConn, error) { - return net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)}) - } - - done := make(chan struct{}) - go func() { - defer close(done) - buf := make([]byte, 1500) - _ = rc.SetReadDeadline(time.Now().Add(2 * time.Second)) - n, from, err := rc.ReadFromUDP(buf) - if err != nil { - return - } - if answers == nil { - return // silent responder: exercises the negative path - } - var p dnsmessage.Parser - hdr, err := p.Start(buf[:n]) - if err != nil { - return - } - q, err := p.Question() - if err != nil { - return - } - resp := dnsmessage.Message{ - Header: dnsmessage.Header{ID: hdr.ID, Response: true, Authoritative: true}, - Questions: []dnsmessage.Question{q}, - Answers: answers, - } - packed, err := resp.Pack() - if err != nil { - return - } - _, _ = rc.WriteToUDP(packed, from) - }() - - t.Cleanup(func() { - _ = rc.Close() - <-done - mdnsAddr, listenPacket = origAddr, origListen - Flush() - }) -} - -func TestLookupResolvesLocalName(t *testing.T) { - Flush() - startResponder(t, []dnsmessage.Resource{aResource(t, "inverter.local.", [4]byte{192, 168, 1, 42}, 60)}) - - addrs, err := Lookup(context.Background(), "inverter.local") - if err != nil { - t.Fatalf("Lookup: %v", err) - } - if len(addrs) != 1 || addrs[0] != netip.MustParseAddr("192.168.1.42") { - t.Fatalf("addrs = %v, want [192.168.1.42]", addrs) - } - - // The answer must now be cached: a second call cannot need the responder, - // which has already stopped. - again, err := Lookup(context.Background(), "INVERTER.local") - if err != nil { - t.Fatalf("cached Lookup: %v", err) - } - if len(again) != 1 || again[0] != addrs[0] { - t.Fatalf("cached addrs = %v, want %v", again, addrs) - } -} - -func TestLookupCachesNegativeAnswer(t *testing.T) { - Flush() - startResponder(t, nil) // never answers - - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - if _, err := Lookup(ctx, "missing.local"); err == nil { - t.Fatal("expected a lookup failure when nothing answers") - } - - addrs, ok := cacheLookup("missing.local") - if !ok { - t.Fatal("a failed lookup should be negatively cached") - } - if len(addrs) != 0 { - t.Fatalf("negative cache holds %v, want no addresses", addrs) - } -} - -func TestCacheExpires(t *testing.T) { - Flush() - base := time.Now() - orig := now - now = func() time.Time { return base } - t.Cleanup(func() { now = orig; Flush() }) - - cacheStore("inverter.local", []netip.Addr{netip.MustParseAddr("192.168.1.9")}, 30*time.Second) - if _, ok := cacheLookup("inverter.local"); !ok { - t.Fatal("entry should be live immediately after store") - } - - now = func() time.Time { return base.Add(31 * time.Second) } - if _, ok := cacheLookup("inverter.local"); ok { - t.Fatal("entry should have expired") - } -} - -func TestDialerSkipsResolutionForPlainHosts(t *testing.T) { - orig := listenPacket - listenPacket = func() (*net.UDPConn, error) { - t.Error("issued an mDNS query for a host that is not a .local name") - return nil, errors.New("should not be called") - } - t.Cleanup(func() { listenPacket = orig }) - - d := Dialer{Dialer: net.Dialer{Timeout: 500 * time.Millisecond}} - // Nothing listens on port 1; the point is that the failure comes from the - // dial, not from resolution. - if _, err := d.Dial("tcp", "127.0.0.1:1"); err == nil { - t.Fatal("expected the dial to fail") - } else if strings.Contains(err.Error(), "mDNS") { - t.Fatalf("plain IP dial went through mDNS: %v", err) - } -} - -func TestDialerReportsResolutionFailure(t *testing.T) { - Flush() - startResponder(t, nil) // never answers - - d := Dialer{Dialer: net.Dialer{Timeout: 100 * time.Millisecond}} - _, err := d.Dial("tcp", "missing.local:502") - if err == nil { - t.Fatal("expected a failure") - } - // The error must name the mechanism — an operator reading the log has to be - // able to tell resolution apart from an unreachable device. - if !strings.Contains(err.Error(), "mDNS") { - t.Fatalf("error %q does not mention mDNS", err) - } -} diff --git a/go/internal/modbus/tcp_client.go b/go/internal/modbus/tcp_client.go index b5b790ab..78b7e7ba 100644 --- a/go/internal/modbus/tcp_client.go +++ b/go/internal/modbus/tcp_client.go @@ -7,8 +7,6 @@ import ( "io" "net" "time" - - "github.com/srcfl/ftw/go/internal/mdnsresolve" ) const ( @@ -41,13 +39,10 @@ func newTCPClient(addr string, timeout, keepAlive time.Duration) *tcpClient { } func (c *tcpClient) Open() error { - // Resolution happens here, on every Open, so a device configured by its - // ".local" name is found again after a DHCP lease moves it — the reconnect - // path rebuilds the client from c.addr and picks up the new address. - dialer := mdnsresolve.Dialer{Dialer: net.Dialer{ + dialer := net.Dialer{ Timeout: modbusDialTimeout, KeepAlive: c.keepAlive, - }} + } conn, err := dialer.Dial("tcp", c.addr) if err != nil { return err diff --git a/go/internal/mqtt/client.go b/go/internal/mqtt/client.go index a711dd96..fb1ec167 100644 --- a/go/internal/mqtt/client.go +++ b/go/internal/mqtt/client.go @@ -4,15 +4,12 @@ package mqtt import ( "fmt" "log/slog" - "net" - "net/url" "sync" "time" paho "github.com/eclipse/paho.mqtt.golang" "github.com/srcfl/ftw/go/internal/drivers" - "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // Capability wraps a paho client to match drivers.MQTTCap. @@ -54,14 +51,6 @@ func Dial(host string, port int, username, password, clientID string) (*Capabili } opts := paho.NewClientOptions(). AddBroker(fmt.Sprintf("tcp://%s:%d", host, port)). - // paho's built-in dialer goes through the stdlib resolver, which never - // answers a ".local" name. Every broker URL built here is tcp://, so a - // TCP-only replacement is complete; non-".local" hosts fall through to - // a plain dial inside mdnsresolve. - SetCustomOpenConnectionFn(func(uri *url.URL, o paho.ClientOptions) (net.Conn, error) { - d := mdnsresolve.Dialer{Dialer: net.Dialer{Timeout: o.ConnectTimeout}} - return d.Dial("tcp", uri.Host) - }). SetClientID(clientID). SetAutoReconnect(true). SetConnectRetry(true). diff --git a/go/internal/mdnsresolve/reverse.go b/go/internal/scanner/mdns.go similarity index 51% rename from go/internal/mdnsresolve/reverse.go rename to go/internal/scanner/mdns.go index b1206136..4963ed52 100644 --- a/go/internal/mdnsresolve/reverse.go +++ b/go/internal/scanner/mdns.go @@ -1,19 +1,20 @@ -package mdnsresolve +package scanner import ( "context" "fmt" "net" "strings" + "time" "golang.org/x/net/dns/dnsmessage" ) -// ReverseLookup asks the device at ip to name itself, so a discovered device -// can be shown and stored by its self-broadcast ".local" name rather than a -// DHCP-assigned address. Returns "" when nothing answers — callers treat a -// missing name as ordinary, not as an error. -func ReverseLookup(ctx context.Context, ip string) string { +var mdnsAddr = &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353} + +// reverseMDNS sends a reverse PTR query with the QU bit set so devices reply +// directly to our ephemeral socket instead of requiring a bind to port 5353. +func reverseMDNS(ctx context.Context, ip string) string { v4 := net.ParseIP(ip).To4() if v4 == nil { return "" @@ -24,19 +25,35 @@ func ReverseLookup(ctx context.Context, ip string) string { return "" } msg := dnsmessage.Message{Questions: []dnsmessage.Question{{ - Name: name, Type: dnsmessage.TypePTR, Class: classQU, + Name: name, Type: dnsmessage.TypePTR, Class: dnsmessage.Class(0x8001), }}} packed, err := msg.Pack() if err != nil { return "" } - - var host string - _ = exchange(ctx, packed, func(packet []byte) bool { - host = parsePTRAnswer(packet, qname) - return host != "" - }) - return host + conn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4zero}) + if err != nil { + return "" + } + defer conn.Close() + deadline := time.Now().Add(900 * time.Millisecond) + if d, ok := ctx.Deadline(); ok && d.Before(deadline) { + deadline = d + } + _ = conn.SetDeadline(deadline) + if _, err := conn.WriteToUDP(packed, mdnsAddr); err != nil { + return "" + } + buf := make([]byte, 1500) + for { + n, _, err := conn.ReadFromUDP(buf) + if err != nil { + return "" + } + if host := parsePTRAnswer(buf[:n], qname); host != "" { + return host + } + } } func parsePTRAnswer(packet []byte, qname string) string { diff --git a/go/internal/mdnsresolve/reverse_test.go b/go/internal/scanner/mdns_test.go similarity index 98% rename from go/internal/mdnsresolve/reverse_test.go rename to go/internal/scanner/mdns_test.go index af8801bc..5a77c8f6 100644 --- a/go/internal/mdnsresolve/reverse_test.go +++ b/go/internal/scanner/mdns_test.go @@ -1,4 +1,4 @@ -package mdnsresolve +package scanner import ( "testing" diff --git a/go/internal/scanner/scanner.go b/go/internal/scanner/scanner.go index 612d8207..0d8dea31 100644 --- a/go/internal/scanner/scanner.go +++ b/go/internal/scanner/scanner.go @@ -15,8 +15,6 @@ import ( "strings" "sync" "time" - - "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // FoundDevice is one open port discovered on the local network. @@ -131,7 +129,7 @@ func resolveHostnames(ctx context.Context, devices []FoundDevice) { } if name == "" && ctx.Err() == nil { mdnsCtx, cancel := context.WithTimeout(ctx, 900*time.Millisecond) - name = mdnsresolve.ReverseLookup(mdnsCtx, ip) + name = reverseMDNS(mdnsCtx, ip) cancel() } if name != "" {