diff --git a/modules/lmdbbackend/Makefile.am b/modules/lmdbbackend/Makefile.am index 91c0fbde1df1..fc75cda0a791 100644 --- a/modules/lmdbbackend/Makefile.am +++ b/modules/lmdbbackend/Makefile.am @@ -1,4 +1,5 @@ -AM_CPPFLAGS += $(LMDB_CFLAGS) $(LIBCRYPTO_INCLUDES) +AM_CPPFLAGS += $(LMDB_CFLAGS) $(LIBCRYPTO_INCLUDES) -I$(top_srcdir)/ext/protozero/include + pkglib_LTLIBRARIES = liblmdbbackend.la diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc index 8d95c1cbc89e..f7897447d84f 100644 --- a/modules/lmdbbackend/lmdbbackend.cc +++ b/modules/lmdbbackend/lmdbbackend.cc @@ -34,6 +34,7 @@ #include "pdns/logger.hh" #include "pdns/misc.hh" #include "pdns/pdnsexception.hh" +#include "pdns/sha.hh" #include "pdns/uuid-utils.hh" #include #include @@ -42,6 +43,8 @@ #include #include #include +#include +#include #include #include #include @@ -889,7 +892,7 @@ void LMDBBackend::openAllTheDatabases() unsigned int LMDBBackend::getCapabilities() { - unsigned int caps = CAP_DNSSEC | CAP_DIRECT | CAP_LIST | CAP_CREATE | CAP_SEARCH; + unsigned int caps = CAP_DNSSEC | CAP_DIRECT | CAP_LIST | CAP_CREATE | CAP_SEARCH | CAP_COMMENTS; if (d_views) { caps |= CAP_VIEWS; } @@ -1161,6 +1164,35 @@ static std::string serializeContent(uint16_t qtype, const DNSName& domain, const return drc->serialize(domain, false); } +// comment serialization: +// 8 bytes timestamp, network order +// comment content, \0 terminated +// account name, \0 terminated + +// perhaps this can also create the compound order name for us +std::pair LMDBBackend::serializeComment(const Comment& comment) +{ + compoundOrdername co; + auto qname = comment.qname.makeRelative(d_transactiondomain); + string key = co(comment.domain_id, qname, comment.qtype); + + // we serialized domain_id, qname, qtype + // that leaves us with content, modified_at, account + + string val; + + protozero::pbf_writer val_writer{val}; + val_writer.add_sfixed64(1, comment.modified_at); + val_writer.add_string(2, comment.account); + val_writer.add_string(3, comment.content); + + auto hash = pdns::sha256sum(val); + + key.append(hash); + + return {key, val}; +} + static std::shared_ptr deserializeContentZR(uint16_t qtype, const DNSName& qname, const std::string& content) { if (qtype == QType::A && content.size() == 4) { @@ -1244,7 +1276,7 @@ static uint32_t peekAtTtl(const string_view& buffer) void LMDBBackend::deleteDomainRecords(RecordsRWTransaction& txn, const std::string& match, QType qtype) { - auto cursor = txn.txn->getCursor(txn.db->dbi); + auto cursor = txn.txn->getCursor(txn.db->rdbi); MDBOutVal key{}; MDBOutVal val{}; @@ -1381,13 +1413,13 @@ void LMDBBackend::deleteNSEC3RecordPair(const std::shared_ptrtxn->get(txn->db->dbi, key, val) == 0) { + if (txn->txn->get(txn->db->rdbi, key, val) == 0) { LMDBResourceRecord lrr; if (deserializeFromBuffer(val.get(), lrr)) { DNSName ordername(lrr.content.c_str(), lrr.content.size(), 0, false); - txn->txn->del(txn->db->dbi, co(domain_id, ordername, QType::NSEC3)); + txn->txn->del(txn->db->rdbi, co(domain_id, ordername, QType::NSEC3)); } - txn->txn->del(txn->db->dbi, key); + txn->txn->del(txn->db->rdbi, key); } } @@ -1408,14 +1440,14 @@ void LMDBBackend::writeNSEC3RecordPair(const std::shared_ptrtxn->get(txn->db->dbi, co(domain_id, qname, QType::NSEC3), val) == 0) { + if (txn->txn->get(txn->db->rdbi, co(domain_id, qname, QType::NSEC3), val) == 0) { LMDBResourceRecord lrr; if (deserializeFromBuffer(val.get(), lrr)) { DNSName prevordername(lrr.content.c_str(), lrr.content.size(), 0, false); if (prevordername == ordername) { return; // nothing to do! (assuming the other record also exists) } - txn->txn->del(txn->db->dbi, co(domain_id, prevordername, QType::NSEC3)); + txn->txn->del(txn->db->rdbi, co(domain_id, prevordername, QType::NSEC3)); } } @@ -1427,14 +1459,14 @@ void LMDBBackend::writeNSEC3RecordPair(const std::shared_ptrtxn->put_header_in_place(txn->db->dbi, co(domain_id, ordername, QType::NSEC3), ser); + txn->txn->put_header_in_place(txn->db->rdbi, co(domain_id, ordername, QType::NSEC3), ser); // Write qname -> ordername forward chain record with ttl set to 1 lrr.ttl = 1; lrr.content = ordername.toDNSString(); ser = MDBRWTransactionImpl::stringWithEmptyHeader(); serializeToBuffer(ser, lrr); - txn->txn->put_header_in_place(txn->db->dbi, co(domain_id, qname, QType::NSEC3), ser); + txn->txn->put_header_in_place(txn->db->rdbi, co(domain_id, qname, QType::NSEC3), ser); } // Check if the only records found for this particular name are a single NSEC3 @@ -1479,11 +1511,11 @@ bool LMDBBackend::feedRecord(const DNSResourceRecord& r, const DNSName& ordernam string rrs = MDBRWTransactionImpl::stringWithEmptyHeader(); MDBOutVal _rrs; - if (!d_rwtxn->txn->get(d_rwtxn->db->dbi, matchName, _rrs)) { + if (!d_rwtxn->txn->get(d_rwtxn->db->rdbi, matchName, _rrs)) { rrs.append(_rrs.get()); } serializeToBuffer(rrs, lrr); - d_rwtxn->txn->put_header_in_place(d_rwtxn->db->dbi, matchName, rrs); + d_rwtxn->txn->put_header_in_place(d_rwtxn->db->rdbi, matchName, rrs); if (lrr.hasOrderName) { writeNSEC3RecordPair(d_rwtxn, lrr.domain_id, lrr.qname, ordername); @@ -1491,6 +1523,16 @@ bool LMDBBackend::feedRecord(const DNSResourceRecord& r, const DNSName& ordernam return true; } +// d_rwtxn must be set here +bool LMDBBackend::feedComment(const Comment& comment) +{ + auto [key, val] = serializeComment(comment); + + d_rwtxn->txn->put(d_rwtxn->db->cdbi, key, val); + + return true; +} + bool LMDBBackend::feedEnts(domainid_t domain_id, map& nonterm) { LMDBResourceRecord lrr; @@ -1503,7 +1545,7 @@ bool LMDBBackend::feedEnts(domainid_t domain_id, map& nonterm) std::string ser = MDBRWTransactionImpl::stringWithEmptyHeader(); serializeToBuffer(ser, lrr); - d_rwtxn->txn->put_header_in_place(d_rwtxn->db->dbi, co(domain_id, lrr.qname, QType::ENT), ser); + d_rwtxn->txn->put_header_in_place(d_rwtxn->db->rdbi, co(domain_id, lrr.qname, QType::ENT), ser); } return true; } @@ -1520,7 +1562,7 @@ bool LMDBBackend::feedEnts3(domainid_t domain_id, const DNSName& domain, maptxn->put_header_in_place(d_rwtxn->db->dbi, co(domain_id, lrr.qname, QType::ENT), ser); + d_rwtxn->txn->put_header_in_place(d_rwtxn->db->rdbi, co(domain_id, lrr.qname, QType::ENT), ser); if (lrr.hasOrderName) { ordername = DNSName(toBase32Hex(hashQNameWithSalt(ns3prc, nt.first))); @@ -1571,7 +1613,7 @@ bool LMDBBackend::replaceRRSet(domainid_t domain_id, const DNSName& qname, const deleteNSEC3RecordPair(txn, domain_id, relative); } else { - auto cursor = txn->txn->getCursor(txn->db->dbi); + auto cursor = txn->txn->getCursor(txn->db->rdbi); MDBOutVal key{}; MDBOutVal val{}; bool hadOrderName{false}; @@ -1605,7 +1647,7 @@ bool LMDBBackend::replaceRRSet(domainid_t domain_id, const DNSName& qname, const } std::string ser = MDBRWTransactionImpl::stringWithEmptyHeader(); serializeToBuffer(ser, adjustedRRSet); - txn->txn->put_header_in_place(txn->db->dbi, match, ser); + txn->txn->put_header_in_place(txn->db->rdbi, match, ser); } if (needCommit) @@ -1614,12 +1656,31 @@ bool LMDBBackend::replaceRRSet(domainid_t domain_id, const DNSName& qname, const return true; } -// NOLINTNEXTLINE(readability-identifier-length) -bool LMDBBackend::replaceComments([[maybe_unused]] domainid_t domain_id, [[maybe_unused]] const DNSName& qname, [[maybe_unused]] const QType& qt, const vector& comments) +bool LMDBBackend::replaceComments(const domainid_t domain_id, const DNSName& qname, const QType& qtype, const vector& comments) { - // if the vector is empty, good, that's what we do here (LMDB does not store comments) - // if it's not, report failure - return comments.empty(); + // delete all existing comments for the RRset + // this could be smarter and not del+replace unchanged comments + auto cursor = d_rwtxn->txn->getCursor(d_rwtxn->db->cdbi); + MDBOutVal key{}; + MDBOutVal val{}; + + compoundOrdername co; + + auto relqname = qname.makeRelative(d_transactiondomain); + + string match = co(domain_id, relqname, qtype); + + if (cursor.prefix(match, key, val) == 0) { + do { + cursor.del(key); + } while (cursor.next(key, val) == 0); + } + + for (const auto& comment : comments) { + feedComment(comment); + } + + return true; } // FIXME: this is not very efficient @@ -1807,7 +1868,8 @@ std::shared_ptr LMDBBackend::getRecordsRWTran if (!shard.env) { shard.env = getMDBEnv((getArg("filename") + "-" + std::to_string(id % s_shards)).c_str(), MDB_NOSUBDIR | MDB_NORDAHEAD | d_asyncFlag, 0600, d_mapsize_shards); - shard.dbi = shard.env->openDB("records_v5", MDB_CREATE); + shard.rdbi = shard.env->openDB("records_v5", MDB_CREATE); + shard.cdbi = shard.env->openDB("comments_v7", MDB_CREATE); } auto ret = std::make_shared(shard.env->getRWTransaction()); ret->db = std::make_shared(shard); @@ -1825,7 +1887,8 @@ std::shared_ptr LMDBBackend::getRecordsROTran } shard.env = getMDBEnv((getArg("filename") + "-" + std::to_string(id % s_shards)).c_str(), MDB_NOSUBDIR | MDB_NORDAHEAD | d_asyncFlag, 0600, d_mapsize_shards); - shard.dbi = shard.env->openDB("records_v5", MDB_CREATE); + shard.rdbi = shard.env->openDB("records_v5", MDB_CREATE); + shard.cdbi = shard.env->openDB("comments_v7", MDB_CREATE); } if (rwtxn) { @@ -1917,11 +1980,30 @@ bool LMDBBackend::deleteDomain(const ZoneName& domain) return true; } +bool LMDBBackend::listComments(domainid_t domain_id) +{ + DomainInfo info; + if (!findDomain(domain_id, info)) { + throw DBException("Domain with id '" + std::to_string(domain_id) + "' not found"); + } + + d_lookupstate.domain = info.zone; + d_lookupstate.submatch.clear(); + d_lookupstate.comments = true; + + compoundOrdername order; + std::string match = order(domain_id); + + lookupStart(domain_id, match, false); + return true; +} + bool LMDBBackend::list(const ZoneName& target, domainid_t domain_id, bool include_disabled) { d_lookupstate.domain = target; d_lookupstate.submatch.clear(); d_lookupstate.includedisabled = include_disabled; + d_lookupstate.comments = false; compoundOrdername order; std::string match = order(domain_id); @@ -1949,6 +2031,7 @@ bool LMDBBackend::listSubZone(const ZoneName& target, domainid_t domain_id) d_lookupstate.domain = std::move(info.zone); d_lookupstate.submatch = std::move(relqname); d_lookupstate.includedisabled = true; + d_lookupstate.comments = false; compoundOrdername order; std::string match = order(domain_id); @@ -1996,6 +2079,7 @@ void LMDBBackend::lookupInternal(const QType& type, const DNSName& qdomain, doma d_lookupstate.domain = std::move(info.zone); d_lookupstate.submatch.clear(); d_lookupstate.includedisabled = include_disabled; + d_lookupstate.comments = false; compoundOrdername order; std::string match; @@ -2013,15 +2097,18 @@ void LMDBBackend::lookupStart(domainid_t domain_id, const std::string& match, bo { d_rotxn = getRecordsROTransaction(domain_id, d_rwtxn); d_txnorder = true; - d_lookupstate.cursor = std::make_shared(d_rotxn->txn->getCursor(d_rotxn->db->dbi)); + if (d_lookupstate.comments) { + d_lookupstate.cursor = std::make_shared(d_rotxn->txn->getCursor(d_rotxn->db->cdbi)); + } + else { + d_lookupstate.cursor = std::make_shared(d_rotxn->txn->getCursor(d_rotxn->db->rdbi)); + } // Make sure we start with fresh data d_lookupstate.rrset.clear(); d_lookupstate.rrsetpos = 0; - MDBOutVal key{}; - MDBOutVal val{}; - if (d_lookupstate.cursor->prefix(match, key, val) != 0) { + if (d_lookupstate.cursor->prefix(match, d_lookupstate.key, d_lookupstate.val) != 0) { d_lookupstate.reset(); // will cause get() to fail if (dolog) { SLOG(g_log << Logger::Warning << "Query " << ((long)(void*)this) << ": " << d_dtime.udiffNoReset() << " us to execute (found nothing)" << endl, @@ -2038,6 +2125,44 @@ void LMDBBackend::lookupStart(domainid_t domain_id, const std::string& match, bo bool LMDBBackend::getInternal(DNSName& basename, std::string_view& key) { + // FIXME: bit of duplication from below here, but much simpler + if (d_lookupstate.comments) { + if (!d_lookupstate.cursor) { + d_rotxn.reset(); + return false; + } + + key = d_lookupstate.key.getNoStripHeader(); + // remove hash from the key so compoundOrdername::get* works + key = key.substr(0, key.size() - 256 / 8); + + basename = compoundOrdername::getQName(key); + + const auto& val = d_lookupstate.val.get(); + + d_lookupstate.comment.domain_id = compoundOrdername::getDomainID(key); + d_lookupstate.comment.qname = basename + d_lookupstate.domain.operator const DNSName&(); + d_lookupstate.comment.qtype = compoundOrdername::getQType(key); + try { + protozero::pbf_reader message{val}; + message.next(1); + d_lookupstate.comment.modified_at = message.get_sfixed64(); + message.next(2); + d_lookupstate.comment.account = message.get_string(); + message.next(3); + d_lookupstate.comment.content = message.get_string(); + } + catch (protozero::exception& e) { + throw DBException("got invalid serialized comment: " + e.what()); + } + + if (d_lookupstate.cursor && d_lookupstate.cursor->next(d_lookupstate.key, d_lookupstate.val) != 0) { + d_lookupstate.reset(); // this invalidates cursor and makes us return false on the next round + } + + return true; + } + for (;;) { if (!d_lookupstate.rrset.empty()) { if (++d_lookupstate.rrsetpos >= d_lookupstate.rrset.size()) { @@ -2104,6 +2229,19 @@ bool LMDBBackend::getInternal(DNSName& basename, std::string_view& key) return true; } +bool LMDBBackend::getComment(Comment& comment) // NOLINT(readability-identifier-length) +{ + DNSName basename; + std::string_view key; + + if (!getInternal(basename, key)) { + return false; + } + comment = d_lookupstate.comment; + + return true; +} + bool LMDBBackend::get(DNSZoneRecord& zr) // NOLINT(readability-identifier-length) { DNSName basename; @@ -2159,7 +2297,7 @@ bool LMDBBackend::getSerial(DomainInfo& di) auto txn = getRecordsROTransaction(di.id); compoundOrdername co; MDBOutVal val; - if (!txn->txn->get(txn->db->dbi, co(di.id, g_rootdnsname, QType::SOA), val)) { + if (!txn->txn->get(txn->db->rdbi, co(di.id, g_rootdnsname, QType::SOA), val)) { LMDBResourceRecord lrr; if (deserializeFromBuffer(val.get(), lrr)) { if (lrr.content.size() >= sizeof(soatimes)) { @@ -2345,7 +2483,7 @@ void LMDBBackend::getUnfreshSecondaryInfos(vector* domains) auto txn2 = getRecordsROTransaction(di.id); compoundOrdername co; MDBOutVal val; - if (!txn2->txn->get(txn2->db->dbi, co(di.id, g_rootdnsname, QType::SOA), val)) { + if (!txn2->txn->get(txn2->db->rdbi, co(di.id, g_rootdnsname, QType::SOA), val)) { LMDBResourceRecord lrr; if (deserializeFromBuffer(val.get(), lrr)) { if (lrr.content.size() >= sizeof(soatimes)) { @@ -2719,7 +2857,7 @@ bool LMDBBackend::getBeforeAndAfterNamesAbsolute(domainid_t id, const DNSName& q compoundOrdername co; auto txn = getRecordsROTransaction(id); - auto cursor = txn->txn->getCursor(txn->db->dbi); + auto cursor = txn->txn->getCursor(txn->db->rdbi); MDBOutVal key, val; string matchkey = co(id, qname, QType::NSEC3); @@ -2862,7 +3000,7 @@ bool LMDBBackend::getBeforeAndAfterNames(domainid_t domainId, const ZoneName& zo compoundOrdername co; auto txn = getRecordsROTransaction(domainId); - auto cursor = txn->txn->getCursor(txn->db->dbi); + auto cursor = txn->txn->getCursor(txn->db->rdbi); MDBOutVal key, val; DNSName qname2 = qname.makeRelative(zonename); @@ -3003,7 +3141,7 @@ bool LMDBBackend::updateDNSSECOrderNameAndAuth(domainid_t domain_id, const DNSNa compoundOrdername co; string matchkey = co(domain_id, rel); - auto cursor = txn->txn->getCursor(txn->db->dbi); + auto cursor = txn->txn->getCursor(txn->db->rdbi); MDBOutVal key, val; if (cursor.prefix(matchkey, key, val) != 0) { // cout << "Could not find anything"<& in // deleteDomainRecords() would do, as we also need to remove // NSEC3 records for these ENT, if any. { - auto cursor = txn->txn->getCursor(txn->db->dbi); + auto cursor = txn->txn->getCursor(txn->db->rdbi); MDBOutVal key{}; MDBOutVal val{}; std::vector names; @@ -3133,9 +3271,9 @@ bool LMDBBackend::updateEmptyNonTerminals(domainid_t domain_id, set& in name.makeUsRelative(info.zone); std::string match = order(domain_id, name, QType::ENT); MDBOutVal val{}; - if (txn->txn->get(txn->db->dbi, match, val) == 0) { + if (txn->txn->get(txn->db->rdbi, match, val) == 0) { bool hadOrderName = peekAtHasOrderName(val.get()); - txn->txn->del(txn->db->dbi, match); + txn->txn->del(txn->db->rdbi, match); if (hadOrderName) { deleteNSEC3RecordPair(txn, domain_id, name); } @@ -3149,7 +3287,7 @@ bool LMDBBackend::updateEmptyNonTerminals(domainid_t domain_id, set& in lrr.auth = true; std::string ser = MDBRWTransactionImpl::stringWithEmptyHeader(); serializeToBuffer(ser, lrr); - txn->txn->put_header_in_place(txn->db->dbi, order(domain_id, lrr.qname, QType::ENT), ser); + txn->txn->put_header_in_place(txn->db->rdbi, order(domain_id, lrr.qname, QType::ENT), ser); // cout <<" +"<& argv) // without disturbing the current get() cursor. compoundOrdername order; MDBOutVal val{}; - if (d_rotxn->txn->get(d_rotxn->db->dbi, order(info.id, basename, QType::NSEC3), val) == 0) { + if (d_rotxn->txn->get(d_rotxn->db->rdbi, order(info.id, basename, QType::NSEC3), val) == 0) { LMDBResourceRecord nsec3rr; if (deserializeFromBuffer(val.get(), nsec3rr)) { DNSName ordername(nsec3rr.content.c_str(), nsec3rr.content.size(), 0, false); @@ -3464,6 +3602,8 @@ bool LMDBBackend::hasCreatedLocalFiles() const // not all of them did. // But since this information is for the sake of pdnsutil, this is not // really a problem. + // However, there is a false positive if we make new databases (dbis) inside + // existing LMDB files on disk. This is not easy to avoid. return MDBDbi::d_creationCount != 0; } diff --git a/modules/lmdbbackend/lmdbbackend.hh b/modules/lmdbbackend/lmdbbackend.hh index 9aa1a9c132d8..096fa306367b 100644 --- a/modules/lmdbbackend/lmdbbackend.hh +++ b/modules/lmdbbackend/lmdbbackend.hh @@ -84,6 +84,10 @@ public: bool commitTransaction() override; bool abortTransaction() override; bool feedRecord(const DNSResourceRecord& r, const DNSName& ordername, bool ordernameIsNSEC3 = false) override; + bool feedComment(const Comment& c) override; + bool listComments(domainid_t domain_id) override; + bool getComment(Comment& comment) override; + bool feedEnts(domainid_t domain_id, map& nonterm) override; bool feedEnts3(domainid_t domain_id, const DNSName& domain, map& nonterm, const NSEC3PARAMRecordContent& ns3prc, bool narrow) override; bool replaceRRSet(domainid_t domain_id, const DNSName& qname, const QType& qt, const vector& rrset) override; @@ -299,7 +303,8 @@ private: struct RecordsDB { shared_ptr env; - MDBDbi dbi; + MDBDbi rdbi; // records + MDBDbi cdbi; // comments }; struct RecordsROTransaction @@ -360,6 +365,8 @@ private: static void deleteNSEC3RecordPair(const std::shared_ptr& txn, domainid_t domain_id, const DNSName& qname); void writeNSEC3RecordPair(const std::shared_ptr& txn, domainid_t domain_id, const DNSName& qname, const DNSName& ordername); + std::pair serializeComment(const Comment& c); + string directBackendCmd_list(std::vector& argv); // Transient DomainInfo data, not necessarily synchronized with the @@ -405,6 +412,10 @@ private: MDBOutVal val; // whether to include disabled records in the results bool includedisabled; + // whether we are doing comments (false=records, true=comments) + bool comments; + // comment found by last call to getInternal + Comment comment; void reset() { @@ -413,6 +424,7 @@ private: rrset.clear(); rrsetpos = 0; cursor.reset(); + // comments bool explicitly not reset here, so getInternal can note absence the right way } } d_lookupstate; diff --git a/pdns/dnsbackend.cc b/pdns/dnsbackend.cc index 738f4b107467..42ee1e8ebe69 100644 --- a/pdns/dnsbackend.cc +++ b/pdns/dnsbackend.cc @@ -109,6 +109,42 @@ bool DNSBackend::searchRecords(const string& pattern, size_t maxResults, vector< return true; } +// Default search logic, for backends which can enumerate their comments. +bool DNSBackend::searchComments(const string& pattern, size_t maxResults, vector& result) +{ + // We depend upon working list(), but also getAllDomains(), which is why we + // are checking explicitly for CAP_SEARCH in addition to CAP_LIST - the + // default getAllDomains() implementation below is not safe to use here. + if ((getCapabilities() & (CAP_LIST | CAP_SEARCH)) != (CAP_LIST | CAP_SEARCH)) { + return false; + } + + SimpleMatch simpleMatch(pattern, true); + std::vector domains; + getAllDomains(&domains, false, true); + for (const auto& info : domains) { + if (!listComments(info.id)) { + return false; + } + Comment comment; + while (getComment(comment)) { + if (maxResults == 0) { + // No need to look any further + lookupEnd(); + break; + } + if (simpleMatch.match(comment.qname) || simpleMatch.match(comment.content)) { + result.emplace_back(comment); + --maxResults; + } + } + if (maxResults == 0) { + break; + } + } + return true; +} + void BackendFactory::declare(const string& suffix, const string& param, const string& explanation, const string& value) { string fullname = d_name + suffix + "-" + param; diff --git a/pdns/dnsbackend.hh b/pdns/dnsbackend.hh index bc1f2dbfb0fc..71c48cd793bc 100644 --- a/pdns/dnsbackend.hh +++ b/pdns/dnsbackend.hh @@ -475,10 +475,7 @@ public: virtual bool searchRecords(const string& pattern, size_t maxResults, vector& result); //! Search for comments, returns true if search was done successfully. - virtual bool searchComments(const string& /* pattern */, size_t /* maxResults */, vector& /* result */) - { - return false; - } + virtual bool searchComments(const string& /* pattern */, size_t /* maxResults */, vector& /* result */); virtual void viewList(vector& /* result */) { diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc index 69d021403f97..4c7af8e88306 100644 --- a/pdns/pdnsutil.cc +++ b/pdns/pdnsutil.cc @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -86,6 +87,8 @@ static int activateTSIGKey(vector& cmds, std::string_view synopsis); static int activateZoneKey(vector& cmds, std::string_view synopsis); static int addAutoprimary(vector& cmds, std::string_view synopsis); static int addMeta(vector& cmds, std::string_view synopsis); +static int addComment(vector& cmds, std::string_view synopsis); +static int listComments(vector& cmds, std::string_view synopsis); static int addRecord(vector& cmds, std::string_view synopsis); static int addZoneKey(vector& cmds, std::string_view synopsis); static int backendCmd(vector& cmds, std::string_view synopsis); @@ -511,6 +514,16 @@ static const groupCommandDispatcher zoneKeyCommands{ "\tUnpublish the zone key with key id KEY_ID in ZONE"}}} }; +static const groupCommandDispatcher commentCommands{ + "Comment", + {{"add", {true, addComment, + "ZONE NAME TYPE COMMENT [ACCOUNT]", + "\tAdd a comment"}}, + {"list", {true, listComments, + "ZONE", + "\tList comments for a zone"}}} +}; + // OTHER (NO OBJECT NAME PREFIX) static const groupCommandDispatcher otherCommands{ @@ -580,6 +593,7 @@ using commandDispatcher = std::mapgetCapabilities() & DNSBackend::CAP_COMMENTS) == 0) { + cerr << "Backend for zone '" << zone << "' does not support listing its comments." << endl; + return EXIT_FAILURE; + } + + Comment comment; + + di.backend->listComments(di.id); + while(di.backend->getComment(comment)) { + cout<& cmds, const std::string_vi } } +static int addComment(vector& cmds, const std::string_view synopsis) +{ + if(cmds.size() < 4) { + return usage(synopsis); + } + + UtilBackend B; //NOLINT(readability-identifier-length) + DomainInfo di; //NOLINT(readability-identifier-length) + ZoneName zone(cmds.at(0)); + if (!B.getDomainInfo(zone, di)) { + cerr << "Zone '" << zone << "' doesn't exist" << endl; + return EXIT_FAILURE; + } + + Comment comment; + + comment.domain_id = di.id; + comment.qname = DNSName(cmds.at(1)); + comment.qtype = cmds.at(2); + comment.content = cmds.at(3); + if(cmds.size() > 4) { + comment.account = cmds.at(4); + } + comment.modified_at = time(nullptr); + + if (!comment.qname.isPartOf(zone)) { + throw PDNSException("Name \"" + comment.qname.toString() + "\" to add comment to is not part of zone \"" + zone.toString() + "\"."); + } + + di.backend->startTransaction(zone, UnknownDomainID); + if (!di.backend->feedComment(comment)) { + cerr << "Backend does not support comments" << endl; + di.backend->abortTransaction(); + return EXIT_FAILURE; + } + + di.backend->commitTransaction(); + return EXIT_SUCCESS; +} + +static int listComments(vector& cmds, const std::string_view synopsis) +{ + if(cmds.size() != 1) { + return usage(synopsis); + } + if (cmds.at(0) == ".") { + cmds.at(0).clear(); + } + + return listComments(ZoneName(cmds.at(0))); +} + + static int addRecord(vector& cmds, const std::string_view synopsis) { if(cmds.size() < 4) { diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py index 172bed4e842f..e3453a8f360d 100644 --- a/regression-tests.api/test_Zones.py +++ b/regression-tests.api/test_Zones.py @@ -430,11 +430,6 @@ def test_create_zone_with_comments(self): }, ] - if is_auth_lmdb(): - # No comments in LMDB - self.create_zone(name=name, rrsets=rrsets, expect_error="Hosting backend does not support editing comments.") - return - name, _, data = self.create_zone(name=name, rrsets=rrsets) # NS records have been created self.assertEqual(len(data['rrsets']), len(rrsets) + 1) @@ -2461,11 +2456,7 @@ def test_zone_comment_create(self): self.url("/api/v1/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) - if is_auth_lmdb(): - self.assert_error_json(r) # No comments in LMDB - return - else: - self.assert_success(r) + self.assert_success(r) # make sure the comments have been set, and that the NS # records are still present data = self.get_zone(name, rrset_name=name, rrset_type="NS") @@ -2487,7 +2478,12 @@ def test_zone_comment_delete(self): 'changetype': 'replace', 'name': name, 'type': 'NS', - 'comments': [] + 'comments': [ + { + 'account': 'test4', + 'content': 'this should not show up after delete' + } + ] } payload = {'rrsets': [rrset]} r = self.session.patch( @@ -2495,6 +2491,17 @@ def test_zone_comment_delete(self): data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assert_success(r) + data = self.get_zone(name) + serverset = get_rrset(data, name, 'NS') + print(serverset) + self.assertNotEqual(serverset['records'], []) + self.assertNotEqual(serverset['comments'], []) + + payload['rrsets'][0]['comments'] = [] + r = self.session.patch( + self.url("/api/v1/servers/localhost/zones/" + name), + data=json.dumps(payload), + headers={'content-type': 'application/json'}) # make sure the NS records are still present data = self.get_zone(name) serverset = get_rrset(data, name, 'NS') @@ -2502,7 +2509,6 @@ def test_zone_comment_delete(self): self.assertNotEqual(serverset['records'], []) self.assertEqual(serverset['comments'], []) - @unittest.skipIf(is_auth_lmdb(), "No comments in LMDB") def test_zone_comment_out_of_range_modified_at(self): # Test if a modified_at outside of the 32 bit range throws an error name, payload, zone = self.create_zone() @@ -2526,7 +2532,6 @@ def test_zone_comment_out_of_range_modified_at(self): self.assertEqual(r.status_code, 422) self.assert_in_json_error("Key 'modified_at' is out of range", r.json()) - @unittest.skipIf(is_auth_lmdb(), "No comments in LMDB") def test_zone_comment_stay_intact(self): # Test if comments on an rrset stay intact if the rrset is replaced name, payload, zone = self.create_zone() @@ -2647,7 +2652,6 @@ def test_search_rr_case_insensitive(self): # should return zone, SOA, ns1, ns2 self.assertEqual(len(r.json()), 4) - @unittest.skipIf(is_auth_lmdb(), "No comments in LMDB") def test_search_rr_comment(self): name = unique_zone_name() rrsets = [{