dnsdist: Apply suggestions from Otto's reviews of the new configuration

pdns/dnsdistdist/dnsdist-backend.cc

@@ -310,12 +310,12 @@ DownstreamState::DownstreamState(DownstreamState::Config&& config, std::shared_p
       setupDoHClientProtocolNegotiation(d_tlsCtx);
 
       auto outgoingDoHWorkerThreads = dnsdist::configuration::getImmutableConfiguration().d_outgoingDoHWorkers;
-      if (dnsdist::configuration::isConfigurationDone() && outgoingDoHWorkerThreads && *outgoingDoHWorkerThreads == 0) {
+      if (dnsdist::configuration::isImmutableConfigurationDone() && outgoingDoHWorkerThreads && *outgoingDoHWorkerThreads == 0) {
         throw std::runtime_error("Error: setOutgoingDoHWorkerThreads() is set to 0 so no outgoing DoH worker thread is available to serve queries");
       }
 
-      if (!dnsdist::configuration::isConfigurationDone() && (!outgoingDoHWorkerThreads || *outgoingDoHWorkerThreads == 0)) {
-        dnsdist::configuration::updateImmutableConfiguration([](dnsdist::configuration::Configuration& immutableConfig) {
+      if (!dnsdist::configuration::isImmutableConfigurationDone() && (!outgoingDoHWorkerThreads || *outgoingDoHWorkerThreads == 0)) {
+        dnsdist::configuration::updateImmutableConfiguration([](dnsdist::configuration::ImmutableConfiguration& immutableConfig) {
           immutableConfig.d_outgoingDoHWorkers = 1;
         });
       }

pdns/dnsdistdist/dnsdist-configuration.cc

@@ -26,8 +26,8 @@
 namespace dnsdist::configuration
 {
 static GlobalStateHolder<RuntimeConfiguration> s_currentRuntimeConfiguration;
-static Configuration s_configuration;
-static std::atomic<bool> s_configurationDone{false};
+static ImmutableConfiguration s_immutableConfiguration;
+static std::atomic<bool> s_immutableConfigurationDone{false};
 
 const RuntimeConfiguration& getCurrentRuntimeConfiguration()
 {
@@ -40,27 +40,29 @@ void updateRuntimeConfiguration(const std::function<void(RuntimeConfiguration&)>
   s_currentRuntimeConfiguration.modify(mutator);
 }
 
-void updateImmutableConfiguration(const std::function<void(Configuration&)>& mutator)
+void updateImmutableConfiguration(const std::function<void(ImmutableConfiguration&)>& mutator)
 {
-  if (isConfigurationDone()) {
+  if (isImmutableConfigurationDone()) {
     throw std::runtime_error("Trying to update an immutable setting at runtime!");
   }
 
-  mutator(s_configuration);
+  mutator(s_immutableConfiguration);
 }
 
-const Configuration& getImmutableConfiguration()
+const ImmutableConfiguration& getImmutableConfiguration()
 {
-  return s_configuration;
+  return s_immutableConfiguration;
 }
 
-bool isConfigurationDone()
+bool isImmutableConfigurationDone()
 {
-  return s_configurationDone.load();
+  return s_immutableConfigurationDone.load();
 }
 
-void setConfigurationDone()
+void setImmutableConfigurationDone()
 {
-  s_configurationDone.store(true);
+  if (s_immutableConfigurationDone.exchange(true)) {
+    throw std::runtime_error("Trying to seal the runtime-immutable configuration a second time");
+  }
 }
 }

pdns/dnsdistdist/dnsdist-configuration.hh

@@ -44,6 +44,7 @@ using servers_t = std::vector<std::shared_ptr<DownstreamState>>;
 
 namespace dnsdist::configuration
 {
+/* This part of the configuration is compile-time only */
 /* when we add EDNS to a query, we don't want to advertise
    a large buffer size */
 static constexpr size_t s_EdnsUDPPayloadSize{512};
@@ -53,7 +54,7 @@ static_assert(s_defaultPayloadSizeSelfGenAnswers < s_udpIncomingBufferSize, "The
 
 /* this part of the configuration can only be updated at configuration
    time, and is immutable once the configuration phase is over */
-struct Configuration
+struct ImmutableConfiguration
 {
   std::set<std::string> d_capabilitiesToRetain;
   std::vector<uint32_t> d_tcpFastOpenKey;
@@ -167,13 +168,13 @@ struct RuntimeConfiguration
 */
 const RuntimeConfiguration& getCurrentRuntimeConfiguration();
 /* Get the runtime-immutable configuration */
-const Configuration& getImmutableConfiguration();
+const ImmutableConfiguration& getImmutableConfiguration();
 /* Update the runtime-immutable part of the configuration. This function can only be called
    during configuration time (isConfigurationDone() returns false), and will throw otherwise. */
-void updateImmutableConfiguration(const std::function<void(Configuration&)>& mutator);
+void updateImmutableConfiguration(const std::function<void(ImmutableConfiguration&)>& mutator);
 void updateRuntimeConfiguration(const std::function<void(RuntimeConfiguration&)>& mutator);
 /* Whether parsing the configuration is done, meaning the runtime-immutable part of the
    configuration is now sealed */
-bool isConfigurationDone();
-void setConfigurationDone();
+bool isImmutableConfigurationDone();
+void setImmutableConfigurationDone();
 }

pdns/dnsdistdist/dnsdist-lua-bindings.cc

@@ -670,7 +670,7 @@ void setupLuaBindings(LuaContext& luaCtx, bool client, bool configCheck)
 
   luaCtx.registerFunction<void (std::shared_ptr<BPFFilter>::*)()>("attachToAllBinds", [](std::shared_ptr<BPFFilter>& bpf) {
     std::string res;
-    if (!dnsdist::configuration::isConfigurationDone()) {
+    if (!dnsdist::configuration::isImmutableConfigurationDone()) {
       throw std::runtime_error("attachToAllBinds() cannot be used at configuration time!");
       return;
     }
@@ -738,7 +738,7 @@ void setupLuaBindings(LuaContext& luaCtx, bool client, bool configCheck)
 #ifdef HAVE_XSK
   using xskopt_t = LuaAssociativeTable<boost::variant<uint32_t, std::string>>;
   luaCtx.writeFunction("newXsk", [client](xskopt_t opts) {
-    if (dnsdist::configuration::isConfigurationDone()) {
+    if (dnsdist::configuration::isImmutableConfigurationDone()) {
       throw std::runtime_error("newXsk() only can be used at configuration time!");
     }
     if (client) {