From: Kees Monshouwer <mind04@monshouwer.org>
Date: Sun, 9 Jan 2022 13:24:23 +0000 (+0100)
Subject: auth: add zone removal to the zonecache
X-Git-Tag: auth-4.7.0-alpha1~75^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b72b74c5c22a59cf216e815323d1bf9248c440ac;p=thirdparty%2Fpdns.git

auth: add zone removal to the zonecache
---

diff --git a/docs/.gitignore b/docs/.gitignore
index 0d9cf88df4..276818114e 100644
--- a/docs/.gitignore
+++ b/docs/.gitignore
@@ -17,3 +17,4 @@
 /doctrees
 html-docs.tar.bz2
 /mans
+/mans.tmp
diff --git a/pdns/auth-zonecache.cc b/pdns/auth-zonecache.cc
index f5e5adec3b..80a0e469cd 100644
--- a/pdns/auth-zonecache.cc
+++ b/pdns/auth-zonecache.cc
@@ -98,15 +98,28 @@ void AuthZoneCache::replace(const vector<tuple<DNSName, int>>& zone_indices)
   }
 
   {
+    // process zone updates done while data collection for replace() was already in progress.
     auto pending = d_pending.lock();
-    if (pending->d_replacePending) {
-      // add/replace all zones created while data collection for replace() was already in progress.
-      for (const tuple<DNSName, int>& tup : pending->d_pendingAdds) {
-        const DNSName& zone = tup.get<0>();
-        CacheValue val;
-        val.zoneId = tup.get<1>();
-        auto& mc = newMaps[getMapIndex(zone)];
-        mc[zone] = val;
+    assert(pending->d_replacePending); // make sure we never forget to call setReplacePending()
+    for (const tuple<DNSName, int, bool>& tup : pending->d_pendingUpdates) {
+      const DNSName& zone = tup.get<0>();
+      CacheValue val;
+      val.zoneId = tup.get<1>();
+      bool insert = tup.get<2>();
+      auto& mc = newMaps[getMapIndex(zone)];
+      auto iter = mc.find(zone);
+      if (iter != mc.end()) {
+        if (insert) {
+          iter->second = std::move(val);
+        }
+        else {
+          mc.erase(iter);
+          count--;
+        }
+      }
+      else if (insert) {
+        mc.emplace(zone, val);
+        count++;
       }
     }
 
@@ -116,11 +129,11 @@ void AuthZoneCache::replace(const vector<tuple<DNSName, int>>& zone_indices)
       *map = std::move(newMaps[mapIndex]);
     }
 
-    pending->d_pendingAdds.clear();
+    pending->d_pendingUpdates.clear();
     pending->d_replacePending = false;
-  }
 
-  d_statnumentries->store(count);
+    d_statnumentries->store(count);
+  }
 }
 
 void AuthZoneCache::add(const DNSName& zone, const int zoneId)
@@ -131,7 +144,7 @@ void AuthZoneCache::add(const DNSName& zone, const int zoneId)
   {
     auto pending = d_pending.lock();
     if (pending->d_replacePending) {
-      pending->d_pendingAdds.emplace_back(zone, zoneId);
+      pending->d_pendingUpdates.emplace_back(zone, zoneId, true);
     }
   }
 
@@ -148,6 +161,29 @@ void AuthZoneCache::add(const DNSName& zone, const int zoneId)
     }
     else {
       map->emplace(zone, val);
+      (*d_statnumentries)++;
+    }
+  }
+}
+
+void AuthZoneCache::remove(const DNSName& zone)
+{
+  if (!d_refreshinterval)
+    return;
+
+  {
+    auto pending = d_pending.lock();
+    if (pending->d_replacePending) {
+      pending->d_pendingUpdates.emplace_back(zone, -1, false);
+    }
+  }
+
+  int mapIndex = getMapIndex(zone);
+  {
+    auto& mc = d_maps[mapIndex];
+    auto map = mc.d_map.write_lock();
+    if (map->erase(zone)) {
+      (*d_statnumentries)--;
     }
   }
 }
@@ -160,6 +196,6 @@ void AuthZoneCache::setReplacePending()
   {
     auto pending = d_pending.lock();
     pending->d_replacePending = true;
-    pending->d_pendingAdds.clear();
+    pending->d_pendingUpdates.clear();
   }
 }
diff --git a/pdns/auth-zonecache.hh b/pdns/auth-zonecache.hh
index e3968e1f89..474b94641a 100644
--- a/pdns/auth-zonecache.hh
+++ b/pdns/auth-zonecache.hh
@@ -34,6 +34,7 @@ public:
 
   void replace(const vector<tuple<DNSName, int>>& zone);
   void add(const DNSName& zone, const int zoneId);
+  void remove(const DNSName& zone);
   void setReplacePending(); //!< call this when data collection for the subsequent replace() call starts.
 
   bool getEntry(const DNSName& zone, int& zoneId);
@@ -90,7 +91,7 @@ private:
 
   struct PendingData
   {
-    std::vector<tuple<DNSName, int>> d_pendingAdds;
+    std::vector<tuple<DNSName, int, bool>> d_pendingUpdates;
     bool d_replacePending{false};
   };
   LockGuarded<PendingData> d_pending;
diff --git a/pdns/test-auth-zonecache_cc.cc b/pdns/test-auth-zonecache_cc.cc
index d2734e200c..330f145201 100644
--- a/pdns/test-auth-zonecache_cc.cc
+++ b/pdns/test-auth-zonecache_cc.cc
@@ -43,6 +43,7 @@ BOOST_AUTO_TEST_CASE(test_replace)
   vector<tuple<DNSName, int>> zone_indices{
     {DNSName("example.org."), 1},
   };
+  cache.setReplacePending();
   cache.replace(zone_indices);
 
   int zoneId = 0;
@@ -70,6 +71,24 @@ BOOST_AUTO_TEST_CASE(test_add_while_pending_replace)
   }
 }
 
+BOOST_AUTO_TEST_CASE(test_remove_while_pending_replace)
+{
+  AuthZoneCache cache;
+  cache.setRefreshInterval(3600);
+
+  vector<tuple<DNSName, int>> zone_indices{
+    {DNSName("powerdns.org."), 1}};
+  cache.setReplacePending();
+  cache.remove(DNSName("powerdns.org."));
+  cache.replace(zone_indices);
+
+  int zoneId = 0;
+  bool found = cache.getEntry(DNSName("example.org."), zoneId);
+  if (found) {
+    BOOST_FAIL("zone removed while replace was pending is found");
+  }
+}
+
 // Add zone using .add(), but also in the .replace() data
 BOOST_AUTO_TEST_CASE(test_add_while_pending_replace_duplicate)
 {
diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 03ed5c3f13..45779e0a2d 100644
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -1844,6 +1844,8 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) {
         throw ApiException("Deleting domain '"+zonename.toString()+"' failed: backend delete failed/unsupported");
 
       di.backend->commitTransaction();
+
+      g_zoneCache.remove(zonename);
     } catch (...) {
       di.backend->abortTransaction();
       throw;
@@ -2278,6 +2280,9 @@ static void apiServerCacheFlush(HttpRequest* req, HttpResponse* resp) {
       // Handle this first, to avoid concurrent queries re-populating the other caches.
       g_zoneCache.add(di.zone, di.id);
     }
+    else {
+      g_zoneCache.remove(di.zone);
+    }
   }
 
   // purge entire zone from cache, not just zone-level records.