diff --git a/CHANGELOG.md b/CHANGELOG.md index 041accf87..e0aa59c3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ keys remain all-access. - Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324) - Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341) - Headscale now requires Go 1.27 to build +- Fix extra-records filewatcher hanging on shutdown after the watched file is deleted, and leaking the watcher when setup fails ## 0.29.3 (2026-07-29) diff --git a/hscontrol/dns/extrarecords.go b/hscontrol/dns/extrarecords.go index 05f42bc9d..29d2430a5 100644 --- a/hscontrol/dns/extrarecords.go +++ b/hscontrol/dns/extrarecords.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "sync" "github.com/cenkalti/backoff/v5" @@ -37,17 +38,24 @@ func NewExtraRecordsManager(path string) (*ExtraRecordsMan, error) { return nil, fmt.Errorf("creating watcher: %w", err) } + closeWatcher := func() { + _ = watcher.Close() + } + fi, err := os.Stat(path) if err != nil { + closeWatcher() return nil, fmt.Errorf("getting file info: %w", err) } if fi.IsDir() { + closeWatcher() return nil, fmt.Errorf("%w: %s", ErrPathIsDirectory, path) } records, hash, err := readExtraRecordsFromPath(path) if err != nil { + closeWatcher() return nil, fmt.Errorf("reading extra records from path: %w", err) } @@ -62,6 +70,8 @@ func NewExtraRecordsManager(path string) (*ExtraRecordsMan, error) { err = watcher.Add(path) if err != nil { + closeWatcher() + return nil, fmt.Errorf("adding path to watcher: %w", err) } @@ -101,15 +111,21 @@ func (e *ExtraRecordsMan) Run() { // If a file is removed or renamed, fsnotify will lose track of it // and not watch it. We will therefore attempt to re-add it with a backoff. case fsnotify.Remove, fsnotify.Rename: - _, err := backoff.Retry(context.Background(), func() (struct{}, error) { - if _, err := os.Stat(e.path); err != nil { //nolint:noinlineerr - return struct{}{}, err + err := e.waitUntilPathExists() + if err != nil { + select { + case <-e.closeCh: + return + default: } - return struct{}{}, nil - }, backoff.WithBackOff(backoff.NewExponentialBackOff())) - if err != nil { log.Error().Caller().Err(err).Msgf("extra records filewatcher retrying to find file after delete") + + addErr := e.watcher.Add(filepath.Dir(e.path)) + if addErr != nil { + log.Error().Caller().Err(addErr).Msgf("extra records filewatcher watching parent after delete failed") + } + continue } @@ -139,6 +155,29 @@ func (e *ExtraRecordsMan) Close() { close(e.closeCh) } +func (e *ExtraRecordsMan) waitUntilPathExists() error { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go func() { + select { + case <-e.closeCh: + cancel() + case <-ctx.Done(): + } + }() + + _, err := backoff.Retry(ctx, func() (struct{}, error) { + if _, err := os.Stat(e.path); err != nil { //nolint:noinlineerr + return struct{}{}, err + } + + return struct{}{}, nil + }, backoff.WithBackOff(backoff.NewExponentialBackOff())) + + return err +} + func (e *ExtraRecordsMan) UpdateCh() <-chan []tailcfg.DNSRecord { return e.updateCh } diff --git a/hscontrol/dns/extrarecords_shutdown_test.go b/hscontrol/dns/extrarecords_shutdown_test.go index 457d58c5e..ea6a015da 100644 --- a/hscontrol/dns/extrarecords_shutdown_test.go +++ b/hscontrol/dns/extrarecords_shutdown_test.go @@ -43,3 +43,32 @@ func TestUpdateRecordsDoesNotBlockShutdown(t *testing.T) { t.Fatal("updateRecords parked on a blocking send and did not return after Close") } } + +// TestWaitUntilPathExistsReturnsOnClose ensures Close unblocks the +// Remove/Rename retry. A missing extra-records file must not pin Run +// for the 15-minute backoff budget after shutdown. +func TestWaitUntilPathExistsReturnsOnClose(t *testing.T) { + path := filepath.Join(t.TempDir(), "extra.json") + require.NoError(t, os.WriteFile(path, + []byte(`[{"name":"a.example.com","type":"A","value":"100.64.0.1"}]`), 0o600)) + + er, err := NewExtraRecordsManager(path) + require.NoError(t, err) + + require.NoError(t, os.Remove(path)) + + done := make(chan error, 1) + + go func() { + done <- er.waitUntilPathExists() + }() + + er.Close() + + select { + case err := <-done: + require.Error(t, err) + case <-time.After(2 * time.Second): + t.Fatal("waitUntilPathExists did not return after Close while the extra-records file was missing") + } +}