From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Fri, 13 Sep 2024 09:00:14 +0000 (+0200)
Subject: rec: make chain test more robust and fix max chain size accounting
X-Git-Tag: rec-5.2.0-alpha1~81^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F14669%2Fhead;p=thirdparty%2Fpdns.git

rec: make chain test more robust and fix max chain size accounting
---

diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
index 01d2ecea56..d39fcd6d7d 100644
--- a/pdns/recursordist/pdns_recursor.cc
+++ b/pdns/recursordist/pdns_recursor.cc
@@ -313,8 +313,8 @@ LWResult::Result asendto(const void* data, size_t len, int /* flags */,
         }
         chain.first->key->authReqChain.emplace(*fileDesc, qid); // we can chain
         auto maxLength = t_Counters.at(rec::Counter::maxChainLength);
-        if (currentChainSize > maxLength) {
-          t_Counters.at(rec::Counter::maxChainLength) = currentChainSize;
+        if (currentChainSize + 1 > maxLength) {
+          t_Counters.at(rec::Counter::maxChainLength) = currentChainSize + 1;
         }
         return LWResult::Result::Success;
       }
diff --git a/regression-tests.recursor-dnssec/recursortests.py b/regression-tests.recursor-dnssec/recursortests.py
index 63fa230ead..bb97883624 100644
--- a/regression-tests.recursor-dnssec/recursortests.py
+++ b/regression-tests.recursor-dnssec/recursortests.py
@@ -12,6 +12,7 @@ import time
 import unittest
 import dns
 import dns.message
+import requests
 
 from proxyprotocol import ProxyProtocol
 
@@ -1197,3 +1198,23 @@ distributor-threads={threads}""".format(confdir=confdir,
         if data:
             message = dns.message.from_wire(data)
         return message
+
+    def checkMetrics(self, map):
+        self.waitForTCPSocket("127.0.0.1", self._wsPort)
+        headers = {'x-api-key': self._apiKey}
+        url = 'http://127.0.0.1:' + str(self._wsPort) + '/api/v1/servers/localhost/statistics'
+        r = requests.get(url, headers=headers, timeout=self._wsTimeout)
+        self.assertEqual(r.status_code, 200)
+        self.assertTrue(r.json())
+        content = r.json()
+        count = 0
+        for entry in content:
+            for key, expected in map.items():
+                if entry['name'] == key:
+                    value = int(entry['value'])
+                    if callable(expected):
+                        self.assertTrue(expected(value))
+                    else:
+                        self.assertEqual(value, expected)
+                    count += 1
+        self.assertEqual(count, len(map))
diff --git a/regression-tests.recursor-dnssec/test_Chain.py b/regression-tests.recursor-dnssec/test_Chain.py
index e962e1395a..6c20937b31 100644
--- a/regression-tests.recursor-dnssec/test_Chain.py
+++ b/regression-tests.recursor-dnssec/test_Chain.py
@@ -8,29 +8,47 @@ class ChainTest(RecursorTest):
     These regression tests test the chaining of outgoing requests.
     """
     _confdir = 'Chain'
+    _wsPort = 8042
+    _wsTimeout = 2
+    _wsPassword = 'secretpassword'
+    _apiKey = 'secretapikey'
 
     _config_template = """dnssec=validate
     trace=no
-"""
+    devonly-regression-test-mode
+    webserver=yes
+    webserver-port=%d
+    webserver-address=127.0.0.1
+    webserver-password=%s
+    api-key=%s
+""" % (_wsPort, _wsPassword, _apiKey)
 
     def testBasic(self):
         """
         Tests the case of #14624. Sending many equal requests could lead to ServFail because of clashing
         waiter ids.
         """
+        # We actually do not check all responses, as experience show that packets may be dropped by the OS
+        # Instead, we check if a few counters in rec have the expected values.
         count = 200
-        name = '1.delay1.example.'
+        name = '9.delay1.example.'
         exp = dns.rrset.from_text(name, 0, dns.rdataclass.IN, 'TXT', 'a')
         for i in range(count):
             query = dns.message.make_query(name, 'TXT', want_dnssec=True)
             query.flags |= dns.flags.AD
             self._sock.send(query.to_wire())
 
-        for i in range(count):
-            data = self._sock.recv(4096)
-            res = dns.message.from_wire(data)
-            self.assertRcodeEqual(res, dns.rcode.NOERROR)
-            self.assertMessageIsAuthenticated(res)
-            self.assertRRsetInAnswer(res, exp)
-            self.assertMatchingRRSIGInAnswer(res, exp)
-        self._sock.settimeout(None)
+        # Just check one, as OS emptying of socket buffers can work against us
+        data = self._sock.recv(4096)
+        res = dns.message.from_wire(data)
+        self.assertRcodeEqual(res, dns.rcode.NOERROR)
+        self.assertMessageIsAuthenticated(res)
+        self.assertRRsetInAnswer(res, exp)
+        self.assertMatchingRRSIGInAnswer(res, exp)
+        time.sleep(1)
+
+        self.checkMetrics({
+            'max-chain-length': (lambda x: x <= count-1), # first request has count - 1 requests chained to it
+            'servfail-answers': 0,
+            'noerror-answers': (lambda x: x <= count),
+        })