Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pdns/recursordist/pdns_recursor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
21 changes: 21 additions & 0 deletions regression-tests.recursor-dnssec/recursortests.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import unittest
import dns
import dns.message
import requests

from proxyprotocol import ProxyProtocol

Expand Down Expand Up @@ -1197,3 +1198,23 @@ def sendTCPQueryWithProxyProtocol(cls, query, v6, source, destination, sourcePor
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))
38 changes: 28 additions & 10 deletions regression-tests.recursor-dnssec/test_Chain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
Loading