From b59997c11c31812d9b138aa143583f2c3e034159 Mon Sep 17 00:00:00 2001 From: Fata Nugraha Date: Tue, 7 Jan 2025 20:08:07 +0800 Subject: [PATCH 1/7] Add filewatcher utility Signed-off-by: Fata Nugraha --- pkg/utils/filewatcher.go | 78 +++++++++++++++++++++++++++++++++++ pkg/utils/filewatcher_test.go | 67 ++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+) create mode 100644 pkg/utils/filewatcher.go create mode 100644 pkg/utils/filewatcher_test.go diff --git a/pkg/utils/filewatcher.go b/pkg/utils/filewatcher.go new file mode 100644 index 00000000..30798af9 --- /dev/null +++ b/pkg/utils/filewatcher.go @@ -0,0 +1,78 @@ +package utils + +import ( + "fmt" + "path/filepath" + "time" + + "github.com/fsnotify/fsnotify" +) + +// FileWatcher is an utility that +type FileWatcher struct { + w *fsnotify.Watcher + path string + + writeGracePeriod time.Duration + timer *time.Timer +} + +func NewFileWatcher(path string) (*FileWatcher, error) { + watcher, err := fsnotify.NewWatcher() + if err != nil { + return nil, err + } + + return &FileWatcher{w: watcher, path: path, writeGracePeriod: 200 * time.Millisecond}, nil +} + +func (fw *FileWatcher) Start(changeHandler func()) error { + // watch the directory instead of the individual file to ensure the notification still works when the file is modified + // through moving/renaming rather than writing into it directly (like what most modern editor does by default). + // ref: https://github.com/fsnotify/fsnotify/blob/a9bc2e01792f868516acf80817f7d7d7b3315409/README.md#watching-a-file-doesnt-work-well + err := fw.w.Add(filepath.Dir(fw.path)) + if err != nil { + return fmt.Errorf("adding watcher failed: %s", err) + } + + go func() { + for { + select { + case _, ok := <-fw.w.Errors: + if !ok { + return // watcher is closed. + } + case event, ok := <-fw.w.Events: + if !ok { + return // watcher is closed. + } + + if event.Name != fw.path { + continue // we don't care about this file. + } + + // Create may not always followed by Write e.g. when we replace the file with mv. + if event.Op.Has(fsnotify.Create) || event.Op.Has(fsnotify.Write) { + // as per the documentation, receiving Write does not mean that the write is finished. + // we try our best here to ignore "unfinished" write by assuming that after [writeGracePeriod] of + // inactivity the write has been finished. + fw.debounce(changeHandler) + } + } + } + }() + + return nil +} + +func (fw *FileWatcher) debounce(fn func()) { + if fw.timer != nil { + fw.timer.Stop() + } + + fw.timer = time.AfterFunc(fw.writeGracePeriod, fn) +} + +func (fw *FileWatcher) Stop() error { + return fw.w.Close() +} diff --git a/pkg/utils/filewatcher_test.go b/pkg/utils/filewatcher_test.go new file mode 100644 index 00000000..2dda5efb --- /dev/null +++ b/pkg/utils/filewatcher_test.go @@ -0,0 +1,67 @@ +package utils + +import ( + "os" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestFileWatcher(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "file.txt") + os.WriteFile(path, []byte("1"), 0o644) + + fw, err := NewFileWatcher(path) + fw.writeGracePeriod = 50 * time.Millisecond // reduce the delay to make the test runs faster. + assert.NoError(t, err) + fw.w.Add(path) + + var numTriggered atomic.Int64 + assertNumTriggered := func(expected int) { + time.Sleep(fw.writeGracePeriod + 50*time.Millisecond) + assert.Equal(t, int64(expected), numTriggered.Load()) + } + + fw.Start(func() { + numTriggered.Add(1) + }) + + // CASE: can detect changes to the file. + if err := os.WriteFile(path, []byte("2"), 0o644); err != nil { + panic(err) + } + assertNumTriggered(1) + + // CASE: can detect "swap"-based file modification. + tmpFile := filepath.Join(dir, "tmp.txt") + if err := os.WriteFile(tmpFile, []byte("lol"), 0o644); err != nil { + panic(err) + } + if err := os.Rename(tmpFile, path); err != nil { + panic(err) + } + assertNumTriggered(2) + + // CASE: combine multiple partial writes into single event. + fd, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0o644) + if err != nil { + panic(err) + } + // we assume these writes happens in less than 50ms. + _, _ = fd.Write([]byte("a")) + fd.Sync() + _, _ = fd.Write([]byte("b")) + fd.Close() + assertNumTriggered(3) + + // CASE: closed file watcher should not call the callback after Stop() is called. + assert.NoError(t, fw.Stop()) + if err := os.WriteFile(path, []byte("2"), 0o644); err != nil { + panic(err) + } + assertNumTriggered(3) // does not change. +} From b232c586b38392dc9242b3f57876addac645cd05 Mon Sep 17 00:00:00 2001 From: Fata Nugraha Date: Tue, 7 Jan 2025 20:17:28 +0800 Subject: [PATCH 2/7] Use filewatcher utils for hosts file reloader Signed-off-by: Fata Nugraha --- pkg/services/dns/hosts_file.go | 55 ++++++++++++---------------------- 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/pkg/services/dns/hosts_file.go b/pkg/services/dns/hosts_file.go index d8d8c7d8..99abf223 100644 --- a/pkg/services/dns/hosts_file.go +++ b/pkg/services/dns/hosts_file.go @@ -2,11 +2,10 @@ package dns import ( "net" - "path/filepath" "sync" "github.com/areYouLazy/libhosty" - "github.com/fsnotify/fsnotify" + "github.com/containers/gvisor-tap-vsock/pkg/utils" log "github.com/sirupsen/logrus" ) @@ -23,48 +22,31 @@ func NewHostsFile(hostsPath string) (*HostsFile, error) { if err != nil { return nil, err } - watcher, err := fsnotify.NewWatcher() - if err != nil { - return nil, err - } h := &HostsFile{ hostsFile: hostsFile, hostsFilePath: hostsFile.Config.FilePath, } - go func() { - h.startWatch(watcher) - }() + if err := h.startWatch(); err != nil { + return nil, err + } + return h, nil } -func (h *HostsFile) startWatch(w *fsnotify.Watcher) { - err := w.Add(filepath.Dir(h.hostsFilePath)) - +func (h *HostsFile) startWatch() error { + watcher, err := utils.NewFileWatcher(h.hostsFilePath) if err != nil { - log.Errorf("Hosts file adding watcher error:%s", err) - return + log.Errorf("Hosts file adding watcher error: %s", err) + return err } - for { - select { - case err, ok := <-w.Errors: - if !ok { - return - } - log.Errorf("Hosts file watcher error:%s", err) - case event, ok := <-w.Events: - if !ok { - return - } - if event.Name == h.hostsFilePath && event.Op&fsnotify.Write == fsnotify.Write { - err := h.updateHostsFile() - if err != nil { - log.Errorf("Hosts file read error:%s", err) - return - } - } - } + + if err := watcher.Start(h.updateHostsFile); err != nil { + log.Errorf("Hosts file adding watcher error: %s", err) + return err } + + return nil } func (h *HostsFile) LookupByHostname(name string) (net.IP, error) { @@ -75,17 +57,18 @@ func (h *HostsFile) LookupByHostname(name string) (net.IP, error) { return ip, err } -func (h *HostsFile) updateHostsFile() error { +func (h *HostsFile) updateHostsFile() { newHosts, err := readHostsFile(h.hostsFilePath) if err != nil { - return err + log.Errorf("Hosts file read error:%s", err) + return } h.hostsReadLock.Lock() defer h.hostsReadLock.Unlock() h.hostsFile = newHosts - return nil + return } func readHostsFile(hostsFilePath string) (*libhosty.HostsFile, error) { From ef520a2ef0223d0f2e5d52df30c16b01d5cfdae2 Mon Sep 17 00:00:00 2001 From: Fata Nugraha Date: Thu, 9 Jan 2025 16:00:38 +0800 Subject: [PATCH 3/7] Ensure nameserver list are synced Signed-off-by: Fata Nugraha --- pkg/services/dns/dns.go | 28 +++++++++---------- pkg/services/dns/dns_config.go | 24 ++++++++++++++++ pkg/services/dns/dns_config_unix.go | 38 ++++++++++++++++++++++++-- pkg/services/dns/dns_config_windows.go | 11 +++++++- 4 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 pkg/services/dns/dns_config.go diff --git a/pkg/services/dns/dns.go b/pkg/services/dns/dns.go index e50bbc52..9168dff0 100644 --- a/pkg/services/dns/dns.go +++ b/pkg/services/dns/dns.go @@ -16,17 +16,16 @@ import ( ) type dnsHandler struct { - zones []types.Zone - zonesLock sync.RWMutex - udpClient *dns.Client - tcpClient *dns.Client - hostsFile *HostsFile - nameservers []string + zones []types.Zone + zonesLock sync.RWMutex + udpClient *dns.Client + tcpClient *dns.Client + hostsFile *HostsFile + dnsConfig *dnsConfig } func newDNSHandler(zones []types.Zone) (*dnsHandler, error) { - - nameservers, err := getDNSHostAndPort() + dnsConfig, err := newDNSConfig() if err != nil { return nil, err } @@ -37,13 +36,12 @@ func newDNSHandler(zones []types.Zone) (*dnsHandler, error) { } return &dnsHandler{ - zones: zones, - tcpClient: &dns.Client{Net: "tcp"}, - udpClient: &dns.Client{Net: "udp"}, - nameservers: nameservers, - hostsFile: hostsFile, + zones: zones, + tcpClient: &dns.Client{Net: "tcp"}, + udpClient: &dns.Client{Net: "udp"}, + dnsConfig: dnsConfig, + hostsFile: hostsFile, }, nil - } func (h *dnsHandler) handle(w dns.ResponseWriter, dnsClient *dns.Client, r *dns.Msg, responseMessageSize int) { @@ -145,7 +143,7 @@ func (h *dnsHandler) addAnswers(dnsClient *dns.Client, r *dns.Msg) *dns.Msg { return m } } - for _, nameserver := range h.nameservers { + for _, nameserver := range h.dnsConfig.Nameservers() { msg := r.Copy() r, _, err := dnsClient.Exchange(msg, nameserver) // return first good answer diff --git a/pkg/services/dns/dns_config.go b/pkg/services/dns/dns_config.go new file mode 100644 index 00000000..1abdfe0c --- /dev/null +++ b/pkg/services/dns/dns_config.go @@ -0,0 +1,24 @@ +package dns + +import "sync" + +type dnsConfig struct { + mu sync.RWMutex + nameservers []string +} + +func newDNSConfig() (*dnsConfig, error) { + r := &dnsConfig{nameservers: []string{}} + if err := r.init(); err != nil { + return nil, err + } + + return r, nil +} + +func (r *dnsConfig) Nameservers() []string { + r.mu.RLock() + defer r.mu.RUnlock() + + return r.nameservers +} diff --git a/pkg/services/dns/dns_config_unix.go b/pkg/services/dns/dns_config_unix.go index ef66be39..09fbe51c 100644 --- a/pkg/services/dns/dns_config_unix.go +++ b/pkg/services/dns/dns_config_unix.go @@ -7,18 +7,50 @@ import ( "net" "net/netip" + "github.com/containers/gvisor-tap-vsock/pkg/utils" "github.com/miekg/dns" log "github.com/sirupsen/logrus" ) +func (r *dnsConfig) init() error { + if err := r.refreshNameservers(); err != nil { + return err + } + + w, err := utils.NewFileWatcher(etcResolvConfPath) + if err != nil { + return err + } + + if err := w.Start(func() { _ = r.refreshNameservers() }); err != nil { + return err + } + + return nil +} + +func (r *dnsConfig) refreshNameservers() error { + nsList, err := getDNSHostAndPort(etcResolvConfPath) + if err != nil { + return err + } + + r.mu.Lock() + r.nameservers = nsList + r.mu.Unlock() + return nil +} + +const etcResolvConfPath = "/etc/resolv.conf" + var errEmptyResolvConf = errors.New("no DNS servers configured in /etc/resolv.conf") -func getDNSHostAndPort() ([]string, error) { - conf, err := dns.ClientConfigFromFile("/etc/resolv.conf") +func getDNSHostAndPort(path string) ([]string, error) { + conf, err := dns.ClientConfigFromFile(path) if err != nil { return []string{}, err } - var hosts = make([]string, len(conf.Servers)) + hosts := make([]string, len(conf.Servers)) for _, server := range conf.Servers { dnsIP, err := netip.ParseAddr(server) if err != nil { diff --git a/pkg/services/dns/dns_config_windows.go b/pkg/services/dns/dns_config_windows.go index 5f1166d1..431727df 100644 --- a/pkg/services/dns/dns_config_windows.go +++ b/pkg/services/dns/dns_config_windows.go @@ -9,6 +9,16 @@ import ( qdmDns "github.com/qdm12/dns/v2/pkg/nameserver" ) +func (r *dnsConfig) init() error { + nsList, err := getDNSHostAndPort() + if err != nil { + return err + } + + r.nameservers = nsList + return nil +} + func getDNSHostAndPort() ([]string, error) { nameservers := qdmDns.GetDNSServers() @@ -21,5 +31,4 @@ func getDNSHostAndPort() ([]string, error) { } return dnsServers, nil - } From 32fa26afd1f9fc41b57ca2cdbc09fff76741cb11 Mon Sep 17 00:00:00 2001 From: Fata Nugraha Date: Thu, 9 Jan 2025 16:43:34 +0800 Subject: [PATCH 4/7] Fix lint issues Signed-off-by: Fata Nugraha --- pkg/services/dns/hosts_file.go | 1 - pkg/utils/filewatcher_test.go | 16 ++++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/services/dns/hosts_file.go b/pkg/services/dns/hosts_file.go index 99abf223..106129ee 100644 --- a/pkg/services/dns/hosts_file.go +++ b/pkg/services/dns/hosts_file.go @@ -68,7 +68,6 @@ func (h *HostsFile) updateHostsFile() { defer h.hostsReadLock.Unlock() h.hostsFile = newHosts - return } func readHostsFile(hostsFilePath string) (*libhosty.HostsFile, error) { diff --git a/pkg/utils/filewatcher_test.go b/pkg/utils/filewatcher_test.go index 2dda5efb..a13950b9 100644 --- a/pkg/utils/filewatcher_test.go +++ b/pkg/utils/filewatcher_test.go @@ -13,12 +13,12 @@ import ( func TestFileWatcher(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "file.txt") - os.WriteFile(path, []byte("1"), 0o644) + _ = os.WriteFile(path, []byte("1"), 0o600) fw, err := NewFileWatcher(path) fw.writeGracePeriod = 50 * time.Millisecond // reduce the delay to make the test runs faster. assert.NoError(t, err) - fw.w.Add(path) + _ = fw.w.Add(path) var numTriggered atomic.Int64 assertNumTriggered := func(expected int) { @@ -26,19 +26,19 @@ func TestFileWatcher(t *testing.T) { assert.Equal(t, int64(expected), numTriggered.Load()) } - fw.Start(func() { + _ = fw.Start(func() { numTriggered.Add(1) }) // CASE: can detect changes to the file. - if err := os.WriteFile(path, []byte("2"), 0o644); err != nil { + if err := os.WriteFile(path, []byte("2"), 0o600); err != nil { panic(err) } assertNumTriggered(1) // CASE: can detect "swap"-based file modification. tmpFile := filepath.Join(dir, "tmp.txt") - if err := os.WriteFile(tmpFile, []byte("lol"), 0o644); err != nil { + if err := os.WriteFile(tmpFile, []byte("lol"), 0o600); err != nil { panic(err) } if err := os.Rename(tmpFile, path); err != nil { @@ -47,20 +47,20 @@ func TestFileWatcher(t *testing.T) { assertNumTriggered(2) // CASE: combine multiple partial writes into single event. - fd, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0o644) + fd, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC, 0o600) if err != nil { panic(err) } // we assume these writes happens in less than 50ms. _, _ = fd.Write([]byte("a")) - fd.Sync() + _ = fd.Sync() _, _ = fd.Write([]byte("b")) fd.Close() assertNumTriggered(3) // CASE: closed file watcher should not call the callback after Stop() is called. assert.NoError(t, fw.Stop()) - if err := os.WriteFile(path, []byte("2"), 0o644); err != nil { + if err := os.WriteFile(path, []byte("2"), 0o600); err != nil { panic(err) } assertNumTriggered(3) // does not change. From 79a7988c6e53a38b35d5265683a34b4c4140c617 Mon Sep 17 00:00:00 2001 From: Fata Nugraha Date: Thu, 9 Jan 2025 18:06:10 +0800 Subject: [PATCH 5/7] resolve symlinks Signed-off-by: Fata Nugraha --- pkg/utils/filewatcher.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/utils/filewatcher.go b/pkg/utils/filewatcher.go index 30798af9..0993d38e 100644 --- a/pkg/utils/filewatcher.go +++ b/pkg/utils/filewatcher.go @@ -27,11 +27,17 @@ func NewFileWatcher(path string) (*FileWatcher, error) { } func (fw *FileWatcher) Start(changeHandler func()) error { + // Ensure that the target that we're watching is not a symlink as we won't get any events when we're watching + // a symlink. + fileRealPath, err := filepath.EvalSymlinks(fw.path) + if err != nil { + return fmt.Errorf("adding watcher failed: %s", err) + } + // watch the directory instead of the individual file to ensure the notification still works when the file is modified // through moving/renaming rather than writing into it directly (like what most modern editor does by default). // ref: https://github.com/fsnotify/fsnotify/blob/a9bc2e01792f868516acf80817f7d7d7b3315409/README.md#watching-a-file-doesnt-work-well - err := fw.w.Add(filepath.Dir(fw.path)) - if err != nil { + if err = fw.w.Add(filepath.Dir(fileRealPath)); err != nil { return fmt.Errorf("adding watcher failed: %s", err) } @@ -47,7 +53,7 @@ func (fw *FileWatcher) Start(changeHandler func()) error { return // watcher is closed. } - if event.Name != fw.path { + if event.Name != fileRealPath { continue // we don't care about this file. } From ac7404b6a88cee8091bf43a23c9642baaa1e6c8d Mon Sep 17 00:00:00 2001 From: Fata Nugraha Date: Thu, 9 Jan 2025 18:16:31 +0800 Subject: [PATCH 6/7] Remove blank entry in nameservers Signed-off-by: Fata Nugraha --- pkg/services/dns/dns_config_unix.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/dns/dns_config_unix.go b/pkg/services/dns/dns_config_unix.go index 09fbe51c..1d8e1783 100644 --- a/pkg/services/dns/dns_config_unix.go +++ b/pkg/services/dns/dns_config_unix.go @@ -50,7 +50,7 @@ func getDNSHostAndPort(path string) ([]string, error) { if err != nil { return []string{}, err } - hosts := make([]string, len(conf.Servers)) + hosts := make([]string, 0, len(conf.Servers)) for _, server := range conf.Servers { dnsIP, err := netip.ParseAddr(server) if err != nil { From e7bf941be5aafe4ce7ec933de9456c52a5f33c40 Mon Sep 17 00:00:00 2001 From: Fata Nugraha Date: Thu, 9 Jan 2025 18:30:48 +0800 Subject: [PATCH 7/7] Add logs Signed-off-by: Fata Nugraha --- pkg/services/dns/dns_config_unix.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/services/dns/dns_config_unix.go b/pkg/services/dns/dns_config_unix.go index 1d8e1783..9d795a83 100644 --- a/pkg/services/dns/dns_config_unix.go +++ b/pkg/services/dns/dns_config_unix.go @@ -32,9 +32,12 @@ func (r *dnsConfig) init() error { func (r *dnsConfig) refreshNameservers() error { nsList, err := getDNSHostAndPort(etcResolvConfPath) if err != nil { + log.Errorf("can't load dns nameservers: %v", err) return err } + log.Infof("reloading dns nameservers to %v", nsList) + r.mu.Lock() r.nameservers = nsList r.mu.Unlock()