diff --git a/docs/changelog/4.9.rst b/docs/changelog/4.9.rst index 920522f3bb86f..9621e3ed2d32f 100644 --- a/docs/changelog/4.9.rst +++ b/docs/changelog/4.9.rst @@ -1,6 +1,20 @@ Changelogs for 4.9.x ==================== +.. changelog:: + :version: 4.9.3 + :released: 17th of December 2024 + + This is release 4.9.3 of the Authoritative Server. + + Please review the :doc:`Upgrade Notes <../upgrading>` before upgrading from versions < 4.9.x. + + .. change:: + :tags: Bug Fixes + :pullreq: 14968 + + SVCB parser: allow quotes around port= + .. changelog:: :version: 4.9.2 :released: 1st of October 2024 diff --git a/docs/secpoll.zone b/docs/secpoll.zone index d1c208964e5f0..ba37b4f72ece8 100644 --- a/docs/secpoll.zone +++ b/docs/secpoll.zone @@ -1,4 +1,4 @@ -@ 86400 IN SOA pdns-public-ns1.powerdns.com. peter\.van\.dijk.powerdns.com. 2024121300 10800 3600 604800 10800 +@ 86400 IN SOA pdns-public-ns1.powerdns.com. peter\.van\.dijk.powerdns.com. 2024121700 10800 3600 604800 10800 @ 3600 IN NS pdns-public-ns1.powerdns.com. @ 3600 IN NS pdns-public-ns2.powerdns.com. @@ -130,6 +130,7 @@ auth-4.9.0-beta2.security-status 60 IN TXT "2 Unsupported auth-4.9.0.security-status 60 IN TXT "1 OK" auth-4.9.1.security-status 60 IN TXT "1 OK" auth-4.9.2.security-status 60 IN TXT "1 OK" +auth-4.9.3.security-status 60 IN TXT "1 OK" ; Auth Debian auth-3.4.1-2.debian.security-status 60 IN TXT "3 Upgrade now, see https://doc.powerdns.com/3/security/powerdns-advisory-2015-01/ and https://doc.powerdns.com/3/security/powerdns-advisory-2015-02/ and https://doc.powerdns.com/3/security/powerdns-advisory-2016-02/ and https://doc.powerdns.com/3/security/powerdns-advisory-2016-03/ and https://doc.powerdns.com/3/security/powerdns-advisory-2016-04/ and https://doc.powerdns.com/3/security/powerdns-advisory-2016-05/" diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc index 36841e619a683..8b6860ee5a089 100644 --- a/modules/lmdbbackend/lmdbbackend.cc +++ b/modules/lmdbbackend/lmdbbackend.cc @@ -1009,7 +1009,7 @@ static std::shared_ptr deserializeContentZR(uint16_t qtype, co if (qtype == QType::A && content.size() == 4) { return std::make_shared(*((uint32_t*)content.c_str())); } - return DNSRecordContent::deserialize(qname, qtype, content); + return DNSRecordContent::deserialize(qname, qtype, content, QClass::IN, true); } /* design. If you ask a question without a zone id, we lookup the best diff --git a/pdns/dnsdistdist/dnsdist-carbon.cc b/pdns/dnsdistdist/dnsdist-carbon.cc index 040783b68a956..66ec734250e4c 100644 --- a/pdns/dnsdistdist/dnsdist-carbon.cc +++ b/pdns/dnsdistdist/dnsdist-carbon.cc @@ -57,6 +57,11 @@ static bool doOneCarbonExport(const Carbon::Endpoint& endpoint) { auto entries = dnsdist::metrics::g_stats.entries.read_lock(); for (const auto& entry : *entries) { + // Skip non-empty labels, since labels are not supported in Carbon + if (!entry.d_labels.empty()) { + continue; + } + str << namespace_name << "." << hostname << "." << instance_name << "." << entry.d_name << ' '; if (const auto& val = std::get_if(&entry.d_value)) { str << (*val)->load(); diff --git a/pdns/dnsdistdist/dnsdist-idstate.hh b/pdns/dnsdistdist/dnsdist-idstate.hh index 259c55e92a15c..48fdeefc30565 100644 --- a/pdns/dnsdistdist/dnsdist-idstate.hh +++ b/pdns/dnsdistdist/dnsdist-idstate.hh @@ -157,8 +157,8 @@ struct InternalQueryState int32_t d_streamID{-1}; // 4 uint32_t cacheKey{0}; // 4 uint32_t cacheKeyNoECS{0}; // 4 - // DoH-only */ - uint32_t cacheKeyUDP{0}; // 4 + // DoH-only: if we received a TC=1 answer, we had to retry over TCP and thus we need the TCP cache key */ + uint32_t cacheKeyTCP{0}; // 4 uint32_t ttlCap{0}; // cap the TTL _after_ inserting into the packet cache // 4 int backendFD{-1}; // 4 int delayMsec{0}; diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index 192e8f1cfd10c..03ffb98a26022 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc @@ -260,7 +260,7 @@ void dnsdist_ffi_dnsquestion_get_sni(const dnsdist_ffi_dnsquestion_t* dq, const const char* dnsdist_ffi_dnsquestion_get_tag(const dnsdist_ffi_dnsquestion_t* dq, const char* label) { - const char * result = nullptr; + const char* result = nullptr; if (dq != nullptr && dq->dq != nullptr && dq->dq->ids.qTag != nullptr) { const auto it = dq->dq->ids.qTag->find(label); @@ -456,7 +456,6 @@ size_t dnsdist_ffi_dnsquestion_get_tag_array(dnsdist_ffi_dnsquestion_t* dq, cons ++pos; } - if (!dq->tagsVect->empty()) { *out = dq->tagsVect->data(); } @@ -1007,7 +1006,7 @@ static constexpr char s_lua_ffi_code[] = R"FFICodeContent( ffi.cdef[[ )FFICodeContent" #include "dnsdist-lua-ffi-interface.inc" -R"FFICodeContent( + R"FFICodeContent( ]] )FFICodeContent"; @@ -1057,7 +1056,8 @@ size_t dnsdist_ffi_generate_proxy_protocol_payload(const size_t addrSize, const if (valuesCount > 0) { valuesVect.reserve(valuesCount); for (size_t idx = 0; idx < valuesCount; idx++) { - valuesVect.push_back({ std::string(values[idx].value, values[idx].size), values[idx].type }); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + valuesVect.push_back({std::string(values[idx].value, values[idx].size), values[idx].type}); } } @@ -1086,7 +1086,8 @@ size_t dnsdist_ffi_dnsquestion_generate_proxy_protocol_payload(const dnsdist_ffi if (valuesCount > 0) { valuesVect.reserve(valuesCount); for (size_t idx = 0; idx < valuesCount; idx++) { - valuesVect.push_back({ std::string(values[idx].value, values[idx].size), values[idx].type }); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + valuesVect.push_back({std::string(values[idx].value, values[idx].size), values[idx].type}); } } @@ -1113,7 +1114,7 @@ bool dnsdist_ffi_dnsquestion_add_proxy_protocol_values(dnsdist_ffi_dnsquestion_t dnsQuestion->dq->proxyProtocolValues->reserve(dnsQuestion->dq->proxyProtocolValues->size() + valuesCount); for (size_t idx = 0; idx < valuesCount; idx++) { // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic): the Lua FFI API is a C API.. - dnsQuestion->dq->proxyProtocolValues->push_back({ std::string(values[idx].value, values[idx].size), values[idx].type }); + dnsQuestion->dq->proxyProtocolValues->push_back({std::string(values[idx].value, values[idx].size), values[idx].type}); } return true; @@ -1146,7 +1147,8 @@ struct dnsdist_ffi_domain_list_t { std::vector d_domains; }; -struct dnsdist_ffi_address_list_t { +struct dnsdist_ffi_address_list_t +{ std::vector d_addresses; }; @@ -1476,7 +1478,8 @@ void dnsdist_ffi_ring_entry_list_free(dnsdist_ffi_ring_entry_list_t* list) delete list; } -template static void addRingEntryToList(std::unique_ptr& list, const struct timespec& now, const T& entry) +template +static void addRingEntryToList(std::unique_ptr& list, const struct timespec& now, const T& entry) { auto age = DiffTime(entry.when, now); @@ -1805,13 +1808,13 @@ bool dnsdist_ffi_metric_declare(const char* name, size_t nameLen, const char* ty if (name == nullptr || nameLen == 0 || type == nullptr || description == nullptr) { return false; } - auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName != nullptr ? std::optional(customName) : std::nullopt); + auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName != nullptr ? std::optional(customName) : std::nullopt, false); return !result; } void dnsdist_ffi_metric_inc(const char* metricName, size_t metricNameLen) { - auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), 1U); + auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, {}); if (std::get_if(&result) != nullptr) { return; } @@ -1819,7 +1822,7 @@ void dnsdist_ffi_metric_inc(const char* metricName, size_t metricNameLen) void dnsdist_ffi_metric_inc_by(const char* metricName, size_t metricNameLen, uint64_t value) { - auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), value); + auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), value, {}); if (std::get_if(&result) != nullptr) { return; } @@ -1827,7 +1830,7 @@ void dnsdist_ffi_metric_inc_by(const char* metricName, size_t metricNameLen, uin void dnsdist_ffi_metric_dec(const char* metricName, size_t metricNameLen) { - auto result = dnsdist::metrics::decrementCustomCounter(std::string_view(metricName, metricNameLen), 1U); + auto result = dnsdist::metrics::decrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, {}); if (std::get_if(&result) != nullptr) { return; } @@ -1835,7 +1838,7 @@ void dnsdist_ffi_metric_dec(const char* metricName, size_t metricNameLen) void dnsdist_ffi_metric_set(const char* metricName, size_t metricNameLen, double value) { - auto result = dnsdist::metrics::setCustomGauge(std::string_view(metricName, metricNameLen), value); + auto result = dnsdist::metrics::setCustomGauge(std::string_view(metricName, metricNameLen), value, {}); if (std::get_if(&result) != nullptr) { return; } @@ -1843,7 +1846,7 @@ void dnsdist_ffi_metric_set(const char* metricName, size_t metricNameLen, double double dnsdist_ffi_metric_get(const char* metricName, size_t metricNameLen, bool isCounter) { - auto result = dnsdist::metrics::getCustomMetric(std::string_view(metricName, metricNameLen)); + auto result = dnsdist::metrics::getCustomMetric(std::string_view(metricName, metricNameLen), {}); if (std::get_if(&result) != nullptr) { return 0.; } diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.hh b/pdns/dnsdistdist/dnsdist-lua-ffi.hh index 1369c2a07cd11..644114d699711 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.hh +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.hh @@ -23,27 +23,31 @@ #include "dnsdist.hh" -extern "C" { +extern "C" +{ #include "dnsdist-lua-ffi-interface.h" } #include "ext/luawrapper/include/LuaContext.hpp" // dnsdist_ffi_dnsquestion_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_dnsquestion_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_dnsquestion_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_dnsquestion_t { - dnsdist_ffi_dnsquestion_t(DNSQuestion* dq_): dq(dq_) + dnsdist_ffi_dnsquestion_t(DNSQuestion* dq_) : + dq(dq_) { } @@ -63,20 +67,23 @@ struct dnsdist_ffi_dnsquestion_t }; // dnsdist_ffi_dnsresponse_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_dnsresponse_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_dnsresponse_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_dnsresponse_t { - dnsdist_ffi_dnsresponse_t(DNSResponse* dr_): dr(dr_) + dnsdist_ffi_dnsresponse_t(DNSResponse* dr_) : + dr(dr_) { } @@ -85,20 +92,23 @@ struct dnsdist_ffi_dnsresponse_t }; // dnsdist_ffi_server_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_server_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_server_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_server_t { - dnsdist_ffi_server_t(const std::shared_ptr& server_): server(server_) + dnsdist_ffi_server_t(const std::shared_ptr& server_) : + server(server_) { } @@ -106,23 +116,26 @@ struct dnsdist_ffi_server_t }; // dnsdist_ffi_servers_list_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_servers_list_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_servers_list_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_servers_list_t { - dnsdist_ffi_servers_list_t(const ServerPolicy::NumberedServerVector& servers_): servers(servers_) + dnsdist_ffi_servers_list_t(const ServerPolicy::NumberedServerVector& servers_) : + servers(servers_) { ffiServers.reserve(servers.size()); - for (const auto& server: servers) { + for (const auto& server : servers) { ffiServers.push_back(dnsdist_ffi_server_t(server.second)); } } @@ -132,20 +145,23 @@ struct dnsdist_ffi_servers_list_t }; // dnsdist_ffi_network_message_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_network_message_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_network_message_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_network_message_t { - dnsdist_ffi_network_message_t(const std::string& payload_ ,const std::string& from_, uint16_t endpointID_): payload(payload_), from(from_), endpointID(endpointID_) + dnsdist_ffi_network_message_t(const std::string& payload_, const std::string& from_, uint16_t endpointID_) : + payload(payload_), from(from_), endpointID(endpointID_) { } diff --git a/pdns/dnsdistdist/dnsdist-lua-inspection.cc b/pdns/dnsdistdist/dnsdist-lua-inspection.cc index 2ad3999f7c2e5..26d37f712254f 100644 --- a/pdns/dnsdistdist/dnsdist-lua-inspection.cc +++ b/pdns/dnsdistdist/dnsdist-lua-inspection.cc @@ -19,7 +19,9 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include #include +#include #include "dnsdist.hh" #include "dnsdist-console.hh" @@ -811,12 +813,18 @@ void setupLuaInspection(LuaContext& luaCtx) boost::format fmt("%-35s\t%+11s"); g_outputBuffer.clear(); auto entries = *dnsdist::metrics::g_stats.entries.read_lock(); - sort(entries.begin(), entries.end(), + + // Filter entries to just the ones without label, for clearer output + std::vector> unlabeledEntries; + std::copy_if(entries.begin(), entries.end(), std::back_inserter(unlabeledEntries), [](const decltype(entries)::value_type& triple) { return triple.d_labels.empty(); }); + + sort(unlabeledEntries.begin(), unlabeledEntries.end(), [](const decltype(entries)::value_type& lhs, const decltype(entries)::value_type& rhs) { return lhs.d_name < rhs.d_name; }); boost::format flt(" %9.1f"); - for (const auto& entry : entries) { + for (const auto& entryRef : unlabeledEntries) { + const auto& entry = entryRef.get(); string second; if (const auto& val = std::get_if(&entry.d_value)) { second = std::to_string((*val)->load()); @@ -828,7 +836,7 @@ void setupLuaInspection(LuaContext& luaCtx) second = std::to_string((*func)(entry.d_name)); } - if (leftcolumn.size() < entries.size() / 2) { + if (leftcolumn.size() < unlabeledEntries.size() / 2) { leftcolumn.push_back((fmt % entry.d_name % second).str()); } else { diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index 09818d982107a..d97c8329e0dec 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -85,6 +85,9 @@ using std::thread; +using update_metric_opts_t = LuaAssociativeTable>>; +using declare_metric_opts_t = LuaAssociativeTable>; + static boost::tribool s_noLuaSideEffect; /* this is a best effort way to prevent logging calls with no side-effects in the output of delta() @@ -1264,7 +1267,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) }); luaCtx.writeFunction("getPoolServers", [](const string& pool) { - //coverity[auto_causes_copy] + // coverity[auto_causes_copy] const auto poolServers = getDownstreamCandidates(pool); return *poolServers; }); @@ -1873,9 +1876,9 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) // 1 2 3 4 ret << (fmt % "Name" % "Cache" % "ServerPolicy" % "Servers") << endl; - //coverity[auto_causes_copy] + // coverity[auto_causes_copy] const auto defaultPolicyName = dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy->getName(); - //coverity[auto_causes_copy] + // coverity[auto_causes_copy] const auto pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools; for (const auto& entry : pools) { const string& name = entry.first; @@ -3410,8 +3413,22 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) newThread.detach(); }); - luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, boost::optional customName) { - auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName ? std::optional(*customName) : std::nullopt); + luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, boost::optional> opts) { + bool withLabels = false; + std::optional customName = std::nullopt; + if (opts) { + auto* optCustomName = boost::get(&opts.get()); + if (optCustomName != nullptr) { + customName = std::optional(*optCustomName); + } + if (!customName) { + boost::optional vars = {boost::get(opts.get())}; + getOptionalValue(vars, "customName", customName); + getOptionalValue(vars, "withLabels", withLabels); + checkAllParametersConsumed("declareMetric", vars); + } + } + auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName, withLabels); if (result) { g_outputBuffer += *result + "\n"; errlog("Error in declareMetric: %s", *result); @@ -3419,8 +3436,21 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return true; }); - luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional step) { - auto result = dnsdist::metrics::incrementCustomCounter(name, step ? *step : 1); + // NOLINTNEXTLINE(performance-unnecessary-value-param) + luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional> opts) { + auto incOpts = opts.get_value_or(1); + uint64_t step = 1; + std::unordered_map labels; + if (auto* custom_step = boost::get(&incOpts)) { + step = *custom_step; + } + else { + boost::optional vars = {boost::get(incOpts)}; + getOptionalValue(vars, "step", step); + getOptionalValue>(vars, "labels", labels); + checkAllParametersConsumed("incMetric", vars); + } + auto result = dnsdist::metrics::incrementCustomCounter(name, step, labels); if (const auto* errorStr = std::get_if(&result)) { g_outputBuffer = *errorStr + "'\n"; errlog("Error in incMetric: %s", *errorStr); @@ -3428,8 +3458,21 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional step) { - auto result = dnsdist::metrics::decrementCustomCounter(name, step ? *step : 1); + // NOLINTNEXTLINE(performance-unnecessary-value-param) + luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional> opts) { + auto decOpts = opts.get_value_or(1); + uint64_t step = 1; + std::unordered_map labels; + if (auto* custom_step = boost::get(&decOpts)) { + step = *custom_step; + } + else { + boost::optional vars = {boost::get(decOpts)}; + getOptionalValue(vars, "step", step); + getOptionalValue>(vars, "labels", labels); + checkAllParametersConsumed("decMetric", vars); + } + auto result = dnsdist::metrics::decrementCustomCounter(name, step, labels); if (const auto* errorStr = std::get_if(&result)) { g_outputBuffer = *errorStr + "'\n"; errlog("Error in decMetric: %s", *errorStr); @@ -3437,8 +3480,13 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("setMetric", [](const std::string& name, const double value) -> double { - auto result = dnsdist::metrics::setCustomGauge(name, value); + luaCtx.writeFunction("setMetric", [](const std::string& name, const double value, boost::optional opts) -> double { + std::unordered_map labels; + if (opts) { + getOptionalValue>(opts, "labels", labels); + } + checkAllParametersConsumed("setMetric", opts); + auto result = dnsdist::metrics::setCustomGauge(name, value, labels); if (const auto* errorStr = std::get_if(&result)) { g_outputBuffer = *errorStr + "'\n"; errlog("Error in setMetric: %s", *errorStr); @@ -3446,8 +3494,13 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("getMetric", [](const std::string& name) { - auto result = dnsdist::metrics::getCustomMetric(name); + luaCtx.writeFunction("getMetric", [](const std::string& name, boost::optional opts) { + std::unordered_map labels; + if (opts) { + getOptionalValue>(opts, "labels", labels); + } + checkAllParametersConsumed("getMetric", opts); + auto result = dnsdist::metrics::getCustomMetric(name, labels); if (const auto* errorStr = std::get_if(&result)) { g_outputBuffer = *errorStr + "'\n"; errlog("Error in getMetric: %s", *errorStr); diff --git a/pdns/dnsdistdist/dnsdist-metrics.cc b/pdns/dnsdistdist/dnsdist-metrics.cc index 434088df4e2f1..293be6d95c4d5 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.cc +++ b/pdns/dnsdistdist/dnsdist-metrics.cc @@ -19,7 +19,10 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include +#include #include +#include #include "dnsdist-metrics.hh" #include "dnsdist.hh" @@ -67,107 +70,107 @@ struct MutableGauge mutable pdns::stat_double_t d_value{0}; }; -static SharedLockGuarded>> s_customCounters; -static SharedLockGuarded>> s_customGauges; +static SharedLockGuarded, std::less<>>> s_customCounters; +static SharedLockGuarded, std::less<>>> s_customGauges; Stats::Stats() : - entries{std::vector{ - {"responses", &responses}, - {"servfail-responses", &servfailResponses}, - {"queries", &queries}, - {"frontend-nxdomain", &frontendNXDomain}, - {"frontend-servfail", &frontendServFail}, - {"frontend-noerror", &frontendNoError}, - {"acl-drops", &aclDrops}, - {"rule-drop", &ruleDrop}, - {"rule-nxdomain", &ruleNXDomain}, - {"rule-refused", &ruleRefused}, - {"rule-servfail", &ruleServFail}, - {"rule-truncated", &ruleTruncated}, - {"self-answered", &selfAnswered}, - {"downstream-timeouts", &downstreamTimeouts}, - {"downstream-send-errors", &downstreamSendErrors}, - {"trunc-failures", &truncFail}, - {"no-policy", &noPolicy}, - {"latency0-1", &latency0_1}, - {"latency1-10", &latency1_10}, - {"latency10-50", &latency10_50}, - {"latency50-100", &latency50_100}, - {"latency100-1000", &latency100_1000}, - {"latency-slow", &latencySlow}, - {"latency-avg100", &latencyAvg100}, - {"latency-avg1000", &latencyAvg1000}, - {"latency-avg10000", &latencyAvg10000}, - {"latency-avg1000000", &latencyAvg1000000}, - {"latency-tcp-avg100", &latencyTCPAvg100}, - {"latency-tcp-avg1000", &latencyTCPAvg1000}, - {"latency-tcp-avg10000", &latencyTCPAvg10000}, - {"latency-tcp-avg1000000", &latencyTCPAvg1000000}, - {"latency-dot-avg100", &latencyDoTAvg100}, - {"latency-dot-avg1000", &latencyDoTAvg1000}, - {"latency-dot-avg10000", &latencyDoTAvg10000}, - {"latency-dot-avg1000000", &latencyDoTAvg1000000}, - {"latency-doh-avg100", &latencyDoHAvg100}, - {"latency-doh-avg1000", &latencyDoHAvg1000}, - {"latency-doh-avg10000", &latencyDoHAvg10000}, - {"latency-doh-avg1000000", &latencyDoHAvg1000000}, - {"latency-doq-avg100", &latencyDoQAvg100}, - {"latency-doq-avg1000", &latencyDoQAvg1000}, - {"latency-doq-avg10000", &latencyDoQAvg10000}, - {"latency-doq-avg1000000", &latencyDoQAvg1000000}, - {"latency-doh3-avg100", &latencyDoH3Avg100}, - {"latency-doh3-avg1000", &latencyDoH3Avg1000}, - {"latency-doh3-avg10000", &latencyDoH3Avg10000}, - {"latency-doh3-avg1000000", &latencyDoH3Avg1000000}, - {"uptime", uptimeOfProcess}, - {"real-memory-usage", getRealMemoryUsage}, - {"special-memory-usage", getSpecialMemoryUsage}, - {"udp-in-errors", [](const std::string&) { return udpErrorStats("udp-in-errors"); }}, - {"udp-noport-errors", [](const std::string&) { return udpErrorStats("udp-noport-errors"); }}, - {"udp-recvbuf-errors", [](const std::string&) { return udpErrorStats("udp-recvbuf-errors"); }}, - {"udp-sndbuf-errors", [](const std::string&) { return udpErrorStats("udp-sndbuf-errors"); }}, - {"udp-in-csum-errors", [](const std::string&) { return udpErrorStats("udp-in-csum-errors"); }}, - {"udp6-in-errors", [](const std::string&) { return udp6ErrorStats("udp6-in-errors"); }}, - {"udp6-recvbuf-errors", [](const std::string&) { return udp6ErrorStats("udp6-recvbuf-errors"); }}, - {"udp6-sndbuf-errors", [](const std::string&) { return udp6ErrorStats("udp6-sndbuf-errors"); }}, - {"udp6-noport-errors", [](const std::string&) { return udp6ErrorStats("udp6-noport-errors"); }}, - {"udp6-in-csum-errors", [](const std::string&) { return udp6ErrorStats("udp6-in-csum-errors"); }}, - {"tcp-listen-overflows", [](const std::string&) { return tcpErrorStats("ListenOverflows"); }}, - {"noncompliant-queries", &nonCompliantQueries}, - {"noncompliant-responses", &nonCompliantResponses}, - {"proxy-protocol-invalid", &proxyProtocolInvalid}, - {"rdqueries", &rdQueries}, - {"empty-queries", &emptyQueries}, - {"cache-hits", &cacheHits}, - {"cache-misses", &cacheMisses}, - {"cpu-iowait", getCPUIOWait}, - {"cpu-steal", getCPUSteal}, - {"cpu-sys-msec", getCPUTimeSystem}, - {"cpu-user-msec", getCPUTimeUser}, - {"fd-usage", getOpenFileDescriptors}, - {"dyn-blocked", &dynBlocked}, + entries{std::vector{ + {"responses", "", &responses}, + {"servfail-responses", "", &servfailResponses}, + {"queries", "", &queries}, + {"frontend-nxdomain", "", &frontendNXDomain}, + {"frontend-servfail", "", &frontendServFail}, + {"frontend-noerror", "", &frontendNoError}, + {"acl-drops", "", &aclDrops}, + {"rule-drop", "", &ruleDrop}, + {"rule-nxdomain", "", &ruleNXDomain}, + {"rule-refused", "", &ruleRefused}, + {"rule-servfail", "", &ruleServFail}, + {"rule-truncated", "", &ruleTruncated}, + {"self-answered", "", &selfAnswered}, + {"downstream-timeouts", "", &downstreamTimeouts}, + {"downstream-send-errors", "", &downstreamSendErrors}, + {"trunc-failures", "", &truncFail}, + {"no-policy", "", &noPolicy}, + {"latency0-1", "", &latency0_1}, + {"latency1-10", "", &latency1_10}, + {"latency10-50", "", &latency10_50}, + {"latency50-100", "", &latency50_100}, + {"latency100-1000", "", &latency100_1000}, + {"latency-slow", "", &latencySlow}, + {"latency-avg100", "", &latencyAvg100}, + {"latency-avg1000", "", &latencyAvg1000}, + {"latency-avg10000", "", &latencyAvg10000}, + {"latency-avg1000000", "", &latencyAvg1000000}, + {"latency-tcp-avg100", "", &latencyTCPAvg100}, + {"latency-tcp-avg1000", "", &latencyTCPAvg1000}, + {"latency-tcp-avg10000", "", &latencyTCPAvg10000}, + {"latency-tcp-avg1000000", "", &latencyTCPAvg1000000}, + {"latency-dot-avg100", "", &latencyDoTAvg100}, + {"latency-dot-avg1000", "", &latencyDoTAvg1000}, + {"latency-dot-avg10000", "", &latencyDoTAvg10000}, + {"latency-dot-avg1000000", "", &latencyDoTAvg1000000}, + {"latency-doh-avg100", "", &latencyDoHAvg100}, + {"latency-doh-avg1000", "", &latencyDoHAvg1000}, + {"latency-doh-avg10000", "", &latencyDoHAvg10000}, + {"latency-doh-avg1000000", "", &latencyDoHAvg1000000}, + {"latency-doq-avg100", "", &latencyDoQAvg100}, + {"latency-doq-avg1000", "", &latencyDoQAvg1000}, + {"latency-doq-avg10000", "", &latencyDoQAvg10000}, + {"latency-doq-avg1000000", "", &latencyDoQAvg1000000}, + {"latency-doh3-avg100", "", &latencyDoH3Avg100}, + {"latency-doh3-avg1000", "", &latencyDoH3Avg1000}, + {"latency-doh3-avg10000", "", &latencyDoH3Avg10000}, + {"latency-doh3-avg1000000", "", &latencyDoH3Avg1000000}, + {"uptime", "", uptimeOfProcess}, + {"real-memory-usage", "", getRealMemoryUsage}, + {"special-memory-usage", "", getSpecialMemoryUsage}, + {"udp-in-errors", "", [](const std::string&) { return udpErrorStats("udp-in-errors"); }}, + {"udp-noport-errors", "", [](const std::string&) { return udpErrorStats("udp-noport-errors"); }}, + {"udp-recvbuf-errors", "", [](const std::string&) { return udpErrorStats("udp-recvbuf-errors"); }}, + {"udp-sndbuf-errors", "", [](const std::string&) { return udpErrorStats("udp-sndbuf-errors"); }}, + {"udp-in-csum-errors", "", [](const std::string&) { return udpErrorStats("udp-in-csum-errors"); }}, + {"udp6-in-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-in-errors"); }}, + {"udp6-recvbuf-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-recvbuf-errors"); }}, + {"udp6-sndbuf-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-sndbuf-errors"); }}, + {"udp6-noport-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-noport-errors"); }}, + {"udp6-in-csum-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-in-csum-errors"); }}, + {"tcp-listen-overflows", "", [](const std::string&) { return tcpErrorStats("ListenOverflows"); }}, + {"noncompliant-queries", "", &nonCompliantQueries}, + {"noncompliant-responses", "", &nonCompliantResponses}, + {"proxy-protocol-invalid", "", &proxyProtocolInvalid}, + {"rdqueries", "", &rdQueries}, + {"empty-queries", "", &emptyQueries}, + {"cache-hits", "", &cacheHits}, + {"cache-misses", "", &cacheMisses}, + {"cpu-iowait", "", getCPUIOWait}, + {"cpu-steal", "", getCPUSteal}, + {"cpu-sys-msec", "", getCPUTimeSystem}, + {"cpu-user-msec", "", getCPUTimeUser}, + {"fd-usage", "", getOpenFileDescriptors}, + {"dyn-blocked", "", &dynBlocked}, #ifndef DISABLE_DYNBLOCKS - {"dyn-block-nmg-size", [](const std::string&) { return dnsdist::DynamicBlocks::getClientAddressDynamicRules().size(); }}, + {"dyn-block-nmg-size", "", [](const std::string&) { return dnsdist::DynamicBlocks::getClientAddressDynamicRules().size(); }}, #endif /* DISABLE_DYNBLOCKS */ - {"security-status", &securityStatus}, - {"doh-query-pipe-full", &dohQueryPipeFull}, - {"doh-response-pipe-full", &dohResponsePipeFull}, - {"doq-response-pipe-full", &doqResponsePipeFull}, - {"doh3-response-pipe-full", &doh3ResponsePipeFull}, - {"outgoing-doh-query-pipe-full", &outgoingDoHQueryPipeFull}, - {"tcp-query-pipe-full", &tcpQueryPipeFull}, - {"tcp-cross-protocol-query-pipe-full", &tcpCrossProtocolQueryPipeFull}, - {"tcp-cross-protocol-response-pipe-full", &tcpCrossProtocolResponsePipeFull}, + {"security-status", "", &securityStatus}, + {"doh-query-pipe-full", "", &dohQueryPipeFull}, + {"doh-response-pipe-full", "", &dohResponsePipeFull}, + {"doq-response-pipe-full", "", &doqResponsePipeFull}, + {"doh3-response-pipe-full", "", &doh3ResponsePipeFull}, + {"outgoing-doh-query-pipe-full", "", &outgoingDoHQueryPipeFull}, + {"tcp-query-pipe-full", "", &tcpQueryPipeFull}, + {"tcp-cross-protocol-query-pipe-full", "", &tcpCrossProtocolQueryPipeFull}, + {"tcp-cross-protocol-response-pipe-full", "", &tcpCrossProtocolResponsePipeFull}, // Latency histogram - {"latency-sum", &latencySum}, - {"latency-count", &latencyCount}, + {"latency-sum", "", &latencySum}, + {"latency-count", "", &latencyCount}, }} { } struct Stats g_stats; -std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName) +std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName, bool withLabels) { if (!std::regex_match(name, std::regex("^[a-z0-9-]+$"))) { return std::string("Unable to declare metric '") + std::string(name) + std::string("': invalid name\n"); @@ -176,18 +179,24 @@ std::optional declareCustomMetric(const std::string& name, const st const std::string finalCustomName(customName ? *customName : ""); if (type == "counter") { auto customCounters = s_customCounters.write_lock(); - auto itp = customCounters->insert({name, MutableCounter()}); + auto itp = customCounters->emplace(name, std::map()); if (itp.second) { - g_stats.entries.write_lock()->emplace_back(Stats::EntryPair{name, &(*customCounters)[name].d_value}); + if (!withLabels) { + auto counter = itp.first->second.emplace("", MutableCounter()); + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{name, "", &counter.first->second.d_value}); + } dnsdist::prometheus::PrometheusMetricDefinition def{name, type, description, finalCustomName}; dnsdist::webserver::addMetricDefinition(def); } } else if (type == "gauge") { auto customGauges = s_customGauges.write_lock(); - auto itp = customGauges->insert({name, MutableGauge()}); + auto itp = customGauges->emplace(name, std::map()); if (itp.second) { - g_stats.entries.write_lock()->emplace_back(Stats::EntryPair{name, &(*customGauges)[name].d_value}); + if (!withLabels) { + auto gauge = itp.first->second.emplace("", MutableGauge()); + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{name, "", &gauge.first->second.d_value}); + } dnsdist::prometheus::PrometheusMetricDefinition def{name, type, description, finalCustomName}; dnsdist::webserver::addMetricDefinition(def); } @@ -198,57 +207,107 @@ std::optional declareCustomMetric(const std::string& name, const st return std::nullopt; } -std::variant incrementCustomCounter(const std::string_view& name, uint64_t step) +static string prometheusLabelValueEscape(const string& value) { - auto customCounters = s_customCounters.read_lock(); + string ret; + + for (char lblchar : value) { + if (lblchar == '"' || lblchar == '\\') { + ret += '\\'; + ret += lblchar; + } + else if (lblchar == '\n') { + ret += '\\'; + ret += 'n'; + } + else { + ret += lblchar; + } + } + return ret; +} + +static std::string generateCombinationOfLabels(const std::unordered_map& labels) +{ + auto ordered = std::map(labels.begin(), labels.end()); + return std::accumulate(ordered.begin(), ordered.end(), std::string(), [](const std::string& acc, const std::pair& label) { + return acc + (acc.empty() ? std::string() : ",") + label.first + "=" + "\"" + prometheusLabelValueEscape(label.second) + "\""; + }); +} + +template +static T& initializeOrGetMetric(const std::string_view& name, std::map& metricEntries, const std::unordered_map& labels) +{ + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = metricEntries.find(combinationOfLabels); + if (metricEntry == metricEntries.end()) { + metricEntry = metricEntries.emplace(std::piecewise_construct, std::forward_as_tuple(combinationOfLabels), std::forward_as_tuple()).first; + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), combinationOfLabels, &metricEntry->second.d_value}); + } + return metricEntry->second; +} + +std::variant incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels) +{ + auto customCounters = s_customCounters.write_lock(); auto metric = customCounters->find(name); if (metric != customCounters->end()) { - metric->second.d_value += step; - return metric->second.d_value.load(); + auto& metricEntry = initializeOrGetMetric(name, metric->second, labels); + metricEntry.d_value += step; + return metricEntry.d_value.load(); } return std::string("Unable to increment custom metric '") + std::string(name) + "': no such counter"; } -std::variant decrementCustomCounter(const std::string_view& name, uint64_t step) +std::variant decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels) { - auto customCounters = s_customCounters.read_lock(); + auto customCounters = s_customCounters.write_lock(); auto metric = customCounters->find(name); if (metric != customCounters->end()) { - metric->second.d_value -= step; - return metric->second.d_value.load(); + auto& metricEntry = initializeOrGetMetric(name, metric->second, labels); + metricEntry.d_value -= step; + return metricEntry.d_value.load(); } return std::string("Unable to decrement custom metric '") + std::string(name) + "': no such counter"; } -std::variant setCustomGauge(const std::string_view& name, const double value) +std::variant setCustomGauge(const std::string_view& name, const double value, const std::unordered_map& labels) { - auto customGauges = s_customGauges.read_lock(); + auto customGauges = s_customGauges.write_lock(); auto metric = customGauges->find(name); if (metric != customGauges->end()) { - metric->second.d_value = value; + auto& metricEntry = initializeOrGetMetric(name, metric->second, labels); + metricEntry.d_value = value; return value; } return std::string("Unable to set metric '") + std::string(name) + "': no such gauge"; } -std::variant getCustomMetric(const std::string_view& name) +std::variant getCustomMetric(const std::string_view& name, const std::unordered_map& labels) { { auto customCounters = s_customCounters.read_lock(); auto counter = customCounters->find(name); if (counter != customCounters->end()) { - return static_cast(counter->second.d_value.load()); + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = counter->second.find(combinationOfLabels); + if (metricEntry != counter->second.end()) { + return static_cast(metricEntry->second.d_value.load()); + } } } { auto customGauges = s_customGauges.read_lock(); auto gauge = customGauges->find(name); if (gauge != customGauges->end()) { - return gauge->second.d_value.load(); + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = gauge->second.find(combinationOfLabels); + if (metricEntry != gauge->second.end()) { + return metricEntry->second.d_value.load(); + } } } return std::string("Unable to get metric '") + std::string(name) + "': no such metric"; } - } diff --git a/pdns/dnsdistdist/dnsdist-metrics.hh b/pdns/dnsdistdist/dnsdist-metrics.hh index 47a3fb84078bb..6cab90527284c 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.hh +++ b/pdns/dnsdistdist/dnsdist-metrics.hh @@ -25,6 +25,7 @@ #include #include #include +#include #include #include "lock.hh" @@ -34,11 +35,11 @@ namespace dnsdist::metrics { using Error = std::string; -[[nodiscard]] std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName); -[[nodiscard]] std::variant incrementCustomCounter(const std::string_view& name, uint64_t step); -[[nodiscard]] std::variant decrementCustomCounter(const std::string_view& name, uint64_t step); -[[nodiscard]] std::variant setCustomGauge(const std::string_view& name, const double value); -[[nodiscard]] std::variant getCustomMetric(const std::string_view& name); +[[nodiscard]] std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName, bool withLabels); +[[nodiscard]] std::variant incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels); +[[nodiscard]] std::variant decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels); +[[nodiscard]] std::variant setCustomGauge(const std::string_view& name, const double value, const std::unordered_map& labels); +[[nodiscard]] std::variant getCustomMetric(const std::string_view& name, const std::unordered_map& labels); using pdns::stat_t; @@ -89,13 +90,14 @@ struct Stats pdns::stat_double_t latencyDoH3Avg100{0}, latencyDoH3Avg1000{0}, latencyDoH3Avg10000{0}, latencyDoH3Avg1000000{0}; using statfunction_t = std::function; using entry_t = std::variant; - struct EntryPair + struct EntryTriple { std::string d_name; + std::string d_labels; entry_t d_value; }; - SharedLockGuarded> entries; + SharedLockGuarded> entries; }; extern struct Stats g_stats; diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc index 5cdfead09cd06..26c34edc4d148 100644 --- a/pdns/dnsdistdist/dnsdist-web.cc +++ b/pdns/dnsdistdist/dnsdist-web.cc @@ -500,6 +500,10 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp) prometheusMetricName = metricDetails.customName; } + if (!entry.d_labels.empty()) { + prometheusMetricName += "{" + entry.d_labels + "}"; + } + // for these we have the help and types encoded in the sources // but we need to be careful about labels in custom metrics std::string helpName = prometheusMetricName.substr(0, prometheusMetricName.find('{')); @@ -927,6 +931,9 @@ static void addStatsToJSONObject(Json::object& obj) if (entry.d_name == "special-memory-usage") { continue; // Too expensive for get-all } + if (!entry.d_labels.empty()) { + continue; // Skip labeled metrics to prevent duplicates + } if (const auto& val = std::get_if(&entry.d_value)) { obj.emplace(entry.d_name, (double)(*val)->load()); } @@ -1908,7 +1915,7 @@ void setMaxConcurrentConnections(size_t max) void WebserverThread(Socket sock) { setThreadName("dnsdist/webserv"); - //coverity[auto_causes_copy] + // coverity[auto_causes_copy] const auto local = *dnsdist::configuration::getCurrentRuntimeConfiguration().d_webServerAddress; infolog("Webserver launched on %s", local.toStringWithPort()); diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc index 75ec0dd29a637..758e0b7dadada 100644 --- a/pdns/dnsdistdist/dnsdist.cc +++ b/pdns/dnsdistdist/dnsdist.cc @@ -501,14 +501,15 @@ bool processResponseAfterRules(PacketBuffer& response, DNSResponse& dnsResponse, zeroScope = false; } uint32_t cacheKey = dnsResponse.ids.cacheKey; - if (dnsResponse.ids.protocol == dnsdist::Protocol::DoH && dnsResponse.ids.forwardedOverUDP) { - cacheKey = dnsResponse.ids.cacheKeyUDP; + if (dnsResponse.ids.protocol == dnsdist::Protocol::DoH && !dnsResponse.ids.forwardedOverUDP) { + cacheKey = dnsResponse.ids.cacheKeyTCP; + // disable zeroScope in that case, as we only have the "no-ECS" cache key for UDP + zeroScope = false; } - else if (zeroScope) { + if (zeroScope) { // if zeroScope, pass the pre-ECS hash-key and do not pass the subnet to the cache cacheKey = dnsResponse.ids.cacheKeyNoECS; } - dnsResponse.ids.packetCache->insert(cacheKey, zeroScope ? boost::none : dnsResponse.ids.subnet, dnsResponse.ids.cacheFlags, dnsResponse.ids.dnssecOK, dnsResponse.ids.qname, dnsResponse.ids.qtype, dnsResponse.ids.qclass, response, dnsResponse.ids.forwardedOverUDP, dnsResponse.getHeader()->rcode, dnsResponse.ids.tempFailureTTL); const auto& chains = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ruleChains; @@ -1418,6 +1419,10 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ std::shared_ptr serverPool = getPool(dnsQuestion.ids.poolName); dnsQuestion.ids.packetCache = serverPool->packetCache; selectBackendForOutgoingQuery(dnsQuestion, serverPool, selectedBackend); + bool willBeForwardedOverUDP = !dnsQuestion.overTCP() || dnsQuestion.ids.protocol == dnsdist::Protocol::DoH; + if (selectedBackend && selectedBackend->isTCPOnly()) { + willBeForwardedOverUDP = false; + } uint32_t allowExpired = selectedBackend ? 0 : dnsdist::configuration::getCurrentRuntimeConfiguration().d_staleCacheEntriesTTL; @@ -1430,7 +1435,7 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ // we need ECS parsing (parseECS) to be true so we can be sure that the initial incoming query did not have an existing // ECS option, which would make it unsuitable for the zero-scope feature. if (dnsQuestion.ids.packetCache && !dnsQuestion.ids.skipCache && (!selectedBackend || !selectedBackend->d_config.disableZeroScope) && dnsQuestion.ids.packetCache->isECSParsingEnabled()) { - if (dnsQuestion.ids.packetCache->get(dnsQuestion, dnsQuestion.getHeader()->id, &dnsQuestion.ids.cacheKeyNoECS, dnsQuestion.ids.subnet, dnsQuestion.ids.dnssecOK, !dnsQuestion.overTCP(), allowExpired, false, true, false)) { + if (dnsQuestion.ids.packetCache->get(dnsQuestion, dnsQuestion.getHeader()->id, &dnsQuestion.ids.cacheKeyNoECS, dnsQuestion.ids.subnet, dnsQuestion.ids.dnssecOK, willBeForwardedOverUDP, allowExpired, false, true, false)) { vinfolog("Packet cache hit for query for %s|%s from %s (%s, %d bytes)", dnsQuestion.ids.qname.toLogString(), QType(dnsQuestion.ids.qtype).toString(), dnsQuestion.ids.origRemote.toStringWithPort(), dnsQuestion.ids.protocol.toString(), dnsQuestion.getData().size()); @@ -1456,14 +1461,11 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ } if (dnsQuestion.ids.packetCache && !dnsQuestion.ids.skipCache) { - bool forwardedOverUDP = !dnsQuestion.overTCP(); - if (selectedBackend && selectedBackend->isTCPOnly()) { - forwardedOverUDP = false; - } - - /* we do not record a miss for queries received over DoH and forwarded over TCP + /* First lookup, which takes into account how the protocol over which the query will be forwarded. + For DoH, this lookup is done with the protocol set to TCP but we will retry over UDP below, + therefore we do not record a miss for queries received over DoH and forwarded over TCP yet, as we will do a second-lookup */ - if (dnsQuestion.ids.packetCache->get(dnsQuestion, dnsQuestion.getHeader()->id, &dnsQuestion.ids.cacheKey, dnsQuestion.ids.subnet, dnsQuestion.ids.dnssecOK, forwardedOverUDP, allowExpired, false, true, dnsQuestion.ids.protocol != dnsdist::Protocol::DoH || forwardedOverUDP)) { + if (dnsQuestion.ids.packetCache->get(dnsQuestion, dnsQuestion.getHeader()->id, dnsQuestion.ids.protocol == dnsdist::Protocol::DoH ? &dnsQuestion.ids.cacheKeyTCP : &dnsQuestion.ids.cacheKey, dnsQuestion.ids.subnet, dnsQuestion.ids.dnssecOK, dnsQuestion.ids.protocol != dnsdist::Protocol::DoH && willBeForwardedOverUDP, allowExpired, false, true, dnsQuestion.ids.protocol != dnsdist::Protocol::DoH || !willBeForwardedOverUDP)) { dnsdist::PacketMangling::editDNSHeaderFromPacket(dnsQuestion.getMutableData(), [flags = dnsQuestion.ids.origFlags](dnsheader& header) { restoreFlags(&header, flags); @@ -1480,9 +1482,10 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dnsQuestion, std::shared_ ++dnsQuestion.ids.cs->responses; return ProcessQueryResult::SendAnswer; } - if (dnsQuestion.ids.protocol == dnsdist::Protocol::DoH && !forwardedOverUDP) { - /* do a second-lookup for UDP responses, but we do not want TC=1 answers */ - if (dnsQuestion.ids.packetCache->get(dnsQuestion, dnsQuestion.getHeader()->id, &dnsQuestion.ids.cacheKeyUDP, dnsQuestion.ids.subnet, dnsQuestion.ids.dnssecOK, true, allowExpired, false, false, true)) { + if (dnsQuestion.ids.protocol == dnsdist::Protocol::DoH && willBeForwardedOverUDP) { + /* do a second-lookup for responses received over UDP, but we do not want TC=1 answers */ + /* we need to be careful to keep the existing cache-key (TCP) */ + if (dnsQuestion.ids.packetCache->get(dnsQuestion, dnsQuestion.getHeader()->id, &dnsQuestion.ids.cacheKey, dnsQuestion.ids.subnet, dnsQuestion.ids.dnssecOK, true, allowExpired, false, false, true)) { if (!prepareOutgoingResponse(*dnsQuestion.ids.cs, dnsQuestion, true)) { return ProcessQueryResult::Drop; } diff --git a/pdns/dnsdistdist/docs/reference/custommetrics.rst b/pdns/dnsdistdist/docs/reference/custommetrics.rst index b2f5b517be3d2..d688a0a1c16ce 100644 --- a/pdns/dnsdistdist/docs/reference/custommetrics.rst +++ b/pdns/dnsdistdist/docs/reference/custommetrics.rst @@ -10,13 +10,19 @@ Then you can update those at runtime using the following functions, depending on * manipulate counters using :func:`incMetric` and :func:`decMetric` * update a gauge using :func:`setMetric` -.. function:: declareMetric(name, type, description [, prometheusName]) -> bool +.. function:: declareMetric(name, type, description [, prometheusName|options]) -> bool .. versionadded:: 1.8.0 .. versionchanged:: 1.8.1 This function can now be used at runtime, instead of only at configuration time. + .. versionchanged:: 2.0.0 + This function now takes options, with ``withLabels`` option added. ``prometheusName`` can now be provided in options. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Re-declaring an existing metric with the same name and type will not reset it. Re-declaring with the same name but a different type will cause one of them to be masked. @@ -25,51 +31,103 @@ Then you can update those at runtime using the following functions, depending on :param str name: The name of the metric, lowercase alphanumerical characters and dashes (-) only :param str type: The desired type in ``gauge`` or ``counter`` :param str description: The description of the metric - :param str prometheusName: The name to use in the prometheus metrics, if supplied. Otherwise the regular name will be used, prefixed with ``dnsdist_`` and ``-`` replaced by ``_``. + :param str prometheusName: The name to use in the prometheus metrics, if supplied. Otherwise the regular name will be used, prefixed with ``dnsdist_`` and ``-`` replaced by ``_`` + :param table options: A table with key: value pairs with metric options. + + Options: -.. function:: incMetric(name [, step]) -> int + * ``name``: str - The name to use in the prometheus metrics, if supplied. Otherwise the regular name will be used, prefixed with ``dnsdist_`` and ``-`` replaced by ``_`` + * ``withLabels=false``: bool - If set to true, labels will be expected when updating this metric and it will not be automatically created without labels. Defaults to ``false``, which automatically creates this metric without labels with default value. + +.. function:: incMetric(name [, step|options]) -> int .. versionadded:: 1.8.0 .. versionchanged:: 1.8.1 Optional ``step`` parameter added. + .. versionchanged:: 2.0.0 + This function now takes options, with ``labels`` option added. ``step`` can now be provided in options. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Increment counter by one (or more, see the ``step`` parameter), will issue an error if the metric is not declared or not a ``counter``. Returns the new value. :param str name: The name of the metric - :param int step: By how much the counter should be incremented, default to 1. + :param int step: By how much the counter should be incremented, default to 1 + :param table options: A table with key: value pairs with metric options. + + Options: + + * ``step``: int - By how much the counter should be incremented, default to 1 + * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to increment the metric. Different combinations of labels have different metric values. -.. function:: decMetric(name) -> int +.. function:: decMetric(name [, step|options]) -> int .. versionadded:: 1.8.0 .. versionchanged:: 1.8.1 Optional ``step`` parameter added. + .. versionchanged:: 2.0.0 + This function now takes options, with ``labels`` option added. ``step`` can now be provided in options. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Decrement counter by one (or more, see the ``step`` parameter), will issue an error if the metric is not declared or not a ``counter``. Returns the new value. :param str name: The name of the metric :param int step: By how much the counter should be decremented, default to 1. + :param table options: A table with key: value pairs with metric options. -.. function:: getMetric(name) -> double + Options: + + * ``step``: int - By how much the counter should be decremented, default to 1 + * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to decrement the metric. Different combinations of labels have different metric values. + +.. function:: getMetric(name [, options]) -> double .. versionadded:: 1.8.0 + .. versionchanged:: 2.0.0 + This function now takes options, with ``labels`` option added. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Get metric value. :param str name: The name of the metric + :param table options: A table with key: value pairs with metric options. + + Options: -.. function:: setMetric(name, value) -> double + * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to read the metric. Different combinations of labels have different metric values. + +.. function:: setMetric(name, value [, options]) -> double .. versionadded:: 1.8.0 + .. versionchanged:: 2.0.0 + This function now takes options, with ``labels`` option added. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Set the new value, will issue an error if the metric is not declared or not a ``gauge``. Return the new value. :param str name: The name of the metric :param double value: The new value + :param table options: A table with key: value pairs with metric options. + + Options: + + * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to set the metric. Different combinations of labels have different metric values. diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc index e1f81a14d3e86..12974ae57cea7 100644 --- a/pdns/dnsparser.cc +++ b/pdns/dnsparser.cc @@ -80,7 +80,7 @@ void UnknownRecordContent::toPacket(DNSPacketWriter& pw) const pw.xfrBlob(string(d_record.begin(),d_record.end())); } -shared_ptr DNSRecordContent::deserialize(const DNSName& qname, uint16_t qtype, const string& serialized, uint16_t qclass) +shared_ptr DNSRecordContent::deserialize(const DNSName& qname, uint16_t qtype, const string& serialized, uint16_t qclass, bool internalRepresentation) { dnsheader dnsheader; memset(&dnsheader, 0, sizeof(dnsheader)); @@ -118,10 +118,11 @@ shared_ptr DNSRecordContent::deserialize(const DNSName& qname, dr.d_type = qtype; dr.d_name = qname; dr.d_clen = serialized.size(); - PacketReader pr(std::string_view(reinterpret_cast(packet.data()), packet.size()), packet.size() - serialized.size() - sizeof(dnsrecordheader)); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): packet.data() is uint8_t * + PacketReader reader(std::string_view(reinterpret_cast(packet.data()), packet.size()), packet.size() - serialized.size() - sizeof(dnsrecordheader), internalRepresentation); /* needed to get the record boundaries right */ - pr.getDnsrecordheader(drh); - auto content = DNSRecordContent::make(dr, pr, Opcode::Query); + reader.getDnsrecordheader(drh); + auto content = DNSRecordContent::make(dr, reader, Opcode::Query); return content; } @@ -665,7 +666,13 @@ void PacketReader::xfrSvcParamKeyVals(set &kvs) { xfrCAWithoutPort(key, addr); addresses.push_back(addr); } - kvs.insert(SvcParam(key, std::move(addresses))); + // If there were no addresses, and the input comes from internal + // representation, we can reasonably assume this is the serialization + // of "auto". + bool doAuto{d_internal && len == 0}; + auto param = SvcParam(key, std::move(addresses)); + param.setAutoHint(doAuto); + kvs.insert(param); break; } case SvcParam::ech: { diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh index 21ad0205c14a7..fd2f5b7112f3a 100644 --- a/pdns/dnsparser.hh +++ b/pdns/dnsparser.hh @@ -68,8 +68,8 @@ class MOADNSParser; class PacketReader { public: - PacketReader(const std::string_view& content, uint16_t initialPos=sizeof(dnsheader)) - : d_pos(initialPos), d_startrecordpos(initialPos), d_content(content) + PacketReader(const std::string_view& content, uint16_t initialPos=sizeof(dnsheader), bool internalRepresentation = false) + : d_pos(initialPos), d_startrecordpos(initialPos), d_content(content), d_internal(internalRepresentation) { if(content.size() > std::numeric_limits::max()) throw std::out_of_range("packet too large"); @@ -186,6 +186,7 @@ private: uint16_t d_recordlen; // ditto uint16_t not_used; // Aligns the whole class on 8-byte boundaries const std::string_view d_content; + bool d_internal; }; struct DNSRecord; @@ -225,8 +226,10 @@ public: return typeid(*this)==typeid(rhs) && this->getZoneRepresentation() == rhs.getZoneRepresentation(); } - // parse the content in wire format, possibly including compressed pointers pointing to the owner name - static shared_ptr deserialize(const DNSName& qname, uint16_t qtype, const string& serialized, uint16_t qclass=QClass::IN); + // parse the content in wire format, possibly including compressed pointers pointing to the owner name. + // internalRepresentation is set when the data comes from an internal source, + // such as the LMDB backend. + static shared_ptr deserialize(const DNSName& qname, uint16_t qtype, const string& serialized, uint16_t qclass=QClass::IN, bool internalRepresentation = false); void doRecordCheck(const struct DNSRecord&){} diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index 9a318dfc46ba8..b8a24c5c8506c 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -1885,6 +1885,32 @@ unsigned int SyncRes::getAdjustedRecursionBound() const return bound; } +static bool haveFinalAnswer(const DNSName& qname, QType qtype, int res, const vector& ret) +{ + if (res != RCode::NoError) { + return false; + } + + // This loop assumes the CNAME's records are in-order + DNSName target(qname); + for (const auto& record : ret) { + if (record.d_place == DNSResourceRecord::ANSWER && record.d_name == target) { + if (record.d_type == qtype) { + return true; + } + if (record.d_type == QType::CNAME) { + if (auto ptr = getRR(record)) { + target = ptr->getTarget(); + } + else { + return false; + } + } + } + } + return false; +} + /*! This function will check the cache and go out to the internet if the answer is not in cache * * \param qname The name we need an answer for @@ -1986,6 +2012,11 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName& qname, const QType qtyp } } } + // This handles the case mentioned above: if the full CNAME chain leading to the answer was + // constructed from the cache, indicate that. + if (fromCache != nullptr && !*fromCache && haveFinalAnswer(qname, qtype, res, ret)) { + *fromCache = true; + } return res; } @@ -2035,7 +2066,9 @@ int SyncRes::doResolveNoQNameMinimization(const DNSName& qname, const QType qtyp } } } - + if (fromCache != nullptr && !*fromCache && haveFinalAnswer(qname, qtype, res, ret)) { + *fromCache = true; + } return res; } } diff --git a/pdns/recursordist/test-syncres_cc1.cc b/pdns/recursordist/test-syncres_cc1.cc index d51953fd2ff71..af5fccaa28c59 100644 --- a/pdns/recursordist/test-syncres_cc1.cc +++ b/pdns/recursordist/test-syncres_cc1.cc @@ -1737,6 +1737,117 @@ BOOST_AUTO_TEST_CASE(test_cname_long_loop) } } +BOOST_AUTO_TEST_CASE(test_cname_long_step0_shortcut) +{ + // This tets the case fixed /optimizaed in #14973 + std::unique_ptr resolver; + initSR(resolver); + resolver->setQNameMinimization(); + primeHints(); + resolver->setLogMode(SyncRes::Store); + + size_t count = 0; + const DNSName target1("cname1.powerdns.com."); + const DNSName target2("cname2.powerdns.com."); + const DNSName target3("cname3.powerdns.com."); + const DNSName target4("cname4.powerdns.com."); + + resolver->setAsyncCallback([&](const ComboAddress& address, const DNSName& domain, int qtype, bool /* doTCP */, bool /* sendRDQuery */, int /* EDNS0Level */, struct timeval* /* now */, boost::optional& /* srcmask */, const ResolveContext& /* context */, LWResult* res, bool* /* chained */) { + count++; + + if (domain == DNSName("com.")) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, domain, QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + if (domain == DNSName("powerdns.com.")) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, domain, QType::NS, "ns.powerdns.com.", DNSResourceRecord::AUTHORITY, 172800); + addRecordToLW(res, "ns.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + if (address == ComboAddress("192.0.2.2:53")) { + + if (domain == target1) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::CNAME, target2.toString()); + return LWResult::Result::Success; + } + if (domain == target2) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::CNAME, target3.toString()); + return LWResult::Result::Success; + } + if (domain == target3) { + setLWResult(res, 0, true, false, false); + addRecordToLW(res, domain, QType::CNAME, target4.toString()); + return LWResult::Result::Success; + } + if (domain == target4) { + setLWResult(res, 0, true, false, false); + if (qtype == QType::A) { + addRecordToLW(res, domain, QType::A, "1.2.3.4"); + } + else if (qtype == QType::AAAA) { + addRecordToLW(res, domain, QType::AAAA, "::1234"); + } + return LWResult::Result::Success; + } + + return LWResult::Result::Success; + } + + return LWResult::Result::Timeout; + }); + + // We analyze the trace to see how many cases of a failed Step0 occurs. This is a bit dirty, but + // we have no other way of telling how the resolving took place, as the number of cache lookups is + // not recorded by the record cache. Also, we like to know if something in Syncres changed that + // influences the number of failing Step0 lookups. + auto counter = [](const string& str) { + const std::string key = "Step0 Not cached"; + size_t occurences = 0; + auto pos = str.find(key); + while (pos != std::string::npos) { + occurences++; + pos = str.find(key, pos + 1); + } + return occurences; + }; + vector ret; + int res = resolver->beginResolve(target1, QType(QType::AAAA), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(ret.size(), 4U); + BOOST_CHECK_EQUAL(count, 6U); + BOOST_CHECK_EQUAL(resolver->d_maxdepth, 3U); + auto trace = resolver->getTrace(); + trace = resolver->getTrace(); + BOOST_CHECK_EQUAL(counter(trace), 4U); + + // And again to check cache, all Step0 cases should succeed + ret.clear(); + res = resolver->beginResolve(target1, QType(QType::AAAA), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(ret.size(), 4U); + BOOST_CHECK_EQUAL(count, 6U); + BOOST_CHECK_EQUAL(resolver->d_maxdepth, 3U); + trace = resolver->getTrace(); + BOOST_CHECK_EQUAL(counter(trace), 4U); + + // And again to check a name that does not fully resolve, we expect Step0 failures to increase + g_recCache->doWipeCache(target4, false, QType::AAAA); + ret.clear(); + res = resolver->beginResolve(target1, QType(QType::AAAA), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_CHECK_EQUAL(ret.size(), 4U); + BOOST_CHECK_EQUAL(count, 7U); + BOOST_CHECK_EQUAL(resolver->d_maxdepth, 3U); + trace = resolver->getTrace(); + // XXX This number feels pretty high (15 extra, same as before #14973, there seems to be room for more improvement). + BOOST_CHECK_EQUAL(counter(trace), 19U); +} + BOOST_AUTO_TEST_CASE(test_cname_length) { std::unique_ptr sr; diff --git a/regression-tests.auth-py/test_SVCB.py b/regression-tests.auth-py/test_SVCB.py index 7bb10ac4ce79f..12f35c4b67190 100644 --- a/regression-tests.auth-py/test_SVCB.py +++ b/regression-tests.auth-py/test_SVCB.py @@ -1,10 +1,23 @@ from authtests import AuthTest import dns +import os +import subprocess +class SVCBRecordsBase(AuthTest): + # Copied from AuthTest, without the bind-config and bind-dnssec fields. + _config_template_default = """ +module-dir={PDNS_MODULE_DIR} +daemon=no +socket-dir={confdir} +cache-ttl=0 +negquery-cache-ttl=0 +query-cache-ttl=0 +log-dns-queries=yes +log-dns-details=yes +loglevel=9 +distributor-threads=1""" -class TestSVCBRecords(AuthTest): _config_template = """ -launch=bind svc-autohints """ @@ -39,7 +52,7 @@ class TestSVCBRecords(AuthTest): """, } - def testWithoutAlias(self): + def impl_testWithoutAlias(self): query = dns.message.make_query('www.example.org', 'HTTPS') res = self.sendUDPQuery(query) expected_ans = dns.rrset.from_text( @@ -50,7 +63,7 @@ def testWithoutAlias(self): self.assertRRsetInAnswer(res, expected_ans) self.assertEqual(len(res.additional), 2) - def testWithAlias(self): + def impl_testWithAlias(self): """ Ensure additional processing happens for HTTPS AliasMode """ @@ -70,7 +83,7 @@ def testWithAlias(self): self.assertRRsetInAdditional(res, expected_addl) self.assertEqual(len(res.additional), 3) - def testWithMissingA(self): + def impl_testWithMissingA(self): """ Ensure PowerDNS removes the ipv4hint if there's no A record """ @@ -84,7 +97,7 @@ def testWithMissingA(self): self.assertRRsetInAnswer(res, expected_ans) self.assertEqual(len(res.additional), 1) - def testWithMissingAAAA(self): + def impl_testWithMissingAAAA(self): """ Ensure PowerDNS removes the ipv6hint if there's no AAAA record """ @@ -98,7 +111,7 @@ def testWithMissingAAAA(self): self.assertRRsetInAnswer(res, expected_ans) self.assertEqual(len(res.additional), 1) - def testNoAuto(self): + def impl_testNoAuto(self): """ Ensure we send the actual hints, not generated ones """ @@ -113,7 +126,7 @@ def testNoAuto(self): self.assertRRsetInAnswer(res, expected_ans) self.assertEqual(len(res.additional), 2) - def testAutoA(self): + def impl_testAutoA(self): """ Ensure we send a generated A hint, but keep the existing AAAA hint """ @@ -128,7 +141,7 @@ def testAutoA(self): self.assertRRsetInAnswer(res, expected_ans) self.assertEqual(len(res.additional), 2) - def testAutoAAAA(self): + def impl_testAutoAAAA(self): """ Ensure we send a generated AAAA hint, but keep the existing A hint """ @@ -142,3 +155,128 @@ def testAutoAAAA(self): print(res) self.assertRRsetInAnswer(res, expected_ans) self.assertEqual(len(res.additional), 2) + +class TestSVCBRecordsBind(SVCBRecordsBase): + _config_template_default = ( + SVCBRecordsBase._config_template_default + + """ +bind-config={confdir}/named.conf +bind-dnssec-db={bind_dnssec_db} +""" + ) + + _config_template = ( + SVCBRecordsBase._config_template + + """ +launch=bind +""" + ) + + def testWithoutAlias(self): + self.impl_testWithoutAlias() + + def testWithAlias(self): + """ + Ensure additional processing happens for HTTPS AliasMode + """ + self.impl_testWithAlias() + + def testWithMissingA(self): + """ + Ensure PowerDNS removes the ipv4hint if there's no A record + """ + self.impl_testWithMissingA() + + def testWithMissingAAAA(self): + """ + Ensure PowerDNS removes the ipv6hint if there's no AAAA record + """ + self.impl_testWithMissingAAAA() + + def testNoAuto(self): + """ + Ensure we send the actual hints, not generated ones + """ + self.impl_testNoAuto() + + def testAutoA(self): + """ + Ensure we send a generated A hint, but keep the existing AAAA hint + """ + self.impl_testAutoA() + + def testAutoAAAA(self): + """ + Ensure we send a generated AAAA hint, but keep the existing A hint + """ + self.impl_testAutoAAAA() + +class TestSVCBRecordsLMDB(SVCBRecordsBase): + _config_template = ( + SVCBRecordsBase._config_template + + """ +launch=lmdb +""" + ) + + @classmethod + def generateAllAuthConfig(cls, confdir): + # This is very similar to AuthTest.generateAllAuthConfig, + # but for lmdb backend, we ignore auth keys but need to load-zone + # into lmdb storage. + cls.generateAuthConfig(confdir) + + for zonename, zonecontent in cls._zones.items(): + cls.generateAuthZone(confdir, + zonename, + zonecontent) + pdnsutilCmd = [os.environ['PDNSUTIL'], + '--config-dir=%s' % confdir, + 'load-zone', + zonename, + os.path.join(confdir, '%s.zone' % zonename)] + + print(' '.join(pdnsutilCmd)) + try: + subprocess.check_output(pdnsutilCmd, stderr=subprocess.STDOUT) + except subprocess.CalledProcessError as e: + raise AssertionError('%s failed (%d): %s' % (pdnsutilCmd, e.returncode, e.output)) + + def testWithoutAlias(self): + self.impl_testWithoutAlias() + + def testWithAlias(self): + """ + Ensure additional processing happens for HTTPS AliasMode + """ + self.impl_testWithAlias() + + def testWithMissingA(self): + """ + Ensure PowerDNS removes the ipv4hint if there's no A record + """ + self.impl_testWithMissingA() + + def testWithMissingAAAA(self): + """ + Ensure PowerDNS removes the ipv6hint if there's no AAAA record + """ + self.impl_testWithMissingAAAA() + + def testNoAuto(self): + """ + Ensure we send the actual hints, not generated ones + """ + self.impl_testNoAuto() + + def testAutoA(self): + """ + Ensure we send a generated A hint, but keep the existing AAAA hint + """ + self.impl_testAutoA() + + def testAutoAAAA(self): + """ + Ensure we send a generated AAAA hint, but keep the existing A hint + """ + self.impl_testAutoAAAA() diff --git a/regression-tests.dnsdist/test_API.py b/regression-tests.dnsdist/test_API.py index dd0e9fde85a72..dfdda817f8a33 100644 --- a/regression-tests.dnsdist/test_API.py +++ b/regression-tests.dnsdist/test_API.py @@ -878,6 +878,10 @@ class TestAPICustomStatistics(APITestsBase): declareMetric("my-custom-metric", "counter", "Number of statistics") declareMetric("my-other-metric", "counter", "Another number of statistics") declareMetric("my-gauge", "gauge", "Current memory usage") + declareMetric("my-labeled-gauge", "gauge", "Custom gauge with labels", { withLabels = true }) + setMetric("my-labeled-gauge", 123, { labels = { foo = "bar" } }) + declareMetric("my-labeled-counter", "counter", "Custom counter with labels", { withLabels = true }) + incMetric("my-labeled-counter", { labels = { foo = "bar" } }) setWebserverConfig({password="%s", apiKey="%s"}) """ @@ -899,3 +903,8 @@ def testCustomStats(self): for key in expected: self.assertIn(key, content) self.assertTrue(content[key] >= 0) + + unexpected = ['my-labeled-gauge', 'my-labeled-counter'] + + for key in unexpected: + self.assertNotIn(key, content) diff --git a/regression-tests.dnsdist/test_Caching.py b/regression-tests.dnsdist/test_Caching.py index f9421812b3632..0f0f1195b4778 100644 --- a/regression-tests.dnsdist/test_Caching.py +++ b/regression-tests.dnsdist/test_Caching.py @@ -2396,6 +2396,12 @@ def testCacheCollisionWithECSParsing(self): class TestCachingScopeZero(DNSDistTest): + _serverKey = 'server.key' + _serverCert = 'server.chain' + _serverName = 'tls.tests.dnsdist.org' + _caCert = 'ca.pem' + _dohServerPort = pickAvailablePort() + _dohBaseURL = ("https://%s:%d/" % (_serverName, _dohServerPort)) _config_template = """ -- Be careful to enable ECS parsing in the packet cache, otherwise scope zero is disabled pc = newPacketCache(100, {maxTTL=86400, minTTL=1, temporaryFailureTTL=60, staleTTL=60, dontAge=false, numberOfShards=1, deferrableInsertLock=true, maxNegativeTTL=3600, parseECS=true}) @@ -2406,7 +2412,11 @@ class TestCachingScopeZero(DNSDistTest): -- to unset it using rules before the first cache lookup) addAction(RDRule(), SetECSAction("192.0.2.1/32")) addAction(RDRule(), SetNoRecurseAction()) + + -- test the DoH special case (query received over TCP, forwarded over UDP) + addDOHLocal("127.0.0.1:%d", "%s", "%s", { "/" }, {library='nghttp2'}) """ + _config_params = ['_testServerPort', '_dohServerPort', '_serverCert', '_serverKey'] def testScopeZero(self): """ @@ -2582,6 +2592,38 @@ def testNoECS(self): self.checkMessageEDNSWithECS(expectedQuery2, receivedQuery) self.checkMessageNoEDNS(receivedResponse, response) + def testScopeZeroIncomingDoH(self): + """ + Cache: Test the scope-zero feature with a query received over DoH, backend returns a scope of zero + """ + ttl = 600 + name = 'scope-zero-incoming-doh.cache.tests.powerdns.com.' + query = dns.message.make_query(name, 'AAAA', 'IN') + query.flags &= ~dns.flags.RD + ecso = clientsubnetoption.ClientSubnetOption('127.0.0.0', 24) + expectedQuery = dns.message.make_query(name, 'AAAA', 'IN', use_edns=True, options=[ecso], payload=4096) + expectedQuery.flags &= ~dns.flags.RD + ecsoResponse = clientsubnetoption.ClientSubnetOption('127.0.0.1', 24, 0) + expectedResponse = dns.message.make_response(query) + scopedResponse = dns.message.make_response(query) + scopedResponse.use_edns(edns=True, payload=4096, options=[ecsoResponse]) + rrset = dns.rrset.from_text(name, + ttl, + dns.rdataclass.IN, + dns.rdatatype.AAAA, + '::1') + scopedResponse.answer.append(rrset) + expectedResponse.answer.append(rrset) + + (receivedQuery, receivedResponse) = self.sendDOHQueryWrapper(query, scopedResponse) + receivedQuery.id = expectedQuery.id + self.checkMessageEDNSWithECS(expectedQuery, receivedQuery) + self.checkMessageNoEDNS(receivedResponse, expectedResponse) + + # next query should hit the cache + (receivedQuery, receivedResponse) = self.sendDOHQueryWrapper(query, response=None, useQueue=False) + self.checkMessageNoEDNS(receivedResponse, expectedResponse) + class TestCachingScopeZeroButNoSubnetcheck(DNSDistTest): _config_template = """ diff --git a/regression-tests.dnsdist/test_Prometheus.py b/regression-tests.dnsdist/test_Prometheus.py index f6d334038b0fa..c4559f5d9b4eb 100644 --- a/regression-tests.dnsdist/test_Prometheus.py +++ b/regression-tests.dnsdist/test_Prometheus.py @@ -30,8 +30,9 @@ class TestPrometheus(DNSDistTest): declareMetric('custom-metric3', 'counter', 'Custom counter', 'custom_prometheus_name') -- test prometheus labels in custom metrics - declareMetric('custom-metric-foo-x-bar-y-xyz', 'counter', 'Custom counter with labels', 'custom_metric_foo{x="bar",y="xyz"}') - declareMetric('custom-metric-foo-x-baz-y-abc', 'counter', 'Custom counter with labels', 'custom_metric_foo{x="baz",y="abc"}') + declareMetric('custom-metric-foo', 'counter', 'Custom counter with labels', { withLabels = true }) + incMetric('custom-metric-foo', { labels = { x = 'bar', y = 'xyz' } }) + incMetric('custom-metric-foo', { labels = { x = 'baz', y = 'abc' } }) """ def checkPrometheusContentBasic(self, content): @@ -47,12 +48,16 @@ def checkPrometheusContentBasic(self, content): tokens = line.split(' ') self.assertEqual(len(tokens), 2) if not line.startswith('dnsdist_') and not line.startswith('custom_'): - raise AssertionError('Expecting prometheus metric to be prefixed by \'dnsdist_\', got: "%s"' % (line)) + raise AssertionError( + 'Expecting prometheus metric to be prefixed by \'dnsdist_\', got: "%s"' % (line)) - def checkMetric(self, content, name, expectedType, expectedValue): + def checkMetric(self, content, name, expectedType, expectedValue, expectedLabels=""): typeFound = False helpFound = False valueFound = False + labelsFound = False + if expectedLabels == "": + labelsFound = True for line in content.splitlines(): if name in str(line): tokens = line.split(' ') @@ -70,10 +75,15 @@ def checkMetric(self, content, name, expectedType, expectedValue): if tokens[0] == name: valueFound = True self.assertEqual(int(tokens[1]), expectedValue) + elif tokens[0] == name + expectedLabels: + valueFound = True + labelsFound = True + self.assertEqual(int(tokens[1]), expectedValue) self.assertTrue(typeFound) self.assertTrue(helpFound) self.assertTrue(valueFound) + self.assertTrue(labelsFound) def checkPrometheusContentPromtool(self, content): output = None @@ -106,3 +116,5 @@ def testMetrics(self): self.checkMetric(r.text, 'dnsdist_custom_metric1', 'counter', 1) self.checkMetric(r.text, 'dnsdist_custom_metric2', 'gauge', 0) self.checkMetric(r.text, 'custom_prometheus_name', 'counter', 0) + self.checkMetric(r.text, 'dnsdist_custom_metric_foo', 'counter', 1, '{x="bar",y="xyz"}') + self.checkMetric(r.text, 'dnsdist_custom_metric_foo', 'counter', 1, '{x="baz",y="abc"}') diff --git a/regression-tests.recursor-dnssec/pytest.ini b/regression-tests.recursor-dnssec/pytest.ini new file mode 100644 index 0000000000000..261099a05f0e3 --- /dev/null +++ b/regression-tests.recursor-dnssec/pytest.ini @@ -0,0 +1,4 @@ +[pytest] +markers = + external: uses external web servers + unreliable_on_gh: test is fine when run locally, but shows issues on GitHub diff --git a/regression-tests.recursor-dnssec/runtests b/regression-tests.recursor-dnssec/runtests index adee05c461732..3543dd2b79c52 100755 --- a/regression-tests.recursor-dnssec/runtests +++ b/regression-tests.recursor-dnssec/runtests @@ -61,9 +61,13 @@ if ! "$PDNSRECURSOR" --version 2>&1 | grep Features | grep -q dnstap-framestream export NODNSTAPTESTS=1 fi +# Run with -m 'not external' to skip test that require external connectivity +# Run with -m 'not unreliable_on_gh' to skip tests that are unreliable on GitHUb +# Run with -m 'not (external or unreliable_on_gh)' to skip both categories + # LIBFAKETIME is only added to LD_PRELOAD by the pyton code when needed if [ "${LIBASAN}" != "" -o "${LIBAUTHBIND}" != "" ]; then -LD_PRELOAD="${LIBASAN} ${LIBAUTHBIND}" pytest --ignore=test_WellKnown.py --junitxml=pytest.xml $@ +LD_PRELOAD="${LIBASAN} ${LIBAUTHBIND}" pytest --junitxml=pytest.xml "$@" else -pytest --ignore=test_WellKnown.py --junitxml=pytest.xml $@ +pytest --junitxml=pytest.xml "$@" fi diff --git a/regression-tests.recursor-dnssec/test_Chain.py b/regression-tests.recursor-dnssec/test_Chain.py index 6c20937b31e32..3c20da0fc7589 100644 --- a/regression-tests.recursor-dnssec/test_Chain.py +++ b/regression-tests.recursor-dnssec/test_Chain.py @@ -1,3 +1,4 @@ +import pytest import dns import os import time @@ -23,6 +24,7 @@ class ChainTest(RecursorTest): api-key=%s """ % (_wsPort, _wsPassword, _apiKey) + @pytest.mark.unreliable_on_gh def testBasic(self): """ Tests the case of #14624. Sending many equal requests could lead to ServFail because of clashing diff --git a/regression-tests.recursor-dnssec/test_ExtendedErrors.py b/regression-tests.recursor-dnssec/test_ExtendedErrors.py index 635c2c579c4c4..a684665a7935d 100644 --- a/regression-tests.recursor-dnssec/test_ExtendedErrors.py +++ b/regression-tests.recursor-dnssec/test_ExtendedErrors.py @@ -83,6 +83,7 @@ def generateRecursorConfig(cls, confdir): super(ExtendedErrorsTest, cls).generateRecursorConfig(confdir) + @pytest.mark.external def testNotIncepted(self): qname = 'signotincepted.bad-dnssec.wb.sidnlabs.nl.' query = dns.message.make_query(qname, 'A', want_dnssec=True) @@ -96,6 +97,7 @@ def testNotIncepted(self): self.assertEqual(res.options[0].otype, 15) self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(8, b'')) + @pytest.mark.external def testExpired(self): qname = 'sigexpired.bad-dnssec.wb.sidnlabs.nl.' query = dns.message.make_query(qname, 'A', want_dnssec=True) @@ -109,6 +111,7 @@ def testExpired(self): self.assertEqual(res.options[0].otype, 15) self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(7, b'')) + @pytest.mark.external def testAllExpired(self): qname = 'servfail.nl.' query = dns.message.make_query(qname, 'AAAA', want_dnssec=True) @@ -122,6 +125,7 @@ def testAllExpired(self): self.assertEqual(res.options[0].otype, 15) self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(6, b'')) + @pytest.mark.external def testBogus(self): qname = 'bogussig.ok.bad-dnssec.wb.sidnlabs.nl.' query = dns.message.make_query(qname, 'A', want_dnssec=True) @@ -135,6 +139,7 @@ def testBogus(self): self.assertEqual(res.options[0].otype, 15) self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(6, b'')) + @pytest.mark.external def testMissingRRSIG(self): qname = 'brokendnssec.net.' query = dns.message.make_query(qname, 'A', want_dnssec=True) @@ -236,6 +241,7 @@ def tearDownClass(cls): def generateRecursorConfig(cls, confdir): super(NoExtendedErrorsTest, cls).generateRecursorConfig(confdir) + @pytest.mark.external def testNotIncepted(self): qname = 'signotincepted.bad-dnssec.wb.sidnlabs.nl.' query = dns.message.make_query(qname, 'A', want_dnssec=True) diff --git a/regression-tests.recursor-dnssec/test_SimpleDoT.py b/regression-tests.recursor-dnssec/test_SimpleDoT.py index 4802e4ef4c5c3..20eaea132d295 100644 --- a/regression-tests.recursor-dnssec/test_SimpleDoT.py +++ b/regression-tests.recursor-dnssec/test_SimpleDoT.py @@ -1,3 +1,4 @@ +import pytest import dns import os import subprocess @@ -30,6 +31,7 @@ def setUpClass(cls): cls.generateRecursorConfig(confdir) cls.startRecursor(confdir, cls._recursorPort) + @pytest.mark.external def testTXT(self): expected = dns.rrset.from_text('dot-test-target.powerdns.org.', 0, dns.rdataclass.IN, 'TXT', 'https://github.com/PowerDNS/pdns/pull/12825') query = dns.message.make_query('dot-test-target.powerdns.org', 'TXT', want_dnssec=True) diff --git a/regression-tests.recursor-dnssec/test_SimpleForwardOverDoT.py b/regression-tests.recursor-dnssec/test_SimpleForwardOverDoT.py index d7b85e648c21e..8de66060b5d8a 100644 --- a/regression-tests.recursor-dnssec/test_SimpleForwardOverDoT.py +++ b/regression-tests.recursor-dnssec/test_SimpleForwardOverDoT.py @@ -1,3 +1,4 @@ +import pytest import dns import os import subprocess @@ -27,6 +28,7 @@ def setUpClass(cls): cls.generateRecursorConfig(confdir) cls.startRecursor(confdir, cls._recursorPort) + @pytest.mark.external def testA(self): expected = dns.rrset.from_text('dns.google.', 0, dns.rdataclass.IN, 'A', '8.8.8.8', '8.8.4.4') query = dns.message.make_query('dns.google', 'A', want_dnssec=True) diff --git a/regression-tests.recursor-dnssec/test_WellKnown.py b/regression-tests.recursor-dnssec/test_WellKnown.py index feaaa6f29d0e5..fa04a9692c924 100644 --- a/regression-tests.recursor-dnssec/test_WellKnown.py +++ b/regression-tests.recursor-dnssec/test_WellKnown.py @@ -1,7 +1,9 @@ +import pytest import dns from recursortests import RecursorTest -class TestWellKnown(RecursorTest): +@pytest.mark.external +class WellKnownTest(RecursorTest): _auths_zones = None _confdir = 'WellKnown' _roothints = None