From 72160e6e20c4932e5501d141bb8d6129a1f6c510 Mon Sep 17 00:00:00 2001 From: jxj Date: Mon, 28 Sep 2026 15:22:39 +0800 Subject: [PATCH] plugin/file: track zone mtime for reload_by_mtime (#8556) * plugin/file: track zone mtime for reload_by_mtime Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com> * plugin/file: capture initial mtime before parsing Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com> * plugin/file: keep unloaded zones retryable after open error Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com> --------- Signed-off-by: git-jxj <65210887+git-jxj@users.noreply.github.com> --- plugin/file/reload.go | 12 +++- plugin/file/reload_test.go | 37 +++++++++++ plugin/file/setup.go | 16 ++++- plugin/file/setup_test.go | 130 +++++++++++++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 2 deletions(-) diff --git a/plugin/file/reload.go b/plugin/file/reload.go index f822e5cdb..150121054 100644 --- a/plugin/file/reload.go +++ b/plugin/file/reload.go @@ -21,6 +21,7 @@ func (z *Zone) Reload(t *transfer.Transfer) error { case <-tick.C: zFile := z.File() serial := z.SOASerialIfDefined() + var fileMtime time.Time if z.ReloadByMtime { fi, err := os.Stat(zFile) @@ -28,9 +29,13 @@ func (z *Zone) Reload(t *transfer.Transfer) error { log.Errorf("Failed to stat zone %q in %q: %v", z.origin, zFile, err) continue } - if !fi.ModTime().After(z.file_mtime) { + z.RLock() + lastMtime := z.file_mtime + z.RUnlock() + if !fi.ModTime().After(lastMtime) { continue } + fileMtime = fi.ModTime() serial = 0 // force reload of the zone } @@ -50,6 +55,11 @@ func (z *Zone) Reload(t *transfer.Transfer) error { } z.setData(zone.Apex, zone.Tree) + if z.ReloadByMtime { + z.Lock() + z.file_mtime = fileMtime + z.Unlock() + } log.Infof("Successfully reloaded zone %q in %q with %d SOA serial", z.origin, zFile, zone.SOA.Serial) if t != nil { diff --git a/plugin/file/reload_test.go b/plugin/file/reload_test.go index d14f3552a..893439c19 100644 --- a/plugin/file/reload_test.go +++ b/plugin/file/reload_test.go @@ -307,6 +307,43 @@ func TestZoneReloadByMtime(t *testing.T) { t.Fatalf("Zone should still serve queries after file deletion, got result %d", res) } }) + + // Test 5: A successful reload advances the mtime baseline. + t.Run("MtimeBaselineAdvancesAfterReload", func(t *testing.T) { + z, fileName, cleanup := prepareMtimeZone(t, reloadZoneTest) + defer cleanup() + + if err := os.WriteFile(fileName, []byte(reloadZone2Test), 0644); err != nil { + t.Fatalf("Failed to write new zone data: %s", err) + } + + var rrs []dns.RR + for start := time.Now(); time.Since(start) < 2*time.Second; { + var err error + rrs, err = z.ApexIfDefined() + if err != nil { + t.Fatal(err) + } + if len(rrs) == 3 { + break + } + time.Sleep(2 * time.Millisecond) + } + if len(rrs) != 3 { + t.Fatalf("Expected 3 RRs after reload, got %d", len(rrs)) + } + + fi, err := os.Stat(fileName) + if err != nil { + t.Fatalf("Failed to stat zone: %s", err) + } + z.RLock() + mtime := z.file_mtime + z.RUnlock() + if !mtime.Equal(fi.ModTime()) { + t.Fatalf("file mtime = %s after reload, want %s", mtime, fi.ModTime()) + } + }) } // prepareMtimeZone creates a zone with mtime-based reload enabled. diff --git a/plugin/file/setup.go b/plugin/file/setup.go index 049ae868a..21db8ef87 100644 --- a/plugin/file/setup.go +++ b/plugin/file/setup.go @@ -2,6 +2,7 @@ package file import ( "errors" + "io" "os" "path/filepath" "time" @@ -69,6 +70,10 @@ func setup(c *caddy.Controller) error { } func fileParse(c *caddy.Controller) (Zones, fall.F, error) { + return fileParseWithParser(c, Parse) +} + +func fileParseWithParser(c *caddy.Controller, parse func(io.Reader, string, string, int64) (*Zone, error)) (Zones, fall.F, error) { z := make(map[string]*Zone) names := []string{} fall := fall.F{} @@ -95,6 +100,12 @@ func fileParse(c *caddy.Controller) (Zones, fall.F, error) { if err != nil { openErr = err } + var openedFileInfo os.FileInfo + if err == nil { + // Capture the version represented by the opened reader, before + // parsing can race with a replacement at the same pathname. + openedFileInfo, _ = reader.Stat() + } err = func() error { defer reader.Close() @@ -103,7 +114,7 @@ func fileParse(c *caddy.Controller) (Zones, fall.F, error) { z[origins[i]] = NewZone(origins[i], fileName) if openErr == nil { reader.Seek(0, 0) - zone, err := Parse(reader, origins[i], fileName, 0) + zone, err := parse(reader, origins[i], fileName, 0) if err != nil { return err } @@ -147,6 +158,9 @@ func fileParse(c *caddy.Controller) (Zones, fall.F, error) { z[origins[i]].ReloadInterval = reload z[origins[i]].Upstream = upstream.New() z[origins[i]].ReloadByMtime = reload_by_mtime + if reload_by_mtime && openErr == nil && openedFileInfo != nil { + z[origins[i]].file_mtime = openedFileInfo.ModTime() + } } } diff --git a/plugin/file/setup_test.go b/plugin/file/setup_test.go index f612140e2..1afc65378 100644 --- a/plugin/file/setup_test.go +++ b/plugin/file/setup_test.go @@ -1,8 +1,10 @@ package file import ( + "bytes" "context" "fmt" + "io" "os" "path/filepath" "strings" @@ -247,3 +249,131 @@ func TestFileParseRelativeZones(t *testing.T) { } } } + +func TestFileParseReloadByMtimeInitializesMtime(t *testing.T) { + name, rm, err := test.TempFile(".", dbMiekNL) + if err != nil { + t.Fatal(err) + } + defer rm() + + c := caddy.NewTestController("dns", "file "+name+" miek.nl. {\n\treload_by_mtime\n}") + zones, _, err := fileParse(c) + if err != nil { + t.Fatal(err) + } + + fi, err := os.Stat(name) + if err != nil { + t.Fatal(err) + } + z := zones.Z["miek.nl."] + if z.file_mtime.IsZero() { + t.Fatal("file mtime was not initialized for reload_by_mtime") + } + if !z.file_mtime.Equal(fi.ModTime()) { + t.Fatalf("file mtime = %s, want %s", z.file_mtime, fi.ModTime()) + } +} + +func TestFileParseReloadByMtimeMissingFileLeavesBaselineUnset(t *testing.T) { + name := filepath.Join(t.TempDir(), "missing.db") + c := caddy.NewTestController("dns", "file "+name+" example.org. {\n\treload_by_mtime\n}") + zones, _, err := fileParse(c) + if err != nil { + t.Fatal(err) + } + if !zones.Z["example.org."].file_mtime.IsZero() { + t.Fatal("missing initial file must leave the mtime baseline unset") + } +} + +func TestFileParseReloadByMtimeRecoversLaterZoneAfterMissingFile(t *testing.T) { + dir := t.TempDir() + missing := filepath.Join(dir, "missing.db") + existing := filepath.Join(dir, "existing.db") + if err := os.WriteFile(existing, []byte(dbMiekNL), 0600); err != nil { + t.Fatal(err) + } + + corefile := fmt.Sprintf(`file %s example.org. { + reload 10ms + reload_by_mtime +} +file %s miek.nl. { + reload 10ms + reload_by_mtime +}`, missing, existing) + zones, _, err := fileParse(caddy.NewTestController("dns", corefile)) + if err != nil { + t.Fatal(err) + } + z := zones.Z["miek.nl."] + if z == nil { + t.Fatal("later zone was not configured") + } + if err := z.Reload(nil); err != nil { + t.Fatal(err) + } + defer z.OnShutdown() + + const wantSerial = 1282630057 + for start := time.Now(); time.Since(start) < 2*time.Second; { + if z.SOASerialIfDefined() == wantSerial { + return + } + time.Sleep(2 * time.Millisecond) + } + t.Fatalf("later zone never loaded: SOA serial = %d, want %d", z.SOASerialIfDefined(), wantSerial) +} + +func TestFileParseReloadByMtimeUsesOpenedFileMtime(t *testing.T) { + name, rm, err := test.TempFile(".", dbRelative) + if err != nil { + t.Fatal(err) + } + defer rm() + + initialMtime := time.Now().Add(-2 * time.Hour).Truncate(time.Second) + if err := os.Chtimes(name, initialMtime, initialMtime); err != nil { + t.Fatal(err) + } + + c := caddy.NewTestController("dns", "file "+name+" example.org. {\n\treload_by_mtime\n}") + zones, _, err := fileParseWithParser(c, func(reader io.Reader, origin, fileName string, serial int64) (*Zone, error) { + // Simulate the opened reader consuming the old zone before a writer + // replaces its contents while the initial load is still in progress. + oldContents, err := io.ReadAll(reader) + if err != nil { + return nil, err + } + newContents := strings.Replace(dbRelative, "192.0.2.1", "192.0.2.2", 1) + if err := os.WriteFile(fileName, []byte(newContents), 0644); err != nil { + return nil, err + } + newMtime := initialMtime.Add(time.Hour) + if err := os.Chtimes(fileName, newMtime, newMtime); err != nil { + return nil, err + } + return Parse(bytes.NewReader(oldContents), origin, fileName, serial) + }) + if err != nil { + t.Fatal(err) + } + + z := zones.Z["example.org."] + entry, ok := z.Search("foo.example.org.") + if !ok || len(entry.Type(dns.TypeA)) != 1 || entry.Type(dns.TypeA)[0].(*dns.A).A.String() != "192.0.2.1" { + t.Fatal("initial load did not retain the old zone record") + } + if !z.file_mtime.Equal(initialMtime) { + t.Fatalf("initial mtime = %s, want opened file's %s", z.file_mtime, initialMtime) + } + fi, err := os.Stat(name) + if err != nil { + t.Fatal(err) + } + if !fi.ModTime().After(z.file_mtime) { + t.Fatalf("changed zone mtime %s must be newer than loaded baseline %s", fi.ModTime(), z.file_mtime) + } +}