From e3257bdce0aebe1bedce2c0eb7ef028bb436ca34 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Tue, 22 Feb 2022 10:50:04 +0100 Subject: [PATCH] Add unit tests for taskqueue, removing the old stub code and using the real stuff --- pdns/recursordist/Makefile.am | 2 + pdns/recursordist/rec-taskqueue.cc | 17 +++++ pdns/recursordist/rec-taskqueue.hh | 7 +- pdns/recursordist/taskqueue.hh | 5 ++ pdns/recursordist/test-rec-taskqueue.cc | 47 +++++++++++++ pdns/recursordist/test-syncres_cc.cc | 17 ++--- pdns/recursordist/test-syncres_cc.hh | 3 - pdns/recursordist/test-syncres_cc1.cc | 89 ++++++++++++++++++++++++- pdns/recursordist/test-syncres_cc2.cc | 5 +- 9 files changed, 172 insertions(+), 20 deletions(-) create mode 100644 pdns/recursordist/test-rec-taskqueue.cc diff --git a/pdns/recursordist/Makefile.am b/pdns/recursordist/Makefile.am index 012af7a363..9217e88cfe 100644 --- a/pdns/recursordist/Makefile.am +++ b/pdns/recursordist/Makefile.am @@ -283,6 +283,7 @@ testrunner_SOURCES = \ query-local-address.hh query-local-address.cc \ rcpgenerator.cc \ rec-eventtrace.cc rec-eventtrace.hh \ + rec-taskqueue.cc rec-taskqueue.hh \ rec-zonetocache.cc rec-zonetocache.hh \ recpacketcache.cc recpacketcache.hh \ recursor_cache.cc recursor_cache.hh \ @@ -321,6 +322,7 @@ testrunner_SOURCES = \ test-negcache_cc.cc \ test-packetcache_hh.cc \ test-rcpgenerator_cc.cc \ + test-rec-taskqueue.cc \ test-rec-zonetocache.cc \ test-recpacketcache_cc.cc \ test-recursorcache_cc.cc \ diff --git a/pdns/recursordist/rec-taskqueue.cc b/pdns/recursordist/rec-taskqueue.cc index b9886983ad..4180f0146d 100644 --- a/pdns/recursordist/rec-taskqueue.cc +++ b/pdns/recursordist/rec-taskqueue.cc @@ -71,6 +71,11 @@ public: return inserted; } + void clear() + { + d_set.clear(); + } + private: struct Entry { @@ -207,6 +212,18 @@ uint64_t getTaskSize() return s_taskQueue.lock()->queue.size(); } +void taskQueueClear() +{ + auto lock = s_taskQueue.lock(); + lock->queue.clear(); + lock->rateLimitSet.clear(); +} + +pdns::ResolveTask taskQueuePop() +{ + return s_taskQueue.lock()->queue.pop(); +} + uint64_t getAlmostExpiredTasksPushed() { return s_almost_expired_tasks.pushed; diff --git a/pdns/recursordist/rec-taskqueue.hh b/pdns/recursordist/rec-taskqueue.hh index c1e664a4af..84f165b7e1 100644 --- a/pdns/recursordist/rec-taskqueue.hh +++ b/pdns/recursordist/rec-taskqueue.hh @@ -25,10 +25,15 @@ #include class DNSName; - +namespace pdns +{ +struct ResolveTask; +} void runTaskOnce(bool logErrors); void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline); void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t deadline); +void taskQueueClear(); +pdns::ResolveTask taskQueuePop(); // General task stats uint64_t getTaskPushes(); diff --git a/pdns/recursordist/taskqueue.hh b/pdns/recursordist/taskqueue.hh index b677fab30c..1faaa30ef1 100644 --- a/pdns/recursordist/taskqueue.hh +++ b/pdns/recursordist/taskqueue.hh @@ -86,6 +86,11 @@ public: d_expired++; } + void clear() + { + d_queue.clear(); + } + private: struct HashTag { diff --git a/pdns/recursordist/test-rec-taskqueue.cc b/pdns/recursordist/test-rec-taskqueue.cc new file mode 100644 index 0000000000..8c997e0696 --- /dev/null +++ b/pdns/recursordist/test-rec-taskqueue.cc @@ -0,0 +1,47 @@ +#define BOOST_TEST_DYN_LINK +#include + +#include + +#include "dnsname.hh" +#include "qtype.hh" +#include "taskqueue.hh" +#include "rec-taskqueue.hh" +#include "test-syncres_cc.hh" + +BOOST_AUTO_TEST_SUITE(rec_taskqueue) + +BOOST_AUTO_TEST_CASE(test_almostexpired_queue_no_dups) +{ + taskQueueClear(); + pushAlmostExpiredTask(DNSName("foo"), QType::AAAA, 0); + pushAlmostExpiredTask(DNSName("foo"), QType::AAAA, 0); + pushAlmostExpiredTask(DNSName("foo"), QType::A, 0); + + BOOST_CHECK_EQUAL(getTaskSize(), 2U); + taskQueuePop(); + taskQueuePop(); + BOOST_CHECK_EQUAL(getTaskSize(), 0U); + // AE queue is not rate limited + pushAlmostExpiredTask(DNSName("foo"), QType::A, 0); + BOOST_CHECK_EQUAL(getTaskSize(), 1U); +} + +BOOST_AUTO_TEST_CASE(test_resolve_queue_rate_limit) +{ + taskQueueClear(); + pushResolveTask(DNSName("foo"), QType::AAAA, 0, 1); + BOOST_CHECK_EQUAL(getTaskSize(), 1U); + taskQueuePop(); + BOOST_CHECK_EQUAL(getTaskSize(), 0U); + + // Should hit rate limiting + pushResolveTask(DNSName("foo"), QType::AAAA, 0, 1); + BOOST_CHECK_EQUAL(getTaskSize(), 0U); + + // Should not hit rate limiting as time has passed + pushResolveTask(DNSName("foo"), QType::AAAA, 61, 62); + BOOST_CHECK_EQUAL(getTaskSize(), 1U); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 6ead553219..8f05f922e2 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -17,7 +17,10 @@ GlobalStateHolder g_DoTToAuthNames; std::unique_ptr g_recCache; std::unique_ptr g_negCache; bool g_lowercaseOutgoing = false; - +#if 0 +pdns::TaskQueue g_test_tasks; +pdns::TaskQueue g_resolve_tasks; +#endif /* Fake some required functions we didn't want the trouble to link with */ ArgvMap& arg() @@ -207,6 +210,7 @@ void initSR(bool debug) g_maxNSEC3Iterations = 2500; g_aggressiveNSECCache.reset(); + taskQueueClear(); ::arg().set("version-string", "string reported on version.pdns or version.bind") = "PowerDNS Unit Tests"; ::arg().set("rng") = "auto"; @@ -551,14 +555,3 @@ LWResult::Result basicRecordsForQnameMinimization(LWResult* res, const DNSName& } return LWResult::Result::Timeout; } - -pdns::TaskQueue g_test_tasks; - -void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline) -{ - g_test_tasks.push({qname, qtype, deadline, true, nullptr}); -} - -void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t now, time_t deadline) -{ -} diff --git a/pdns/recursordist/test-syncres_cc.hh b/pdns/recursordist/test-syncres_cc.hh index f33abd77cf..11d99c6f30 100644 --- a/pdns/recursordist/test-syncres_cc.hh +++ b/pdns/recursordist/test-syncres_cc.hh @@ -28,7 +28,6 @@ #include "syncres.hh" #include "test-common.hh" #include "validate-recursor.hh" -#include "taskqueue.hh" extern GlobalStateHolder g_luaconfs; @@ -75,5 +74,3 @@ void generateKeyMaterial(const DNSName& name, unsigned int algo, uint8_t digest, LWResult::Result genericDSAndDNSKEYHandler(LWResult* res, const DNSName& domain, DNSName auth, int type, const testkeysset_t& keys, bool proveCut = true, boost::optional now = boost::none, bool nsec3 = false, bool optOut = false); LWResult::Result basicRecordsForQnameMinimization(LWResult* res, const DNSName& domain, int type); - -extern pdns::TaskQueue g_test_tasks; diff --git a/pdns/recursordist/test-syncres_cc1.cc b/pdns/recursordist/test-syncres_cc1.cc index d31c24babe..e71c7381a3 100644 --- a/pdns/recursordist/test-syncres_cc1.cc +++ b/pdns/recursordist/test-syncres_cc1.cc @@ -2,6 +2,8 @@ #include #include "test-syncres_cc.hh" +#include "taskqueue.hh" +#include "rec-taskqueue.hh" BOOST_AUTO_TEST_SUITE(syncres_cc1) @@ -1000,6 +1002,89 @@ BOOST_AUTO_TEST_CASE(test_glueless_referral) BOOST_CHECK_EQUAL(ret[0].d_name, target); } +BOOST_AUTO_TEST_CASE(test_glueless_referral_aaaa_task) +{ + std::unique_ptr sr; + initSR(sr); + + primeHints(); + + const DNSName target("powerdns.com."); + + sr->setAsyncCallback([target](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, LWResult* res, bool* chained) { + if (isRootServer(ip)) { + setLWResult(res, 0, false, false, true); + + if (domain.isPartOf(DNSName("com."))) { + addRecordToLW(res, "com.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + } + else if (domain.isPartOf(DNSName("org."))) { + addRecordToLW(res, "org.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + } + else { + setLWResult(res, RCode::NXDomain, false, false, true); + return LWResult::Result::Success; + } + + addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + addRecordToLW(res, "a.gtld-servers.net.", QType::AAAA, "2001:DB8::1", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + else if (ip == ComboAddress("192.0.2.1:53") || ip == ComboAddress("[2001:DB8::1]:53")) { + if (domain == target) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, "powerdns.com.", QType::NS, "pdns-public-ns1.powerdns.org.", DNSResourceRecord::AUTHORITY, 172800); + addRecordToLW(res, "powerdns.com.", QType::NS, "pdns-public-ns2.powerdns.org.", DNSResourceRecord::AUTHORITY, 172800); + return LWResult::Result::Success; + } + else if (domain == DNSName("pdns-public-ns1.powerdns.org.")) { + setLWResult(res, 0, true, false, true); + if (type == QType::A) { + addRecordToLW(res, "pdns-public-ns1.powerdns.org.", QType::A, "192.0.2.2"); + } + else { + addRecordToLW(res, "pdns-public-ns1.powerdns.org.", QType::AAAA, "2001:DB8::2"); + } + return LWResult::Result::Success; + } + else if (domain == DNSName("pdns-public-ns2.powerdns.org.")) { + setLWResult(res, 0, true, false, true); + if (type == QType::A) { + addRecordToLW(res, "pdns-public-ns2.powerdns.org.", QType::A, "192.0.2.3"); + } + else { + addRecordToLW(res, "pdns-public-ns2.powerdns.org.", QType::AAAA, "2001:DB8::3"); + } + return LWResult::Result::Success; + } + + setLWResult(res, RCode::NXDomain, false, false, true); + return LWResult::Result::Success; + } + else if (ip == ComboAddress("192.0.2.2:53") || ip == ComboAddress("192.0.2.3:53") || ip == ComboAddress("[2001:DB8::2]:53") || ip == ComboAddress("[2001:DB8::3]:53")) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, target, QType::A, "192.0.2.4"); + return LWResult::Result::Success; + } + else { + return LWResult::Result::Timeout; + } + }); + + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_REQUIRE_EQUAL(ret.size(), 1U); + BOOST_CHECK(ret[0].d_type == QType::A); + BOOST_CHECK_EQUAL(ret[0].d_name, target); + + // One task should be submitted + BOOST_REQUIRE_EQUAL(getTaskSize(), 1U); + auto task = taskQueuePop(); + BOOST_CHECK(task.d_qname == DNSName("pdns-public-ns1.powerdns.org") || task.d_qname == DNSName("pdns-public-ns2.powerdns.org")); + BOOST_CHECK_EQUAL(task.d_qtype, QType::AAAA); +} + BOOST_AUTO_TEST_CASE(test_edns_subnet_by_domain) { std::unique_ptr sr; @@ -1812,8 +1897,8 @@ BOOST_AUTO_TEST_CASE(test_dname_dnssec_secure) sr->setAsyncCallback([dnameOwner, dnameTarget, target, cnameTarget, keys, &queries](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, LWResult* res, bool* chained) { queries++; /* We don't use the genericDSAndDNSKEYHandler here, as it would deny names existing at the wrong level of the tree, due to the way computeZoneCuts works - * As such, we need to do some more work to make the answers correct. - */ + * As such, we need to do some more work to make the answers correct. + */ if (isRootServer(ip)) { if (domain.countLabels() == 0 && type == QType::DNSKEY) { // .|DNSKEY diff --git a/pdns/recursordist/test-syncres_cc2.cc b/pdns/recursordist/test-syncres_cc2.cc index 8e46deb05f..4cc535743e 100644 --- a/pdns/recursordist/test-syncres_cc2.cc +++ b/pdns/recursordist/test-syncres_cc2.cc @@ -2,6 +2,7 @@ #include #include "test-syncres_cc.hh" +#include "taskqueue.hh" #include "rec-taskqueue.hh" BOOST_AUTO_TEST_SUITE(syncres_cc2) @@ -1822,9 +1823,9 @@ BOOST_AUTO_TEST_CASE(test_cache_almost_expired_ttl) BOOST_CHECK_EQUAL(ttl, 29U); // One task should be submitted - BOOST_CHECK_EQUAL(g_test_tasks.size(), 1U); + BOOST_CHECK_EQUAL(getTaskSize(), 1U); - auto task = g_test_tasks.pop(); + auto task = taskQueuePop(); // Refresh the almost expired record, its NS records also gets updated sr->setRefreshAlmostExpired(task.d_refreshMode); -- 2.47.2