From b8043e4f05989e3ffed398a3fd5c5478cabb8f5e Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Wed, 30 Jul 2025 17:53:09 +0300 Subject: [PATCH] Pull request 2440: AGDNS-3060-imp-gocognit-logs Merge in DNS/adguard-home from AGDNS-3060-imp-gocognit-logs to master Squashed commit of the following: commit 3026dc35662c71c125c60198f828e7da7e8e5b4f Merge: 2b56f4236 df258512d Author: Stanislav Chzhen Date: Wed Jul 30 13:00:44 2025 +0300 Merge branch 'master' into AGDNS-3060-imp-gocognit-logs commit 2b56f423649ef0e5734a3a1d9c0b5df178462e29 Author: Stanislav Chzhen Date: Fri Jul 25 14:41:40 2025 +0300 all: imp docs commit 101d043c85e8b813f6b28751edeeb6c6b43578a9 Author: Stanislav Chzhen Date: Wed Jul 23 20:09:38 2025 +0300 all: imp code commit 87cfa502f7abf5b8f8a18b71f4a99aab9a066961 Author: Stanislav Chzhen Date: Wed Jul 23 14:51:33 2025 +0300 all: imp code commit 07c1a04a4022f4e38282a538670d575fba949bb2 Author: Stanislav Chzhen Date: Tue Jul 22 20:04:58 2025 +0300 all: imp gocognit, logs --- internal/aghnet/hostscontainer.go | 5 +- internal/aghnet/hostscontainer_test.go | 29 ++-- internal/aghos/aghos_test.go | 11 -- internal/aghos/filewalker_test.go | 42 ++--- internal/aghos/fswatcher.go | 74 ++++---- internal/aghos/os.go | 15 +- internal/aghtest/interface.go | 18 +- internal/aghtls/aghtls.go | 11 +- internal/aghtls/aghtls_test.go | 9 +- internal/aghtls/root.go | 6 +- internal/aghtls/root_linux.go | 75 ++++---- internal/aghtls/root_others.go | 8 +- .../dnsforward/dnsforward_internal_test.go | 5 +- internal/dnsforward/http_internal_test.go | 9 +- internal/filtering/hosts_test.go | 9 +- internal/home/clients_internal_test.go | 2 +- internal/home/home.go | 62 ++++--- internal/home/service.go | 160 +++++++++++------- internal/home/signal.go | 25 +-- internal/home/tls.go | 2 +- internal/updater/updater.go | 21 ++- scripts/make/go-lint.sh | 12 +- 22 files changed, 348 insertions(+), 262 deletions(-) delete mode 100644 internal/aghos/aghos_test.go diff --git a/internal/aghnet/hostscontainer.go b/internal/aghnet/hostscontainer.go index 9390ac45..14e2c5d4 100644 --- a/internal/aghnet/hostscontainer.go +++ b/internal/aghnet/hostscontainer.go @@ -1,6 +1,7 @@ package aghnet import ( + "context" "fmt" "io" "io/fs" @@ -102,7 +103,9 @@ func NewHostsContainer( func (hc *HostsContainer) Close() (err error) { log.Debug("%s: closing", hostsContainerPrefix) - err = errors.Annotate(hc.watcher.Close(), "closing fs watcher: %w") + // TODO(s.chzhen): Pass context. + ctx := context.TODO() + err = errors.Annotate(hc.watcher.Shutdown(ctx), "closing fs watcher: %w") // Go on and close the container either way. close(hc.done) diff --git a/internal/aghnet/hostscontainer_test.go b/internal/aghnet/hostscontainer_test.go index 9393155d..b86e95ca 100644 --- a/internal/aghnet/hostscontainer_test.go +++ b/internal/aghnet/hostscontainer_test.go @@ -1,6 +1,7 @@ package aghnet_test import ( + "context" "net/netip" "path" "sync/atomic" @@ -67,10 +68,10 @@ func TestNewHostsContainer(t *testing.T) { } hc, err := aghnet.NewHostsContainer(testFS, &aghtest.FSWatcher{ - OnStart: func() (_ error) { panic("not implemented") }, - OnEvents: onEvents, - OnAdd: onAdd, - OnClose: func() (err error) { return nil }, + OnStart: func(_ context.Context) (_ error) { panic("not implemented") }, + OnEvents: onEvents, + OnAdd: onAdd, + OnShutdown: func(_ context.Context) (err error) { return nil }, }, tc.paths...) if tc.wantErr != nil { require.ErrorIs(t, err, tc.wantErr) @@ -94,11 +95,11 @@ func TestNewHostsContainer(t *testing.T) { t.Run("nil_fs", func(t *testing.T) { require.Panics(t, func() { _, _ = aghnet.NewHostsContainer(nil, &aghtest.FSWatcher{ - OnStart: func() (_ error) { panic("not implemented") }, + OnStart: func(_ context.Context) (_ error) { panic("not implemented") }, // Those shouldn't panic. - OnEvents: func() (e <-chan struct{}) { return nil }, - OnAdd: func(name string) (err error) { return nil }, - OnClose: func() (err error) { return nil }, + OnEvents: func() (e <-chan struct{}) { return nil }, + OnAdd: func(name string) (err error) { return nil }, + OnShutdown: func(_ context.Context) (err error) { return nil }, }, p) }) }) @@ -113,10 +114,10 @@ func TestNewHostsContainer(t *testing.T) { const errOnAdd errors.Error = "error" errWatcher := &aghtest.FSWatcher{ - OnStart: func() (_ error) { panic("not implemented") }, - OnEvents: func() (e <-chan struct{}) { panic("not implemented") }, - OnAdd: func(name string) (err error) { return errOnAdd }, - OnClose: func() (err error) { return nil }, + OnStart: func(_ context.Context) (_ error) { panic("not implemented") }, + OnEvents: func() (e <-chan struct{}) { panic("not implemented") }, + OnAdd: func(name string) (err error) { return errOnAdd }, + OnShutdown: func(_ context.Context) (err error) { return nil }, } hc, err := aghnet.NewHostsContainer(testFS, errWatcher, p) @@ -158,14 +159,14 @@ func TestHostsContainer_refresh(t *testing.T) { t.Cleanup(func() { close(eventsCh) }) w := &aghtest.FSWatcher{ - OnStart: func() (_ error) { panic("not implemented") }, + OnStart: func(_ context.Context) (_ error) { panic("not implemented") }, OnEvents: func() (e <-chan event) { return eventsCh }, OnAdd: func(name string) (err error) { assert.Equal(t, "dir", name) return nil }, - OnClose: func() (err error) { return nil }, + OnShutdown: func(_ context.Context) (err error) { return nil }, } hc, err := aghnet.NewHostsContainer(testFS, w, "dir") diff --git a/internal/aghos/aghos_test.go b/internal/aghos/aghos_test.go deleted file mode 100644 index 3916d98e..00000000 --- a/internal/aghos/aghos_test.go +++ /dev/null @@ -1,11 +0,0 @@ -package aghos_test - -import ( - "testing" - - "github.com/AdguardTeam/golibs/testutil" -) - -func TestMain(m *testing.M) { - testutil.DiscardLogOutput(m) -} diff --git a/internal/aghos/filewalker_test.go b/internal/aghos/filewalker_test.go index 94443831..ac0960e4 100644 --- a/internal/aghos/filewalker_test.go +++ b/internal/aghos/filewalker_test.go @@ -13,29 +13,33 @@ import ( "github.com/stretchr/testify/require" ) -func TestFileWalker_Walk(t *testing.T) { - const attribute = `000` +// Common file-walker constants. +const ( + attribute = "000" + nl = "\n" +) - makeFileWalker := func(_ string) (fw aghos.FileWalker) { - return func(r io.Reader) (patterns []string, cont bool, err error) { - s := bufio.NewScanner(r) - for s.Scan() { - line := s.Text() - if line == attribute { - return nil, false, nil - } - - if len(line) != 0 { - patterns = append(patterns, path.Join(".", line)) - } +// newFileWalker returns a new file-walker function that reads patterns from an +// [io.Reader]. +func newFileWalker() (fw aghos.FileWalker) { + return func(r io.Reader) (patterns []string, cont bool, err error) { + s := bufio.NewScanner(r) + for s.Scan() { + line := s.Text() + if line == attribute { + return nil, false, nil } - return patterns, true, s.Err() + if len(line) != 0 { + patterns = append(patterns, path.Join(".", line)) + } } + + return patterns, true, s.Err() } +} - const nl = "\n" - +func TestFileWalker_Walk(t *testing.T) { testCases := []struct { testFS fstest.MapFS want assert.BoolAssertionFunc @@ -88,7 +92,7 @@ func TestFileWalker_Walk(t *testing.T) { }} for _, tc := range testCases { - fw := makeFileWalker("") + fw := newFileWalker() t.Run(tc.name, func(t *testing.T) { ok, err := fw.Walk(tc.testFS, tc.initPattern) @@ -100,7 +104,7 @@ func TestFileWalker_Walk(t *testing.T) { t.Run("pattern_malformed", func(t *testing.T) { f := fstest.MapFS{} - ok, err := makeFileWalker("").Walk(f, "[]") + ok, err := newFileWalker().Walk(f, "[]") require.Error(t, err) assert.False(t, ok) diff --git a/internal/aghos/fswatcher.go b/internal/aghos/fswatcher.go index de95e94c..a22293fc 100644 --- a/internal/aghos/fswatcher.go +++ b/internal/aghos/fswatcher.go @@ -1,15 +1,17 @@ package aghos import ( + "context" "fmt" - "io" "io/fs" + "log/slog" "path/filepath" "github.com/AdguardTeam/golibs/container" "github.com/AdguardTeam/golibs/errors" - "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/osutil" + "github.com/AdguardTeam/golibs/service" "github.com/fsnotify/fsnotify" ) @@ -23,11 +25,7 @@ type event = struct{} // // TODO(e.burkov): Add tests. type FSWatcher interface { - // Start starts watching the added files. - Start() (err error) - - // Close stops watching the files and closes an update channel. - io.Closer + service.Interface // Events returns the channel to notify about the file system events. Events() (e <-chan event) @@ -39,6 +37,9 @@ type FSWatcher interface { // osWatcher tracks the file system provided by the OS. type osWatcher struct { + // logger is used for logging the operations of the osWatcher. + logger *slog.Logger + // watcher is the actual notifier that is handled by osWatcher. watcher *fsnotify.Watcher @@ -54,8 +55,8 @@ type osWatcher struct { const osWatcherPref = "os watcher" // NewOSWritesWatcher creates FSWatcher that tracks the real file system of the -// OS and notifies only about writing events. -func NewOSWritesWatcher() (w FSWatcher, err error) { +// OS and notifies only about writing events. l must not be nil. +func NewOSWritesWatcher(l *slog.Logger) (w FSWatcher, err error) { defer func() { err = errors.Annotate(err, "%s: %w", osWatcherPref) }() var watcher *fsnotify.Watcher @@ -65,6 +66,7 @@ func NewOSWritesWatcher() (w FSWatcher, err error) { } return &osWatcher{ + logger: l, watcher: watcher, events: make(chan event, 1), files: container.NewMapSet[string](), @@ -74,16 +76,16 @@ func NewOSWritesWatcher() (w FSWatcher, err error) { // type check var _ FSWatcher = (*osWatcher)(nil) -// Start implements the FSWatcher interface for *osWatcher. -func (w *osWatcher) Start() (err error) { - go w.handleErrors() - go w.handleEvents() +// Start implements the [FSWatcher] interface for *osWatcher. +func (w *osWatcher) Start(ctx context.Context) (err error) { + go w.handleErrors(ctx) + go w.handleEvents(ctx) return nil } -// Close implements the FSWatcher interface for *osWatcher. -func (w *osWatcher) Close() (err error) { +// Shutdown implements the [FSWatcher] interface for *osWatcher. +func (w *osWatcher) Shutdown(_ context.Context) (err error) { return w.watcher.Close() } @@ -120,8 +122,8 @@ func (w *osWatcher) Add(name string) (err error) { // handleEvents notifies about the received file system's event if needed. It // is intended to be used as a goroutine. -func (w *osWatcher) handleEvents() { - defer log.OnPanic(fmt.Sprintf("%s: handling events", osWatcherPref)) +func (w *osWatcher) handleEvents(ctx context.Context) { + defer slogutil.RecoverAndLog(ctx, w.logger) defer close(w.events) @@ -131,33 +133,37 @@ func (w *osWatcher) handleEvents() { continue } - // Skip the following events assuming that sometimes the same event - // occurs several times. - for ok := true; ok; { - select { - case _, ok = <-ch: - // Go on. - default: - ok = false - } - } + skipDuplicates(ch) select { case w.events <- event{}: // Go on. default: - log.Debug("%s: events buffer is full", osWatcherPref) + w.logger.DebugContext(ctx, "events buffer is full") + } + } +} + +// skipDuplicates drains the given channel of events, assuming that some events +// might occur multiple times. +func skipDuplicates(ch <-chan fsnotify.Event) { + for { + select { + case <-ch: + // Go on. + default: + return } } } // handleErrors handles accompanying errors. It used to be called in a separate // goroutine. -func (w *osWatcher) handleErrors() { - defer log.OnPanic(fmt.Sprintf("%s: handling errors", osWatcherPref)) +func (w *osWatcher) handleErrors(ctx context.Context) { + defer slogutil.RecoverAndLog(ctx, w.logger) for err := range w.watcher.Errors { - log.Error("%s: %s", osWatcherPref, err) + w.logger.ErrorContext(ctx, "handling error", slogutil.KeyError, err) } } @@ -170,13 +176,13 @@ var _ FSWatcher = EmptyFSWatcher{} // Start implements the [FSWatcher] interface for EmptyFSWatcher. It always // returns nil error. -func (EmptyFSWatcher) Start() (err error) { +func (EmptyFSWatcher) Start(_ context.Context) (err error) { return nil } -// Close implements the [FSWatcher] interface for EmptyFSWatcher. It always +// Shutdown implements the [FSWatcher] interface for EmptyFSWatcher. It always // returns nil error. -func (EmptyFSWatcher) Close() (err error) { +func (EmptyFSWatcher) Shutdown(_ context.Context) (err error) { return nil } diff --git a/internal/aghos/os.go b/internal/aghos/os.go index 693cf7d2..029df885 100644 --- a/internal/aghos/os.go +++ b/internal/aghos/os.go @@ -5,9 +5,11 @@ package aghos import ( "bufio" + "context" "fmt" "io" "io/fs" + "log/slog" "os" "os/exec" "path" @@ -17,7 +19,6 @@ import ( "strings" "github.com/AdguardTeam/golibs/errors" - "github.com/AdguardTeam/golibs/log" ) // Default file, binary, and directory permissions. @@ -67,8 +68,14 @@ func RunCommand(command string, arguments ...string) (code int, output []byte, e } // PIDByCommand searches for process named command and returns its PID ignoring -// the PIDs from except. If no processes found, the error returned. -func PIDByCommand(command string, except ...int) (pid int, err error) { +// the PIDs from except. If no processes found, the error returned. l must not +// be nil. +func PIDByCommand( + ctx context.Context, + l *slog.Logger, + command string, + except ...int, +) (pid int, err error) { // Don't use -C flag here since it's a feature of linux's ps // implementation. Use POSIX-compatible flags instead. // @@ -101,7 +108,7 @@ func PIDByCommand(command string, except ...int) (pid int, err error) { case 1: // Go on. default: - log.Info("warning: %d %s instances found", instNum, command) + l.WarnContext(ctx, "instances found", "num", instNum, "command", command) } if code := cmd.ProcessState.ExitCode(); code != 0 { diff --git a/internal/aghtest/interface.go b/internal/aghtest/interface.go index 00d537bd..3b07257d 100644 --- a/internal/aghtest/interface.go +++ b/internal/aghtest/interface.go @@ -24,23 +24,23 @@ import ( // FSWatcher is a fake [aghos.FSWatcher] implementation for tests. type FSWatcher struct { - OnStart func() (err error) - OnClose func() (err error) - OnEvents func() (e <-chan struct{}) - OnAdd func(name string) (err error) + OnStart func(ctx context.Context) (err error) + OnShutdown func(ctx context.Context) (err error) + OnEvents func() (e <-chan struct{}) + OnAdd func(name string) (err error) } // type check var _ aghos.FSWatcher = (*FSWatcher)(nil) // Start implements the [aghos.FSWatcher] interface for *FSWatcher. -func (w *FSWatcher) Start() (err error) { - return w.OnStart() +func (w *FSWatcher) Start(ctx context.Context) (err error) { + return w.OnStart(ctx) } -// Close implements the [aghos.FSWatcher] interface for *FSWatcher. -func (w *FSWatcher) Close() (err error) { - return w.OnClose() +// Shutdown implements the [aghos.FSWatcher] interface for *FSWatcher. +func (w *FSWatcher) Shutdown(ctx context.Context) (err error) { + return w.OnShutdown(ctx) } // Events implements the [aghos.FSWatcher] interface for *FSWatcher. diff --git a/internal/aghtls/aghtls.go b/internal/aghtls/aghtls.go index 017bfc12..a1d2c71d 100644 --- a/internal/aghtls/aghtls.go +++ b/internal/aghtls/aghtls.go @@ -2,26 +2,29 @@ package aghtls import ( + "context" "crypto/tls" "crypto/x509" "fmt" + "log/slog" "slices" - "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" ) -// init makes sure that the cipher name map is filled. +// Init populates the cipherSuites map with the name-to-ID mapping of cipher +// suites from crypto/tls. It must be called only once, and it must be called +// before any function that calls [ParseCiphers]. // // TODO(a.garipov): Propose a similar API to crypto/tls. -func init() { +func Init(ctx context.Context, l *slog.Logger) { suites := tls.CipherSuites() cipherSuites = make(map[string]uint16, len(suites)) for _, s := range suites { cipherSuites[s.Name] = s.ID } - log.Debug("tls: known ciphers: %q", cipherSuites) + l.DebugContext(ctx, "known ciphers", "ciphers", cipherSuites) } // cipherSuites are a name-to-ID mapping of cipher suites from crypto/tls. It diff --git a/internal/aghtls/aghtls_test.go b/internal/aghtls/aghtls_test.go index 7e5b99f9..6707e2e7 100644 --- a/internal/aghtls/aghtls_test.go +++ b/internal/aghtls/aghtls_test.go @@ -3,17 +3,20 @@ package aghtls_test import ( "crypto/tls" "testing" + "time" "github.com/AdguardTeam/AdGuardHome/internal/aghtls" + "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" ) -func TestMain(m *testing.M) { - testutil.DiscardLogOutput(m) -} +// testTimeout is a common timeout for tests and contexts. +const testTimeout time.Duration = 1 * time.Second func TestParseCiphers(t *testing.T) { + aghtls.Init(testutil.ContextWithTimeout(t, testTimeout), slogutil.NewDiscardLogger()) + testCases := []struct { name string wantErrMsg string diff --git a/internal/aghtls/root.go b/internal/aghtls/root.go index d81db143..12d96e1d 100644 --- a/internal/aghtls/root.go +++ b/internal/aghtls/root.go @@ -1,7 +1,9 @@ package aghtls import ( + "context" "crypto/x509" + "log/slog" ) // SystemRootCAs tries to load root certificates from the operating system. It @@ -9,6 +11,6 @@ import ( // default algorithm to find system root CA list. // // See https://github.com/AdguardTeam/AdGuardHome/issues/1311. -func SystemRootCAs() (roots *x509.CertPool) { - return rootCAs() +func SystemRootCAs(ctx context.Context, l *slog.Logger) (roots *x509.CertPool) { + return rootCAs(ctx, l) } diff --git a/internal/aghtls/root_linux.go b/internal/aghtls/root_linux.go index 0805f198..781f03f1 100644 --- a/internal/aghtls/root_linux.go +++ b/internal/aghtls/root_linux.go @@ -3,15 +3,17 @@ package aghtls import ( + "context" "crypto/x509" + "log/slog" "os" "path/filepath" "github.com/AdguardTeam/golibs/errors" - "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/logutil/slogutil" ) -func rootCAs() (roots *x509.CertPool) { +func rootCAs(ctx context.Context, l *slog.Logger) (roots *x509.CertPool) { // Directories with the system root certificates, which aren't supported by // Go's crypto/x509. dirs := []string{ @@ -21,36 +23,51 @@ func rootCAs() (roots *x509.CertPool) { roots = x509.NewCertPool() for _, dir := range dirs { - dirEnts, err := os.ReadDir(dir) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - continue - } - - // TODO(a.garipov): Improve error handling here and in other places. - log.Error("aghtls: opening directory %q: %s", dir, err) - } - - var rootsAdded bool - for _, de := range dirEnts { - var certData []byte - rootFile := filepath.Join(dir, de.Name()) - certData, err = os.ReadFile(rootFile) - if err != nil { - log.Error("aghtls: reading root cert: %s", err) - } else { - if roots.AppendCertsFromPEM(certData) { - rootsAdded = true - } else { - log.Error("aghtls: could not add root from %q", rootFile) - } - } - } - - if rootsAdded { + if addCertsFromDir(ctx, l, roots, dir) { return roots } } return nil } + +// addCertsFromDir appends all readable PEM files from dir to pool. It returns +// true if at least one certificate was accepted. +func addCertsFromDir( + ctx context.Context, + l *slog.Logger, + pool *x509.CertPool, + dir string, +) (ok bool) { + dirEnts, err := os.ReadDir(dir) + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + // TODO(a.garipov): Improve error handling here and in other places. + l.ErrorContext(ctx, "opening directory", slogutil.KeyError, err) + } + + return false + } + + var rootsAdded bool + for _, de := range dirEnts { + var certData []byte + rootFile := filepath.Join(dir, de.Name()) + certData, err = os.ReadFile(rootFile) + if err != nil { + l.ErrorContext(ctx, "reading root cert", slogutil.KeyError, err) + + continue + } + + if !pool.AppendCertsFromPEM(certData) { + l.ErrorContext(ctx, "adding root cert", "file", rootFile, slogutil.KeyError, err) + + continue + } + + rootsAdded = true + } + + return rootsAdded +} diff --git a/internal/aghtls/root_others.go b/internal/aghtls/root_others.go index 38a50630..2de146bf 100644 --- a/internal/aghtls/root_others.go +++ b/internal/aghtls/root_others.go @@ -2,8 +2,12 @@ package aghtls -import "crypto/x509" +import ( + "context" + "crypto/x509" + "log/slog" +) -func rootCAs() (roots *x509.CertPool) { +func rootCAs(_ context.Context, _ *slog.Logger) (roots *x509.CertPool) { return nil } diff --git a/internal/dnsforward/dnsforward_internal_test.go b/internal/dnsforward/dnsforward_internal_test.go index 53be7bbc..adb08537 100644 --- a/internal/dnsforward/dnsforward_internal_test.go +++ b/internal/dnsforward/dnsforward_internal_test.go @@ -2,6 +2,7 @@ package dnsforward import ( "cmp" + "context" "crypto/ecdsa" "crypto/rand" "crypto/rsa" @@ -1451,7 +1452,7 @@ func TestPTRResponseFromHosts(t *testing.T) { var eventsCalledCounter uint32 hc, err := aghnet.NewHostsContainer(testFS, &aghtest.FSWatcher{ - OnStart: func() (_ error) { panic("not implemented") }, + OnStart: func(_ context.Context) (_ error) { panic("not implemented") }, OnEvents: func() (e <-chan struct{}) { assert.Equal(t, uint32(1), atomic.AddUint32(&eventsCalledCounter, 1)) @@ -1462,7 +1463,7 @@ func TestPTRResponseFromHosts(t *testing.T) { return nil }, - OnClose: func() (err error) { panic("not implemented") }, + OnShutdown: func(_ context.Context) (err error) { panic("not implemented") }, }, hostsFilename) require.NoError(t, err) t.Cleanup(func() { diff --git a/internal/dnsforward/http_internal_test.go b/internal/dnsforward/http_internal_test.go index f57783c7..2b076281 100644 --- a/internal/dnsforward/http_internal_test.go +++ b/internal/dnsforward/http_internal_test.go @@ -2,6 +2,7 @@ package dnsforward import ( "bytes" + "context" "encoding/json" "io" "net" @@ -355,10 +356,10 @@ func TestServer_HandleTestUpstreamDNS(t *testing.T) { }, }, &aghtest.FSWatcher{ - OnStart: func() (_ error) { panic("not implemented") }, - OnEvents: func() (e <-chan struct{}) { return nil }, - OnAdd: func(_ string) (err error) { return nil }, - OnClose: func() (err error) { return nil }, + OnStart: func(_ context.Context) (_ error) { panic("not implemented") }, + OnEvents: func() (e <-chan struct{}) { return nil }, + OnAdd: func(_ string) (err error) { return nil }, + OnShutdown: func(_ context.Context) (err error) { return nil }, }, hostsFileName, ) diff --git a/internal/filtering/hosts_test.go b/internal/filtering/hosts_test.go index 5c692814..de93394e 100644 --- a/internal/filtering/hosts_test.go +++ b/internal/filtering/hosts_test.go @@ -1,6 +1,7 @@ package filtering_test import ( + "context" "fmt" "net/netip" "testing" @@ -43,10 +44,10 @@ func TestDNSFilter_CheckHost_hostsContainer(t *testing.T) { }, } watcher := &aghtest.FSWatcher{ - OnStart: func() (_ error) { panic("not implemented") }, - OnEvents: func() (e <-chan struct{}) { return nil }, - OnAdd: func(name string) (err error) { return nil }, - OnClose: func() (err error) { return nil }, + OnStart: func(_ context.Context) (_ error) { panic("not implemented") }, + OnEvents: func() (e <-chan struct{}) { return nil }, + OnAdd: func(name string) (err error) { return nil }, + OnShutdown: func(_ context.Context) (err error) { return nil }, } hc, err := aghnet.NewHostsContainer(files, watcher, "hosts") require.NoError(t, err) diff --git a/internal/home/clients_internal_test.go b/internal/home/clients_internal_test.go index 8bbfd185..917b20e0 100644 --- a/internal/home/clients_internal_test.go +++ b/internal/home/clients_internal_test.go @@ -29,7 +29,7 @@ func newClientsContainer(t *testing.T) (c *clientsContainer) { &filtering.Config{ Logger: testLogger, }, - newSignalHandler(nil, nil), + newSignalHandler(testLogger, nil, nil), ) require.NoError(t, err) diff --git a/internal/home/home.go b/internal/home/home.go index 4e377f28..7acd29ad 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -22,6 +22,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/aghslog" + "github.com/AdguardTeam/AdGuardHome/internal/aghtls" "github.com/AdguardTeam/AdGuardHome/internal/arpdb" "github.com/AdguardTeam/AdGuardHome/internal/dhcpd" "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" @@ -114,13 +115,19 @@ func Main(clientBuildFS fs.FS) { // package flag. opts := loadCmdLineOpts() + ls := getLogSettings(opts) + + // TODO(a.garipov): Use slog everywhere. + baseLogger := newSlogLogger(ls) + done := make(chan struct{}) signals := make(chan os.Signal, 1) signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGQUIT) ctx := context.Background() - sigHdlr := newSignalHandler(signals, func(ctx context.Context) { + sigHdlrLogger := baseLogger.With(slogutil.KeyPrefix, "signalhdlr") + sigHdlr := newSignalHandler(sigHdlrLogger, signals, func(ctx context.Context) { cleanup(ctx) cleanupAlways() close(done) @@ -129,24 +136,34 @@ func Main(clientBuildFS fs.FS) { go sigHdlr.handle(ctx) if opts.serviceControlAction != "" { - handleServiceControlAction(opts, clientBuildFS, signals, done, sigHdlr) + svcLogger := baseLogger.With(slogutil.KeyPrefix, "service") + handleServiceControlAction( + ctx, + baseLogger, + svcLogger, + opts, + clientBuildFS, + signals, + done, + sigHdlr, + ) return } // run the protection - run(opts, clientBuildFS, done, sigHdlr) + run(ctx, baseLogger, opts, clientBuildFS, done, sigHdlr) } // setupContext initializes [globalContext] fields. It also reads and upgrades -// config file if necessary. -func setupContext(opts options) (err error) { +// config file if necessary. baseLogger must not be nil. +func setupContext(ctx context.Context, baseLogger *slog.Logger, opts options) (err error) { globalContext.firstRun = detectFirstRun() globalContext.mux = http.NewServeMux() if !opts.noEtcHosts { - err = setupHostsContainer() + err = setupHostsContainer(ctx, baseLogger) if err != nil { // Don't wrap the error, because it's informative enough as is. return err @@ -230,9 +247,9 @@ func configureOS(conf *configuration) (err error) { } // setupHostsContainer initializes the structures to keep up-to-date the hosts -// provided by the OS. -func setupHostsContainer() (err error) { - hostsWatcher, err := aghos.NewOSWritesWatcher() +// provided by the OS. baseLogger must not be nil. +func setupHostsContainer(ctx context.Context, baseLogger *slog.Logger) (err error) { + hostsWatcher, err := aghos.NewOSWritesWatcher(baseLogger.With(slogutil.KeyPrefix, "oswatcher")) if err != nil { log.Info("WARNING: initializing filesystem watcher: %s; not watching for changes", err) @@ -246,7 +263,7 @@ func setupHostsContainer() (err error) { globalContext.etcHosts, err = aghnet.NewHostsContainer(osutil.RootDirFS(), hostsWatcher, paths...) if err != nil { - closeErr := hostsWatcher.Close() + closeErr := hostsWatcher.Shutdown(ctx) if errors.Is(err, aghnet.ErrNoHostsPaths) { log.Info("warning: initing hosts container: %s", err) @@ -256,7 +273,7 @@ func setupHostsContainer() (err error) { return errors.Join(fmt.Errorf("initializing hosts container: %w", err), closeErr) } - return hostsWatcher.Start() + return hostsWatcher.Start(ctx) } // setupOpts sets up command-line options. @@ -603,7 +620,14 @@ func fatalOnError(err error) { // run configures and starts AdGuard Home. // // TODO(e.burkov): Make opts a pointer. -func run(opts options, clientBuildFS fs.FS, done chan struct{}, sigHdlr *signalHandler) { +func run( + ctx context.Context, + slogLogger *slog.Logger, + opts options, + clientBuildFS fs.FS, + done chan struct{}, + sigHdlr *signalHandler, +) { // Configure working dir. err := initWorkingDir(opts) fatalOnError(err) @@ -617,10 +641,6 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}, sigHdlr *signalH err = configureLogger(ls) fatalOnError(err) - // TODO(a.garipov): Use slog everywhere. - slogLogger := newSlogLogger(ls) - sigHdlr.swapLogger(slogLogger) - // Print the first message after logger is configured. log.Info("%s", version.Full()) log.Debug("current working directory is %s", globalContext.workDir) @@ -628,15 +648,14 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}, sigHdlr *signalH log.Info("AdGuard Home is running as a service") } - err = setupContext(opts) + aghtls.Init(ctx, slogLogger.With(slogutil.KeyPrefix, "aghtls")) + + err = setupContext(ctx, slogLogger, opts) fatalOnError(err) err = configureOS(config) fatalOnError(err) - // TODO(s.chzhen): Use it for the entire initialization process. - ctx := context.Background() - // Clients package uses filtering package's static data // (filtering.BlockedSvcKnown()), so we have to initialize filtering static // data first, but also to avoid relying on automatic Go init() function. @@ -646,6 +665,7 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}, sigHdlr *signalH fatalOnError(err) tlsMgrLogger := slogLogger.With(slogutil.KeyPrefix, "tls_manager") + tlsMgr, err := newTLSManager(ctx, &tlsManagerConfig{ logger: tlsMgrLogger, configModified: onConfigModified, @@ -1131,7 +1151,7 @@ func cmdlineUpdate( err = upd.Update(ctx, globalContext.firstRun) fatalOnError(err) - err = restartService() + err = restartService(ctx, l) if err != nil { l.DebugContext(ctx, "restarting service", slogutil.KeyError, err) l.InfoContext(ctx, "AdGuard Home was not installed as a service. "+ diff --git a/internal/home/service.go b/internal/home/service.go index e3e1cab6..af3d4a70 100644 --- a/internal/home/service.go +++ b/internal/home/service.go @@ -1,8 +1,10 @@ package home import ( + "context" "fmt" "io/fs" + "log/slog" "os" "runtime" "strconv" @@ -13,8 +15,9 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/AdGuardHome/internal/version" "github.com/AdguardTeam/golibs/errors" - "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/netutil/urlutil" + "github.com/AdguardTeam/golibs/osutil" "github.com/kardianos/service" ) @@ -32,10 +35,14 @@ const ( // program represents the program that will be launched by as a service or a // daemon. type program struct { + // TODO(s.chzhen): Remove this. + ctx context.Context clientBuildFS fs.FS signals chan os.Signal done chan struct{} opts options + baseLogger *slog.Logger + logger *slog.Logger sigHdlr *signalHandler } @@ -48,14 +55,14 @@ func (p *program) Start(_ service.Service) (err error) { args := p.opts args.runningAsService = true - go run(args, p.clientBuildFS, p.done, p.sigHdlr) + go run(p.ctx, p.baseLogger, args, p.clientBuildFS, p.done, p.sigHdlr) return nil } // Stop implements service.Interface interface for *program. func (p *program) Stop(_ service.Service) (err error) { - log.Info("service: stopping: waiting for cleanup") + p.logger.InfoContext(p.ctx, "stopping: waiting for cleanup") aghos.SendShutdownSignal(p.signals) @@ -84,14 +91,14 @@ func svcStatus(s service.Service) (status service.Status, err error) { return status, err } -// svcAction performs the action on the service. +// svcAction performs the action on the service. l must not be nil. // // On OpenWrt, the service utility may not exist. We use our service script // directly in this case. -func svcAction(s service.Service, action string) (err error) { +func svcAction(ctx context.Context, l *slog.Logger, s service.Service, action string) (err error) { if action == "start" { if err = aghos.PreCheckActionStart(); err != nil { - log.Error("starting service: %s", err) + l.ErrorContext(ctx, "starting service", slogutil.KeyError, err) } } @@ -105,10 +112,10 @@ func svcAction(s service.Service, action string) (err error) { } // Send SIGHUP to a process with PID taken from our .pid file. If it doesn't -// exist, find our PID using 'ps' command. -func sendSigReload() { +// exist, find our PID using 'ps' command. baseLogger and l must not be nil. +func sendSigReload(ctx context.Context, baseLogger, l *slog.Logger) { if runtime.GOOS == "windows" { - log.Error("service: not implemented on windows") + l.ErrorContext(ctx, "not implemented on windows") return } @@ -117,25 +124,26 @@ func sendSigReload() { var pid int data, err := os.ReadFile(pidFile) if errors.Is(err, os.ErrNotExist) { - if pid, err = aghos.PIDByCommand(serviceName, os.Getpid()); err != nil { - log.Error("service: finding AdGuardHome process: %s", err) + aghosLogger := baseLogger.With(slogutil.KeyPrefix, "aghos") + if pid, err = aghos.PIDByCommand(ctx, aghosLogger, serviceName, os.Getpid()); err != nil { + l.ErrorContext(ctx, "finding adguardhome process", slogutil.KeyError, err) return } } else if err != nil { - log.Error("service: reading pid file %s: %s", pidFile, err) + l.ErrorContext(ctx, "reading", "pid_file", pidFile, slogutil.KeyError, err) return } else { parts := strings.SplitN(string(data), "\n", 2) if len(parts) == 0 { - log.Error("service: parsing pid file %s: bad value", pidFile) + l.ErrorContext(ctx, "splitting", "pid_file", pidFile, slogutil.KeyError, "bad value") return } if pid, err = strconv.Atoi(strings.TrimSpace(parts[0])); err != nil { - log.Error("service: parsing pid from file %s: %s", pidFile, err) + l.ErrorContext(ctx, "parsing", "pid_file", pidFile, slogutil.KeyError, err) return } @@ -143,23 +151,23 @@ func sendSigReload() { var proc *os.Process if proc, err = os.FindProcess(pid); err != nil { - log.Error("service: finding process for pid %d: %s", pid, err) + l.ErrorContext(ctx, "finding process for", "pid", pid, slogutil.KeyError, err) return } if err = proc.Signal(syscall.SIGHUP); err != nil { - log.Error("service: sending signal HUP to pid %d: %s", pid, err) + l.ErrorContext(ctx, "sending sighup to", "pid", pid, slogutil.KeyError, err) return } - log.Debug("service: sent signal to pid %d", pid) + l.DebugContext(ctx, "sent sighup to", "pid", pid) } // restartService restarts the service. It returns error if the service is not -// running. -func restartService() (err error) { +// running. l must not be nil. +func restartService(ctx context.Context, l *slog.Logger) (err error) { // Call chooseSystem explicitly to introduce OpenBSD support for service // package. It's a noop for other GOOS values. chooseSystem() @@ -182,7 +190,7 @@ func restartService() (err error) { return fmt.Errorf("initializing service: %w", err) } - if err = svcAction(s, "restart"); err != nil { + if err = svcAction(ctx, l, s, "restart"); err != nil { return fmt.Errorf("restarting service: %w", err) } @@ -201,6 +209,9 @@ func restartService() (err error) { // it is specified when we register a service, and it indicates to the app // that it is being run as a service/daemon. func handleServiceControlAction( + ctx context.Context, + baseLogger *slog.Logger, + l *slog.Logger, opts options, clientBuildFS fs.FS, signals chan os.Signal, @@ -212,25 +223,26 @@ func handleServiceControlAction( chooseSystem() action := opts.serviceControlAction - log.Info("%s", version.Full()) - log.Info("service: control action: %s", action) + l.InfoContext(ctx, version.Full()) + l.InfoContext(ctx, "control", "action", action) if action == "reload" { - sendSigReload() + sendSigReload(ctx, baseLogger, l) return } pwd, err := os.Getwd() if err != nil { - log.Fatalf("service: getting current directory: %s", err) + l.ErrorContext(ctx, "getting current directory", slogutil.KeyError, err) + os.Exit(osutil.ExitCodeFailure) } runOpts := opts runOpts.serviceControlAction = "run" args := optsToArgs(runOpts) - log.Debug("service: using args %q", args) + l.DebugContext(ctx, "using", "args", args) svcConfig := &service.Config{ Name: serviceName, @@ -242,33 +254,45 @@ func handleServiceControlAction( configureService(svcConfig) s, err := service.New(&program{ + ctx: ctx, clientBuildFS: clientBuildFS, signals: signals, done: done, opts: runOpts, + baseLogger: l, + logger: l.With(slogutil.KeyPrefix, "service"), sigHdlr: sigHdlr, }, svcConfig) if err != nil { - log.Fatalf("service: initializing service: %s", err) + l.ErrorContext(ctx, "initializing service", slogutil.KeyError, err) + os.Exit(osutil.ExitCodeFailure) } - err = handleServiceCommand(s, action, opts) + err = handleServiceCommand(ctx, l, s, action, opts) if err != nil { - log.Fatalf("service: %s", err) + l.ErrorContext(ctx, "handling command", slogutil.KeyError, err) + os.Exit(osutil.ExitCodeFailure) } - log.Printf( - "service: action %s has been done successfully on %s", - action, - service.ChosenSystem(), + l.InfoContext( + ctx, + "action has been done successfully", + "action", action, + "system", service.ChosenSystem(), ) } // handleServiceCommand handles service command. -func handleServiceCommand(s service.Service, action string, opts options) (err error) { +func handleServiceCommand( + ctx context.Context, + l *slog.Logger, + s service.Service, + action string, + opts options, +) (err error) { switch action { case "status": - handleServiceStatusCommand(s) + handleServiceStatusCommand(ctx, l, s) case "run": if err = s.Run(); err != nil { return fmt.Errorf("failed to run service: %w", err) @@ -280,11 +304,11 @@ func handleServiceCommand(s service.Service, action string, opts options) (err e initConfigFilename(opts) - handleServiceInstallCommand(s) + handleServiceInstallCommand(ctx, l, s) case "uninstall": - handleServiceUninstallCommand(s) + handleServiceUninstallCommand(ctx, l, s) default: - if err = svcAction(s, action); err != nil { + if err = svcAction(ctx, l, s, action); err != nil { return fmt.Errorf("executing action %q: %w", action, err) } } @@ -297,29 +321,35 @@ func handleServiceCommand(s service.Service, action string, opts options) (err e const statusRestartOnFail = service.StatusStopped + 1 // handleServiceStatusCommand handles service "status" command. -func handleServiceStatusCommand(s service.Service) { +func handleServiceStatusCommand( + ctx context.Context, + l *slog.Logger, + s service.Service, +) { status, errSt := svcStatus(s) if errSt != nil { - log.Fatalf("service: failed to get service status: %s", errSt) + l.ErrorContext(ctx, "failed to get service status", slogutil.KeyError, errSt) + os.Exit(osutil.ExitCodeFailure) } switch status { case service.StatusUnknown: - log.Printf("service: status is unknown") + l.InfoContext(ctx, "status is unknown") case service.StatusStopped: - log.Printf("service: stopped") + l.InfoContext(ctx, "stopped") case service.StatusRunning: - log.Printf("service: running") + l.InfoContext(ctx, "running") case statusRestartOnFail: - log.Printf("service: restarting after failed start") + l.InfoContext(ctx, "restarting after failed start") } } // handleServiceInstallCommand handles service "install" command. -func handleServiceInstallCommand(s service.Service) { - err := svcAction(s, "install") +func handleServiceInstallCommand(ctx context.Context, l *slog.Logger, s service.Service) { + err := svcAction(ctx, l, s, "install") if err != nil { - log.Fatalf("service: executing action %q: %s", "install", err) + l.ErrorContext(ctx, "executing install", slogutil.KeyError, err) + os.Exit(osutil.ExitCodeFailure) } if aghos.IsOpenWrt() { @@ -328,56 +358,60 @@ func handleServiceInstallCommand(s service.Service) { // startup. _, err = runInitdCommand("enable") if err != nil { - log.Fatalf("service: running init enable: %s", err) + l.ErrorContext(ctx, "running init enable", slogutil.KeyError, err) + os.Exit(osutil.ExitCodeFailure) } } // Start automatically after install. - err = svcAction(s, "start") + err = svcAction(ctx, l, s, "start") if err != nil { - log.Fatalf("service: starting: %s", err) + l.ErrorContext(ctx, "starting", slogutil.KeyError, err) + os.Exit(osutil.ExitCodeFailure) } - log.Printf("service: started") + l.InfoContext(ctx, "started") if detectFirstRun() { - log.Printf(`Almost ready! -AdGuard Home is successfully installed and will automatically start on boot. -There are a few more things that must be configured before you can use it. -Click on the link below and follow the Installation Wizard steps to finish setup. -AdGuard Home is now available at the following addresses:`) + slogutil.PrintLines(ctx, l, slog.LevelInfo, "", "Almost ready!\n"+ + "AdGuard Home is successfully installed and will automatically start on boot.\n"+ + "There are a few more things that must be configured before you can use it.\n"+ + "Click on the link below and follow the Installation Wizard steps to finish setup.\n"+ + "AdGuard Home is now available at the following addresses:") printHTTPAddresses(urlutil.SchemeHTTP, nil) } } // handleServiceUninstallCommand handles service "uninstall" command. -func handleServiceUninstallCommand(s service.Service) { +func handleServiceUninstallCommand(ctx context.Context, l *slog.Logger, s service.Service) { if aghos.IsOpenWrt() { // On OpenWrt it is important to run disable command first // as it will remove the symlink _, err := runInitdCommand("disable") if err != nil { - log.Fatalf("service: running init disable: %s", err) + l.ErrorContext(ctx, "running init disable", slogutil.KeyError, err) + os.Exit(osutil.ExitCodeFailure) } } - if err := svcAction(s, "stop"); err != nil { - log.Debug("service: executing action %q: %s", "stop", err) + if err := svcAction(ctx, l, s, "stop"); err != nil { + l.DebugContext(ctx, "executing action stop", slogutil.KeyError, err) } - if err := svcAction(s, "uninstall"); err != nil { - log.Fatalf("service: executing action %q: %s", "uninstall", err) + if err := svcAction(ctx, l, s, "uninstall"); err != nil { + l.ErrorContext(ctx, "executing action uninstall", slogutil.KeyError, err) + os.Exit(osutil.ExitCodeFailure) } if runtime.GOOS == "darwin" { // Remove log files on cleanup and log errors. err := os.Remove(launchdStdoutPath) if err != nil && !errors.Is(err, os.ErrNotExist) { - log.Info("service: warning: removing stdout file: %s", err) + l.WarnContext(ctx, "removing stdout file", slogutil.KeyError, err) } err = os.Remove(launchdStderrPath) if err != nil && !errors.Is(err, os.ErrNotExist) { - log.Info("service: warning: removing stderr file: %s", err) + l.WarnContext(ctx, "removing stderr file", slogutil.KeyError, err) } } } diff --git a/internal/home/signal.go b/internal/home/signal.go index 638d3632..04822ff5 100644 --- a/internal/home/signal.go +++ b/internal/home/signal.go @@ -5,7 +5,6 @@ import ( "log/slog" "os" "sync" - "sync/atomic" "syscall" "github.com/AdguardTeam/AdGuardHome/internal/client" @@ -16,10 +15,8 @@ import ( // signalHandler processes incoming signals. It reloads configurations of // stored entities on SIGHUP and performs cleanup on all other signals. type signalHandler struct { - // logger is used to log the operation of the signal handler. Initially, - // [slog.Default] is used, but it should be swapped later using - // [signalHandler.swapLogger]. - logger *atomic.Pointer[slog.Logger] + // logger is used to log the operation of the signal handler. + logger *slog.Logger // mu protects clientStorage and tlsManager. mu *sync.Mutex @@ -41,24 +38,16 @@ type signalHandler struct { // newSignalHandler returns a new properly initialized *signalHandler. func newSignalHandler( + l *slog.Logger, signals <-chan os.Signal, cleanup func(ctx context.Context), ) (h *signalHandler) { - h = &signalHandler{ - logger: &atomic.Pointer[slog.Logger]{}, + return &signalHandler{ + logger: l, mu: &sync.Mutex{}, signals: signals, cleanup: cleanup, } - - h.logger.Store(slog.Default()) - - return h -} - -// swapLogger replaces the stored logger with the given logger. -func (h *signalHandler) swapLogger(logger *slog.Logger) { - h.logger.Swap(logger) } // addClientStorage stores the client storage. @@ -89,14 +78,14 @@ func (h *signalHandler) handle(ctx context.Context) { return } - slogutil.PrintRecovered(ctx, h.logger.Load(), v) + slogutil.PrintRecovered(ctx, h.logger, v) os.Exit(osutil.ExitCodeFailure) }() for { sig := <-h.signals - h.logger.Load().InfoContext(ctx, "received signal", "signal", sig) + h.logger.InfoContext(ctx, "received signal", "signal", sig) switch sig { case syscall.SIGHUP: h.reloadConfig(ctx) diff --git a/internal/home/tls.go b/internal/home/tls.go index 058a4ba0..25ce29da 100644 --- a/internal/home/tls.go +++ b/internal/home/tls.go @@ -99,7 +99,7 @@ func newTLSManager(ctx context.Context, conf *tlsManagerConfig) (m *tlsManager, servePlainDNS: conf.servePlainDNS, } - m.rootCerts = aghtls.SystemRootCAs() + m.rootCerts = aghtls.SystemRootCAs(ctx, conf.logger) if len(conf.tlsSettings.OverrideTLSCiphers) > 0 { m.customCipherIDs, err = aghtls.ParseCiphers(config.TLS.OverrideTLSCiphers) diff --git a/internal/updater/updater.go b/internal/updater/updater.go index 93138230..22c44e31 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -145,19 +145,15 @@ func NewUpdater(conf *Config) *Updater { } } -// Update performs the auto-update. It returns an error if the update failed. +// Update performs the auto-update. It returns an error if the update fails. // If firstRun is true, it assumes the configuration file doesn't exist. func (u *Updater) Update(ctx context.Context, firstRun bool) (err error) { u.mu.Lock() defer u.mu.Unlock() - u.logger.InfoContext(ctx, "staring update", "first_run", firstRun) + u.logger.InfoContext(ctx, "starting update", "first_run", firstRun) defer func() { - if err != nil { - u.logger.ErrorContext(ctx, "update failed", slogutil.KeyError, err) - } else { - u.logger.InfoContext(ctx, "update finished") - } + u.logUpdateResult(ctx, err) }() err = u.prepare(ctx) @@ -197,6 +193,17 @@ func (u *Updater) Update(ctx context.Context, firstRun bool) (err error) { return nil } +// logUpdateResult logs the result of the update operation. +func (u *Updater) logUpdateResult(ctx context.Context, err error) { + if err != nil { + u.logger.ErrorContext(ctx, "update failed", slogutil.KeyError, err) + + return + } + + u.logger.InfoContext(ctx, "update finished") +} + // NewVersion returns the available new version. func (u *Updater) NewVersion() (nv string) { u.mu.RLock() diff --git a/scripts/make/go-lint.sh b/scripts/make/go-lint.sh index 84a4e5b8..62bda9a9 100644 --- a/scripts/make/go-lint.sh +++ b/scripts/make/go-lint.sh @@ -169,12 +169,7 @@ run_linter gocognit --over='19' \ ./internal/home/ \ ; -run_linter gocognit --over='18' \ - ./internal/aghtls/ \ - ; - run_linter gocognit --over='15' \ - ./internal/aghos/ \ ./internal/filtering/ \ ; @@ -190,15 +185,13 @@ run_linter gocognit --over='12' \ ./internal/filtering/rewrite/ \ ; -run_linter gocognit --over='11' \ - ./internal/updater/ \ - ; - run_linter gocognit --over='10' \ ./internal/aghalg/ \ ./internal/aghhttp/ \ + ./internal/aghos/ \ ./internal/aghrenameio/ \ ./internal/aghtest/ \ + ./internal/aghtls/ \ ./internal/aghuser/ \ ./internal/arpdb/ \ ./internal/client/ \ @@ -213,6 +206,7 @@ run_linter gocognit --over='10' \ ./internal/rdns/ \ ./internal/schedule/ \ ./internal/stats/ \ + ./internal/updater/ \ ./internal/version/ \ ./internal/whois/ \ ./scripts/ \