Pull request: upd-dnsproxy

Squashed commit of the following:

commit 4638117
Merge: 3de6224 130560b
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed Jul 10 15:06:01 2024 +0300

    Merge remote-tracking branch 'origin/master' into upd-dnsproxy

commit 3de6224
Merge: e2de50b e269260
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed Jul 10 09:13:40 2024 +0300

    Merge remote-tracking branch 'origin/master' into upd-dnsproxy

commit e2de50b
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed Jul 10 09:11:25 2024 +0300

    home: todos

commit 58fe497
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Jul 9 13:29:19 2024 +0300

    home: imp code

commit 4db7cdc
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Jul 9 11:31:12 2024 +0300

    all: imp code

commit 7e8d3b5
Merge: 559c3b7 9a6dd0d
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jul 8 10:56:14 2024 +0300

    Merge remote-tracking branch 'origin/master' into upd-dnsproxy

commit 559c3b7
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jul 8 10:54:03 2024 +0300

    dnsforward: imp code

commit ba4a7e1
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jul 8 10:49:46 2024 +0300

    aghos: imp code

commit cdf9ccd
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Jul 5 16:19:27 2024 +0300

    all: partial revert slog logger usage

commit f16cddb
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Jul 5 13:01:37 2024 +0300

    all: upd dnsproxy

commit 5932c8d
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Jul 5 11:49:37 2024 +0300

    dnsforward: slog logger

commit 3d7f734
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Jul 5 11:05:14 2024 +0300

    all: slog logger

commit 9a74d5d
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu Jul 4 12:16:21 2024 +0300

    all: upd dnsproxy

commit 537bdac
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu Jul 4 12:10:30 2024 +0300

    all: upd dnsproxy

commit 38e10de
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu Jul 4 10:37:50 2024 +0300

    dnsforward: upstream mode

go.mod

@@ -3,8 +3,8 @@ module github.com/AdguardTeam/AdGuardHome
 go 1.22.5
 
 require (
-	github.com/AdguardTeam/dnsproxy v0.71.2
-	github.com/AdguardTeam/golibs v0.24.0
+	github.com/AdguardTeam/dnsproxy v0.72.0
+	github.com/AdguardTeam/golibs v0.24.1
 	github.com/AdguardTeam/urlfilter v0.19.0
 	github.com/NYTimes/gziphandler v1.1.1
 	github.com/ameshkov/dnscrypt/v2 v2.3.0

go.sum

@@ -1,7 +1,7 @@
-github.com/AdguardTeam/dnsproxy v0.71.2 h1:dFG2wga4GDdj1eI3rU2wqjQ6QGQm9MjLRb5ZzyH3Vgg=
-github.com/AdguardTeam/dnsproxy v0.71.2/go.mod h1:huI5zyWhlimHBhg0jt2CMinXzsEHymI+WlvxIfmfEGA=
-github.com/AdguardTeam/golibs v0.24.0 h1:qAnOq7BQtwSVo7Co9q703/n+nZ2Ap6smkugU9G9MomY=
-github.com/AdguardTeam/golibs v0.24.0/go.mod h1:9/vJcYznW7RlmCT/Qzi8XNZGj+ZbWfHZJmEXKnRpCAU=
+github.com/AdguardTeam/dnsproxy v0.72.0 h1:Psn7uCMVR/dCx8Te2Iy05bWWRNArSBF9j38VXNtt6+4=
+github.com/AdguardTeam/dnsproxy v0.72.0/go.mod h1:5ehzbfInAu07not4beAM+FlFPqntw18T1sQCK/kIQR8=
+github.com/AdguardTeam/golibs v0.24.1 h1:/ulkfm65wi33p72ybxiOt3lSdP0nr1GggSoaT4sHbns=
+github.com/AdguardTeam/golibs v0.24.1/go.mod h1:9/vJcYznW7RlmCT/Qzi8XNZGj+ZbWfHZJmEXKnRpCAU=
 github.com/AdguardTeam/urlfilter v0.19.0 h1:q7eH13+yNETlpD/VD3u5rLQOripcUdEktqZFy+KiQLk=
 github.com/AdguardTeam/urlfilter v0.19.0/go.mod h1:+N54ZvxqXYLnXuvpaUhK2exDQW+djZBRSb6F6j0rkBY=
 github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I=

internal/aghos/syslog.go

@@ -1,6 +1,6 @@
 package aghos
 
 // ConfigureSyslog reroutes standard logger output to syslog.
-func ConfigureSyslog(serviceName string) error {
+func ConfigureSyslog(serviceName string) (err error) {
 	return configureSyslog(serviceName)
 }

internal/aghos/syslog_others.go

@@ -8,11 +8,15 @@ import (
 	"github.com/AdguardTeam/golibs/log"
 )
 
-func configureSyslog(serviceName string) error {
+// configureSyslog sets standard log output to syslog.
+func configureSyslog(serviceName string) (err error) {
 	w, err := syslog.New(syslog.LOG_NOTICE|syslog.LOG_USER, serviceName)
 	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
 		return err
 	}
+
 	log.SetOutput(w)
+
 	return nil
 }

internal/aghos/syslog_windows.go

@@ -19,23 +19,30 @@ func (w *eventLogWriter) Write(b []byte) (int, error) {
 	return len(b), w.el.Info(1, string(b))
 }
 
-func configureSyslog(serviceName string) error {
-	// Note that the eventlog src is the same as the service name
-	// Otherwise, we will get "the description for event id cannot be found" warning in every log record
+// configureSyslog sets standard log output to event log.
+func configureSyslog(serviceName string) (err error) {
+	// Note that the eventlog src is the same as the service name, otherwise we
+	// will get "the description for event id cannot be found" warning in every
+	// log record.
 
 	// Continue if we receive "registry key already exists" or if we get
 	// ERROR_ACCESS_DENIED so that we can log without administrative permissions
 	// for pre-existing eventlog sources.
-	if err := eventlog.InstallAsEventCreate(serviceName, eventlog.Info|eventlog.Warning|eventlog.Error); err != nil {
-		if !strings.Contains(err.Error(), "registry key already exists") && err != windows.ERROR_ACCESS_DENIED {
-			return err
-		}
+	err = eventlog.InstallAsEventCreate(serviceName, eventlog.Info|eventlog.Warning|eventlog.Error)
+	if err != nil &&
+		!strings.Contains(err.Error(), "registry key already exists") &&
+		err != windows.ERROR_ACCESS_DENIED {
+		// Don't wrap the error, because it's informative enough as is.
+		return err
 	}
+
 	el, err := eventlog.Open(serviceName)
 	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
 		return err
 	}
 
 	log.SetOutput(&eventLogWriter{el: el})
+
 	return nil
 }

internal/dnsforward/clientid_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/AdguardTeam/dnsproxy/proxy"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/quic-go/quic-go"
 	"github.com/stretchr/testify/assert"
@@ -217,7 +218,8 @@ func TestServer_clientIDFromDNSContext(t *testing.T) {
 			}
 
 			srv := &Server{
-				conf: ServerConfig{TLSConfig: tlsConf},
+				conf:   ServerConfig{TLSConfig: tlsConf},
+				logger: slogutil.NewDiscardLogger(),
 			}
 
 			var (

internal/dnsforward/config.go

@@ -22,6 +22,7 @@ import (
 	"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/netutil"
 	"github.com/AdguardTeam/golibs/stringutil"
 	"github.com/AdguardTeam/golibs/timeutil"
@@ -301,6 +302,8 @@ type ServerConfig struct {
 
 // UpstreamMode is a enumeration of upstream mode representations.  See
 // [proxy.UpstreamModeType].
+//
+// TODO(d.kolyshev): Consider using [proxy.UpstreamMode].
 type UpstreamMode string
 
 const (
@@ -339,6 +342,10 @@ func (s *Server) newProxyConfig() (conf *proxy.Config, err error) {
 		MessageConstructor:        s,
 	}
 
+	if s.logger != nil {
+		conf.Logger = s.logger.With(slogutil.KeyPrefix, "dnsproxy")
+	}
+
 	if srvConf.EDNSClientSubnet.UseCustom {
 		// TODO(s.chzhen):  Use netip.Addr instead of net.IP inside dnsproxy.
 		conf.EDNSAddr = net.IP(srvConf.EDNSClientSubnet.CustomIP.AsSlice())

internal/dnsforward/dnsforward.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"log/slog"
 	"net"
 	"net/http"
 	"net/netip"
@@ -27,6 +28,7 @@ import (
 	"github.com/AdguardTeam/golibs/cache"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/netutil/sysresolv"
 	"github.com/AdguardTeam/golibs/stringutil"
@@ -121,6 +123,12 @@ type Server struct {
 	// access drops disallowed clients.
 	access *accessManager
 
+	// logger is used for logging during server routines.
+	//
+	// TODO(d.kolyshev): Make it never nil.
+	// TODO(d.kolyshev): Use this logger.
+	logger *slog.Logger
+
 	// localDomainSuffix is the suffix used to detect internal hosts.  It
 	// must be a valid domain name plus dots on each side.
 	localDomainSuffix string
@@ -197,6 +205,10 @@ type DNSCreateParams struct {
 	PrivateNets netutil.SubnetSet
 	Anonymizer  *aghnet.IPMut
 	EtcHosts    *aghnet.HostsContainer
+
+	// Logger is used as a base logger.  It must not be nil.
+	Logger *slog.Logger
+
 	LocalDomain string
 }
 
@@ -233,6 +245,7 @@ func NewServer(p DNSCreateParams) (s *Server, err error) {
 		stats:       p.Stats,
 		queryLog:    p.QueryLog,
 		privateNets: p.PrivateNets,
+		logger:      p.Logger.With(slogutil.KeyPrefix, "dnsforward"),
 		// TODO(e.burkov):  Use some case-insensitive string comparison.
 		localDomainSuffix: strings.ToLower(localDomainSuffix),
 		etcHosts:          etcHosts,
@@ -719,6 +732,10 @@ func (s *Server) prepareInternalProxy() (err error) {
 		MessageConstructor:        s,
 	}
 
+	if s.logger != nil {
+		conf.Logger = s.logger.With(slogutil.KeyPrefix, "dnsproxy")
+	}
+
 	err = setProxyUpstreamMode(conf, srvConf.UpstreamMode, srvConf.FastestTimeout.Duration)
 	if err != nil {
 		return fmt.Errorf("invalid upstream mode: %w", err)

internal/dnsforward/dnsforward_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering/safesearch"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/AdguardTeam/golibs/timeutil"
@@ -99,6 +100,7 @@ func createTestServer(
 		DHCPServer:  dhcp,
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
+		Logger:      slogutil.NewDiscardLogger(),
 	})
 	require.NoError(t, err)
 
@@ -339,7 +341,10 @@ func TestServer_timeout(t *testing.T) {
 			ServePlainDNS: true,
 		}
 
-		s, err := NewServer(DNSCreateParams{DNSFilter: createTestDNSFilter(t)})
+		s, err := NewServer(DNSCreateParams{
+			DNSFilter: createTestDNSFilter(t),
+			Logger:    slogutil.NewDiscardLogger(),
+		})
 		require.NoError(t, err)
 
 		err = s.Prepare(srvConf)
@@ -349,7 +354,10 @@ func TestServer_timeout(t *testing.T) {
 	})
 
 	t.Run("default", func(t *testing.T) {
-		s, err := NewServer(DNSCreateParams{DNSFilter: createTestDNSFilter(t)})
+		s, err := NewServer(DNSCreateParams{
+			DNSFilter: createTestDNSFilter(t),
+			Logger:    slogutil.NewDiscardLogger(),
+		})
 		require.NoError(t, err)
 
 		s.conf.Config.UpstreamMode = UpstreamModeLoadBalance
@@ -376,7 +384,9 @@ func TestServer_Prepare_fallbacks(t *testing.T) {
 		ServePlainDNS: true,
 	}
 
-	s, err := NewServer(DNSCreateParams{})
+	s, err := NewServer(DNSCreateParams{
+		Logger: slogutil.NewDiscardLogger(),
+	})
 	require.NoError(t, err)
 
 	err = s.Prepare(srvConf)
@@ -962,6 +972,7 @@ func TestBlockedCustomIP(t *testing.T) {
 		DHCPServer:  dhcp,
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
+		Logger:      slogutil.NewDiscardLogger(),
 	})
 	require.NoError(t, err)
 
@@ -1127,6 +1138,7 @@ func TestRewrite(t *testing.T) {
 		DHCPServer:  dhcp,
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
+		Logger:      slogutil.NewDiscardLogger(),
 	})
 	require.NoError(t, err)
 
@@ -1256,6 +1268,7 @@ func TestPTRResponseFromDHCPLeases(t *testing.T) {
 			},
 		},
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
+		Logger:      slogutil.NewDiscardLogger(),
 		LocalDomain: localDomain,
 	})
 	require.NoError(t, err)
@@ -1341,6 +1354,7 @@ func TestPTRResponseFromHosts(t *testing.T) {
 		DHCPServer:  dhcp,
 		DNSFilter:   flt,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
+		Logger:      slogutil.NewDiscardLogger(),
 	})
 	require.NoError(t, err)
 
@@ -1392,24 +1406,29 @@ func TestNewServer(t *testing.T) {
 		in         DNSCreateParams
 		wantErrMsg string
 	}{{
-		name:       "success",
-		in:         DNSCreateParams{},
+		name: "success",
+		in: DNSCreateParams{
+			Logger: slogutil.NewDiscardLogger(),
+		},
 		wantErrMsg: "",
 	}, {
 		name: "success_local_tld",
 		in: DNSCreateParams{
+			Logger:      slogutil.NewDiscardLogger(),
 			LocalDomain: "mynet",
 		},
 		wantErrMsg: "",
 	}, {
 		name: "success_local_domain",
 		in: DNSCreateParams{
+			Logger:      slogutil.NewDiscardLogger(),
 			LocalDomain: "my.local.net",
 		},
 		wantErrMsg: "",
 	}, {
 		name: "bad_local_domain",
 		in: DNSCreateParams{
+			Logger:      slogutil.NewDiscardLogger(),
 			LocalDomain: "!!!",
 		},
 		wantErrMsg: `local domain: bad domain name "!!!": ` +

internal/dnsforward/filter_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
@@ -57,6 +58,7 @@ func TestHandleDNSRequest_handleDNSRequest(t *testing.T) {
 		},
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
+		Logger:      slogutil.NewDiscardLogger(),
 	})
 	require.NoError(t, err)
 
@@ -229,6 +231,7 @@ func TestHandleDNSRequest_filterDNSResponse(t *testing.T) {
 		DHCPServer:  &testDHCP{},
 		DNSFilter:   f,
 		PrivateNets: netutil.SubnetSetFunc(netutil.IsLocallyServed),
+		Logger:      slogutil.NewDiscardLogger(),
 	})
 	require.NoError(t, err)
 

internal/dnsforward/process.go

@@ -159,7 +159,7 @@ func (s *Server) processInitial(dctx *dnsContext) (rc resultCode) {
 	q := pctx.Req.Question[0]
 	qt := q.Qtype
 	if s.conf.AAAADisabled && qt == dns.TypeAAAA {
-		_ = proxy.CheckDisabledAAAARequest(pctx, true)
+		pctx.Res = s.newMsgNODATA(pctx.Req)
 
 		return resultCodeFinish
 	}

internal/dnsforward/process_internal_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/AdguardTeam/urlfilter/rules"
@@ -430,6 +431,7 @@ func TestServer_ProcessDHCPHosts_localRestriction(t *testing.T) {
 				dnsFilter:         createTestDNSFilter(t),
 				dhcpServer:        dhcp,
 				localDomainSuffix: localDomainSuffix,
+				logger:            slogutil.NewDiscardLogger(),
 			}
 
 			req := &dns.Msg{
@@ -565,6 +567,7 @@ func TestServer_ProcessDHCPHosts(t *testing.T) {
 			dnsFilter:         createTestDNSFilter(t),
 			dhcpServer:        testDHCP,
 			localDomainSuffix: tc.suffix,
+			logger:            slogutil.NewDiscardLogger(),
 		}
 
 		req := &dns.Msg{

internal/dnsforward/stats_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/stats"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -202,6 +203,7 @@ func TestServer_ProcessQueryLogsAndStats(t *testing.T) {
 		ql := &testQueryLog{}
 		st := &testStats{}
 		srv := &Server{
+			logger:     slogutil.NewDiscardLogger(),
 			queryLog:   ql,
 			stats:      st,
 			anonymizer: aghnet.NewIPMut(nil),

internal/dnsforward/upstreams.go

@@ -150,12 +150,12 @@ func setProxyUpstreamMode(
 ) (err error) {
 	switch upstreamMode {
 	case UpstreamModeParallel:
-		conf.UpstreamMode = proxy.UModeParallel
+		conf.UpstreamMode = proxy.UpstreamModeParallel
 	case UpstreamModeFastestAddr:
-		conf.UpstreamMode = proxy.UModeFastestAddr
+		conf.UpstreamMode = proxy.UpstreamModeFastestAddr
 		conf.FastestPingTimeout = fastestTimeout
 	case UpstreamModeLoadBalance:
-		conf.UpstreamMode = proxy.UModeLoadBalance
+		conf.UpstreamMode = proxy.UpstreamModeLoadBalance
 	default:
 		return fmt.Errorf("unexpected value %q", upstreamMode)
 	}