From: Chris Hofstaedtler <chris.hofstaedtler@deduktiva.com>
Date: Mon, 29 Jun 2020 21:38:57 +0000 (+0200)
Subject: backends: Pass masters as vector<ComboAddress>
X-Git-Tag: rec-4.4.0-alpha2~6^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F9280%2Fhead;p=thirdparty%2Fpdns.git

backends: Pass masters as vector<ComboAddress>

And make lmdbbackend set the master on domain creation.
---

diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc
index 7eae04b5c1..b768905076 100644
--- a/modules/lmdbbackend/lmdbbackend.cc
+++ b/modules/lmdbbackend/lmdbbackend.cc
@@ -758,20 +758,14 @@ void LMDBBackend::setNotified(uint32_t domain_id, uint32_t serial)
 }
 
 
-bool LMDBBackend::setMaster(const DNSName &domain, const std::string& ips)
+bool LMDBBackend::setMasters(const DNSName &domain, const vector<ComboAddress> &masters)
 {
-  vector<ComboAddress> masters;
-  vector<string> parts;
-  stringtok(parts, ips, " \t;,");
-  for(const auto& ip : parts) 
-    masters.push_back(ComboAddress(ip, 53));
-  
   return genChangeDomain(domain, [&masters](DomainInfo& di) {
       di.masters = masters;
     });
 }
 
-bool LMDBBackend::createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const string &masters, const string &account)
+bool LMDBBackend::createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const vector<ComboAddress> &masters, const string &account)
 {
   DomainInfo di;
 
@@ -779,9 +773,10 @@ bool LMDBBackend::createDomain(const DNSName &domain, const DomainInfo::DomainKi
   if(txn.get<0>(domain, di)) {
     throw DBException("Domain '"+domain.toLogString()+"' exists already");
   }
-  
+
   di.zone = domain;
   di.kind = kind;
+  di.masters = masters;
   di.account = account;
 
   txn.put(di);
diff --git a/modules/lmdbbackend/lmdbbackend.hh b/modules/lmdbbackend/lmdbbackend.hh
index 05dd2bc935..d37b497879 100644
--- a/modules/lmdbbackend/lmdbbackend.hh
+++ b/modules/lmdbbackend/lmdbbackend.hh
@@ -38,8 +38,8 @@ public:
   bool list(const DNSName &target, int id, bool include_disabled) override;
 
   bool getDomainInfo(const DNSName &domain, DomainInfo &di, bool getSerial=true) override;
-  bool createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const string &masters, const string &account) override;
-  
+  bool createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const vector<ComboAddress> &masters, const string &account) override;
+
   bool startTransaction(const DNSName &domain, int domain_id=-1) override;
   bool commitTransaction() override;
   bool abortTransaction() override;
@@ -55,7 +55,7 @@ public:
 
   void getUnfreshSlaveInfos(vector<DomainInfo>* domains) override;
   
-  bool setMaster(const DNSName &domain, const string &ip) override;
+  bool setMasters(const DNSName &domain, const vector<ComboAddress> &masters) override;
   bool setKind(const DNSName &domain, const DomainInfo::DomainKind kind) override;
   bool getAllDomainMetadata(const DNSName& name, std::map<std::string, std::vector<std::string> >& meta) override;
   bool getDomainMetadata(const DNSName& name, const std::string& kind, std::vector<std::string>& meta) override
diff --git a/pdns/backends/gsql/gsqlbackend.cc b/pdns/backends/gsql/gsqlbackend.cc
index ece61a7fda..4643b95d32 100644
--- a/pdns/backends/gsql/gsqlbackend.cc
+++ b/pdns/backends/gsql/gsqlbackend.cc
@@ -216,19 +216,26 @@ void GSQLBackend::setFresh(uint32_t domain_id)
   }
 }
 
-bool GSQLBackend::setMaster(const DNSName &domain, const string &ip)
+bool GSQLBackend::setMasters(const DNSName &domain, const vector<ComboAddress> &masters)
 {
+  vector<string> masters_s;
+  for (const auto& master : masters) {
+    masters_s.push_back(master.toStringWithPortExcept(53));
+  }
+
+  auto tmp = boost::join(masters_s, ", ");
+
   try {
     reconnectIfNeeded();
 
     d_UpdateMasterOfZoneQuery_stmt->
-      bind("master", ip)->
+      bind("master", tmp)->
       bind("domain", domain)->
       execute()->
       reset();
   }
   catch (SSqlException &e) {
-    throw PDNSException("GSQLBackend unable to set master of domain '"+domain.toLogString()+"' to IP address " + ip + ": "+e.txtReason());
+    throw PDNSException("GSQLBackend unable to set masters of domain '"+domain.toLogString()+"' to " + tmp + ": "+e.txtReason());
   }
   return true;
 }
@@ -1234,15 +1241,20 @@ bool GSQLBackend::superMasterBackend(const string &ip, const DNSName &domain, co
   return false;
 }
 
-bool GSQLBackend::createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const string &masters, const string &account)
+bool GSQLBackend::createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const vector<ComboAddress> &masters, const string &account)
 {
+  vector<string> masters_s;
+  for (const auto& master : masters) {
+    masters_s.push_back(master.toStringWithPortExcept(53));
+  }
+
   try {
     reconnectIfNeeded();
 
     d_InsertZoneQuery_stmt->
       bind("type", toUpper(DomainInfo::getKindString(kind)))->
       bind("domain", domain)->
-      bind("masters", masters)->
+      bind("masters", boost::join(masters_s, ", "))->
       bind("account", account)->
       execute()->
       reset();
@@ -1256,7 +1268,7 @@ bool GSQLBackend::createDomain(const DNSName &domain, const DomainInfo::DomainKi
 bool GSQLBackend::createSlaveDomain(const string &ip, const DNSName &domain, const string &nameserver, const string &account)
 {
   string name;
-  string masters(ip);
+  vector<ComboAddress> masters({ComboAddress(ip, 53)});
   try {
     if (!nameserver.empty()) {
       // figure out all IP addresses for the master
@@ -1270,13 +1282,13 @@ bool GSQLBackend::createSlaveDomain(const string &ip, const DNSName &domain, con
         reset();
       if (!d_result.empty()) {
         // collect all IP addresses
-        vector<string> tmp;
+        vector<ComboAddress> tmp;
         for(const auto& row: d_result) {
           if (account == row[1])
-            tmp.push_back(row[0]);
+            tmp.emplace_back(row[0], 53);
         }
         // set them as domain's masters, comma separated
-        masters = boost::join(tmp, ", ");
+        masters = tmp;
       }
     }
     createDomain(domain, DomainInfo::Slave, masters, account);
diff --git a/pdns/backends/gsql/gsqlbackend.hh b/pdns/backends/gsql/gsqlbackend.hh
index 513e60fb87..b87bdec201 100644
--- a/pdns/backends/gsql/gsqlbackend.hh
+++ b/pdns/backends/gsql/gsqlbackend.hh
@@ -190,7 +190,7 @@ public:
   bool feedRecord(const DNSResourceRecord &r, const DNSName &ordername, bool ordernameIsNSEC3=false) override;
   bool feedEnts(int domain_id, map<DNSName,bool>& nonterm) override;
   bool feedEnts3(int domain_id, const DNSName &domain, map<DNSName,bool> &nonterm, const NSEC3PARAMRecordContent& ns3prc, bool narrow) override;
-  bool createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const string &masters, const string &account) override;
+  bool createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const vector<ComboAddress> &masters, const string &account) override;
   bool createSlaveDomain(const string &ip, const DNSName &domain, const string &nameserver, const string &account) override;
   bool deleteDomain(const DNSName &domain) override;
   bool superMasterBackend(const string &ip, const DNSName &domain, const vector<DNSResourceRecord>&nsset, string *nameserver, string *account, DNSBackend **db) override;
@@ -199,7 +199,7 @@ public:
   void getUpdatedMasters(vector<DomainInfo> *updatedDomains) override;
   bool getDomainInfo(const DNSName &domain, DomainInfo &di, bool getSerial=true) override;
   void setNotified(uint32_t domain_id, uint32_t serial) override;
-  bool setMaster(const DNSName &domain, const string &ip) override;
+  bool setMasters(const DNSName &domain, const vector<ComboAddress> &masters) override;
   bool setKind(const DNSName &domain, const DomainInfo::DomainKind kind) override;
   bool setAccount(const DNSName &domain, const string &account) override;
 
diff --git a/pdns/dnsbackend.hh b/pdns/dnsbackend.hh
index 67b1f3eacf..b599dca391 100644
--- a/pdns/dnsbackend.hh
+++ b/pdns/dnsbackend.hh
@@ -315,8 +315,8 @@ public:
   {
   }
 
-  //! Called when the Master of a domain should be changed
-  virtual bool setMaster(const DNSName &domain, const string &ip)
+  //! Called when the Master list of a domain should be changed
+  virtual bool setMasters(const DNSName &domain, const vector<ComboAddress> &masters)
   {
     return false;
   }
@@ -343,7 +343,7 @@ public:
   }
 
   //! called by PowerDNS to create a new domain
-  virtual bool createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const string &masters, const string &account)
+  virtual bool createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const vector<ComboAddress> &masters, const string &account)
   {
     return false;
   }
diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index e6aa8f3300..ce1fc75625 100644
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -51,6 +51,14 @@ ArgvMap &arg()
   return arg;
 }
 
+static std::string comboAddressVecToString(const std::vector<ComboAddress>& vec) {
+  vector<string> strs;
+  for (const auto& ca : vec) {
+    strs.push_back(ca.toStringWithPortExcept(53));
+  }
+  return boost::join(strs, ",");
+}
+
 static void loadMainConfig(const std::string& configdir)
 {
   ::arg().set("config-dir","Location of configuration directory (pdns.conf)")=configdir;
@@ -1085,7 +1093,7 @@ static int loadZone(DNSName zone, const string& fname) {
   }
   else {
     cerr<<"Creating '"<<zone<<"'"<<endl;
-    B.createDomain(zone, DomainInfo::Native, "", "");
+    B.createDomain(zone, DomainInfo::Native, vector<ComboAddress>(), "");
 
     if(!B.getDomainInfo(zone, di)) {
       cerr<<"Domain '"<<zone<<"' was not created - perhaps backend ("<<::arg()["launch"]<<") does not support storing new zones."<<endl;
@@ -1128,7 +1136,7 @@ static int createZone(const DNSName &zone, const DNSName& nsname) {
     return EXIT_FAILURE;
   }
   cerr<<"Creating empty zone '"<<zone<<"'"<<endl;
-  B.createDomain(zone, DomainInfo::Native, "", "");
+  B.createDomain(zone, DomainInfo::Native, vector<ComboAddress>(), "");
   if(!B.getDomainInfo(zone, di)) {
     cerr<<"Domain '"<<zone<<"' was not created!"<<endl;
     return EXIT_FAILURE;
@@ -1170,13 +1178,12 @@ static int createSlaveZone(const vector<string>& cmds) {
     cerr<<"Domain '"<<zone<<"' exists already"<<endl;
     return EXIT_FAILURE;
   }
-  vector<string> masters;
+  vector<ComboAddress> masters;
   for (unsigned i=2; i < cmds.size(); i++) {
-    ComboAddress master(cmds[i], 53);
-    masters.push_back(master.toStringWithPortExcept(53));
+    masters.emplace_back(cmds[i], 53);
   }
-  cerr<<"Creating slave zone '"<<zone<<"', with master(s) '"<<boost::join(masters, ",")<<"'"<<endl;
-  B.createDomain(zone, DomainInfo::Slave, boost::join(masters, ","), "");
+  cerr<<"Creating slave zone '"<<zone<<"', with master(s) '"<<comboAddressVecToString(masters)<<"'"<<endl;
+  B.createDomain(zone, DomainInfo::Slave, masters, "");
   if(!B.getDomainInfo(zone, di)) {
     cerr<<"Domain '"<<zone<<"' was not created!"<<endl;
     return EXIT_FAILURE;
@@ -1192,14 +1199,13 @@ static int changeSlaveZoneMaster(const vector<string>& cmds) {
     cerr<<"Domain '"<<zone<<"' doesn't exist"<<endl;
     return EXIT_FAILURE;
   }
-  vector<string> masters;
+  vector<ComboAddress> masters;
   for (unsigned i=2; i < cmds.size(); i++) {
-    ComboAddress master(cmds[i], 53);
-    masters.push_back(master.toStringWithPortExcept(53));
+    masters.emplace_back(cmds[i], 53);
   }
-  cerr<<"Updating slave zone '"<<zone<<"', master(s) to '"<<boost::join(masters, ",")<<"'"<<endl;
+  cerr<<"Updating slave zone '"<<zone<<"', master(s) to '"<<comboAddressVecToString(masters)<<"'"<<endl;
   try {
-    di.backend->setMaster(zone, boost::join(masters, ","));
+    di.backend->setMasters(zone, masters);
     return EXIT_SUCCESS;
   }
   catch (PDNSException& e) {
@@ -3307,16 +3313,8 @@ try
       DomainInfo di_new;
       DNSResourceRecord rr;
       cout<<"Processing '"<<di.zone<<"'"<<endl;
-      string masters="";
-      bool first = true;
-      for(const auto& master: di.masters) {
-        if (!first)
-          masters += ", ";
-        first = false;
-        masters += master.toStringWithPortExcept(53);
-      }
       // create zone
-      if (!tgt->createDomain(di.zone, di.kind, masters, di.account)) throw PDNSException("Failed to create zone");
+      if (!tgt->createDomain(di.zone, di.kind, di.masters, di.account)) throw PDNSException("Failed to create zone");
       if (!tgt->getDomainInfo(di.zone, di_new)) throw PDNSException("Failed to create zone");
       // move records
       if (!src->list(di.zone, di.id, true)) throw PDNSException("Failed to list records");
diff --git a/pdns/ueberbackend.cc b/pdns/ueberbackend.cc
index db1c1eb1a8..138f91455b 100644
--- a/pdns/ueberbackend.cc
+++ b/pdns/ueberbackend.cc
@@ -109,7 +109,7 @@ bool UeberBackend::getDomainInfo(const DNSName &domain, DomainInfo &di, bool get
   return false;
 }
 
-bool UeberBackend::createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const string &masters, const string &account)
+bool UeberBackend::createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const vector<ComboAddress> &masters, const string &account)
 {
   for(DNSBackend* mydb :  backends) {
     if(mydb->createDomain(domain, kind, masters, account)) {
diff --git a/pdns/ueberbackend.hh b/pdns/ueberbackend.hh
index 4f1b55f4f2..80f1ee5723 100644
--- a/pdns/ueberbackend.hh
+++ b/pdns/ueberbackend.hh
@@ -109,7 +109,7 @@ public:
   void getUnfreshSlaveInfos(vector<DomainInfo>* domains);
   void getUpdatedMasters(vector<DomainInfo>* domains);
   bool getDomainInfo(const DNSName &domain, DomainInfo &di, bool getSerial=true);
-  bool createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const string &masters, const string &account);
+  bool createDomain(const DNSName &domain, const DomainInfo::DomainKind kind, const vector<ComboAddress> &masters, const string &account);
   
   bool doesDNSSEC();
   bool addDomainKey(const DNSName& name, const DNSBackend::KeyData& key, int64_t& id);
diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 5f6096d729..93a4c26251 100644
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -616,28 +616,25 @@ static void throwUnableToSecure(const DNSName& zonename) {
 }
 
 
-static void extractDomainInfoFromDocument(const Json document, boost::optional<DomainInfo::DomainKind>& kind, boost::optional<string>& masters, boost::optional<string>& account) {
+static void extractDomainInfoFromDocument(const Json document, boost::optional<DomainInfo::DomainKind>& kind, boost::optional<vector<ComboAddress>>& masters, boost::optional<string>& account) {
   if (document["kind"].is_string()) {
     kind = DomainInfo::stringToKind(stringFromJson(document, "kind"));
   } else {
     kind = boost::none;
   }
 
-  vector<string> zonemaster;
-  for(auto value : document["masters"].array_items()) {
-    string master = value.string_value();
-    if (master.empty())
-      throw ApiException("Master can not be an empty string");
-    try {
-      ComboAddress m(master, 53);
-      zonemaster.push_back(m.toStringWithPortExcept(53));
-    } catch (const PDNSException &e) {
-      throw ApiException("Master (" + master + ") is not an IP address: " + e.reason);
+  if (document["masters"].is_array()) {
+    masters = vector<ComboAddress>();
+    for(auto value : document["masters"].array_items()) {
+      string master = value.string_value();
+      if (master.empty())
+        throw ApiException("Master can not be an empty string");
+      try {
+        masters->emplace_back(master, 53);
+      } catch (const PDNSException &e) {
+        throw ApiException("Master (" + master + ") is not an IP address: " + e.reason);
+      }
     }
-  }
-
-  if (zonemaster.size()) {
-    masters = boost::join(zonemaster, ",");
   } else {
     masters = boost::none;
   }
@@ -651,7 +648,8 @@ static void extractDomainInfoFromDocument(const Json document, boost::optional<D
 
 static void updateDomainSettingsFromDocument(UeberBackend& B, const DomainInfo& di, const DNSName& zonename, const Json document, bool rectifyTransaction=true) {
   boost::optional<DomainInfo::DomainKind> kind;
-  boost::optional<string> masters, account;
+  boost::optional<vector<ComboAddress>> masters;
+  boost::optional<string> account;
 
   extractDomainInfoFromDocument(document, kind, masters, account);
 
@@ -659,7 +657,7 @@ static void updateDomainSettingsFromDocument(UeberBackend& B, const DomainInfo&
     di.backend->setKind(zonename, *kind);
   }
   if (masters) {
-    di.backend->setMaster(zonename, *masters);
+    di.backend->setMasters(zonename, *masters);
   }
   if (account) {
     di.backend->setAccount(zonename, *account);
@@ -1678,11 +1676,12 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp) {
     }
 
     boost::optional<DomainInfo::DomainKind> kind;
-    boost::optional<string> masters, account;
+    boost::optional<vector<ComboAddress>> masters;
+    boost::optional<string> account;
     extractDomainInfoFromDocument(document, kind, masters, account);
 
     // no going back after this
-    if(!B.createDomain(zonename, kind.get_value_or(DomainInfo::Native), masters.get_value_or(""), account.get_value_or("")))
+    if(!B.createDomain(zonename, kind.get_value_or(DomainInfo::Native), masters.get_value_or(vector<ComboAddress>()), account.get_value_or("")))
       throw ApiException("Creating domain '"+zonename.toString()+"' failed");
 
     if(!B.getDomainInfo(zonename, di))