From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Wed, 12 Jan 2022 15:24:43 +0000 (+0100)
Subject: dnsdist: Comment on the use of 'poolname', add option to QPSPoolAction as well
X-Git-Tag: auth-4.7.0-alpha1~30^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ab7b796006ee50dd7c9e700f8e380f1689b21c3d;p=thirdparty%2Fpdns.git

dnsdist: Comment on the use of 'poolname', add option to QPSPoolAction as well
---

diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc
index 1ce6cc7500..df7189205a 100644
--- a/pdns/dnsdist-lua-actions.cc
+++ b/pdns/dnsdist-lua-actions.cc
@@ -278,6 +278,7 @@ public:
   DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override
   {
     if (d_stopProcessing) {
+      /* we need to do it that way to keep compatiblity with custom Lua actions returning DNSAction.Pool, 'poolname' */
       *ruleresult = d_pool;
       return Action::Pool;
     }
@@ -289,24 +290,31 @@ public:
 
   std::string toString() const override
   {
-    return "to pool "+d_pool;
+    return "to pool " + d_pool;
   }
 
 private:
-  std::string d_pool;
-  bool d_stopProcessing;
+  const std::string d_pool;
+  const bool d_stopProcessing;
 };
 
 
 class QPSPoolAction : public DNSAction
 {
 public:
-  QPSPoolAction(unsigned int limit, const std::string& pool) : d_qps(QPSLimiter(limit, limit)), d_pool(pool) {}
+  QPSPoolAction(unsigned int limit, const std::string& pool, bool stopProcessing) : d_qps(QPSLimiter(limit, limit)), d_pool(pool), d_stopProcessing(stopProcessing) {}
   DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override
   {
     if (d_qps.lock()->check()) {
-      *ruleresult = d_pool;
-      return Action::Pool;
+      if (d_stopProcessing) {
+        /* we need to do it that way to keep compatiblity with custom Lua actions returning DNSAction.Pool, 'poolname' */
+        *ruleresult = d_pool;
+        return Action::Pool;
+      }
+      else {
+        dq->poolname = d_pool;
+        return Action::None;
+      }
     }
     else {
       return Action::None;
@@ -314,12 +322,13 @@ public:
   }
   std::string toString() const override
   {
-    return "max " +std::to_string(d_qps.lock()->getRate())+" to pool "+d_pool;
+    return "max " + std::to_string(d_qps.lock()->getRate()) + " to pool " + d_pool;
   }
 
 private:
   mutable LockGuarded<QPSLimiter> d_qps;
   const std::string d_pool;
+  const bool d_stopProcessing;
 };
 
 class RCodeAction : public DNSAction
@@ -2151,15 +2160,15 @@ void setupLuaActions(LuaContext& luaCtx)
     });
 
   luaCtx.writeFunction("PoolAction", [](const std::string& a, boost::optional<bool> stopProcessing) {
-    return std::shared_ptr<DNSAction>(new PoolAction(a, stopProcessing.get_value_or(true)));
+      return std::shared_ptr<DNSAction>(new PoolAction(a, stopProcessing.get_value_or(true)));
     });
 
   luaCtx.writeFunction("QPSAction", [](int limit) {
       return std::shared_ptr<DNSAction>(new QPSAction(limit));
     });
 
-  luaCtx.writeFunction("QPSPoolAction", [](int limit, const std::string& a) {
-      return std::shared_ptr<DNSAction>(new QPSPoolAction(limit, a));
+  luaCtx.writeFunction("QPSPoolAction", [](int limit, const std::string& a, boost::optional<bool> stopProcessing) {
+      return std::shared_ptr<DNSAction>(new QPSPoolAction(limit, a, stopProcessing.get_value_or(true)));
     });
 
   luaCtx.writeFunction("SpoofAction", [](boost::variant<std::string,vector<pair<int, std::string>>> inp, boost::optional<responseParams_t> vars) {
diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 31fc9ad247..b63d3c2f49 100644
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -833,7 +833,9 @@ bool processRulesResult(const DNSAction::Action& action, DNSQuestion& dq, std::s
     return true;
     break;
   case DNSAction::Action::Pool:
-    dq.poolname=ruleresult;
+    /* we need to keep this because a custom Lua action can return
+       DNSAction.Spoof, 'poolname' */
+    dq.poolname = ruleresult;
     return true;
     break;
   case DNSAction::Action::NoRecurse:
diff --git a/pdns/dnsdistdist/docs/rules-actions.rst b/pdns/dnsdistdist/docs/rules-actions.rst
index 592cc47a6b..69fd508eda 100644
--- a/pdns/dnsdistdist/docs/rules-actions.rst
+++ b/pdns/dnsdistdist/docs/rules-actions.rst
@@ -1223,11 +1223,15 @@ The following actions exist.
 
 .. function:: QPSPoolAction(maxqps, poolname)
 
-  Send the packet into the specified pool only if it does not exceed the ``maxqps`` queries per second limits.
+  .. versionchanged:: 1.8.0
+    Added the ``stop`` optional parameter.
+
+  Send the packet into the specified pool only if it does not exceed the ``maxqps`` queries per second limits. If ``stop`` is set to false, subsequent rules will be processed after this action.
   Letting the subsequent rules apply otherwise.
 
   :param int maxqps: The QPS limit for that pool
   :param string poolname: The name of the pool
+  :param bool stop: Whether to stop processing rules after this action. Default is true, meaning the remaining rules will not be processed.
 
 .. function:: RCodeAction(rcode [, options])