dnsdist: Add more KVS unit tests

diff --git a/pdns/dnsdistdist/dnsdist-kvs.cc b/pdns/dnsdistdist/dnsdist-kvs.cc
index 80def8d4ab9b6a4b3bb13e3dd2bbcd506bbbfacd..2d0c5dbff8802442fbc648fbf3381cb6bbd2f3ca 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-kvs.cc
+++ b/pdns/dnsdistdist/dnsdist-kvs.cc
@@ -23,6 +23,40 @@
 #include "dnsdist-kvs.hh"
 #include "dolog.hh"
 
+std::vector<std::string> KeyValueLookupKeySourceIP::getKeys(const DNSQuestion& dq)
+{
+  std::vector<std::string> result;
+
+  if (dq.remote->sin4.sin_family == AF_INET) {
+    result.emplace_back(reinterpret_cast<const char*>(&dq.remote->sin4.sin_addr.s_addr), sizeof(dq.remote->sin4.sin_addr.s_addr));
+  }
+  else if (dq.remote->sin4.sin_family == AF_INET6) {
+    result.emplace_back(reinterpret_cast<const char*>(&dq.remote->sin6.sin6_addr.s6_addr), sizeof(dq.remote->sin6.sin6_addr.s6_addr));
+  }
+
+  return result;
+}
+
+std::vector<std::string> KeyValueLookupKeySuffix::getKeys(const DNSName& qname)
+{
+  if (qname.empty() || qname.isRoot()) {
+    return {};
+  }
+
+  auto lowerQName = qname.makeLowerCase();
+  std::vector<std::string> result;
+  result.reserve(lowerQName.countLabels() - 1);
+
+  while(!lowerQName.isRoot()) {
+    result.emplace_back(lowerQName.toDNSString());
+    if (!lowerQName.chopOff()) {
+      break;
+    }
+  }
+
+  return result;
+}
+
 #ifdef HAVE_LMDB
 
 bool LMDBKVStore::getValue(const std::string& key, std::string& value)
@@ -41,7 +75,7 @@ bool LMDBKVStore::getValue(const std::string& key, std::string& value)
     }
   }
   catch(const std::exception& e) {
-    warnlog("Error while looking up key '%s' from LMDB file '%s', database '%s': %s", key, d_fname, d_dbName);
+    warnlog("Error while looking up key '%s' from LMDB file '%s', database '%s': %s", key, d_fname, d_dbName, e.what());
   }
   return false;
 }

diff --git a/pdns/dnsdistdist/dnsdist-kvs.hh b/pdns/dnsdistdist/dnsdist-kvs.hh
index fc2fa2a4ce4001e90721c896ff259e33121af6f2..5edf4794cab78f2a6088e171871f227aa7f6a23f 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-kvs.hh
+++ b/pdns/dnsdistdist/dnsdist-kvs.hh
@@ -36,19 +36,7 @@ public:
 class KeyValueLookupKeySourceIP: public KeyValueLookupKey
 {
 public:
-  std::vector<std::string> getKeys(const DNSQuestion& dq) override
-  {
-    std::vector<std::string> result;
-
-    if (dq.remote->sin4.sin_family == AF_INET) {
-      result.emplace_back(reinterpret_cast<const char*>(&dq.remote->sin4.sin_addr.s_addr), sizeof(dq.remote->sin4.sin_addr.s_addr));
-    }
-    else if (dq.remote->sin4.sin_family == AF_INET6) {
-      result.emplace_back(reinterpret_cast<const char*>(&dq.remote->sin6.sin6_addr.s6_addr), sizeof(dq.remote->sin6.sin6_addr.s6_addr));
-    }
-
-    return result;
-  }
+  std::vector<std::string> getKeys(const DNSQuestion& dq) override;
 
   std::string toString() const override
   {
@@ -59,9 +47,15 @@ public:
 class KeyValueLookupKeyQName: public KeyValueLookupKey
 {
 public:
+
+  std::vector<std::string> getKeys(const DNSName& qname)
+  {
+    return {qname.toDNSStringLC()};
+  }
+
   std::vector<std::string> getKeys(const DNSQuestion& dq) override
   {
-    return {dq.qname->toDNSStringLC()};
+    return getKeys(*dq.qname);
   }
 
   std::string toString() const override
@@ -73,17 +67,11 @@ public:
 class KeyValueLookupKeySuffix: public KeyValueLookupKey
 {
 public:
+  std::vector<std::string> getKeys(const DNSName& qname);
+
   std::vector<std::string> getKeys(const DNSQuestion& dq) override
   {
-    auto lowerQName = dq.qname->makeLowerCase();
-    std::vector<std::string> result(lowerQName.countLabels());
-
-    do {
-      result.emplace_back(lowerQName.toDNSString());
-    }
-    while (lowerQName.chopOff());
-
-    return result;
+    return getKeys(*dq.qname);
   }
 
   std::string toString() const override

diff --git a/pdns/dnsdistdist/test-dnsdistkvs_cc.cc b/pdns/dnsdistdist/test-dnsdistkvs_cc.cc
index 8a92c57a325b03f66840c5af68d7f7f1c6fd58ca..7bc15aa6787e4bccc3b27fb27951694057cc860d 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdistkvs_cc.cc
+++ b/pdns/dnsdistdist/test-dnsdistkvs_cc.cc
@@ -6,6 +6,83 @@
 
 #include "dnsdist-kvs.hh"
 
+static void doKVSChecks(std::unique_ptr<KeyValueStore>& kvs, const ComboAddress& lc, const ComboAddress& rem, const DNSQuestion& dq)
+{
+  /* source IP */
+  {
+    auto lookupKey = make_unique<KeyValueLookupKeySourceIP>();
+    auto keys = lookupKey->getKeys(dq);
+    BOOST_CHECK_EQUAL(keys.size(), 1);
+    for (const auto& key : keys) {
+      std::string value;
+      BOOST_CHECK_EQUAL(kvs->getValue(std::string(reinterpret_cast<const char*>(&lc.sin4.sin_addr.s_addr), sizeof(lc.sin4.sin_addr.s_addr)), value), false);
+      BOOST_CHECK_EQUAL(kvs->getValue(key, value), true);
+      BOOST_CHECK_EQUAL(value, "this is the value for the remote addr");
+    }
+  }
+
+  const DNSName subdomain = DNSName("sub") + *dq.qname;
+  const DNSName notPDNS("not-powerdns.com.");
+
+  /* qname */
+  {
+    std::string value;
+    auto lookupKey = make_unique<KeyValueLookupKeyQName>();
+    auto keys = lookupKey->getKeys(dq);
+    BOOST_CHECK_EQUAL(keys.size(), 1);
+    for (const auto& key : keys) {
+      value.clear();
+      BOOST_CHECK_EQUAL(kvs->getValue(key, value), true);
+      BOOST_CHECK_EQUAL(value, "this is the value for the qname");
+    }
+
+    /* other domain, should not match */
+    keys = lookupKey->getKeys(notPDNS);
+    BOOST_CHECK_EQUAL(keys.size(), 1);
+    for (const auto& key : keys) {
+      value.clear();
+      BOOST_CHECK_EQUAL(kvs->getValue(key, value), false);
+    }
+
+    /* subdomain, should not match */
+    keys = lookupKey->getKeys(subdomain);
+    BOOST_CHECK_EQUAL(keys.size(), 1);
+    for (const auto& key : keys) {
+      value.clear();
+      BOOST_CHECK_EQUAL(kvs->getValue(key, value), false);
+    }
+  }
+
+  /* suffix match */
+  {
+    auto lookupKey = make_unique<KeyValueLookupKeySuffix>();
+    auto keys = lookupKey->getKeys(dq);
+    BOOST_CHECK_EQUAL(keys.size(), dq.qname->countLabels());
+    BOOST_REQUIRE(!keys.empty());
+    BOOST_CHECK_EQUAL(keys.at(0), dq.qname->toDNSStringLC());
+    std::string value;
+    BOOST_CHECK_EQUAL(kvs->getValue(keys.at(0), value), true);
+    BOOST_CHECK_EQUAL(value, "this is the value for the qname");
+    value.clear();
+    BOOST_CHECK_EQUAL(kvs->getValue(keys.at(1), value), false);
+
+    /* other domain, should not match */
+    keys = lookupKey->getKeys(notPDNS);
+    BOOST_CHECK_EQUAL(keys.size(), notPDNS.countLabels());
+    for (const auto& key : keys) {
+      value.clear();
+      BOOST_CHECK_EQUAL(kvs->getValue(key, value), false);
+    }
+
+    /* subdomain, the second key should match */
+    keys = lookupKey->getKeys(subdomain);
+    BOOST_REQUIRE_EQUAL(keys.size(), subdomain.countLabels());
+    BOOST_CHECK_EQUAL(kvs->getValue(keys.at(0), value), false);
+    BOOST_CHECK_EQUAL(kvs->getValue(keys.at(1), value), true);
+    BOOST_CHECK_EQUAL(value, "this is the value for the qname");
+  }
+}
+
 BOOST_AUTO_TEST_SUITE(dnsdistkvs_cc)
 
 #ifdef HAVE_LMDB
@@ -14,8 +91,8 @@ BOOST_AUTO_TEST_CASE(test_LMDB) {
   DNSName qname("powerdns.com.");
   uint16_t qtype = QType::A;
   uint16_t qclass = QClass::IN;
-  ComboAddress lc("127.0.0.1:53");
-  ComboAddress rem("127.0.0.1:42");
+  ComboAddress lc("192.0.2.1:53");
+  ComboAddress rem("192.0.2.128:42");
   struct dnsheader dh;
   memset(&dh, 0, sizeof(dh));
   size_t bufferSize = 0;
@@ -34,13 +111,14 @@ BOOST_AUTO_TEST_CASE(test_LMDB) {
     MDBEnv env(dbPath.c_str(), MDB_NOSUBDIR, 0600);
     auto transaction = env.getRWTransaction();
     auto dbi = transaction.openDB("db-name", MDB_CREATE);
-    transaction.put(dbi, MDBInVal(std::string(reinterpret_cast<const char*>(&dq.remote->sin4.sin_addr.s_addr), sizeof(dq.remote->sin4.sin_addr.s_addr))), MDBInVal("this is the value of the tag"));
+    transaction.put(dbi, MDBInVal(std::string(reinterpret_cast<const char*>(&rem.sin4.sin_addr.s_addr), sizeof(rem.sin4.sin_addr.s_addr))), MDBInVal("this is the value for the remote addr"));
+    transaction.put(dbi, MDBInVal(qname.toDNSStringLC()), MDBInVal("this is the value for the qname"));
     transaction.commit();
   }
 
-  auto lmdb = make_unique<LMDBKVStore>(dbPath, "db-name");
-  auto lookupKey = make_unique<KeyValueLookupKeySourceIP>();
-
+  auto lmdb = std::unique_ptr<KeyValueStore>(new LMDBKVStore(dbPath, "db-name"));
+  doKVSChecks(lmdb, lc, rem, dq);
+  /*
   std::string value;
   DTime dt;
   dt.set();
@@ -53,6 +131,7 @@ BOOST_AUTO_TEST_CASE(test_LMDB) {
     }
   }
   cerr<<dt.udiff()/1000/1000<<endl;
+  */
 }
 #endif /* HAVE_LMDB */
 
@@ -62,8 +141,8 @@ BOOST_AUTO_TEST_CASE(test_CDB) {
   DNSName qname("powerdns.com.");
   uint16_t qtype = QType::A;
   uint16_t qclass = QClass::IN;
-  ComboAddress lc("127.0.0.1:53");
-  ComboAddress rem("127.0.0.1:42");
+  ComboAddress lc("192.0.2.1:53");
+  ComboAddress rem("192.0.2.128:42");
   struct dnsheader dh;
   memset(&dh, 0, sizeof(dh));
   size_t bufferSize = 0;
@@ -82,13 +161,15 @@ BOOST_AUTO_TEST_CASE(test_CDB) {
     int fd = mkstemp(db);
     BOOST_REQUIRE(fd >= 0);
     CDBWriter writer(fd);
-    BOOST_REQUIRE(writer.addEntry(std::string(reinterpret_cast<const char*>(&dq.remote->sin4.sin_addr.s_addr), sizeof(dq.remote->sin4.sin_addr.s_addr)), "this is the value of the tag"));
+    BOOST_REQUIRE(writer.addEntry(std::string(reinterpret_cast<const char*>(&rem.sin4.sin_addr.s_addr), sizeof(rem.sin4.sin_addr.s_addr)), "this is the value for the remote addr"));
+    BOOST_REQUIRE(writer.addEntry(qname.toDNSStringLC(), "this is the value for the qname"));
     writer.close();
   }
 
-  auto cdb = make_unique<CDBKVStore>(db);
-  auto lookupKey = make_unique<KeyValueLookupKeySourceIP>();
+  auto cdb = std::unique_ptr<KeyValueStore>(new CDBKVStore(db));
+  doKVSChecks(cdb, lc, rem, dq);
 
+  /*
   std::string value;
   DTime dt;
   dt.set();
@@ -100,6 +181,7 @@ BOOST_AUTO_TEST_CASE(test_CDB) {
     }
   }
   cerr<<dt.udiff()/1000/1000<<endl;
+  */
 }
 #endif /* HAVE_CDB */