Add test and fix transition from empty to non empty conditions list

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

diff --git a/pdns/recursordist/ws-recursor.cc b/pdns/recursordist/ws-recursor.cc
index b640bac64ebe79914779dc0fc4975408426be900..e5dcaaceb46c4331e62e51579063c0e8f8c45c5d 100644 (file)
--- a/pdns/recursordist/ws-recursor.cc
+++ b/pdns/recursordist/ws-recursor.cc
@@ -580,7 +580,6 @@ static void apiServerOTConditionsGET(HttpRequest* /* req */, HttpResponse* resp)
 
 static void fillOTCondition(const Netmask& netmask, HttpResponse* resp)
 {
-  Json::array doc;
   auto lock = g_initialOpenTelemetryConditions.lock();
   if (*lock) {
     auto condition = (*lock)->lookup(netmask);
@@ -660,39 +659,40 @@ static void apiServerOTConditionDetailPOST(HttpRequest* req, HttpResponse* resp)
       throw ApiException("Required parameter acl missing");
     }
     auto lock = g_initialOpenTelemetryConditions.lock();
-    if (*lock) {
-      auto conditionPtr = (*lock)->lookup(netmask);
-      if (conditionPtr != nullptr && conditionPtr->first == netmask) { // exact match
-        throw ApiException("OTCondition already exists");
-      }
+    if (!*lock) {
+      *lock = std::make_unique<OpenTelemetryTraceConditions>();
+    }
+    auto conditionPtr = (*lock)->lookup(netmask);
+    if (conditionPtr != nullptr && conditionPtr->first == netmask) { // exact match
+      throw ApiException("OTCondition already exists");
+    }
 
-      OpenTelemetryTraceCondition condition;
-      if (auto traceid_only = document["traceid_only"]; traceid_only.is_bool()) {
-        condition.d_traceid_only = traceid_only.bool_value();
-      }
-      if (auto edns = document["edns_option_required"]; edns.is_bool()) {
-        condition.d_edns_option_required = edns.bool_value();
-      }
-      if (auto qnames = document["qnames"]; qnames.is_array() && !qnames.array_items().empty()) {
-        condition.d_qnames = SuffixMatchNode();
-        for (const auto& qname : qnames.array_items()) {
-          condition.d_qnames->add(DNSName(qname.string_value()));
-        }
+    OpenTelemetryTraceCondition condition;
+    if (auto traceid_only = document["traceid_only"]; traceid_only.is_bool()) {
+      condition.d_traceid_only = traceid_only.bool_value();
+    }
+    if (auto edns = document["edns_option_required"]; edns.is_bool()) {
+      condition.d_edns_option_required = edns.bool_value();
+    }
+    if (auto qnames = document["qnames"]; qnames.is_array() && !qnames.array_items().empty()) {
+      condition.d_qnames = SuffixMatchNode();
+      for (const auto& qname : qnames.array_items()) {
+        condition.d_qnames->add(DNSName(qname.string_value()));
       }
-      if (auto qtypes = document["qtypes"]; qtypes.is_array() && !qtypes.array_items().empty()) {
-        condition.d_qtypes = std::unordered_set<QType>();
-        for (const auto& qtype : qtypes.array_items()) {
-          if (auto qcode = QType::chartocode(qtype.string_value().c_str()); qcode > 0) {
-            condition.d_qtypes->insert(qcode);
-          }
+    }
+    if (auto qtypes = document["qtypes"]; qtypes.is_array() && !qtypes.array_items().empty()) {
+      condition.d_qtypes = std::unordered_set<QType>();
+      for (const auto& qtype : qtypes.array_items()) {
+        if (auto qcode = QType::chartocode(qtype.string_value().c_str()); qcode > 0) {
+          condition.d_qtypes->insert(qcode);
         }
       }
-      if (auto qid = document["qid"]; qid.is_number()) {
-        condition.d_qid = qid.int_value();
-      }
-      (*lock)->insert(netmask).second = condition;
-      updateOTConditions(**lock);
     }
+    if (auto qid = document["qid"]; qid.is_number()) {
+      condition.d_qid = qid.int_value();
+    }
+    (*lock)->insert(netmask).second = condition;
+    updateOTConditions(**lock);
   }
   catch (NetmaskException&) {
     throw ApiException("Could not parse netmask");

diff --git a/regression-tests.api/runtests.py b/regression-tests.api/runtests.py
index cb9bcdea1592213a32150c834b5f447ec2d3b92b..8fd736450c4f9c2afb5b465857febeaf6fe5a16d 100755 (executable)
--- a/regression-tests.api/runtests.py
+++ b/regression-tests.api/runtests.py
@@ -161,7 +161,7 @@ if daemon not in ('authoritative', 'recursor') or backend not in ('gmysql', 'gpg
 daemon = sys.argv[1]
 
 pdns_server = os.environ.get("PDNSSERVER", "../pdns/pdns_server")
-pdns_recursor = os.environ.get("PDNSRECURSOR", "../pdns/recursordist/pdns_recursor")
+pdns_recursor = os.environ.get("PDNSRECURSOR", "../pdns/recursordist/build/pdns_recursor")
 common_args = [
     "--daemon=no", "--socket-dir=.", "--config-dir=.",
     "--local-address=127.0.0.1", "--local-port="+str(DNSPORT),

diff --git a/regression-tests.api/test_RecursorOTConditions.py b/regression-tests.api/test_RecursorOTConditions.py
new file mode 100644 (file)
index 0000000..a579f08
--- /dev/null
+++ b/regression-tests.api/test_RecursorOTConditions.py
@@ -0,0 +1,174 @@
+import json
+import unittest
+from test_helper import ApiTestCase, is_recursor
+
+
+@unittest.skipIf(not is_recursor(), "Only applicable to recursors")
+class RecursorOT(ApiTestCase):
+
+    def assert_in_json_error(self, expected, json):
+        error = json['error']
+        if expected not in error:
+            found = False
+            if 'errors' in json:
+                errors = json['errors']
+                for item in errors:
+                    if expected in item:
+                        found = True
+                assert found, "%r not found in %r" % (expected, errors)
+            assert found, "%r not found in %r" % (expected, error)
+
+    def test_basic_ot_conditions(self):
+        # initial list is empty
+        r = self.session.get(
+            self.url("/api/v1/servers/localhost/otconditions"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(r.json(), [])
+
+        # nonexistent condition
+        r = self.session.get(
+            self.url("/api/v1/servers/localhost/otconditions/1.2.3.4%2F32"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 422)
+        self.assert_in_json_error('Could not find otcondition', r.json())
+
+        # malformed netmask
+        r = self.session.get(
+            self.url("/api/v1/servers/localhost/otconditions/1.2.3%2F32"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 422)
+        self.assert_in_json_error('Could not parse netmask', r.json())
+
+        # deleting non-existent netmask
+        r = self.session.delete(
+            self.url("/api/v1/servers/localhost/otconditions/1.2.3.4%2F32"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 422)
+        self.assert_in_json_error('Could not find otcondition', r.json())
+
+        # creating, most simple case
+        payload = {
+            "acl": "1.2.3.4"
+        }
+        r = self.session.post(
+            self.url("/api/v1/servers/localhost/otconditions"),
+            data=json.dumps(payload),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 201)
+        data = r.json()
+        self.assertIn('acl', data)
+        self.assertIn('edns_option_required', data)
+        self.assertIn('traceid_only', data)
+        self.assertEqual(data['acl'], '1.2.3.4/32')
+        self.assertFalse(data['edns_option_required'])
+        self.assertFalse(data['traceid_only'])
+
+        # creating, error because duplicate
+        payload = {
+            "acl": "1.2.3.4"
+        }
+        r = self.session.post(
+            self.url("/api/v1/servers/localhost/otconditions"),
+            data=json.dumps(payload),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 422)
+        self.assert_in_json_error('OTCondition already exists', r.json())
+
+        # list has one element
+        r = self.session.get(
+            self.url("/api/v1/servers/localhost/otconditions"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(len(r.json()), 1)
+
+        # creating, more general case
+        payload = {
+            "acl": "1.2.3.0/24"
+        }
+        r = self.session.post(
+            self.url("/api/v1/servers/localhost/otconditions"),
+            data=json.dumps(payload),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 201)
+        data = r.json()
+        self.assertIn('acl', data)
+        self.assertIn('edns_option_required', data)
+        self.assertIn('traceid_only', data)
+        self.assertEqual(data['acl'], '1.2.3.0/24')
+        self.assertFalse(data['edns_option_required'])
+        self.assertFalse(data['traceid_only'])
+
+        # list has two elements
+        r = self.session.get(
+            self.url("/api/v1/servers/localhost/otconditions"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(len(r.json()), 2)
+
+        # querying by more specific key
+        r = self.session.get(
+            self.url("/api/v1/servers/localhost/otconditions/1.2.3.4%2F31"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 422)
+        self.assert_in_json_error('Could not find otcondition', r.json())
+
+        # deleting specific netmask
+        r = self.session.delete(
+            self.url("/api/v1/servers/localhost/otconditions/1.2.3.4%2F32"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 204)
+
+        # list has one elements
+        r = self.session.get(
+            self.url("/api/v1/servers/localhost/otconditions"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 200)
+        self.assertEqual(len(r.json()), 1)
+
+        # creating, all fields filled in
+        payload = {
+            "acl": "::/0",
+            "qid": 99,
+            "qnames": ["foo.bar", "nl", "com"],
+            "qtypes": ["AAAA", "TXT"],
+            "traceid_only": True,
+            "edns_option_required": True
+        }
+        r = self.session.post(
+            self.url("/api/v1/servers/localhost/otconditions"),
+            data=json.dumps(payload),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 201)
+        data = r.json()
+        self.assertIn('acl', data)
+        self.assertIn('qid', data)
+        self.assertIn('qnames', data)
+        self.assertIn('qtypes', data)
+        self.assertIn('traceid_only', data)
+        self.assertIn('edns_option_required', data)
+        self.assertEqual(data['acl'], '::/0')
+        self.assertEqual(data['qid'], 99)
+        self.assertEqual(len(data['qnames']), 3)
+        self.assertEqual(len(data['qtypes']), 2)
+        self.assertTrue(data['edns_option_required'])
+        self.assertTrue(data['traceid_only'])
+
+        # and GET the newly created one in a separate call
+        r = self.session.get(
+            self.url("/api/v1/servers/localhost/otconditions/::1%2F0"),
+            headers={'content-type': 'application/json'})
+        self.assertEqual(r.status_code, 200)
+        data = r.json()
+        self.assertIn('acl', data)
+        self.assertIn('qid', data)
+        self.assertIn('qnames', data)
+        self.assertIn('qtypes', data)
+        self.assertIn('traceid_only', data)
+        self.assertIn('edns_option_required', data)
+        self.assertEqual(data['acl'], '::/0')
+        self.assertEqual(data['qid'], 99)
+        self.assertEqual(len(data['qnames']), 3)
+        self.assertEqual(len(data['qtypes']), 2)
+        self.assertTrue(data['edns_option_required'])
+        self.assertTrue(data['traceid_only'])