Small stuff from review comments

diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc
index 4d3f10dd94203adc45fe73070aab95dcd7e80181..cc7fe088d2f97c0c3d0eb6318642a5929450af92 100644 (file)
--- a/pdns/rec-lua-conf.cc
+++ b/pdns/rec-lua-conf.cc
@@ -403,7 +403,12 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de
   Lua.writeFunction("zoneToCache", [&delayedThreads](const string& zoneName, const string& method, const boost::variant<string, std::vector<std::pair<int, string>>>& srcs, boost::optional<zoneToCacheOptions_t> options) {
     try {
       RecZoneToCache::Config conf;
+      DNSName validZoneName(zoneName);
       conf.d_zone = zoneName;
+      const set<string> methods = {"axfr", "url", "file"};
+      if (methods.count(method) == 0) {
+        throw std::runtime_error("unknwon method '" + method + "'");
+      }
       conf.d_method = method;
       if (srcs.type() == typeid(std::string)) {
         conf.d_sources.push_back(boost::get<std::string>(srcs));
@@ -413,40 +418,43 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de
           conf.d_sources.push_back(src.second);
         }
       }
+      if (conf.d_sources.size() == 0) {
+        throw std::runtime_error("at least one source required");
+      }
       if (options) {
         auto& have = *options;
         if (have.count("timeout")) {
-          conf.d_timeout = boost::get<uint32_t>(have["timeout"]);
+          conf.d_timeout = boost::get<uint32_t>(have.at("timeout"));
         }
         if (have.count("tsigname")) {
-          conf.d_tt.name = DNSName(toLower(boost::get<string>(have["tsigname"])));
-          conf.d_tt.algo = DNSName(toLower(boost::get<string>(have[ "tsigalgo"])));
-          if (B64Decode(boost::get<string>(have[ "tsigsecret"]), conf.d_tt.secret)) {
+          conf.d_tt.name = DNSName(toLower(boost::get<string>(have.at("tsigname"))));
+          conf.d_tt.algo = DNSName(toLower(boost::get<string>(have.at("tsigalgo"))));
+          if (B64Decode(boost::get<string>(have.at("tsigsecret")), conf.d_tt.secret)) {
             throw std::runtime_error("TSIG secret is not valid Base-64 encoded");
           }
         }
         if (have.count("maxReceivedMBytes")) {
-          conf.d_maxReceivedBytes = static_cast<size_t>(boost::get<uint32_t>(have["maxReceivedMBytes"]));
+          conf.d_maxReceivedBytes = static_cast<size_t>(boost::get<uint32_t>(have.at("maxReceivedMBytes")));
           conf.d_maxReceivedBytes *= 1024 * 1024;
         }
         if (have.count("localAddress")) {
-          conf.d_local = ComboAddress(boost::get<string>(have["localAddress"]));
+          conf.d_local = ComboAddress(boost::get<string>(have.at("localAddress")));
         }
         if (have.count("refreshPeriod")) {
-          conf.d_refreshPeriod = boost::get<uint32_t>(have["refreshPeriod"]);
+          conf.d_refreshPeriod = boost::get<uint32_t>(have.at("refreshPeriod"));
         }
         if (have.count("retryOnErrorPeriod")) {
-          conf.d_retryOnError = boost::get<uint32_t>(have["retryOnErrorPeriod"]);
+          conf.d_retryOnError = boost::get<uint32_t>(have.at("retryOnErrorPeriod"));
         }
       }
 
       delayedThreads.ztcConfigs.push_back(conf);
     }
-    catch (const std::exception&e ) {
-      g_log<<Logger::Error<<"Problem configuring zoneToCache: "<<e.what()<<endl;
+    catch (const std::exception& e) {
+      g_log<<Logger::Error<<"Problem configuring zoneToCache for zone '" << zoneName << "': " << e.what() << endl;
     }
   });
- 
+
   typedef vector<pair<int,boost::variant<string, vector<pair<int, string> > > > > argvec_t;
   Lua.writeFunction("addSortList", 
                    [&lci](const std::string& formask_, 

diff --git a/pdns/recursordist/rec-zonetocache.cc b/pdns/recursordist/rec-zonetocache.cc
index ffdd6777248617ff3ae58c4cb9858decb120ab0c..ea614df3558a7af5f74d144a938ab4e2e7785b98 100644 (file)
--- a/pdns/recursordist/rec-zonetocache.cc
+++ b/pdns/recursordist/rec-zonetocache.cc
@@ -49,13 +49,13 @@ struct ZoneData
   DNSName d_zone;
   shared_ptr<Logr::Logger>& d_log;
 
-  bool isRRSetAuth(const DNSName& qname, QType qtype);
+  bool isRRSetAuth(const DNSName& qname, QType qtype) const;
   void parseDRForCache(DNSRecord& dr);
   void getByAXFR(const RecZoneToCache::Config&);
-  void ZoneToCache(const RecZoneToCache::Config& config);
+  void ZoneToCache(const RecZoneToCache::Config& config, uint64_t gen);
 };
 
-bool ZoneData::isRRSetAuth(const DNSName& qname, QType qtype)
+bool ZoneData::isRRSetAuth(const DNSName& qname, QType qtype) const
 {
   DNSName delegatedZone(qname);
   if (qtype == QType::DS) {
@@ -141,18 +141,18 @@ void ZoneData::getByAXFR(const RecZoneToCache::Config& config)
     }
     axfrNow = time(nullptr);
     if (axfrNow < axfrStart || axfrNow - axfrStart > axfrTimeout) {
-      throw PDNSException("Total AXFR time for zoneToCache exceeded!");
+      throw std::runtime_error("Total AXFR time for zoneToCache exceeded!");
     }
   }
 }
 
-static std::vector<std::string> getLinesFromFile(const RecZoneToCache::Config& config)
+static std::vector<std::string> getLinesFromFile(const std::string& file)
 {
 
   std::vector<std::string> lines;
-  std::ifstream stream(config.d_sources.at(0));
+  std::ifstream stream(file);
   if (!stream) {
-    throw PDNSException("Cannot read file: " + config.d_sources.at(0));
+    throw std::runtime_error("Cannot read file: " + file);
   }
   std::string line;
   while (std::getline(stream, line)) {
@@ -170,7 +170,7 @@ static std::vector<std::string> getURL(const RecZoneToCache::Config& config)
   std::string reply = mc.getURL(config.d_sources.at(0), nullptr, local == ComboAddress() ? nullptr : &local, config.d_timeout, false, true);
   if (config.d_maxReceivedBytes > 0 && reply.size() > config.d_maxReceivedBytes) {
     // We should actually detect this *during* the GET
-    throw PDNSException("Retrieved data exceeds maxReceivedBytes");
+    throw std::runtime_error("Retrieved data exceeds maxReceivedBytes");
   }
   std::istringstream stream(reply);
   string line;
@@ -181,7 +181,7 @@ static std::vector<std::string> getURL(const RecZoneToCache::Config& config)
   return lines;
 }
 
-void ZoneData::ZoneToCache(const RecZoneToCache::Config& config)
+void ZoneData::ZoneToCache(const RecZoneToCache::Config& config, uint64_t configGeneration)
 {
   if (config.d_sources.size() > 1) {
     d_log->info("Multiple sources not yet supported, using first");
@@ -206,11 +206,11 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config)
     }
     else if (config.d_method == "file") {
       d_log->info("Getting zone from file");
-      lines = getLinesFromFile(config);
+      lines = getLinesFromFile(config.d_sources.at(0));
     }
     DNSResourceRecord drr;
     ZoneParserTNG zpt(lines, d_zone);
-    zpt.setMaxGenerateSteps(0);
+    zpt.setMaxGenerateSteps(1);
 
     while (zpt.get(drr)) {
       DNSRecord dr(drr);
@@ -218,6 +218,12 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config)
     }
   }
 
+  // Extra check before we are touching the cache
+  auto luaconfsLocal = g_luaconfs.getLocal();
+  if (luaconfsLocal->generation != configGeneration) {
+    return;
+  }
+
   // Rerun, now inserting the rrsets into the cache with associated sigs
   d_now = time(nullptr);
   for (const auto& [key, v] : d_all) {
@@ -232,9 +238,7 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config)
       vector<shared_ptr<RRSIGRecordContent>> sigsrr;
       auto it = d_sigs.find(key);
       if (it != d_sigs.end()) {
-        for (const auto& sig : it->second) {
-          sigsrr.push_back(sig);
-        }
+        sigsrr = it->second;
       }
       bool auth = isRRSetAuth(qname, qtype);
       // Same decision as updateCacheFromRecords() (we do not test for NSEC since we skip those completely)
@@ -267,7 +271,11 @@ void RecZoneToCache::ZoneToCache(RecZoneToCache::Config config, uint64_t configG
     time_t refresh = config.d_retryOnError;
     try {
       ZoneData data(log);
-      data.ZoneToCache(config);
+      data.ZoneToCache(config, configGeneration);
+      if (luaconfsLocal->generation != configGeneration) {
+        log->info("A more recent configuration has been found, stopping the old update thread");
+        return;
+      }
       refresh = config.d_refreshPeriod;
       log->info("Loaded zone into cache", "refresh", Logging::Loggable(refresh));
     }