rec: handle applicable dynamic parts of YAML only config when reloading Lua config

pdns/recursordist/rec-rust-lib/cxxsupport.cc

@@ -1429,8 +1429,6 @@ bool pdns::settings::rec::luaItemSet(const pdns::rust::settings::rec::Recursorse
   alldefault = alldefault && settings.recordcache.zonetocaches.empty();
   alldefault = alldefault && settings.recursor.allowed_additional_qtypes.empty();
   alldefault = alldefault && settings.incoming.proxymappings.empty();
-  alldefault = alldefault && settings.outgoing.tls_configurations.empty(); // actually not a Lua item, but very much alike
-  alldefault = alldefault && settings.logging.opentelemetry_trace_conditions.empty(); // actually not a Lua item, but very much alike
   return !alldefault;
 }
 

pdns/recursordist/rec_channel_rec.cc

@@ -2075,32 +2075,69 @@ static RecursorControlChannel::Answer help(ArgIterator /* begin */, ArgIterator
   return {0, str.str()};
 }
 
+// The common part of activating the newly read config
+static void activateLua(LuaConfigItems& lci, bool broadcast, ProxyMapping&& proxyMapping, OpenTelemetryTraceConditions&& conditions)
+{
+  extern std::unique_ptr<ProxyMapping> g_proxyMapping;
+
+  activateLuaConfig(lci);
+  lci = g_luaconfs.getCopy();
+  if (broadcast) {
+    startLuaConfigDelayedThreads(lci, lci.generation);
+    broadcastFunction([pmap = std::move(proxyMapping)] { return pleaseSupplantProxyMapping(pmap); });
+    broadcastFunction([conds = std::move(conditions)] { return pleaseSupplantOTConditions(conds); });
+  }
+  else {
+    // Initial proxy mapping and OT conditions
+    g_proxyMapping = proxyMapping.empty() ? nullptr : std::make_unique<ProxyMapping>(proxyMapping);
+    *g_initialOpenTelemetryConditions.lock() = conditions.empty() ? nullptr : std::make_unique<OpenTelemetryTraceConditions>(conditions);
+  }
+  if (broadcast) {
+    g_slog->withName("config")->info(Logr::Info, "Reloaded");
+  }
+}
+
 RecursorControlChannel::Answer luaconfig(bool broadcast)
 {
   ProxyMapping proxyMapping;
   LuaConfigItems lci;
   lci.d_slog = g_slog;
-  extern std::unique_ptr<ProxyMapping> g_proxyMapping;
+  pdns::rust::settings::rec::Recursorsettings settings;
+  OpenTelemetryTraceConditions conditions;
+  pdns::settings::rec::YamlSettingsStatus yamlstat = pdns::settings::rec::YamlSettingsStatus::CannotOpen;
+
+  // If we have a YAML file read it to be able to use (parts of it) later
+  if (g_yamlSettings) {
+    string configname = ::arg()["config-dir"] + "/recursor";
+    if (!::arg()["config-name"].empty()) {
+      configname = ::arg()["config-dir"] + "/recursor-" + ::arg()["config-name"];
+    }
+    bool dummy1{};
+    bool dummy2{};
+    yamlstat = pdns::settings::rec::tryReadYAML(configname + g_yamlSettingsSuffix, false, dummy1, dummy2, settings, g_slog, Logr::Error);
+  }
+
   if (!g_luaSettingsInYAML) {
+    // We might have a lua config file, but also process dynamic YAML parts if applicable, currently those are:
+    // - the OT trace conditions
+    // - the outgoing TLS config
     try {
-      if (::arg()["lua-config-file"].empty()) {
-        return {0, "No Lua or corresponding YAML configuration active\n"};
+      if (yamlstat == pdns::settings::rec::YamlSettingsStatus::OK) {
+        // YAML read above succeeded
+        ProxyMapping dummyProxyMapping; // taken from lua, so ignore YAML
+        LuaConfigItems dummyLuaConfig; // we do not use the converted orm YAML LuaConfigItems, but the "real thing"
+        pdns::settings::rec::fromBridgeStructToLuaConfig(settings, dummyLuaConfig, dummyProxyMapping, conditions);
+        TCPOutConnectionManager::setupOutgoingTLSConfigTables(settings);
       }
-      loadRecursorLuaConfig(::arg()["lua-config-file"], proxyMapping, lci);
-      activateLuaConfig(lci);
-      lci = g_luaconfs.getCopy();
-      if (broadcast) {
-        startLuaConfigDelayedThreads(lci, lci.generation);
-        broadcastFunction([pmap = std::move(proxyMapping)] { return pleaseSupplantProxyMapping(pmap); });
+      if (!::arg()["lua-config-file"].empty()) {
+        loadRecursorLuaConfig(::arg()["lua-config-file"], proxyMapping, lci); // will bump generation
       }
-      else {
-        // Initial proxy mapping
-        g_proxyMapping = proxyMapping.empty() ? nullptr : std::make_unique<ProxyMapping>(proxyMapping);
-      }
-      if (broadcast) {
-        g_slog->withName("config")->info(Logr::Info, "Reloaded");
+      activateLua(lci, broadcast, std::move(proxyMapping), std::move(conditions));
+      string msg = "Reloaded dynamic parts of YAML config";
+      if (!::arg()["lua-config-file"].empty()) {
+        msg += " and Lua configuration file '" + ::arg()["lua-config-file"] + "'";
       }
-      return {0, "Reloaded Lua configuration file '" + ::arg()["lua-config-file"] + "'\n"};
+      return {0, msg + "\n"};
     }
     catch (std::exception& e) {
       return {1, "Unable to load Lua script from '" + ::arg()["lua-config-file"] + "': " + e.what() + "\n"};
@@ -2109,36 +2146,15 @@ RecursorControlChannel::Answer luaconfig(bool broadcast)
       return {1, "Unable to load Lua script from '" + ::arg()["lua-config-file"] + "': " + e.reason + "\n"};
     }
   }
+  // More simple case: we don't have any Lua config
   try {
-    string configname = ::arg()["config-dir"] + "/recursor";
-    if (!::arg()["config-name"].empty()) {
-      configname = ::arg()["config-dir"] + "/recursor-" + ::arg()["config-name"];
-    }
-    bool dummy1{};
-    bool dummy2{};
-    pdns::rust::settings::rec::Recursorsettings settings;
-    auto yamlstat = pdns::settings::rec::tryReadYAML(configname + g_yamlSettingsSuffix, false, dummy1, dummy2, settings, g_slog, Logr::Error);
     if (yamlstat != pdns::settings::rec::YamlSettingsStatus::OK) {
       return {1, "Reloading dynamic part of YAML configuration failed\n"};
     }
     auto generation = g_luaconfs.getLocal()->generation;
     lci.generation = generation + 1;
-    OpenTelemetryTraceConditions conditions;
     pdns::settings::rec::fromBridgeStructToLuaConfig(settings, lci, proxyMapping, conditions);
-    activateLuaConfig(lci);
-    lci = g_luaconfs.getCopy();
-    if (broadcast) {
-      startLuaConfigDelayedThreads(lci, lci.generation);
-
-      *g_initialOpenTelemetryConditions.lock() = conditions.empty() ? nullptr : std::make_unique<OpenTelemetryTraceConditions>(conditions);
-      broadcastFunction([pmap = std::move(proxyMapping)] { return pleaseSupplantProxyMapping(pmap); });
-      broadcastFunction([conds = std::move(conditions)] { return pleaseSupplantOTConditions(conds); });
-    }
-    else {
-      // Initial proxy mapping
-      g_proxyMapping = proxyMapping.empty() ? nullptr : std::make_unique<ProxyMapping>(proxyMapping);
-      *g_initialOpenTelemetryConditions.lock() = conditions.empty() ? nullptr : std::make_unique<OpenTelemetryTraceConditions>(conditions);
-    }
+    activateLua(lci, broadcast, std::move(proxyMapping), std::move(conditions));
     TCPOutConnectionManager::setupOutgoingTLSConfigTables(settings);
 
     return {0, "Reloaded dynamic part of YAML configuration\n"};

regression-tests.recursor-dnssec/test_ForwardOverDoT.py

@@ -11,13 +11,33 @@ class SimpleForwardOverDoTTest(RecursorTest):
 
     _confdir = 'SimpleForwardOverDoT'
     _config_template = """
-dnssec=validate
-forward-zones-recurse=.=1.1.1.1:853;8.8.8.8:853;9.9.9.9:853
-devonly-regression-test-mode
-    """
+dnssec:
+    validation: validate
+recursor:
+    forward_zones_recurse:
+    - zone: .
+      forwarders: [1.1.1.1:853,8.8.8.8:853,9.9.9.9:853]
+    devonly_regression_test_mode: true
+outgoing:
+    tcp_max_queries: 1
+    dont_throttle_netmasks: [0.0.0.0/0, '::/0']
+"""
+
+    @classmethod
+    def generateRecursorConfig(cls, confdir):
+        super(SimpleForwardOverDoTTest, cls).generateRecursorYamlConfig(confdir, False)
+
+    def reloadConfig(self, config):
+      confdir = os.path.join('configs', SimpleForwardOverDoTTest._confdir)
+      SimpleForwardOverDoTTest._config_template = config
+      SimpleForwardOverDoTTest.generateRecursorYamlConfig(confdir, False)
+      SimpleForwardOverDoTTest.recControl(confdir, 'reload-yaml')
 
     @pytest.mark.external
-    def testA(self):
+    def testBasic(self):
+        confdir = 'configs/' + self._confdir
+        self.reloadConfig(self._config_template)
+        self.recControl(confdir, 'reload-zones')
         expected = dns.rrset.from_text('dns.google.', 0, dns.rdataclass.IN, 'A', '8.8.8.8', '8.8.4.4')
         query = dns.message.make_query('dns.google', 'A', want_dnssec=True)
         query.flags |= dns.flags.AD
@@ -28,36 +48,14 @@ def testA(self):
         self.assertRRsetInAnswer(res, expected)
         self.assertMatchingRRSIGInAnswer(res, expected)
 
-        rec_controlCmd = [os.environ['RECCONTROL'],
-                          '--config-dir=%s' % 'configs/' + self._confdir,
-                          'get dot-outqueries']
-        try:
-            ret = subprocess.check_output(rec_controlCmd, stderr=subprocess.STDOUT)
-            self.assertNotEqual(ret, b'UNKNOWN\n')
-            self.assertNotEqual(ret, b'0\n')
-
-        except subprocess.CalledProcessError as e:
-            print(e.output)
-            raise
-
-        rec_controlCmd = [os.environ['RECCONTROL'],
-                          '--config-dir=%s' % 'configs/' + self._confdir,
-                          'get tcp-outqueries']
-        try:
-            ret = subprocess.check_output(rec_controlCmd, stderr=subprocess.STDOUT)
-            self.assertEqual(ret, b'0\n')
-
-        except subprocess.CalledProcessError as e:
-            print(e.output)
-            raise
-
-class ForwardOverDoTTest(RecursorTest):
-    """
-    This is forwarding to DoT servers with validation and is dependent on the forwards working for DoT
-    """
+        ret = self.recControl(confdir, 'get', 'dot-outqueries')
+        self.assertNotEqual(ret, 'UNKNOWN\n')
+        self.assertNotEqual(ret, '0\n')
 
-    _confdir = 'ForwardOverDoT'
-    _config_template = """
+        ret = self.recControl(confdir, 'get', 'tcp-outqueries')
+        self.assertEqual(ret, '0\n')
+
+    _config_template_test2 = """
 dnssec:
     validation: validate
 outgoing:
@@ -71,19 +69,20 @@ class ForwardOverDoTTest(RecursorTest):
       subject_name: dns.google
       validate_certificate: true
       verbose_logging: true
+    tcp_max_queries: 1
+    dont_throttle_netmasks: [0.0.0.0/0, '::/0']
 recursor:
     forward_zones_recurse:
     - zone: .
       forwarders: [1.1.1.1:853,8.8.8.8:853,9.9.9.9:853]
     devonly_regression_test_mode: true
     """
 
-    @classmethod
-    def generateRecursorConfig(cls, confdir):
-        super(ForwardOverDoTTest, cls).generateRecursorYamlConfig(confdir, False)
-
     @pytest.mark.external
-    def testA(self):
+    def testWithVerify(self):
+        confdir = 'configs/' + self._confdir
+        self.reloadConfig(self._config_template_test2)
+        self.recControl(confdir, 'reload-zones')
         expected = dns.rrset.from_text('dns.google.', 0, dns.rdataclass.IN, 'A', '8.8.8.8', '8.8.4.4')
         query = dns.message.make_query('dns.google', 'A', want_dnssec=True)
         query.flags |= dns.flags.AD
@@ -94,26 +93,45 @@ def testA(self):
         self.assertRRsetInAnswer(res, expected)
         self.assertMatchingRRSIGInAnswer(res, expected)
 
-        rec_controlCmd = [os.environ['RECCONTROL'],
-                          '--config-dir=%s' % 'configs/' + self._confdir,
-                          'get dot-outqueries']
-        try:
-            ret = subprocess.check_output(rec_controlCmd, stderr=subprocess.STDOUT)
-            self.assertNotEqual(ret, b'UNKNOWN\n')
-            self.assertNotEqual(ret, b'0\n')
-
-        except subprocess.CalledProcessError as e:
-            print(e.output)
-            raise
-
-        rec_controlCmd = [os.environ['RECCONTROL'],
-                          '--config-dir=%s' % 'configs/' + self._confdir,
-                          'get tcp-outqueries']
-        try:
-            ret = subprocess.check_output(rec_controlCmd, stderr=subprocess.STDOUT)
-            self.assertEqual(ret, b'0\n')
-
-        except subprocess.CalledProcessError as e:
-            print(e.output)
-            raise
+        ret = self.recControl(confdir, 'get', 'dot-outqueries')
+        self.assertNotEqual(ret, 'UNKNOWN\n')
+        self.assertNotEqual(ret, '0\n')
+
+        ret = self.recControl(confdir, 'get', 'tcp-outqueries')
+        self.assertEqual(ret, '0\n')
+
+    _config_template_test3 = """
+dnssec:
+    validation: validate
+outgoing:
+    tls_configurations:
+    - name: fwtopublic
+      subnets: [1.1.1.1,9.9.9.9]
+      validate_certificate: true
+      verbose_logging: true
+      subject_name: WRONG
+    - name: fwtogoogle
+      subnets: [8.8.8.8]
+      subject_name: dns.googleXXX
+      validate_certificate: true
+      verbose_logging: true
+    tcp_max_queries: 1
+    dont_throttle_netmasks: [0.0.0.0/0, '::/0']
+recursor:
+    forward_zones_recurse:
+    - zone: .
+      forwarders: [1.1.1.1:853,8.8.8.8:853,9.9.9.9:853]
+    devonly_regression_test_mode: true
+    """
+
+    @pytest.mark.external
+    def testCertFailed(self):
+        confdir = 'configs/' + self._confdir
+        self.reloadConfig(self._config_template_test3)
+        self.recControl(confdir, 'reload-zones')
+        expected = dns.rrset.from_text('dns.google.', 0, dns.rdataclass.IN, 'A', '8.8.8.8', '8.8.4.4')
+        query = dns.message.make_query('dns.google', 'A', want_dnssec=True)
+        query.flags |= dns.flags.AD
 
+        res = self.sendUDPQuery(query)
+        self.assertRcodeEqual(res, dns.rcode.SERVFAIL)