diff --git a/internal/clientinfo/client_info.go b/internal/clientinfo/client_info.go index acae754b..36412b4a 100644 --- a/internal/clientinfo/client_info.go +++ b/internal/clientinfo/client_info.go @@ -118,7 +118,7 @@ func (t *Table) AddLeaseFile(name string, format ctrld.LeaseFileFormat) { if !t.discoverDHCP() { return } - clientInfoFiles[name] = format + setClientInfoFile(name, format) } // RefreshLoop runs all the refresher to update new client info data. diff --git a/internal/clientinfo/dhcp.go b/internal/clientinfo/dhcp.go index 5d11d5eb..af53883b 100644 --- a/internal/clientinfo/dhcp.go +++ b/internal/clientinfo/dhcp.go @@ -9,6 +9,7 @@ import ( "net" "net/netip" "os" + "path/filepath" "sort" "strings" "sync" @@ -39,7 +40,29 @@ func (d *dhcp) init() error { } d.addSelf() d.watcher = watcher - for file, format := range clientInfoFiles { + + // Install the directory watch before the initial file discovery. This closes + // the race where an SDN lease file is created after the startup snapshot but + // before watchChanges starts consuming fsnotify events. + leaseDir := router.LeaseFilesDir() + if leaseDir != "" { + if err := d.watcher.Add(leaseDir); err != nil { + ctrld.ProxyLogger.Load().Err(err).Str("dir", leaseDir).Msg("could not watch lease dir") + } else { + // First add all already-registered files while the directory watch is + // active, then scan for dynamic dnsmasq files which existed before the + // watcher was installed. + for file, format := range clientInfoFilesSnapshot() { + _ = d.addLeaseFile(file, format) + } + if err := d.discoverDynamicLeaseFiles(leaseDir); err != nil { + ctrld.ProxyLogger.Load().Err(err).Str("dir", leaseDir).Msg("could not scan lease dir") + } + return nil + } + } + + for file, format := range clientInfoFilesSnapshot() { // Ignore errors for default lease files. _ = d.addLeaseFile(file, format) } @@ -50,11 +73,6 @@ func (d *dhcp) watchChanges() { if d.watcher == nil { return } - if dir := router.LeaseFilesDir(); dir != "" { - if err := d.watcher.Add(dir); err != nil { - ctrld.ProxyLogger.Load().Err(err).Str("dir", dir).Msg("could not watch lease dir") - } - } for { select { case event, ok := <-d.watcher.Events: @@ -62,7 +80,11 @@ func (d *dhcp) watchChanges() { return } if event.Has(fsnotify.Create) { - if format, ok := clientInfoFiles[event.Name]; ok { + format, ok := clientInfoFileFormat(event.Name) + if !ok { + format, ok = dynamicDnsmasqLeaseFileFormat(event.Name) + } + if ok { if err := d.addLeaseFile(event.Name, format); err != nil { ctrld.ProxyLogger.Load().Err(err).Str("file", event.Name).Msg("could not add lease file") } @@ -70,7 +92,10 @@ func (d *dhcp) watchChanges() { continue } if event.Has(fsnotify.Write) || event.Has(fsnotify.Rename) || event.Has(fsnotify.Chmod) || event.Has(fsnotify.Remove) { - format := clientInfoFiles[event.Name] + format, ok := clientInfoFileFormat(event.Name) + if !ok { + continue + } if err := d.readLeaseFile(event.Name, format); err != nil && !os.IsNotExist(err) { ctrld.ProxyLogger.Load().Err(err).Str("file", event.Name).Msg("leases file changed but failed to update client info") } @@ -170,6 +195,42 @@ func (d *dhcp) lookupIPByHostname(name string, v6 bool) string { return "" } +func dynamicDnsmasqLeaseFileFormat(name string) (ctrld.LeaseFileFormat, bool) { + matched, err := filepath.Match("dnsmasq-*.leases", filepath.Base(name)) + if err != nil || !matched { + return "", false + } + return ctrld.Dnsmasq, true +} + + +func (d *dhcp) discoverDynamicLeaseFiles(dir string) error { + entries, err := os.ReadDir(dir) + if err != nil { + return err + } + for _, entry := range entries { + if entry.IsDir() { + continue + } + path := filepath.Join(dir, entry.Name()) + if _, registered := clientInfoFileFormat(path); registered { + // A registered file was already attempted above while the directory + // watcher was active. If it appeared during that attempt, its Create + // event is queued and watchChanges will retry it. + continue + } + format, ok := dynamicDnsmasqLeaseFileFormat(path) + if !ok { + continue + } + if err := d.addLeaseFile(path, format); err != nil && !os.IsNotExist(err) { + ctrld.ProxyLogger.Load().Err(err).Str("file", path).Msg("could not add discovered lease file") + } + } + return nil +} + // AddLeaseFile adds given lease file for reading/watching clients info. func (d *dhcp) addLeaseFile(name string, format ctrld.LeaseFileFormat) error { if d.watcher == nil { @@ -178,7 +239,7 @@ func (d *dhcp) addLeaseFile(name string, format ctrld.LeaseFileFormat) error { if err := d.readLeaseFile(name, format); err != nil { return fmt.Errorf("could not read lease file: %w", err) } - clientInfoFiles[name] = format + setClientInfoFile(name, format) return d.watcher.Add(name) } diff --git a/internal/clientinfo/dhcp_lease_files.go b/internal/clientinfo/dhcp_lease_files.go index 34aabf3a..199f496b 100644 --- a/internal/clientinfo/dhcp_lease_files.go +++ b/internal/clientinfo/dhcp_lease_files.go @@ -1,6 +1,10 @@ package clientinfo -import "github.com/Control-D-Inc/ctrld" +import ( + "sync" + + "github.com/Control-D-Inc/ctrld" +) // clientInfoFiles specifies client info files and how to read them on supported platforms. var clientInfoFiles = map[string]ctrld.LeaseFileFormat{ @@ -18,3 +22,30 @@ var clientInfoFiles = map[string]ctrld.LeaseFileFormat{ "/var/lib/kea/dhcp4.leases": ctrld.KeaDHCP4, // Pfsense "/var/db/dnsmasq.leases": ctrld.Dnsmasq, // OPNsense } + + +var clientInfoFilesMu sync.RWMutex + +func setClientInfoFile(name string, format ctrld.LeaseFileFormat) { + clientInfoFilesMu.Lock() + clientInfoFiles[name] = format + clientInfoFilesMu.Unlock() +} + +func clientInfoFileFormat(name string) (ctrld.LeaseFileFormat, bool) { + clientInfoFilesMu.RLock() + format, ok := clientInfoFiles[name] + clientInfoFilesMu.RUnlock() + return format, ok +} + +func clientInfoFilesSnapshot() map[string]ctrld.LeaseFileFormat { + clientInfoFilesMu.RLock() + defer clientInfoFilesMu.RUnlock() + + snapshot := make(map[string]ctrld.LeaseFileFormat, len(clientInfoFiles)) + for name, format := range clientInfoFiles { + snapshot[name] = format + } + return snapshot +} diff --git a/internal/clientinfo/dhcp_test.go b/internal/clientinfo/dhcp_test.go index 07dbf5a7..5b5130e2 100644 --- a/internal/clientinfo/dhcp_test.go +++ b/internal/clientinfo/dhcp_test.go @@ -2,8 +2,12 @@ package clientinfo import ( "io" + "os" + "path/filepath" "strings" "testing" + + "github.com/fsnotify/fsnotify" ) func Test_readClientInfoReader(t *testing.T) { @@ -139,3 +143,88 @@ func Test_dhcp_lookupIPByHostname(t *testing.T) { t.Fatalf("unexpected result, want: %s, got: %s", want, got) } } + + +func TestDynamicDnsmasqLeaseFileFormat(t *testing.T) { + tests := []struct { + name string + path string + ok bool + }{ + {"merlin sdn", "/var/lib/misc/dnsmasq-1.leases", true}, + {"merlin later sdn", "/var/lib/misc/dnsmasq-123.leases", true}, + {"edgeos dnsmasq", "/run/dnsmasq-dhcp.leases", true}, + {"main dnsmasq", "/var/lib/misc/dnsmasq.leases", false}, + {"unrelated lease", "/var/lib/misc/other.leases", false}, + {"similar suffix", "/var/lib/misc/dnsmasq-1.leases.bak", false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + format, ok := dynamicDnsmasqLeaseFileFormat(tc.path) + if ok != tc.ok { + t.Fatalf("dynamicDnsmasqLeaseFileFormat(%q) ok=%v, want %v", tc.path, ok, tc.ok) + } + if ok && format != "dnsmasq" { + t.Fatalf("dynamic format = %q, want dnsmasq", format) + } + }) + } +} + + +func TestDiscoverDynamicLeaseFilesScansExistingFiles(t *testing.T) { + dir := t.TempDir() + lease := filepath.Join(dir, "dnsmasq-7.leases") + if err := os.WriteFile(lease, []byte("1683329857 e6:20:59:b8:c1:6d 192.168.1.186 host1 *\n"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "unrelated.leases"), []byte("ignored\n"), 0644); err != nil { + t.Fatal(err) + } + + watcher, err := fsnotify.NewWatcher() + if err != nil { + t.Fatal(err) + } + defer watcher.Close() + + d := &dhcp{watcher: watcher} + if err := d.discoverDynamicLeaseFiles(dir); err != nil { + t.Fatal(err) + } + + format, ok := clientInfoFileFormat(lease) + if !ok || format != "dnsmasq" { + t.Fatalf("dynamic lease file was not registered: ok=%v format=%q", ok, format) + } + if got := d.LookupHostnameByMac("e6:20:59:b8:c1:6d"); got != "host1" { + t.Fatalf("hostname = %q, want host1", got) + } + + // Keep the package-global registry isolated from subsequent tests. + clientInfoFilesMu.Lock() + delete(clientInfoFiles, lease) + clientInfoFilesMu.Unlock() +} + +func TestDiscoverDynamicLeaseFilesIgnoresUnrelatedFiles(t *testing.T) { + dir := t.TempDir() + unrelated := filepath.Join(dir, "other.leases") + if err := os.WriteFile(unrelated, []byte("ignored\n"), 0644); err != nil { + t.Fatal(err) + } + + watcher, err := fsnotify.NewWatcher() + if err != nil { + t.Fatal(err) + } + defer watcher.Close() + + d := &dhcp{watcher: watcher} + if err := d.discoverDynamicLeaseFiles(dir); err != nil { + t.Fatal(err) + } + if _, ok := clientInfoFileFormat(unrelated); ok { + t.Fatalf("unrelated file was unexpectedly registered: %s", unrelated) + } +} diff --git a/internal/router/router.go b/internal/router/router.go index 2d8c462d..50ea2c69 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -209,10 +209,15 @@ func SelfInterfaces() []*net.Interface { // LeaseFilesDir is the directory which contains lease files. func LeaseFilesDir() string { - if Name() == edgeos.Name { - edgeos.LeaseFileDir() + switch Name() { + case edgeos.Name: + return edgeos.LeaseFileDir() + case merlin.Name: + // Main and Guest Network Pro / SDN dnsmasq instances keep leases here. + return "/var/lib/misc" + default: + return "" } - return "" } // ServiceDependencies returns list of dependencies that ctrld services needs on this router.