rec: allow recursor.conf file to contain YAML

This should us to work around the packaging issues discussed in #13935.
THe idea is that modify the parsing so that .conf files also *may* contain YAML.

The search for a config file then becomes:

1. Try read recuror.yml if it exists. If valid, done. If it is invalid punt.
2. Try read recursor.conf as YAML. If it is valid, done.
3. If it is invalid, try to read as old-style.

This means that the status of recursor.conf as a config file does not change.

This allows us to install a default YAML config into recursor.conf for new installs.
Of course we leave recursor.conf (and recursor.yml) alone for existing installs.

This is a draft. I will add docs and packaging changes after this is deemed
the way to proceed.

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 57004038e9857a306a36a49eaafe2828ec65fdc6..a1295147748853030af07a64b0e629ce8ca65ffd 100644 (file)
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -85,6 +85,7 @@ string g_pidfname;
 RecursorControlChannel g_rcc; // only active in the handler thread
 bool g_regressionTestMode;
 bool g_yamlSettings;
+string g_yamlSettingsSuffix;
 bool g_luaSettingsInYAML;
 
 #ifdef NOD_ENABLED
@@ -1382,7 +1383,7 @@ void parseACLs()
     cleanSlashes(configName);
 
     if (g_yamlSettings) {
-      configName += ".yml";
+      configName += g_yamlSettingsSuffix;
       string msg;
       pdns::rust::settings::rec::Recursorsettings settings;
       // XXX Does ::arg()["include-dir"] have the right value, i.e. potentially overriden by command line?
@@ -3214,11 +3215,20 @@ int main(int argc, char** argv)
 
     ::arg().setSLog(startupLog);
 
-    const string yamlconfigname = configname + ".yml";
+    string yamlconfigname;
     pdns::rust::settings::rec::Recursorsettings settings;
-    auto yamlstatus = pdns::settings::rec::tryReadYAML(yamlconfigname, true, g_yamlSettings, g_luaSettingsInYAML, settings, startupLog);
-    if (yamlstatus == pdns::settings::rec::PresentButFailed) {
-      return 1;
+    pdns::settings::rec::YamlSettingsStatus yamlstatus{};
+
+    for (const string suffix : {".yml", ".conf"}) {
+      yamlconfigname = configname + suffix;
+      yamlstatus = pdns::settings::rec::tryReadYAML(yamlconfigname, true, g_yamlSettings, g_luaSettingsInYAML, settings, startupLog);
+      if (yamlstatus == pdns::settings::rec::YamlSettingsStatus::OK) {
+        g_yamlSettingsSuffix = suffix;
+        break;
+      }
+      if (suffix == ".yml" && yamlstatus == pdns::settings::rec::PresentButFailed) {
+        return 1;
+      }
     }
 
     if (g_yamlSettings) {
@@ -3232,8 +3242,9 @@ int main(int argc, char** argv)
       auto lock = g_yamlStruct.lock();
       *lock = std::move(settings);
     }
-    if (yamlstatus == pdns::settings::rec::YamlSettingsStatus::CannotOpen) {
+    else {
       configname += ".conf";
+      startupLog->info(Logr::Warning, "Trying to read YAML from .yml or .conf failed, failing back to old-style config read", "configname", Logging::Loggable(configname));
       bool mustExit = false;
       std::tie(ret, mustExit) = doConfig(startupLog, configname, argc, argv);
       if (ret != 0 || mustExit) {

diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh
index f9260e592ac71d1b1e9239305412345af457ac06..e30224e4c736d68079d7a30d290dad1ff3603594 100644 (file)
--- a/pdns/recursordist/rec-main.hh
+++ b/pdns/recursordist/rec-main.hh
@@ -193,6 +193,7 @@ extern std::unique_ptr<RecursorPacketCache> g_packetCache;
 using RemoteLoggerStats_t = std::unordered_map<std::string, RemoteLoggerInterface::Stats>;
 
 extern bool g_yamlSettings;
+extern string g_yamlSettingsSuffix;
 extern bool g_logCommonErrors;
 extern size_t g_proxyProtocolMaximumSize;
 extern std::atomic<bool> g_quiet;

diff --git a/pdns/recursordist/rec_control.cc b/pdns/recursordist/rec_control.cc
index 6b2937cf44ab17a081da1cc15825d6177aa1079a..30187a11fe422b7d9105219c4cc9174c0c49c4fd 100644 (file)
--- a/pdns/recursordist/rec_control.cc
+++ b/pdns/recursordist/rec_control.cc
@@ -74,26 +74,33 @@ static void initArguments(int argc, char** argv, Logr::log_t log)
 
   cleanSlashes(configname);
 
-  const string yamlconfigname = configname + ".yml";
   string msg;
   pdns::rust::settings::rec::Recursorsettings settings;
-
-  auto yamlstatus = pdns::settings::rec::readYamlSettings(yamlconfigname, "", settings, msg, g_slog);
-
-  switch (yamlstatus) {
-  case pdns::settings::rec::YamlSettingsStatus::CannotOpen:
-    break;
-  case pdns::settings::rec::YamlSettingsStatus::PresentButFailed:
-    log->error(Logr::Error, msg, "YAML config found, but error ocurred processing it", "configname", Logging::Loggable(yamlconfigname));
-    exit(1); // NOLINT(concurrency-mt-unsafe)
-    break;
-  case pdns::settings::rec::YamlSettingsStatus::OK:
-    log->info(Logr::Notice, "YAML config found and processed", "configname", Logging::Loggable(yamlconfigname));
-    pdns::settings::rec::bridgeStructToOldStyleSettings(settings);
-    break;
+  pdns::settings::rec::YamlSettingsStatus yamlstatus{};
+
+  for (const string suffix : {".yml", ".conf"}) {
+    const string yamlconfigname = configname + suffix;
+    yamlstatus = pdns::settings::rec::readYamlSettings(yamlconfigname, "", settings, msg, g_slog);
+
+    switch (yamlstatus) {
+    case pdns::settings::rec::YamlSettingsStatus::CannotOpen:
+      break;
+    case pdns::settings::rec::YamlSettingsStatus::PresentButFailed:
+      if (suffix == ".yml") {
+        log->error(Logr::Error, msg, "YAML config found, but error ocurred processing it", "configname", Logging::Loggable(yamlconfigname));
+        exit(1); // NOLINT(concurrency-mt-unsafe)
+      }
+      break;
+    case pdns::settings::rec::YamlSettingsStatus::OK:
+      log->info(Logr::Notice, "YAML config found and processed", "configname", Logging::Loggable(yamlconfigname));
+      pdns::settings::rec::bridgeStructToOldStyleSettings(settings);
+      break;
+    }
+    if (yamlstatus == pdns::settings::rec::YamlSettingsStatus::OK) {
+      break;
+    }
   }
-
-  if (yamlstatus == pdns::settings::rec::YamlSettingsStatus::CannotOpen) {
+  if (yamlstatus != pdns::settings::rec::YamlSettingsStatus::OK) {
     configname += ".conf";
     arg().laxFile(configname);
   }

diff --git a/pdns/recursordist/reczones.cc b/pdns/recursordist/reczones.cc
index f8d50bdfb9cb5aecc1c81616f81bb4d08d2592b5..747c11d83d9a4a09fb5511bf9ec2ee4e3b5e9248 100644 (file)
--- a/pdns/recursordist/reczones.cc
+++ b/pdns/recursordist/reczones.cc
@@ -35,8 +35,10 @@
 #include "settings/cxxsettings.hh"
 #include "rec-system-resolve.hh"
 
+// XXX consider including rec-main.hh?
 extern int g_argc;
 extern char** g_argv;
+extern string g_yamlSettingsSuffix;
 
 bool primeHints(time_t now)
 {
@@ -147,7 +149,7 @@ string reloadZoneConfiguration(bool yaml)
          log->info(Logr::Notice, "Reloading zones, purging data from cache"));
 
     if (yaml) {
-      configname += ".yml";
+      configname += g_yamlSettingsSuffix;
       string msg;
       pdns::rust::settings::rec::Recursorsettings settings;
       // XXX Does ::arg()["include-dir"] have the right value, i.e. potentially overriden by command line?