Skip to content

Commit

Permalink
Merge pull request #15015 from omoerbeek/backport-15010-to-rec-5.2.x
Browse files Browse the repository at this point in the history
rec: Backport 15010 to rec 5.2.x: fix protobufServer(.. {taggedOnly=true}) logic for cache-returned responses
  • Loading branch information
omoerbeek authored Jan 10, 2025
2 parents 2919078 + de1a487 commit 544037c
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 49 deletions.
1 change: 0 additions & 1 deletion pdns/recursordist/docs/lua-config/protobuf.rst
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ While :func:`protobufServer` only exports the queries sent to the recursor from
* ``timeout=2``: int - Time in seconds to wait when sending a message
* ``maxQueuedEntries=100``: int - How many entries will be kept in memory if the server becomes unreachable
* ``reconnectWaitTime=1``: int - How long to wait, in seconds, between two reconnection attempts
* ``taggedOnly=false``: bool - Only entries with a policy or a policy tag set will be sent
* ``asyncConnect``: bool - When set to false (default) the first connection to the server during startup will block up to ``timeout`` seconds, otherwise the connection is done in a separate thread, after the first message has been queued
* ``logQueries=true``: bool - Whether to export queries
* ``logResponses=true``: bool - Whether to export responses
Expand Down
2 changes: 1 addition & 1 deletion pdns/recursordist/pdns_recursor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2338,7 +2338,7 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr
int sendErr = sendOnNBSocket(fileDesc, &msgh);
eventTrace.add(RecEventTrace::AnswerSent);

if (t_protobufServers.servers && logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || !pbData || pbData->d_tagged)) {
if (t_protobufServers.servers && logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || (pbData && pbData->d_tagged))) {
protobufLogResponse(dnsheader, luaconfsLocal, pbData, tval, false, source, destination, mappedSource, ednssubnet, uniqueId, requestorId, deviceId, deviceName, meta, eventTrace, policyTags);
}

Expand Down
2 changes: 1 addition & 1 deletion pdns/recursordist/rec-main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2897,7 +2897,7 @@ static void recursorThread()
threadInfo.setMT(g_multiTasker.get());

{
/* start protobuf export threads if needed, don;'t keep a ref to lua config around */
/* start protobuf export threads if needed, don't keep a ref to lua config around */
auto luaconfsLocal = g_luaconfs.getLocal();
checkProtobufExport(luaconfsLocal);
checkOutgoingProtobufExport(luaconfsLocal);
Expand Down
2 changes: 1 addition & 1 deletion pdns/recursordist/rec-tcp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ static void doProcessTCPQuestion(std::unique_ptr<DNSComboWriter>& comboWriter, s
t_Counters.at(rec::Histogram::cumulativeAnswers)(spentUsec);
comboWriter->d_eventTrace.add(RecEventTrace::AnswerSent);

if (t_protobufServers.servers && comboWriter->d_logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || !pbData || pbData->d_tagged)) {
if (t_protobufServers.servers && comboWriter->d_logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || (pbData && pbData->d_tagged))) {
struct timeval tval
{
0, 0
Expand Down
103 changes: 58 additions & 45 deletions regression-tests.recursor-dnssec/test_Protobuf.py
Original file line number Diff line number Diff line change
Expand Up @@ -962,58 +962,71 @@ def testA(self):
expected = dns.rrset.from_text(name, 0, dns.rdataclass.IN, 'A', '192.0.2.42')
query = dns.message.make_query(name, 'A', want_dnssec=True)
query.flags |= dns.flags.CD
res = self.sendUDPQuery(query)
self.assertRRsetInAnswer(res, expected)
for method in ("sendUDPQuery", "sendTCPQuery"):
sender = getattr(self, method)
res = sender(query)
self.assertRRsetInAnswer(res, expected)

# check the protobuf message corresponding to the UDP response
# the first query and answer are not tagged, so there is nothing in the queue
time.sleep(1)
self.checkNoRemainingMessage()
# check the protobuf message corresponding to the UDP response
# the first query and answer are not tagged, so there is nothing in the queue
time.sleep(1)

self.checkNoRemainingMessage()
# Again to check PC case
res = sender(query)
time.sleep(1)
self.checkNoRemainingMessage()

def testTagged(self):
name = 'tagged.example.'
expected = dns.rrset.from_text(name, 0, dns.rdataclass.IN, 'A', '192.0.2.84')
query = dns.message.make_query(name, 'A', want_dnssec=True)
query.flags |= dns.flags.CD
res = self.sendUDPQuery(query)
self.assertRRsetInAnswer(res, expected)

# check the protobuf messages corresponding to the UDP query and answer
msg = self.getFirstProtobufMessage()
self.checkProtobufQuery(msg, dnsmessage_pb2.PBDNSMessage.UDP, query, dns.rdataclass.IN, dns.rdatatype.A, name)
self.checkProtobufTags(msg, [ self._tag_from_gettag ])
# then the response
msg = self.getFirstProtobufMessage()
self.checkProtobufResponse(msg, dnsmessage_pb2.PBDNSMessage.UDP, res)
self.assertEqual(len(msg.response.rrs), 1)
rr = msg.response.rrs[0]
# we have max-cache-ttl set to 15
self.checkProtobufResponseRecord(rr, dns.rdataclass.IN, dns.rdatatype.A, name, 15)
self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.84')
tags = [ self._tag_from_gettag ] + self._tags
#print(msg)
self.checkProtobufTags(msg, tags)
self.checkNoRemainingMessage()

# Again to check PC case
res = self.sendUDPQuery(query)
self.assertRRsetInAnswer(res, expected)

# check the protobuf messages corresponding to the UDP query and answer
msg = self.getFirstProtobufMessage()
self.checkProtobufQuery(msg, dnsmessage_pb2.PBDNSMessage.UDP, query, dns.rdataclass.IN, dns.rdatatype.A, name)
self.checkProtobufTags(msg, [ self._tag_from_gettag ])
# then the response
msg = self.getFirstProtobufMessage()
self.checkProtobufResponse(msg, dnsmessage_pb2.PBDNSMessage.UDP, res)
self.assertEqual(len(msg.response.rrs), 1)
rr = msg.response.rrs[0]
# time may have passed, so do not check TTL
self.checkProtobufResponseRecord(rr, dns.rdataclass.IN, dns.rdatatype.A, name, 15, checkTTL=False)
self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.84')
tags = [ self._tag_from_gettag ] + self._tags
self.checkProtobufTags(msg, tags)
self.checkNoRemainingMessage()
first = True
for method in ("sendUDPQuery", "sendTCPQuery"):
messagetype = dnsmessage_pb2.PBDNSMessage.UDP
if not first:
messagetype = dnsmessage_pb2.PBDNSMessage.TCP
sender = getattr(self, method)
res = sender(query)
self.assertRRsetInAnswer(res, expected)

# check the protobuf messages corresponding to the query and answer
msg = self.getFirstProtobufMessage()
self.checkProtobufQuery(msg, messagetype, query, dns.rdataclass.IN, dns.rdatatype.A, name)
self.checkProtobufTags(msg, [ self._tag_from_gettag ])
# then the response
msg = self.getFirstProtobufMessage()
self.checkProtobufResponse(msg, messagetype, res)
self.assertEqual(len(msg.response.rrs), 1)
rr = msg.response.rrs[0]
# we have max-cache-ttl set to 15, but only check it first iteration
self.checkProtobufResponseRecord(rr, dns.rdataclass.IN, dns.rdatatype.A, name, 15, checkTTL=first)
self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.84')
tags = [ self._tag_from_gettag ] + self._tags
self.checkProtobufTags(msg, tags)
self.checkNoRemainingMessage()

# Again to check PC case
res = sender(query)
self.assertRRsetInAnswer(res, expected)

# check the protobuf messages corresponding to the query and answer
msg = self.getFirstProtobufMessage()
self.checkProtobufQuery(msg, messagetype, query, dns.rdataclass.IN, dns.rdatatype.A, name)
self.checkProtobufTags(msg, [ self._tag_from_gettag ])
# then the response
msg = self.getFirstProtobufMessage()
self.checkProtobufResponse(msg, messagetype, res)
self.assertEqual(len(msg.response.rrs), 1)
rr = msg.response.rrs[0]
# time may have passed, so do not check TTL
self.checkProtobufResponseRecord(rr, dns.rdataclass.IN, dns.rdatatype.A, name, 15, checkTTL=False)
self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.84')
tags = [ self._tag_from_gettag ] + self._tags
self.checkProtobufTags(msg, tags)
self.checkNoRemainingMessage()
first = False

class ProtobufTagCacheBase(TestRecursorProtobuf):
__test__ = False
Expand Down

0 comments on commit 544037c

Please sign in to comment.