Merge branch 'master' into AGDNS-2862-math-rand-v2

Author: schzhn

proxy/proxy.go

@@ -750,21 +750,20 @@ func (p *Proxy) Resolve(dctx *DNSContext) (err error) {
 func (p *Proxy) cacheWorks(dctx *DNSContext) (ok bool) {
 	var reason string
 	switch {
-	case p.cache == nil:
-		reason = "disabled"
-	case dctx.RequestedPrivateRDNS != netip.Prefix{}:
-		// Don't cache the requests intended for local upstream servers, those
-		// should be fast enough as is.
-		reason = "requested address is private"
 	case dctx.CustomUpstreamConfig != nil && dctx.CustomUpstreamConfig.cache == nil:
-		// In case of custom upstream cache is not configured, the global proxy
-		// cache cannot be used because different upstreams can return different
-		// results.
+		// If custom upstreams are used but the custom upstream cache is
+		// disabled, return false to prevent storing results in the global
+		// cache.
 		//
 		// See https://github.com/AdguardTeam/dnsproxy/issues/169.
-		//
-		// TODO(e.burkov):  It probably should be decided after resolve.
 		reason = "custom upstreams cache is not configured"
+	case p.cache == nil &&
+		(dctx.CustomUpstreamConfig == nil || dctx.CustomUpstreamConfig.cache == nil):
+		reason = "caching disabled: neither global cache nor custom upstreams cache is configured"
+	case dctx.RequestedPrivateRDNS != netip.Prefix{}:
+		// Don't cache the requests intended for local upstream servers, those
+		// should be fast enough as is.
+		reason = "requested address is private"
 	case dctx.Req.CheckingDisabled:
 		// Also don't lookup the cache for responses with DNSSEC checking
 		// disabled since only validated responses are cached and those may be

proxy/proxy_test.go

@@ -0,0 +1,145 @@
+package proxy_test
+
+import (
+	"net"
+	"testing"
+
+	"github.com/AdguardTeam/dnsproxy/internal/dnsproxytest"
+	"github.com/AdguardTeam/dnsproxy/proxy"
+	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/testutil"
+	"github.com/miekg/dns"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// newCustomUpstreamConfig is a helper function that returns an initialized
+// [*proxy.CustomUpstreamConfig].
+func newCustomUpstreamConfig(ups upstream.Upstream, enabled bool) (c *proxy.CustomUpstreamConfig) {
+	return proxy.NewCustomUpstreamConfig(
+		&proxy.UpstreamConfig{Upstreams: []upstream.Upstream{ups}},
+		enabled,
+		0,
+		false,
+	)
+}
+
+// isCachedWithCustomConfig is a helper function that returns the caching
+// results of a constructed request using the provided custom upstream
+// configuration and FQDN.
+func isCachedWithCustomConfig(
+	tb testing.TB,
+	p *proxy.Proxy,
+	conf *proxy.CustomUpstreamConfig,
+	fqdn string,
+) (isCached bool) {
+	tb.Helper()
+
+	d := &proxy.DNSContext{
+		CustomUpstreamConfig: conf,
+		Req:                  (&dns.Msg{}).SetQuestion(fqdn, dns.TypeA),
+	}
+
+	err := p.Resolve(d)
+	require.NoError(tb, err)
+
+	qs := d.QueryStatistics()
+	require.NotNil(tb, qs)
+
+	s := qs.Main()
+	require.Len(tb, s, 1)
+
+	return s[0].IsCached
+}
+
+func TestProxy_Resolve_cache(t *testing.T) {
+	const host = "example.test."
+
+	ups := &dnsproxytest.FakeUpstream{
+		OnAddress: func() (addr string) { return "stub" },
+		OnClose:   func() (err error) { return nil },
+	}
+	ups.OnExchange = func(req *dns.Msg) (resp *dns.Msg, err error) {
+		resp = (&dns.Msg{}).SetReply(req)
+		resp.Answer = append(resp.Answer, &dns.A{
+			Hdr: dns.RR_Header{
+				Name:   req.Question[0].Name,
+				Rrtype: dns.TypeA,
+				Class:  dns.ClassINET,
+				Ttl:    60,
+			},
+			A: net.IP{192, 0, 2, 0},
+		})
+
+		return resp, nil
+	}
+
+	upsConf := &proxy.UpstreamConfig{
+		Upstreams: []upstream.Upstream{ups},
+	}
+
+	testCases := []struct {
+		customUpstreamConf *proxy.CustomUpstreamConfig
+		wantCachedWithConf assert.BoolAssertionFunc
+		wantCachedGlobal   assert.BoolAssertionFunc
+		name               string
+		prxCacheEnabled    bool
+	}{{
+		customUpstreamConf: nil,
+		wantCachedWithConf: assert.True,
+		wantCachedGlobal:   assert.True,
+		name:               "global_cache",
+		prxCacheEnabled:    true,
+	}, {
+		customUpstreamConf: newCustomUpstreamConfig(ups, true),
+		wantCachedWithConf: assert.True,
+		wantCachedGlobal:   assert.False,
+		name:               "custom_cache",
+		prxCacheEnabled:    false,
+	}, {
+		customUpstreamConf: newCustomUpstreamConfig(ups, false),
+		wantCachedWithConf: assert.False,
+		wantCachedGlobal:   assert.False,
+		name:               "custom_cache_only_upstreams",
+		prxCacheEnabled:    false,
+	}, {
+		customUpstreamConf: newCustomUpstreamConfig(ups, true),
+		wantCachedWithConf: assert.True,
+		wantCachedGlobal:   assert.False,
+		name:               "two_caches_enabled",
+		prxCacheEnabled:    true,
+	}, {
+		customUpstreamConf: nil,
+		wantCachedWithConf: assert.False,
+		wantCachedGlobal:   assert.False,
+		name:               "two_caches_disabled",
+		prxCacheEnabled:    false,
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			p, err := proxy.New(&proxy.Config{
+				UDPListenAddr:  []*net.UDPAddr{net.UDPAddrFromAddrPort(localhostAnyPort)},
+				UpstreamConfig: upsConf,
+				CacheEnabled:   tc.prxCacheEnabled,
+			})
+			require.NoError(t, err)
+			require.NotNil(t, p)
+
+			ctx := testutil.ContextWithTimeout(t, testTimeout)
+			err = p.Start(ctx)
+			require.NoError(t, err)
+
+			testutil.CleanupAndRequireSuccess(t, func() (err error) { return p.Shutdown(ctx) })
+
+			res := isCachedWithCustomConfig(t, p, tc.customUpstreamConf, host)
+			assert.False(t, res)
+
+			res = isCachedWithCustomConfig(t, p, tc.customUpstreamConf, host)
+			tc.wantCachedWithConf(t, res)
+
+			res = isCachedWithCustomConfig(t, p, nil, host)
+			tc.wantCachedGlobal(t, res)
+		})
+	}
+}