From c229703f9f3a547ebbfd7a5c6ba0722cf3125c7f Mon Sep 17 00:00:00 2001 From: AJ Heller Date: Wed, 6 Jul 2022 16:33:05 -0700 Subject: [PATCH] Reland: Add SRV and TXT record lookup methods to the iomgr API (#30206) * Revert "Revert "Add SRV and TXT record lookup methods to the iomgr API (#30078)" (#30176)" This reverts commit 2c3acbb2b2bbaca2833a923a4003ab6fe52ec5f8. * one way to fix the ares handle race. Another option: work_serializer * replace mu with parent's work serializer * add lock annotations * Revert "replace mu with parent's work serializer" This reverts commit 0fce0ae150d0a2e891cfee26e50ac80ce4de1ca1. * statusor -> optional * Automated change: Fix sanity tests * add missing dep Co-authored-by: drfloob --- BUILD | 3 + .../resolver/dns/c_ares/dns_resolver_ares.cc | 501 ++++++++++++++---- .../resolver/dns/c_ares/grpc_ares_wrapper.cc | 255 +++++---- .../resolver/dns/c_ares/grpc_ares_wrapper.h | 27 +- .../resolver/dns/native/dns_resolver.cc | 7 +- .../transport/chttp2/server/chttp2_server.cc | 3 +- src/core/lib/http/httpcli.cc | 7 +- src/core/lib/iomgr/event_engine/resolver.h | 58 -- src/core/lib/iomgr/resolve_address.h | 35 +- src/core/lib/iomgr/resolve_address_posix.cc | 46 +- src/core/lib/iomgr/resolve_address_posix.h | 24 +- src/core/lib/iomgr/resolve_address_windows.cc | 48 +- src/core/lib/iomgr/resolve_address_windows.h | 24 +- .../resolvers/dns_resolver_cooldown_test.cc | 62 ++- test/core/end2end/dualstack_socket_test.cc | 4 +- .../end2end/fixtures/http_proxy_fixture.cc | 4 +- test/core/end2end/fuzzers/api_fuzzer.cc | 45 +- test/core/end2end/goaway_server_test.cc | 66 ++- test/core/iomgr/resolve_address_posix_test.cc | 8 +- test/core/iomgr/resolve_address_test.cc | 123 +++-- test/core/surface/server_test.cc | 2 +- .../transport/chttp2/settings_timeout_test.cc | 2 +- test/core/util/resolve_localhost_ip46.cc | 2 +- 23 files changed, 958 insertions(+), 398 deletions(-) delete mode 100644 src/core/lib/iomgr/event_engine/resolver.h diff --git a/BUILD b/BUILD index 14f478e6b00..a00bccdd4fd 100644 --- a/BUILD +++ b/BUILD @@ -2754,6 +2754,7 @@ grpc_cc_library( "debug_location", "dual_ref_counted", "error", + "event_engine_base", "exec_ctx", "gpr_base", "gpr_codegen", @@ -4714,6 +4715,8 @@ grpc_cc_library( "absl/status:statusor", "absl/strings", "absl/strings:str_format", + "absl/time", + "absl/types:optional", "address_sorting", "cares", ], diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 63cf9cec0f1..fac6f3f7a9f 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -34,6 +34,7 @@ #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/strings/strip.h" +#include "absl/types/optional.h" #include #include @@ -109,17 +110,43 @@ class AresClientChannelDNSResolver : public PollingResolver { explicit AresRequestWrapper( RefCountedPtr resolver) : resolver_(std::move(resolver)) { - Ref(DEBUG_LOCATION, "OnResolved").release(); - GRPC_CLOSURE_INIT(&on_resolved_, OnResolved, this, nullptr); - request_.reset(grpc_dns_lookup_ares( + // TODO(hork): replace this callback bookkeeping with promises. + // Locking to prevent completion before all records are queried + MutexLock lock(&on_resolved_mu_); + Ref(DEBUG_LOCATION, "OnHostnameResolved").release(); + GRPC_CLOSURE_INIT(&on_hostname_resolved_, OnHostnameResolved, this, + nullptr); + hostname_request_.reset(grpc_dns_lookup_hostname_ares( resolver_->authority().c_str(), resolver_->name_to_resolve().c_str(), - kDefaultSecurePort, resolver_->interested_parties(), &on_resolved_, - &addresses_, - resolver_->enable_srv_queries_ ? &balancer_addresses_ : nullptr, - resolver_->request_service_config_ ? &service_config_json_ : nullptr, - resolver_->query_timeout_ms_)); - GRPC_CARES_TRACE_LOG("resolver:%p Started resolving. request_:%p", - resolver_.get(), request_.get()); + kDefaultSecurePort, resolver_->interested_parties(), + &on_hostname_resolved_, &addresses_, resolver_->query_timeout_ms_)); + GRPC_CARES_TRACE_LOG( + "resolver:%p Started resolving hostnames. hostname_request_:%p", + resolver_.get(), hostname_request_.get()); + if (resolver_->enable_srv_queries_) { + Ref(DEBUG_LOCATION, "OnSRVResolved").release(); + GRPC_CLOSURE_INIT(&on_srv_resolved_, OnSRVResolved, this, nullptr); + srv_request_.reset(grpc_dns_lookup_srv_ares( + resolver_->authority().c_str(), + resolver_->name_to_resolve().c_str(), + resolver_->interested_parties(), &on_srv_resolved_, + &balancer_addresses_, resolver_->query_timeout_ms_)); + GRPC_CARES_TRACE_LOG( + "resolver:%p Started resolving SRV records. srv_request_:%p", + resolver_.get(), srv_request_.get()); + } + if (resolver_->request_service_config_) { + Ref(DEBUG_LOCATION, "OnTXTResolved").release(); + GRPC_CLOSURE_INIT(&on_txt_resolved_, OnTXTResolved, this, nullptr); + txt_request_.reset(grpc_dns_lookup_txt_ares( + resolver_->authority().c_str(), + resolver_->name_to_resolve().c_str(), + resolver_->interested_parties(), &on_txt_resolved_, + &service_config_json_, resolver_->query_timeout_ms_)); + GRPC_CARES_TRACE_LOG( + "resolver:%p Started resolving TXT records. txt_request_:%p", + resolver_.get(), srv_request_.get()); + } } ~AresRequestWrapper() override { @@ -127,22 +154,47 @@ class AresClientChannelDNSResolver : public PollingResolver { resolver_.reset(DEBUG_LOCATION, "dns-resolving"); } - void Orphan() override { - grpc_cancel_ares_request(request_.get()); + // Note that thread safety cannot be analyzed due to this being invoked from + // OrphanablePtr<>, and there's no way to pass the lock annotation through + // there. + void Orphan() override ABSL_NO_THREAD_SAFETY_ANALYSIS { + MutexLock lock(&on_resolved_mu_); + if (hostname_request_ != nullptr) { + grpc_cancel_ares_request(hostname_request_.get()); + } + if (srv_request_ != nullptr) { + grpc_cancel_ares_request(srv_request_.get()); + } + if (txt_request_ != nullptr) { + grpc_cancel_ares_request(txt_request_.get()); + } Unref(DEBUG_LOCATION, "Orphan"); } private: - static void OnResolved(void* arg, grpc_error_handle error); - void OnResolved(grpc_error_handle error); + static void OnHostnameResolved(void* arg, grpc_error_handle error); + static void OnSRVResolved(void* arg, grpc_error_handle error); + static void OnTXTResolved(void* arg, grpc_error_handle error); + absl::optional OnResolvedLocked(grpc_error_handle error) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(on_resolved_mu_); + Mutex on_resolved_mu_; RefCountedPtr resolver_; - std::unique_ptr request_; - grpc_closure on_resolved_; + grpc_closure on_hostname_resolved_; + std::unique_ptr hostname_request_ + ABSL_GUARDED_BY(on_resolved_mu_); + grpc_closure on_srv_resolved_; + std::unique_ptr srv_request_ + ABSL_GUARDED_BY(on_resolved_mu_); + grpc_closure on_txt_resolved_; + std::unique_ptr txt_request_ + ABSL_GUARDED_BY(on_resolved_mu_); // Output fields from ares request. - std::unique_ptr addresses_; - std::unique_ptr balancer_addresses_; - char* service_config_json_ = nullptr; + std::unique_ptr addresses_ + ABSL_GUARDED_BY(on_resolved_mu_); + std::unique_ptr balancer_addresses_ + ABSL_GUARDED_BY(on_resolved_mu_); + char* service_config_json_ ABSL_GUARDED_BY(on_resolved_mu_) = nullptr; }; ~AresClientChannelDNSResolver() override; @@ -276,15 +328,69 @@ std::string ChooseServiceConfig(char* service_config_choice_json, return service_config->Dump(); } -void AresClientChannelDNSResolver::AresRequestWrapper::OnResolved( +void AresClientChannelDNSResolver::AresRequestWrapper::OnHostnameResolved( + void* arg, grpc_error_handle error) { + auto* self = static_cast(arg); + absl::optional result; + { + MutexLock lock(&self->on_resolved_mu_); + self->hostname_request_.reset(); + result = self->OnResolvedLocked(error); + } + if (result.has_value()) { + self->resolver_->OnRequestComplete(std::move(*result)); + } + self->Unref(DEBUG_LOCATION, "OnHostnameResolved"); +} + +void AresClientChannelDNSResolver::AresRequestWrapper::OnSRVResolved( void* arg, grpc_error_handle error) { auto* self = static_cast(arg); - self->OnResolved(error); + absl::optional result; + { + MutexLock lock(&self->on_resolved_mu_); + self->srv_request_.reset(); + result = self->OnResolvedLocked(error); + } + if (result.has_value()) { + self->resolver_->OnRequestComplete(std::move(*result)); + } + self->Unref(DEBUG_LOCATION, "OnSRVResolved"); } -void AresClientChannelDNSResolver::AresRequestWrapper::OnResolved( - grpc_error_handle error) { - GRPC_CARES_TRACE_LOG("resolver:%p OnResolved()", this); +void AresClientChannelDNSResolver::AresRequestWrapper::OnTXTResolved( + void* arg, grpc_error_handle error) { + auto* self = static_cast(arg); + absl::optional result; + { + MutexLock lock(&self->on_resolved_mu_); + self->txt_request_.reset(); + result = self->OnResolvedLocked(error); + } + if (result.has_value()) { + self->resolver_->OnRequestComplete(std::move(*result)); + } + self->Unref(DEBUG_LOCATION, "OnTXTResolved"); +} + +// Returns a Result if resolution is complete. +// callers must release the lock and call OnRequestComplete if a Result is +// returned. This is because OnRequestComplete may Orphan the resolver, which +// requires taking the lock. +absl::optional +AresClientChannelDNSResolver::AresRequestWrapper::OnResolvedLocked( + grpc_error_handle error) ABSL_EXCLUSIVE_LOCKS_REQUIRED(on_resolved_mu_) { + if (hostname_request_ != nullptr || srv_request_ != nullptr || + txt_request_ != nullptr) { + GRPC_CARES_TRACE_LOG( + "resolver:%p OnResolved() waiting for results (hostname: %s, srv: %s, " + "txt: %s)", + this, hostname_request_ != nullptr ? "waiting" : "done", + srv_request_ != nullptr ? "waiting" : "done", + txt_request_ != nullptr ? "waiting" : "done"); + return absl::nullopt; + } + GRPC_CARES_TRACE_LOG("resolver:%p OnResolved() proceeding", this); Result result; absl::InlinedVector new_args; // TODO(roth): Change logic to be able to report failures for addresses @@ -334,8 +440,7 @@ void AresClientChannelDNSResolver::AresRequestWrapper::OnResolved( } result.args = grpc_channel_args_copy_and_add( resolver_->channel_args(), new_args.data(), new_args.size()); - resolver_->OnRequestComplete(std::move(result)); - Unref(DEBUG_LOCATION, "OnResolved"); + return std::move(result); } // @@ -363,51 +468,45 @@ class AresClientChannelDNSResolverFactory : public ResolverFactory { class AresDNSResolver : public DNSResolver { public: + // Abstract class that centralizes common request handling logic via the + // template method pattern. + // This requires a two-phase initialization, where 1) a request is created via + // a subclass constructor, and 2) the request is initiated via Run() class AresRequest { public: - AresRequest( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* interested_parties, - std::function>)> - on_resolve_address_done, - AresDNSResolver* resolver, intptr_t aba_token) - : name_(std::string(name)), - default_port_(std::string(default_port)), - interested_parties_(interested_parties), - pollset_set_(grpc_pollset_set_create()), - on_resolve_address_done_(std::move(on_resolve_address_done)), - completed_(false), - resolver_(resolver), - aba_token_(aba_token) { - GRPC_CARES_TRACE_LOG("AresRequest:%p ctor", this); - GRPC_CLOSURE_INIT(&on_dns_lookup_done_, OnDnsLookupDone, this, - grpc_schedule_on_exec_ctx); - MutexLock lock(&mu_); - grpc_pollset_set_add_pollset_set(pollset_set_, interested_parties); - ares_request_ = std::unique_ptr(grpc_dns_lookup_ares( - /*dns_server=*/"", name_.c_str(), default_port_.c_str(), pollset_set_, - &on_dns_lookup_done_, &addresses_, - /*balancer_addresses=*/nullptr, /*service_config_json=*/nullptr, - GRPC_DNS_ARES_DEFAULT_QUERY_TIMEOUT_MS)); - GRPC_CARES_TRACE_LOG("AresRequest:%p Start ares_request_:%p", this, - ares_request_.get()); - } - - ~AresRequest() { + virtual ~AresRequest() { GRPC_CARES_TRACE_LOG("AresRequest:%p dtor ares_request_:%p", this, - ares_request_.get()); + grpc_ares_request_.get()); resolver_->UnregisterRequest(task_handle()); grpc_pollset_set_destroy(pollset_set_); } + // Initiates the low-level c-ares request and returns its handle. + virtual std::unique_ptr MakeRequestLocked() = 0; + // Called on ares resolution, but not upon cancellation. + // After execution, the AresRequest will perform any final cleanup and + // delete itself. + virtual void OnComplete(grpc_error_handle error) = 0; + + // Called to initiate the request. + void Run() { + MutexLock lock(&mu_); + grpc_ares_request_ = MakeRequestLocked(); + } + bool Cancel() { MutexLock lock(&mu_); - GRPC_CARES_TRACE_LOG("AresRequest:%p Cancel ares_request_:%p", this, - ares_request_.get()); - if (completed_) return false; - // OnDnsLookupDone will still be run - grpc_cancel_ares_request(ares_request_.get()); - completed_ = true; + if (grpc_ares_request_ != nullptr) { + GRPC_CARES_TRACE_LOG("AresRequest:%p Cancel ares_request_:%p", this, + grpc_ares_request_.get()); + if (completed_) return false; + // OnDnsLookupDone will still be run + completed_ = true; + grpc_cancel_ares_request(grpc_ares_request_.get()); + } else { + completed_ = true; + OnDnsLookupDone(this, GRPC_ERROR_CANCELLED); + } grpc_pollset_set_del_pollset_set(pollset_set_, interested_parties_); return true; } @@ -416,65 +515,215 @@ class AresDNSResolver : public DNSResolver { return {reinterpret_cast(this), aba_token_}; } + protected: + AresRequest(absl::string_view name, absl::string_view name_server, + Duration timeout, grpc_pollset_set* interested_parties, + AresDNSResolver* resolver, intptr_t aba_token) + : name_(name), + name_server_(name_server), + timeout_(timeout), + interested_parties_(interested_parties), + completed_(false), + resolver_(resolver), + aba_token_(aba_token), + pollset_set_(grpc_pollset_set_create()) { + GRPC_CLOSURE_INIT(&on_dns_lookup_done_, OnDnsLookupDone, this, + grpc_schedule_on_exec_ctx); + grpc_pollset_set_add_pollset_set(pollset_set_, interested_parties_); + } + + grpc_pollset_set* pollset_set() { return pollset_set_; }; + grpc_closure* on_dns_lookup_done() { return &on_dns_lookup_done_; }; + const std::string& name() { return name_; } + const std::string& name_server() { return name_server_; } + const Duration& timeout() { return timeout_; } + private: // Called by ares when lookup has completed or when cancelled. It is always - // called exactly once. + // called exactly once, and it triggers self-deletion. static void OnDnsLookupDone(void* arg, grpc_error_handle error) { - AresRequest* request = static_cast(arg); - GRPC_CARES_TRACE_LOG("AresRequest:%p OnDnsLookupDone", request); - // This request is deleted and unregistered upon any exit. - std::unique_ptr deleter(request); - std::vector resolved_addresses; + AresRequest* r = static_cast(arg); + auto deleter = std::unique_ptr(r); { - MutexLock lock(&request->mu_); - if (request->completed_) return; - request->completed_ = true; - if (request->addresses_ != nullptr) { - resolved_addresses.reserve(request->addresses_->size()); - for (const auto& server_address : *request->addresses_) { - resolved_addresses.push_back(server_address.address()); - } + MutexLock lock(&r->mu_); + grpc_pollset_set_del_pollset_set(r->pollset_set_, + r->interested_parties_); + if (r->completed_) { + return; } + r->completed_ = true; } - grpc_pollset_set_del_pollset_set(request->pollset_set_, - request->interested_parties_); - if (!GRPC_ERROR_IS_NONE(error)) { - request->on_resolve_address_done_(grpc_error_to_absl_status(error)); - return; - } - request->on_resolve_address_done_(std::move(resolved_addresses)); + r->OnComplete(error); } + // the name to resolve + const std::string name_; + // the name server to query + const std::string name_server_; + // request-specific timeout + Duration timeout_; // mutex to synchronize access to this object (but not to the ares_request // object itself). Mutex mu_; - // the name to resolve - const std::string name_; - // the default port to use if name doesn't have one - const std::string default_port_; // parties interested in our I/O grpc_pollset_set* const interested_parties_; + // underlying cares_request that the query is performed on + std::unique_ptr grpc_ares_request_ ABSL_GUARDED_BY(mu_); + // Set when the callback is either cancelled or executed. + // It is not the subclasses' responsibility to set this flag. + bool completed_ ABSL_GUARDED_BY(mu_); + // Parent resolver that created this request + AresDNSResolver* resolver_; + // Unique token to help distinguish this request from others that may later + // be created in the same memory location. + intptr_t aba_token_; + // closure to call when the ares resolution request completes. Subclasses + // should use this as the ares callback in MakeRequestLocked() + grpc_closure on_dns_lookup_done_ ABSL_GUARDED_BY(mu_); // locally owned pollset_set, required to support cancellation of requests - // while ares still needs a valid pollset_set. + // while ares still needs a valid pollset_set. Subclasses should give this + // pollset to ares in MakeRequestLocked(); grpc_pollset_set* pollset_set_; + }; + + class AresHostnameRequest : public AresRequest { + public: + AresHostnameRequest( + absl::string_view name, absl::string_view default_port, + absl::string_view name_server, Duration timeout, + grpc_pollset_set* interested_parties, + std::function>)> + on_resolve_address_done, + AresDNSResolver* resolver, intptr_t aba_token) + : AresRequest(name, name_server, timeout, interested_parties, resolver, + aba_token), + default_port_(default_port), + on_resolve_address_done_(std::move(on_resolve_address_done)) { + GRPC_CARES_TRACE_LOG("AresHostnameRequest:%p ctor", this); + } + + std::unique_ptr MakeRequestLocked() override { + auto ares_request = + std::unique_ptr(grpc_dns_lookup_hostname_ares( + name_server().c_str(), name().c_str(), default_port_.c_str(), + pollset_set(), on_dns_lookup_done(), &addresses_, + timeout().millis())); + GRPC_CARES_TRACE_LOG("AresHostnameRequest:%p Start ares_request_:%p", + this, ares_request.get()); + return ares_request; + } + + void OnComplete(grpc_error_handle error) override { + GRPC_CARES_TRACE_LOG("AresHostnameRequest:%p OnComplete", this); + if (!GRPC_ERROR_IS_NONE(error)) { + on_resolve_address_done_(grpc_error_to_absl_status(error)); + return; + } + std::vector resolved_addresses; + if (addresses_ != nullptr) { + resolved_addresses.reserve(addresses_->size()); + for (const auto& server_address : *addresses_) { + resolved_addresses.push_back(server_address.address()); + } + } + on_resolve_address_done_(std::move(resolved_addresses)); + } + + // the default port to use if name doesn't have one + const std::string default_port_; // user-provided completion callback const std::function>)> on_resolve_address_done_; // currently resolving addresses - std::unique_ptr addresses_ ABSL_GUARDED_BY(mu_); - // closure to call when the resolve_address_ares request completes - // a closure wrapping on_resolve_address_done, which should be invoked - // when the grpc_dns_lookup_ares operation is done. - grpc_closure on_dns_lookup_done_ ABSL_GUARDED_BY(mu_); - // underlying ares_request that the query is performed on - std::unique_ptr ares_request_ ABSL_GUARDED_BY(mu_); - bool completed_ ABSL_GUARDED_BY(mu_); - // Parent resolver that created this request - AresDNSResolver* resolver_; - // Unique token to help distinguish this request from others that may later - // be created in the same memory location. - intptr_t aba_token_; + std::unique_ptr addresses_; + }; + + class AresSRVRequest : public AresRequest { + public: + AresSRVRequest( + absl::string_view name, absl::string_view name_server, Duration timeout, + grpc_pollset_set* interested_parties, + std::function>)> + on_resolve_address_done, + AresDNSResolver* resolver, intptr_t aba_token) + : AresRequest(name, name_server, timeout, interested_parties, resolver, + aba_token), + on_resolve_address_done_(std::move(on_resolve_address_done)) { + GRPC_CARES_TRACE_LOG("AresSRVRequest:%p ctor", this); + } + + std::unique_ptr MakeRequestLocked() override { + auto ares_request = + std::unique_ptr(grpc_dns_lookup_srv_ares( + name_server().c_str(), name().c_str(), pollset_set(), + on_dns_lookup_done(), &balancer_addresses_, timeout().millis())); + GRPC_CARES_TRACE_LOG("AresSRVRequest:%p Start ares_request_:%p", this, + ares_request.get()); + return ares_request; + } + + void OnComplete(grpc_error_handle error) override { + GRPC_CARES_TRACE_LOG("AresSRVRequest:%p OnComplete", this); + if (!GRPC_ERROR_IS_NONE(error)) { + on_resolve_address_done_(grpc_error_to_absl_status(error)); + return; + } + std::vector resolved_addresses; + if (balancer_addresses_ != nullptr) { + resolved_addresses.reserve(balancer_addresses_->size()); + for (const auto& server_address : *balancer_addresses_) { + resolved_addresses.push_back(server_address.address()); + } + } + on_resolve_address_done_(std::move(resolved_addresses)); + } + + // user-provided completion callback + const std::function>)> + on_resolve_address_done_; + // currently resolving addresses + std::unique_ptr balancer_addresses_; + }; + + class AresTXTRequest : public AresRequest { + public: + AresTXTRequest(absl::string_view name, absl::string_view name_server, + Duration timeout, grpc_pollset_set* interested_parties, + std::function)> on_resolved, + AresDNSResolver* resolver, intptr_t aba_token) + : AresRequest(name, name_server, timeout, interested_parties, resolver, + aba_token), + on_resolved_(std::move(on_resolved)) { + GRPC_CARES_TRACE_LOG("AresTXTRequest:%p ctor", this); + } + + ~AresTXTRequest() override { gpr_free(service_config_json_); } + + std::unique_ptr MakeRequestLocked() override { + auto ares_request = + std::unique_ptr(grpc_dns_lookup_txt_ares( + name_server().c_str(), name().c_str(), pollset_set(), + on_dns_lookup_done(), &service_config_json_, timeout().millis())); + GRPC_CARES_TRACE_LOG("AresSRVRequest:%p Start ares_request_:%p", this, + ares_request.get()); + return ares_request; + } + + void OnComplete(grpc_error_handle error) override { + GRPC_CARES_TRACE_LOG("AresSRVRequest:%p OnComplete", this); + if (!GRPC_ERROR_IS_NONE(error)) { + on_resolved_(grpc_error_to_absl_status(error)); + return; + } + on_resolved_(service_config_json_); + } + + // service config from the TXT record + char* service_config_json_ = nullptr; + // user-provided completion callback + const std::function)> on_resolved_; }; // gets the singleton instance, possibly creating it first @@ -483,26 +732,60 @@ class AresDNSResolver : public DNSResolver { return instance; } - TaskHandle ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* interested_parties, + TaskHandle LookupHostname( std::function>)> - on_done) override { + on_resolved, + absl::string_view name, absl::string_view default_port, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override { MutexLock lock(&mu_); - auto* request = new AresRequest(name, default_port, interested_parties, - std::move(on_done), this, aba_token_++); + auto* request = new AresHostnameRequest( + name, default_port, name_server, timeout, interested_parties, + std::move(on_resolved), this, aba_token_++); + request->Run(); auto handle = request->task_handle(); open_requests_.insert(handle); return handle; } - absl::StatusOr> ResolveNameBlocking( + absl::StatusOr> LookupHostnameBlocking( absl::string_view name, absl::string_view default_port) override { // TODO(apolcyn): change this to wrap the async version of the c-ares // API with a promise, and remove the reference to the previous resolver. - return default_resolver_->ResolveNameBlocking(name, default_port); + return default_resolver_->LookupHostnameBlocking(name, default_port); } + TaskHandle LookupSRV( + std::function>)> + on_resolved, + absl::string_view name, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override { + MutexLock lock(&mu_); + auto* request = + new AresSRVRequest(name, name_server, timeout, interested_parties, + std::move(on_resolved), this, aba_token_++); + request->Run(); + auto handle = request->task_handle(); + open_requests_.insert(handle); + return handle; + }; + + TaskHandle LookupTXT( + std::function)> on_resolved, + absl::string_view name, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override { + MutexLock lock(&mu_); + auto* request = + new AresTXTRequest(name, name_server, timeout, interested_parties, + std::move(on_resolved), this, aba_token_++); + request->Run(); + auto handle = request->task_handle(); + open_requests_.insert(handle); + return handle; + }; + bool Cancel(TaskHandle handle) override { MutexLock lock(&mu_); if (!open_requests_.contains(handle)) { diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc index 1c813152362..a85f7203272 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc @@ -21,6 +21,8 @@ #include #include +#include "absl/strings/string_view.h" + #include "src/core/lib/iomgr/sockaddr.h" // IWYU pragma: no_include @@ -598,12 +600,10 @@ static void grpc_ares_request_unref_locked(grpc_ares_request* r) void grpc_ares_complete_request_locked(grpc_ares_request* r) ABSL_EXCLUSIVE_LOCKS_REQUIRED(r->mu) { - /* Invoke on_done callback and destroy the - request */ + // Invoke on_done callback and destroy the request r->ev_driver = nullptr; - ServerAddressList* addresses = r->addresses_out->get(); - if (addresses != nullptr) { - grpc_cares_wrapper_address_sorting_sort(r, addresses); + if (r->addresses_out != nullptr && *r->addresses_out != nullptr) { + grpc_cares_wrapper_address_sorting_sort(r, r->addresses_out->get()); GRPC_ERROR_UNREF(r->error); r->error = GRPC_ERROR_NONE; // TODO(apolcyn): allow c-ares to return a service config @@ -816,6 +816,7 @@ static void on_txt_done_locked(void* arg, int status, int /*timeouts*/, } // Clean up. ares_free_data(reply); + grpc_ares_request_unref_locked(r); return; fail: std::string error_msg = @@ -827,38 +828,14 @@ fail: r->error = grpc_error_add_child(error, r->error); } -void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( - grpc_ares_request* r, const char* dns_server, const char* name, - const char* default_port, grpc_pollset_set* interested_parties, - int query_timeout_ms) ABSL_EXCLUSIVE_LOCKS_REQUIRED(r->mu) { - grpc_error_handle error = GRPC_ERROR_NONE; - grpc_ares_hostbyname_request* hr = nullptr; - /* parse name, splitting it into host and port parts */ - std::string host; - std::string port; - grpc_core::SplitHostPort(name, &host, &port); - if (host.empty()) { - error = grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("unparseable host:port"), - GRPC_ERROR_STR_TARGET_ADDRESS, name); - goto error_cleanup; - } else if (port.empty()) { - if (default_port == nullptr || strlen(default_port) == 0) { - error = grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("no port in name"), - GRPC_ERROR_STR_TARGET_ADDRESS, name); - goto error_cleanup; - } - port = default_port; - } - error = grpc_ares_ev_driver_create_locked(&r->ev_driver, interested_parties, - query_timeout_ms, r); - if (!GRPC_ERROR_IS_NONE(error)) goto error_cleanup; - // If dns_server is specified, use it. - if (dns_server != nullptr && dns_server[0] != '\0') { - GRPC_CARES_TRACE_LOG("request:%p Using DNS server %s", r, dns_server); +grpc_error_handle set_request_dns_server(grpc_ares_request* r, + absl::string_view dns_server) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(r->mu) { + if (!dns_server.empty()) { + GRPC_CARES_TRACE_LOG("request:%p Using DNS server %s", r, + dns_server.data()); grpc_resolved_address addr; - if (grpc_parse_ipv4_hostport(dns_server, &addr, false /* log_errors */)) { + if (grpc_parse_ipv4_hostport(dns_server, &addr, /*log_errors=*/false)) { r->dns_server_addr.family = AF_INET; struct sockaddr_in* in = reinterpret_cast(addr.addr); memcpy(&r->dns_server_addr.addr.addr4, &in->sin_addr, @@ -866,7 +843,7 @@ void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( r->dns_server_addr.tcp_port = grpc_sockaddr_get_port(&addr); r->dns_server_addr.udp_port = grpc_sockaddr_get_port(&addr); } else if (grpc_parse_ipv6_hostport(dns_server, &addr, - false /* log_errors */)) { + /*log_errors=*/false)) { r->dns_server_addr.family = AF_INET6; struct sockaddr_in6* in6 = reinterpret_cast(addr.addr); @@ -875,51 +852,49 @@ void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( r->dns_server_addr.tcp_port = grpc_sockaddr_get_port(&addr); r->dns_server_addr.udp_port = grpc_sockaddr_get_port(&addr); } else { - error = grpc_error_set_str( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("cannot parse authority"), - GRPC_ERROR_STR_TARGET_ADDRESS, name); - goto error_cleanup; + return GRPC_ERROR_CREATE_FROM_CPP_STRING( + absl::StrCat("cannot parse authority ", dns_server)); } int status = ares_set_servers_ports(r->ev_driver->channel, &r->dns_server_addr); if (status != ARES_SUCCESS) { - error = GRPC_ERROR_CREATE_FROM_CPP_STRING(absl::StrCat( + return GRPC_ERROR_CREATE_FROM_CPP_STRING(absl::StrCat( "C-ares status is not ARES_SUCCESS: ", ares_strerror(status))); - goto error_cleanup; } } - r->pending_queries = 1; - if (grpc_ares_query_ipv6()) { - hr = create_hostbyname_request_locked(r, host.c_str(), - grpc_strhtons(port.c_str()), - /*is_balancer=*/false, "AAAA"); - ares_gethostbyname(r->ev_driver->channel, hr->host, AF_INET6, - on_hostbyname_done_locked, hr); - } - hr = create_hostbyname_request_locked(r, host.c_str(), - grpc_strhtons(port.c_str()), - /*is_balancer=*/false, "A"); - ares_gethostbyname(r->ev_driver->channel, hr->host, AF_INET, - on_hostbyname_done_locked, hr); - if (r->balancer_addresses_out != nullptr) { - /* Query the SRV record */ - std::string service_name = absl::StrCat("_grpclb._tcp.", host); - GrpcAresQuery* srv_query = new GrpcAresQuery(r, service_name); - ares_query(r->ev_driver->channel, service_name.c_str(), ns_c_in, ns_t_srv, - on_srv_query_done_locked, srv_query); - } - if (r->service_config_json_out != nullptr) { - std::string config_name = absl::StrCat("_grpc_config.", host); - GrpcAresQuery* txt_query = new GrpcAresQuery(r, config_name); - ares_search(r->ev_driver->channel, config_name.c_str(), ns_c_in, ns_t_txt, - on_txt_done_locked, txt_query); - } - grpc_ares_ev_driver_start_locked(r->ev_driver); - grpc_ares_request_unref_locked(r); - return; + return GRPC_ERROR_NONE; +} -error_cleanup: - grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, error); +// Common logic for all lookup methods. +// If an error occurs, callers must run the client callback. +grpc_error_handle grpc_dns_lookup_ares_continued( + grpc_ares_request* r, const char* dns_server, const char* name, + const char* default_port, grpc_pollset_set* interested_parties, + int query_timeout_ms, std::string* host, std::string* port, bool check_port) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(r->mu) { + grpc_error_handle error = GRPC_ERROR_NONE; + /* parse name, splitting it into host and port parts */ + grpc_core::SplitHostPort(name, host, port); + if (host->empty()) { + error = grpc_error_set_str( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("unparseable host:port"), + GRPC_ERROR_STR_TARGET_ADDRESS, name); + return error; + } else if (check_port && port->empty()) { + if (default_port == nullptr || strlen(default_port) == 0) { + error = grpc_error_set_str( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("no port in name"), + GRPC_ERROR_STR_TARGET_ADDRESS, name); + return error; + } + *port = default_port; + } + error = grpc_ares_ev_driver_create_locked(&r->ev_driver, interested_parties, + query_timeout_ms, r); + if (!GRPC_ERROR_IS_NONE(error)) return error; + // If dns_server is specified, use it. + error = set_request_dns_server(r, dns_server); + return error; } static bool inner_resolve_as_ip_literal_locked( @@ -1051,21 +1026,18 @@ static bool grpc_ares_maybe_resolve_localhost_manually_locked( } #endif /* GRPC_ARES_RESOLVE_LOCALHOST_MANUALLY */ -static grpc_ares_request* grpc_dns_lookup_ares_impl( +static grpc_ares_request* grpc_dns_lookup_hostname_ares_impl( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, std::unique_ptr* addrs, - std::unique_ptr* balancer_addrs, - char** service_config_json, int query_timeout_ms) { + int query_timeout_ms) { grpc_ares_request* r = new grpc_ares_request(); grpc_core::MutexLock lock(&r->mu); r->ev_driver = nullptr; r->on_done = on_done; r->addresses_out = addrs; - r->balancer_addresses_out = balancer_addrs; - r->service_config_json_out = service_config_json; GRPC_CARES_TRACE_LOG( - "request:%p c-ares grpc_dns_lookup_ares_impl name=%s, " + "request:%p c-ares grpc_dns_lookup_hostname_ares_impl name=%s, " "default_port=%s", r, name, default_port); // Early out if the target is an ipv4 or ipv6 literal. @@ -1079,26 +1051,129 @@ static grpc_ares_request* grpc_dns_lookup_ares_impl( grpc_ares_complete_request_locked(r); return r; } - // Don't query for SRV and TXT records if the target is "localhost", so - // as to cut down on lookups over the network, especially in tests: - // https://github.com/grpc/proposal/pull/79 + // Look up name using c-ares lib. + std::string host; + std::string port; + grpc_error_handle error = grpc_dns_lookup_ares_continued( + r, dns_server, name, default_port, interested_parties, query_timeout_ms, + &host, &port, true); + if (!GRPC_ERROR_IS_NONE(error)) { + grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, error); + return r; + } + r->pending_queries = 1; + grpc_ares_hostbyname_request* hr = nullptr; + if (grpc_ares_query_ipv6()) { + hr = create_hostbyname_request_locked(r, host.c_str(), + grpc_strhtons(port.c_str()), + /*is_balancer=*/false, "AAAA"); + ares_gethostbyname(r->ev_driver->channel, hr->host, AF_INET6, + on_hostbyname_done_locked, hr); + } + hr = create_hostbyname_request_locked(r, host.c_str(), + grpc_strhtons(port.c_str()), + /*is_balancer=*/false, "A"); + ares_gethostbyname(r->ev_driver->channel, hr->host, AF_INET, + on_hostbyname_done_locked, hr); + grpc_ares_ev_driver_start_locked(r->ev_driver); + grpc_ares_request_unref_locked(r); + return r; +} + +grpc_ares_request* grpc_dns_lookup_srv_ares_impl( + const char* dns_server, const char* name, + grpc_pollset_set* interested_parties, grpc_closure* on_done, + std::unique_ptr* balancer_addresses, + int query_timeout_ms) { + grpc_ares_request* r = new grpc_ares_request(); + grpc_core::MutexLock lock(&r->mu); + r->ev_driver = nullptr; + r->on_done = on_done; + r->balancer_addresses_out = balancer_addresses; + GRPC_CARES_TRACE_LOG( + "request:%p c-ares grpc_dns_lookup_srv_ares_impl name=%s", r, name); + grpc_error_handle error = GRPC_ERROR_NONE; + // Don't query for SRV records if the target is "localhost" + if (target_matches_localhost(name)) { + grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, error); + return r; + } + // Look up name using c-ares lib. + std::string host; + std::string port; + error = grpc_dns_lookup_ares_continued(r, dns_server, name, nullptr, + interested_parties, query_timeout_ms, + &host, &port, false); + if (!GRPC_ERROR_IS_NONE(error)) { + grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, error); + return r; + } + r->pending_queries = 1; + /* Query the SRV record */ + std::string service_name = absl::StrCat("_grpclb._tcp.", host); + GrpcAresQuery* srv_query = new GrpcAresQuery(r, service_name); + ares_query(r->ev_driver->channel, service_name.c_str(), ns_c_in, ns_t_srv, + on_srv_query_done_locked, srv_query); + grpc_ares_ev_driver_start_locked(r->ev_driver); + grpc_ares_request_unref_locked(r); + return r; +} + +grpc_ares_request* grpc_dns_lookup_txt_ares_impl( + const char* dns_server, const char* name, + grpc_pollset_set* interested_parties, grpc_closure* on_done, + char** service_config_json, int query_timeout_ms) { + grpc_ares_request* r = new grpc_ares_request(); + grpc_core::MutexLock lock(&r->mu); + r->ev_driver = nullptr; + r->on_done = on_done; + r->service_config_json_out = service_config_json; + GRPC_CARES_TRACE_LOG( + "request:%p c-ares grpc_dns_lookup_txt_ares_impl name=%s", r, name); + grpc_error_handle error = GRPC_ERROR_NONE; + // Don't query for TXT records if the target is "localhost" if (target_matches_localhost(name)) { - r->balancer_addresses_out = nullptr; - r->service_config_json_out = nullptr; + grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, error); + return r; } // Look up name using c-ares lib. - grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( - r, dns_server, name, default_port, interested_parties, query_timeout_ms); + std::string host; + std::string port; + error = grpc_dns_lookup_ares_continued(r, dns_server, name, nullptr, + interested_parties, query_timeout_ms, + &host, &port, false); + if (!GRPC_ERROR_IS_NONE(error)) { + grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, error); + return r; + } + r->pending_queries = 1; + /* Query the TXT record */ + std::string config_name = absl::StrCat("_grpc_config.", host); + GrpcAresQuery* txt_query = new GrpcAresQuery(r, config_name); + ares_search(r->ev_driver->channel, config_name.c_str(), ns_c_in, ns_t_txt, + on_txt_done_locked, txt_query); + grpc_ares_ev_driver_start_locked(r->ev_driver); + grpc_ares_request_unref_locked(r); return r; } -grpc_ares_request* (*grpc_dns_lookup_ares)( +grpc_ares_request* (*grpc_dns_lookup_hostname_ares)( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, std::unique_ptr* addrs, - std::unique_ptr* balancer_addrs, + int query_timeout_ms) = grpc_dns_lookup_hostname_ares_impl; + +grpc_ares_request* (*grpc_dns_lookup_srv_ares)( + const char* dns_server, const char* name, + grpc_pollset_set* interested_parties, grpc_closure* on_done, + std::unique_ptr* balancer_addresses, + int query_timeout_ms) = grpc_dns_lookup_srv_ares_impl; + +grpc_ares_request* (*grpc_dns_lookup_txt_ares)( + const char* dns_server, const char* name, + grpc_pollset_set* interested_parties, grpc_closure* on_done, char** service_config_json, - int query_timeout_ms) = grpc_dns_lookup_ares_impl; + int query_timeout_ms) = grpc_dns_lookup_txt_ares_impl; static void grpc_cancel_ares_request_impl(grpc_ares_request* r) { GPR_ASSERT(r != nullptr); diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h index abb32dc7503..00679784702 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h @@ -78,23 +78,36 @@ struct grpc_ares_request { grpc_error_handle error ABSL_GUARDED_BY(mu) = GRPC_ERROR_NONE; }; -/* Asynchronously resolve \a name. It will try to resolve grpclb SRV records in - addition to the normal address records. For normal address records, it uses - \a default_port if a port isn't designated in \a name, otherwise it uses the - port in \a name. grpc_ares_init() must be called at least once before this - function. The returned grpc_ares_request object is owned by the caller and it - is safe to free after on_done is called back. +/* Asynchronously resolve \a name (A/AAAA records only). + It uses \a default_port if a port isn't designated in \a name, otherwise it + uses the port in \a name. grpc_ares_init() must be called at least once before + this function. The returned grpc_ares_request object is owned by the caller + and it is safe to free after on_done is called back. Note on synchronization: \a as on_done might be called from another thread ~immediately, access to the grpc_ares_request* return value must be synchronized by the caller. TODO(apolcyn): we should remove this requirement by changing this API to use two phase initialization - one API to create the grpc_ares_request* and another to start the async work. */ -extern grpc_ares_request* (*grpc_dns_lookup_ares)( +extern grpc_ares_request* (*grpc_dns_lookup_hostname_ares)( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, std::unique_ptr* addresses, + int query_timeout_ms); + +// Asynchronously resolve a SRV record. +// See \a grpc_dns_lookup_hostname_ares for usage details and caveats. +extern grpc_ares_request* (*grpc_dns_lookup_srv_ares)( + const char* dns_server, const char* name, + grpc_pollset_set* interested_parties, grpc_closure* on_done, std::unique_ptr* balancer_addresses, + int query_timeout_ms); + +// Asynchronously resolve a TXT record. +// See \a grpc_dns_lookup_hostname_ares for usage details and caveats. +extern grpc_ares_request* (*grpc_dns_lookup_txt_ares)( + const char* dns_server, const char* name, + grpc_pollset_set* interested_parties, grpc_closure* on_done, char** service_config_json, int query_timeout_ms); /* Cancel the pending grpc_ares_request \a request */ diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index b7fd8d1c620..f5f753674d8 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -118,9 +118,10 @@ NativeClientChannelDNSResolver::~NativeClientChannelDNSResolver() { OrphanablePtr NativeClientChannelDNSResolver::StartRequest() { Ref(DEBUG_LOCATION, "dns_request").release(); - auto dns_request_handle = GetDNSResolver()->ResolveName( - name_to_resolve(), kDefaultSecurePort, interested_parties(), - absl::bind_front(&NativeClientChannelDNSResolver::OnResolved, this)); + auto dns_request_handle = GetDNSResolver()->LookupHostname( + absl::bind_front(&NativeClientChannelDNSResolver::OnResolved, this), + name_to_resolve(), kDefaultSecurePort, kDefaultDNSRequestTimeout, + interested_parties(), /*name_server=*/""); if (GRPC_TRACE_FLAG_ENABLED(grpc_trace_dns_resolver)) { gpr_log(GPR_DEBUG, "[dns_resolver=%p] starting request=%p", this, DNSResolver::HandleToString(dns_request_handle).c_str()); diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index d979e6bfe86..df0ad45d1df 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -943,7 +943,8 @@ grpc_error_handle Chttp2ServerAddPort(Server* server, const char* addr, resolved_or = grpc_resolve_unix_abstract_domain_address(parsed_addr_unprefixed); } else { - resolved_or = GetDNSResolver()->ResolveNameBlocking(parsed_addr, "https"); + resolved_or = + GetDNSResolver()->LookupHostnameBlocking(parsed_addr, "https"); } if (!resolved_or.ok()) { return absl_status_to_grpc_error(resolved_or.status()); diff --git a/src/core/lib/http/httpcli.cc b/src/core/lib/http/httpcli.cc index c9f22719e37..f97a404dee6 100644 --- a/src/core/lib/http/httpcli.cc +++ b/src/core/lib/http/httpcli.cc @@ -209,9 +209,10 @@ void HttpRequest::Start() { return; } Ref().release(); // ref held by pending DNS resolution - dns_request_handle_ = GetDNSResolver()->ResolveName( - uri_.authority(), uri_.scheme(), pollset_set_, - absl::bind_front(&HttpRequest::OnResolved, this)); + dns_request_handle_ = GetDNSResolver()->LookupHostname( + absl::bind_front(&HttpRequest::OnResolved, this), uri_.authority(), + uri_.scheme(), kDefaultDNSRequestTimeout, pollset_set_, + /*name_server=*/""); } void HttpRequest::Orphan() { diff --git a/src/core/lib/iomgr/event_engine/resolver.h b/src/core/lib/iomgr/event_engine/resolver.h deleted file mode 100644 index c3d9ba9c216..00000000000 --- a/src/core/lib/iomgr/event_engine/resolver.h +++ /dev/null @@ -1,58 +0,0 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -#ifndef GRPC_CORE_LIB_IOMGR_EVENT_ENGINE_RESOLVER_H -#define GRPC_CORE_LIB_IOMGR_EVENT_ENGINE_RESOLVER_H - -#include - -#include -#include - -#include - -#include -#include -#include -#include - -#include "src/core/lib/iomgr/port.h" -#include "src/core/lib/iomgr/resolve_address.h" - -namespace grpc_core { -namespace experimental { - -#ifdef GRPC_USE_EVENT_ENGINE -class EventEngineDNSResolver : public DNSResolver { - public: - // Gets the singleton instance, creating it first if it doesn't exist - static EventEngineDNSResolver* GetOrCreate(); - - OrphanablePtr ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* interested_parties, - std::function>)> - on_done) override; - - absl::StatusOr> ResolveNameBlocking( - absl::string_view name, absl::string_view default_port) override; -}; -#endif // GRPC_USE_EVENT_ENGINE - -} // namespace experimental -} // namespace grpc_core - -#endif // GRPC_CORE_LIB_IOMGR_EVENT_ENGINE_RESOLVER_H diff --git a/src/core/lib/iomgr/resolve_address.h b/src/core/lib/iomgr/resolve_address.h index 77e5ea9fbd8..13a518cb337 100644 --- a/src/core/lib/iomgr/resolve_address.h +++ b/src/core/lib/iomgr/resolve_address.h @@ -37,6 +37,7 @@ namespace grpc_core { extern const char* kDefaultSecurePort; constexpr int kDefaultSecurePortInt = 443; +constexpr Duration kDefaultDNSRequestTimeout = Duration::Minutes(2); // A singleton class used for async and blocking DNS resolution class DNSResolver { @@ -60,17 +61,39 @@ class DNSResolver { // address this. // // \a interested_parties may be deleted after a request is cancelled. - virtual TaskHandle ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* interested_parties, + virtual TaskHandle LookupHostname( std::function>)> - on_done) = 0; + on_resolved, + absl::string_view name, absl::string_view default_port, Duration timeout, + grpc_pollset_set* interested_parties, absl::string_view name_server) = 0; // Resolve name in a blocking fashion. Use \a default_port if a port isn't // designated in \a name, otherwise use the port in \a name. virtual absl::StatusOr> - ResolveNameBlocking(absl::string_view name, - absl::string_view default_port) = 0; + LookupHostnameBlocking(absl::string_view name, + absl::string_view default_port) = 0; + + // Asynchronously resolve an SRV Record to Hostnames. + // On completion, \a on_done is invoked with the result. + // + // The same caveats in \a LookupHostname apply here as well. + // + // TODO(hork): return std::vector and ask the client to do the + // subsequent hostname lookups. + virtual TaskHandle LookupSRV( + std::function>)> + on_resolved, + absl::string_view name, Duration timeout, + grpc_pollset_set* interested_parties, absl::string_view name_server) = 0; + + // Asynchronously resolve a TXT Record. On completion, \a on_done is invoked + // with the resulting string. + // + // The same caveats in \a LookupHostname apply here. + virtual TaskHandle LookupTXT( + std::function)> on_resolved, + absl::string_view name, Duration timeout, + grpc_pollset_set* interested_parties, absl::string_view name_server) = 0; // This shares the same semantics with \a EventEngine::Cancel: successfully // cancelled lookups will not have their callbacks executed, and this diff --git a/src/core/lib/iomgr/resolve_address_posix.cc b/src/core/lib/iomgr/resolve_address_posix.cc index 650696666d9..b292b36d5e8 100644 --- a/src/core/lib/iomgr/resolve_address_posix.cc +++ b/src/core/lib/iomgr/resolve_address_posix.cc @@ -29,6 +29,7 @@ #include #include +#include "src/core/lib/event_engine/event_engine_factory.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/host_port.h" @@ -45,6 +46,8 @@ namespace grpc_core { namespace { +using ::grpc_event_engine::experimental::GetDefaultEventEngine; + class NativeDNSRequest { public: NativeDNSRequest( @@ -58,11 +61,11 @@ class NativeDNSRequest { private: // Callback to be passed to grpc Executor to asynch-ify - // ResolveNameBlocking + // LookupHostnameBlocking static void DoRequestThread(void* rp, grpc_error_handle /*error*/) { NativeDNSRequest* r = static_cast(rp); auto result = - GetDNSResolver()->ResolveNameBlocking(r->name_, r->default_port_); + GetDNSResolver()->LookupHostnameBlocking(r->name_, r->default_port_); // running inline is safe since we've already been scheduled on the executor r->on_done_(std::move(result)); delete r; @@ -82,19 +85,20 @@ NativeDNSResolver* NativeDNSResolver::GetOrCreate() { return instance; } -DNSResolver::TaskHandle NativeDNSResolver::ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* /* interested_parties */, +DNSResolver::TaskHandle NativeDNSResolver::LookupHostname( std::function>)> - on_done) { + on_done, + absl::string_view name, absl::string_view default_port, + Duration /* timeout */, grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) { // self-deleting class new NativeDNSRequest(name, default_port, std::move(on_done)); return kNullHandle; } absl::StatusOr> -NativeDNSResolver::ResolveNameBlocking(absl::string_view name, - absl::string_view default_port) { +NativeDNSResolver::LookupHostnameBlocking(absl::string_view name, + absl::string_view default_port) { ExecCtx exec_ctx; struct addrinfo hints; struct addrinfo *result = nullptr, *resp; @@ -173,6 +177,32 @@ done: return error_result; } +DNSResolver::TaskHandle NativeDNSResolver::LookupSRV( + std::function>)> + on_resolved, + absl::string_view /* name */, Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) { + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Native resolver does not support looking up SRV records")); + }); + return {-1, -1}; +}; + +DNSResolver::TaskHandle NativeDNSResolver::LookupTXT( + std::function)> on_resolved, + absl::string_view /* name */, Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) { + // Not supported + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Native resolver does not support looking up TXT records")); + }); + return {-1, -1}; +}; + bool NativeDNSResolver::Cancel(TaskHandle /*handle*/) { return false; } } // namespace grpc_core diff --git a/src/core/lib/iomgr/resolve_address_posix.h b/src/core/lib/iomgr/resolve_address_posix.h index bd79b71c936..a07163d144e 100644 --- a/src/core/lib/iomgr/resolve_address_posix.h +++ b/src/core/lib/iomgr/resolve_address_posix.h @@ -32,15 +32,29 @@ class NativeDNSResolver : public DNSResolver { // Gets the singleton instance, creating it first if it doesn't exist static NativeDNSResolver* GetOrCreate(); - TaskHandle ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* /* interested_parties */, + TaskHandle LookupHostname( std::function>)> - on_done) override; + on_done, + absl::string_view name, absl::string_view default_port, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override; - absl::StatusOr> ResolveNameBlocking( + absl::StatusOr> LookupHostnameBlocking( absl::string_view name, absl::string_view default_port) override; + TaskHandle LookupSRV( + std::function>)> + on_resolved, + absl::string_view name, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override; + + TaskHandle LookupTXT( + std::function)> on_resolved, + absl::string_view name, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override; + // NativeDNSResolver does not support cancellation. bool Cancel(TaskHandle handle) override; }; diff --git a/src/core/lib/iomgr/resolve_address_windows.cc b/src/core/lib/iomgr/resolve_address_windows.cc index 0051b74093e..3734fa21f15 100644 --- a/src/core/lib/iomgr/resolve_address_windows.cc +++ b/src/core/lib/iomgr/resolve_address_windows.cc @@ -34,6 +34,7 @@ #include #include "src/core/lib/address_utils/sockaddr_utils.h" +#include "src/core/lib/event_engine/event_engine_factory.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/host_port.h" #include "src/core/lib/gprpp/thd.h" @@ -48,6 +49,8 @@ namespace grpc_core { namespace { +using ::grpc_event_engine::experimental::GetDefaultEventEngine; + class NativeDNSRequest { public: NativeDNSRequest( @@ -61,11 +64,11 @@ class NativeDNSRequest { private: // Callback to be passed to grpc Executor to asynch-ify - // ResolveNameBlocking + // LookupHostnameBlocking static void DoRequestThread(void* rp, grpc_error_handle /*error*/) { NativeDNSRequest* r = static_cast(rp); auto result = - GetDNSResolver()->ResolveNameBlocking(r->name_, r->default_port_); + GetDNSResolver()->LookupHostnameBlocking(r->name_, r->default_port_); // running inline is safe since we've already been scheduled on the executor r->on_done_(std::move(result)); delete r; @@ -85,18 +88,19 @@ NativeDNSResolver* NativeDNSResolver::GetOrCreate() { return instance; } -DNSResolver::TaskHandle NativeDNSResolver::ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* /* interested_parties */, +DNSResolver::TaskHandle NativeDNSResolver::LookupHostname( std::function>)> - on_done) { - new NativeDNSRequest(name, default_port, std::move(on_done)); + on_resolved, + absl::string_view name, absl::string_view default_port, + Duration /* timeout */, grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) { + new NativeDNSRequest(name, default_port, std::move(on_resolved)); return kNullHandle; } absl::StatusOr> -NativeDNSResolver::ResolveNameBlocking(absl::string_view name, - absl::string_view default_port) { +NativeDNSResolver::LookupHostnameBlocking(absl::string_view name, + absl::string_view default_port) { ExecCtx exec_ctx; struct addrinfo hints; struct addrinfo *result = NULL, *resp; @@ -157,6 +161,32 @@ done: return error_result; } +DNSResolver::TaskHandle NativeDNSResolver::LookupSRV( + std::function>)> + on_resolved, + absl::string_view /* name */, Duration /* deadline */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) { + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Native resolver does not support looking up SRV records")); + }); + return {-1, -1}; +}; + +DNSResolver::TaskHandle NativeDNSResolver::LookupTXT( + std::function)> on_resolved, + absl::string_view /* name */, Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) { + // Not supported + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Native resolver does not support looking up TXT records")); + }); + return {-1, -1}; +}; + bool NativeDNSResolver::Cancel(TaskHandle /*handle*/) { return false; } } // namespace grpc_core diff --git a/src/core/lib/iomgr/resolve_address_windows.h b/src/core/lib/iomgr/resolve_address_windows.h index 67a0181dba0..901d47331d3 100644 --- a/src/core/lib/iomgr/resolve_address_windows.h +++ b/src/core/lib/iomgr/resolve_address_windows.h @@ -32,15 +32,29 @@ class NativeDNSResolver : public DNSResolver { // Gets the singleton instance, creating it first if it doesn't exist static NativeDNSResolver* GetOrCreate(); - TaskHandle ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* /* interested_parties */, + TaskHandle LookupHostname( std::function>)> - on_done) override; + on_resolved, + absl::string_view name, absl::string_view default_port, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override; - absl::StatusOr> ResolveNameBlocking( + absl::StatusOr> LookupHostnameBlocking( absl::string_view name, absl::string_view default_port) override; + TaskHandle LookupSRV( + std::function>)> + on_resolved, + absl::string_view name, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override; + + TaskHandle LookupTXT( + std::function)> on_resolved, + absl::string_view name, Duration timeout, + grpc_pollset_set* interested_parties, + absl::string_view name_server) override; + // NativeDNSResolver does not support cancellation. bool Cancel(TaskHandle handle) override; }; diff --git a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc index 34d09b6402d..6fe248b84f9 100644 --- a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc @@ -29,6 +29,7 @@ #include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/config/core_configuration.h" +#include "src/core/lib/event_engine/event_engine_factory.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/time.h" #include "src/core/lib/iomgr/resolve_address.h" @@ -45,10 +46,9 @@ static grpc_ares_request* (*g_default_dns_lookup_ares)( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, - char** service_config_json, int query_timeout_ms); + int query_timeout_ms); -// Counter incremented by TestDNSResolver::ResolveName indicating the +// Counter incremented by TestDNSResolver::LookupHostname indicating the // number of times a system-level resolution has happened. static int g_resolution_count; @@ -62,19 +62,23 @@ static struct iomgr_args { namespace { +using ::grpc_event_engine::experimental::GetDefaultEventEngine; + grpc_core::DNSResolver* g_default_dns_resolver; class TestDNSResolver : public grpc_core::DNSResolver { public: // Wrapper around default resolve_address in order to count the number of // times we incur in a system-level name resolution. - TaskHandle ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* interested_parties, + TaskHandle LookupHostname( std::function>)> - on_done) override { - auto result = g_default_dns_resolver->ResolveName( - name, default_port, interested_parties, std::move(on_done)); + on_resolved, + absl::string_view name, absl::string_view default_port, + grpc_core::Duration timeout, grpc_pollset_set* interested_parties, + absl::string_view name_server) override { + auto result = g_default_dns_resolver->LookupHostname( + std::move(on_resolved), name, default_port, timeout, interested_parties, + name_server); ++g_resolution_count; static grpc_core::Timestamp last_resolution_time = grpc_core::Timestamp::ProcessEpoch(); @@ -98,11 +102,37 @@ class TestDNSResolver : public grpc_core::DNSResolver { return result; } - absl::StatusOr> ResolveNameBlocking( + absl::StatusOr> LookupHostnameBlocking( absl::string_view name, absl::string_view default_port) override { - return g_default_dns_resolver->ResolveNameBlocking(name, default_port); + return g_default_dns_resolver->LookupHostnameBlocking(name, default_port); } + TaskHandle LookupSRV( + std::function>)> + on_resolved, + absl::string_view /* name */, grpc_core::Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) override { + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Testing DNS resolver does not support looking up SRV records")); + }); + return {-1, -1}; + }; + + TaskHandle LookupTXT( + std::function)> on_resolved, + absl::string_view /* name */, grpc_core::Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) override { + // Not supported + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Testing DNS resolver does not support looking up TXT records")); + }); + return {-1, -1}; + }; + // Not cancellable bool Cancel(TaskHandle /*handle*/) override { return false; } }; @@ -113,11 +143,11 @@ static grpc_ares_request* test_dns_lookup_ares( const char* dns_server, const char* name, const char* default_port, grpc_pollset_set* /*interested_parties*/, grpc_closure* on_done, std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, - char** service_config_json, int query_timeout_ms) { + int query_timeout_ms) { + // A records should suffice grpc_ares_request* result = g_default_dns_lookup_ares( dns_server, name, default_port, g_iomgr_args.pollset_set, on_done, - addresses, balancer_addresses, service_config_json, query_timeout_ms); + addresses, query_timeout_ms); ++g_resolution_count; static auto last_resolution_time = grpc_core::Timestamp::ProcessEpoch(); auto now = @@ -345,8 +375,8 @@ TEST(DnsResolverCooldownTest, MainTest) { auto work_serializer = std::make_shared(); g_work_serializer = &work_serializer; - g_default_dns_lookup_ares = grpc_dns_lookup_ares; - grpc_dns_lookup_ares = test_dns_lookup_ares; + g_default_dns_lookup_ares = grpc_dns_lookup_hostname_ares; + grpc_dns_lookup_hostname_ares = test_dns_lookup_ares; g_default_dns_resolver = grpc_core::GetDNSResolver(); grpc_core::SetDNSResolver(new TestDNSResolver()); diff --git a/test/core/end2end/dualstack_socket_test.cc b/test/core/end2end/dualstack_socket_test.cc index 8e9ca0c5a04..970ea595d27 100644 --- a/test/core/end2end/dualstack_socket_test.cc +++ b/test/core/end2end/dualstack_socket_test.cc @@ -62,7 +62,7 @@ static void drain_cq(grpc_completion_queue* cq) { static void log_resolved_addrs(const char* label, const char* hostname) { absl::StatusOr> addresses_or = - grpc_core::GetDNSResolver()->ResolveNameBlocking(hostname, "80"); + grpc_core::GetDNSResolver()->LookupHostnameBlocking(hostname, "80"); if (!addresses_or.ok()) { GRPC_LOG_IF_ERROR(hostname, absl_status_to_grpc_error(addresses_or.status())); @@ -281,7 +281,7 @@ void test_connect(const char* server_host, const char* client_host, int port, int external_dns_works(const char* host) { auto addresses_or = - grpc_core::GetDNSResolver()->ResolveNameBlocking(host, "80"); + grpc_core::GetDNSResolver()->LookupHostnameBlocking(host, "80"); if (!addresses_or.ok()) { return 0; } diff --git a/test/core/end2end/fixtures/http_proxy_fixture.cc b/test/core/end2end/fixtures/http_proxy_fixture.cc index 2394bcc0f33..f44cb6e7807 100644 --- a/test/core/end2end/fixtures/http_proxy_fixture.cc +++ b/test/core/end2end/fixtures/http_proxy_fixture.cc @@ -532,8 +532,8 @@ static void on_read_request_done_locked(void* arg, grpc_error_handle error) { } // Resolve address. absl::StatusOr> addresses_or = - grpc_core::GetDNSResolver()->ResolveNameBlocking(conn->http_request.path, - "80"); + grpc_core::GetDNSResolver()->LookupHostnameBlocking( + conn->http_request.path, "80"); if (!addresses_or.ok()) { proxy_connection_failed(conn, SETUP_FAILED, "HTTP proxy DNS lookup", GRPC_ERROR_REF(error)); diff --git a/test/core/end2end/fuzzers/api_fuzzer.cc b/test/core/end2end/fuzzers/api_fuzzer.cc index d3479d25e17..0942d2447fa 100644 --- a/test/core/end2end/fuzzers/api_fuzzer.cc +++ b/test/core/end2end/fuzzers/api_fuzzer.cc @@ -104,6 +104,8 @@ static void finish_resolve(void* arg, grpc_error_handle error) { namespace { +using ::grpc_event_engine::experimental::GetDefaultEventEngine; + class FuzzerDNSResolver : public grpc_core::DNSResolver { public: class FuzzerDNSRequest { @@ -147,21 +149,49 @@ class FuzzerDNSResolver : public grpc_core::DNSResolver { return instance; } - TaskHandle ResolveName( + TaskHandle LookupHostname( + std::function>)> + on_resolved, absl::string_view name, absl::string_view /* default_port */, + grpc_core::Duration /* timeout */, grpc_pollset_set* /* interested_parties */, - std::function>)> - on_done) override { - new FuzzerDNSRequest(name, std::move(on_done)); + absl::string_view /* name_server */) override { + new FuzzerDNSRequest(name, std::move(on_resolved)); return kNullHandle; } - absl::StatusOr> ResolveNameBlocking( + absl::StatusOr> LookupHostnameBlocking( absl::string_view /* name */, absl::string_view /* default_port */) override { GPR_ASSERT(0); } + TaskHandle LookupSRV( + std::function>)> + on_resolved, + absl::string_view /* name */, grpc_core::Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) override { + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Fuzzing DNS resolver does not support looking up SRV records")); + }); + return {-1, -1}; + }; + + TaskHandle LookupTXT( + std::function)> on_resolved, + absl::string_view /* name */, grpc_core::Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) override { + // Not supported + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Fuzing DNS resolver does not support looking up TXT records")); + }); + return {-1, -1}; + }; + // FuzzerDNSResolver does not support cancellation. bool Cancel(TaskHandle /*handle*/) override { return false; } }; @@ -172,8 +202,7 @@ grpc_ares_request* my_dns_lookup_ares( const char* /*dns_server*/, const char* addr, const char* /*default_port*/, grpc_pollset_set* /*interested_parties*/, grpc_closure* on_done, std::unique_ptr* addresses, - std::unique_ptr* /*balancer_addresses*/, - char** /*service_config_json*/, int /*query_timeout*/) { + int /*query_timeout*/) { addr_req* r = new addr_req(); r->addr = gpr_strdup(addr); r->on_done = on_done; @@ -770,7 +799,7 @@ DEFINE_PROTO_FUZZER(const api_fuzzer::Msg& msg) { grpc_core::Executor::SetThreadingAll(false); } grpc_core::SetDNSResolver(FuzzerDNSResolver::GetOrCreate()); - grpc_dns_lookup_ares = my_dns_lookup_ares; + grpc_dns_lookup_hostname_ares = my_dns_lookup_ares; grpc_cancel_ares_request = my_cancel_ares_request; GPR_ASSERT(g_channel == nullptr); diff --git a/test/core/end2end/goaway_server_test.cc b/test/core/end2end/goaway_server_test.cc index 3b315a17988..4f0cbc902ce 100644 --- a/test/core/end2end/goaway_server_test.cc +++ b/test/core/end2end/goaway_server_test.cc @@ -28,6 +28,7 @@ #include #include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h" +#include "src/core/lib/event_engine/event_engine_factory.h" #include "src/core/lib/iomgr/resolve_address.h" #include "src/core/lib/iomgr/resolve_address_impl.h" #include "src/core/lib/iomgr/sockaddr.h" @@ -46,8 +47,7 @@ static grpc_ares_request* (*iomgr_dns_lookup_ares)( const char* dns_server, const char* addr, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, - char** service_config_json, int query_timeout_ms); + int query_timeout_ms); static void (*iomgr_cancel_ares_request)(grpc_ares_request* request); @@ -59,28 +59,58 @@ static void set_resolve_port(int port) { namespace { +using ::grpc_event_engine::experimental::GetDefaultEventEngine; + grpc_core::DNSResolver* g_default_dns_resolver; class TestDNSResolver : public grpc_core::DNSResolver { public: - TaskHandle ResolveName( - absl::string_view name, absl::string_view default_port, - grpc_pollset_set* interested_parties, + TaskHandle LookupHostname( std::function>)> - on_done) override { + on_resolved, + absl::string_view name, absl::string_view default_port, + grpc_core::Duration timeout, grpc_pollset_set* interested_parties, + absl::string_view name_server) override { if (name != "test") { - return g_default_dns_resolver->ResolveName( - name, default_port, interested_parties, std::move(on_done)); + return g_default_dns_resolver->LookupHostname( + std::move(on_resolved), name, default_port, timeout, + interested_parties, name_server); } - MakeDNSRequest(std::move(on_done)); + MakeDNSRequest(std::move(on_resolved)); return kNullHandle; } - absl::StatusOr> ResolveNameBlocking( + absl::StatusOr> LookupHostnameBlocking( absl::string_view name, absl::string_view default_port) override { - return g_default_dns_resolver->ResolveNameBlocking(name, default_port); + return g_default_dns_resolver->LookupHostnameBlocking(name, default_port); } + TaskHandle LookupSRV( + std::function>)> + on_resolved, + absl::string_view /* name */, grpc_core::Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) override { + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Testing DNS resolver does not support looking up SRV records")); + }); + return {-1, -1}; + }; + + TaskHandle LookupTXT( + std::function)> on_resolved, + absl::string_view /* name */, grpc_core::Duration /* timeout */, + grpc_pollset_set* /* interested_parties */, + absl::string_view /* name_server */) override { + // Not supported + GetDefaultEventEngine()->Run([on_resolved] { + on_resolved(absl::UnimplementedError( + "The Testing DNS resolver does not support looking up TXT records")); + }); + return {-1, -1}; + }; + bool Cancel(TaskHandle /*handle*/) override { return false; } private: @@ -114,12 +144,12 @@ static grpc_ares_request* my_dns_lookup_ares( const char* dns_server, const char* addr, const char* default_port, grpc_pollset_set* interested_parties, grpc_closure* on_done, std::unique_ptr* addresses, - std::unique_ptr* balancer_addresses, - char** service_config_json, int query_timeout_ms) { + int query_timeout_ms) { if (0 != strcmp(addr, "test")) { - return iomgr_dns_lookup_ares( - dns_server, addr, default_port, interested_parties, on_done, addresses, - balancer_addresses, service_config_json, query_timeout_ms); + // A records should suffice + return iomgr_dns_lookup_ares(dns_server, addr, default_port, + interested_parties, on_done, addresses, + query_timeout_ms); } grpc_error_handle error = GRPC_ERROR_NONE; @@ -160,9 +190,9 @@ int main(int argc, char** argv) { g_default_dns_resolver = grpc_core::GetDNSResolver(); auto* resolver = new TestDNSResolver(); grpc_core::SetDNSResolver(resolver); - iomgr_dns_lookup_ares = grpc_dns_lookup_ares; + iomgr_dns_lookup_ares = grpc_dns_lookup_hostname_ares; iomgr_cancel_ares_request = grpc_cancel_ares_request; - grpc_dns_lookup_ares = my_dns_lookup_ares; + grpc_dns_lookup_hostname_ares = my_dns_lookup_ares; grpc_cancel_ares_request = my_cancel_ares_request; int was_cancelled1; diff --git a/test/core/iomgr/resolve_address_posix_test.cc b/test/core/iomgr/resolve_address_posix_test.cc index f2049250b4d..900a07232f3 100644 --- a/test/core/iomgr/resolve_address_posix_test.cc +++ b/test/core/iomgr/resolve_address_posix_test.cc @@ -135,11 +135,13 @@ static void resolve_address_must_succeed(const char* target) { args_struct args; args_init(&args); poll_pollset_until_request_done(&args); - grpc_core::GetDNSResolver()->ResolveName( - target, "1" /* port number */, args.pollset_set, + grpc_core::GetDNSResolver()->LookupHostname( [&args](absl::StatusOr> result) { MustSucceed(&args, std::move(result)); - }); + }, + target, /*port number=*/"1", grpc_core::kDefaultDNSRequestTimeout, + args.pollset_set, + /*name_server=*/""); grpc_core::ExecCtx::Get()->Flush(); args_finish(&args); } diff --git a/test/core/iomgr/resolve_address_test.cc b/test/core/iomgr/resolve_address_test.cc index 99946e304fe..b369631e77d 100644 --- a/test/core/iomgr/resolve_address_test.cc +++ b/test/core/iomgr/resolve_address_test.cc @@ -182,18 +182,18 @@ class ResolveAddressTest : public ::testing::Test { TEST_F(ResolveAddressTest, Localhost) { grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - "localhost:1", "", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustSucceed, this), "localhost:1", + "", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); PollPollsetUntilRequestDone(); } TEST_F(ResolveAddressTest, DefaultPort) { grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - "localhost", "1", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustSucceed, this), "localhost", + "1", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); PollPollsetUntilRequestDone(); } @@ -203,9 +203,10 @@ TEST_F(ResolveAddressTest, LocalhostResultHasIPv6First) { GTEST_SKIP() << "this test is only valid with the c-ares resolver"; } grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - "localhost:1", "", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustSucceedWithIPv6First, this)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustSucceedWithIPv6First, this), + "localhost:1", "", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), + ""); grpc_core::ExecCtx::Get()->Flush(); PollPollsetUntilRequestDone(); } @@ -250,45 +251,47 @@ TEST_F(ResolveAddressTest, LocalhostResultHasIPv4FirstWhenIPv6IsntAvalailable) { address_sorting_override_source_addr_factory_for_testing(mock); // run the test grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - "localhost:1", "", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustSucceedWithIPv4First, this)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustSucceedWithIPv4First, this), + "localhost:1", "", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), + ""); grpc_core::ExecCtx::Get()->Flush(); PollPollsetUntilRequestDone(); } TEST_F(ResolveAddressTest, NonNumericDefaultPort) { grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - "localhost", "http", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustSucceed, this), "localhost", + "http", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); PollPollsetUntilRequestDone(); } TEST_F(ResolveAddressTest, MissingDefaultPort) { grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - "localhost", "", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustFail, this)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustFail, this), "localhost", "", + grpc_core::kDefaultDNSRequestTimeout, pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); PollPollsetUntilRequestDone(); } TEST_F(ResolveAddressTest, IPv6WithPort) { grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - "[2001:db8::1]:1", "", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustSucceed, this), + "[2001:db8::1]:1", "", grpc_core::kDefaultDNSRequestTimeout, + pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); PollPollsetUntilRequestDone(); } void TestIPv6WithoutPort(ResolveAddressTest* test, const char* target) { grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - target, "80", test->pollset_set(), - absl::bind_front(&ResolveAddressTest::MustSucceed, test)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustSucceed, test), target, "80", + grpc_core::kDefaultDNSRequestTimeout, test->pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); test->PollPollsetUntilRequestDone(); } @@ -307,9 +310,9 @@ TEST_F(ResolveAddressTest, IPv6WithoutPortV4MappedV6) { void TestInvalidIPAddress(ResolveAddressTest* test, const char* target) { grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - target, "", test->pollset_set(), - absl::bind_front(&ResolveAddressTest::MustFail, test)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustFail, test), target, "", + grpc_core::kDefaultDNSRequestTimeout, test->pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); test->PollPollsetUntilRequestDone(); } @@ -324,9 +327,9 @@ TEST_F(ResolveAddressTest, InvalidIPv6Addresses) { void TestUnparseableHostPort(ResolveAddressTest* test, const char* target) { grpc_core::ExecCtx exec_ctx; - grpc_core::GetDNSResolver()->ResolveName( - target, "1", test->pollset_set(), - absl::bind_front(&ResolveAddressTest::MustFail, test)); + grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustFail, test), target, "1", + grpc_core::kDefaultDNSRequestTimeout, test->pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); test->PollPollsetUntilRequestDone(); } @@ -359,9 +362,9 @@ TEST_F(ResolveAddressTest, UnparseableHostPortsBadLocalhostWithPort) { // test doesn't care what the result is, just that we don't crash etc. TEST_F(ResolveAddressTest, ImmediateCancel) { grpc_core::ExecCtx exec_ctx; - auto request_handle = grpc_core::GetDNSResolver()->ResolveName( - "localhost:1", "1", pollset_set(), - absl::bind_front(&ResolveAddressTest::DontCare, this)); + auto request_handle = grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::DontCare, this), "localhost:1", "1", + grpc_core::kDefaultDNSRequestTimeout, pollset_set(), ""); if (grpc_core::GetDNSResolver()->Cancel(request_handle)) { Finish(); } @@ -372,9 +375,9 @@ TEST_F(ResolveAddressTest, ImmediateCancel) { // Attempt to cancel a request after it has completed. TEST_F(ResolveAddressTest, CancelDoesNotSucceed) { grpc_core::ExecCtx exec_ctx; - auto request_handle = grpc_core::GetDNSResolver()->ResolveName( - "localhost:1", "1", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + auto request_handle = grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustSucceed, this), "localhost:1", + "1", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); PollPollsetUntilRequestDone(); ASSERT_FALSE(grpc_core::GetDNSResolver()->Cancel(request_handle)); @@ -416,9 +419,10 @@ TEST_F(ResolveAddressTest, CancelWithNonResponsiveDNSServer) { grpc_ares_test_only_inject_config = InjectNonResponsiveDNSServer; // Run the test grpc_core::ExecCtx exec_ctx; - auto request_handle = grpc_core::GetDNSResolver()->ResolveName( - "foo.bar.com:1", "1", pollset_set(), - absl::bind_front(&ResolveAddressTest::MustNotBeCalled, this)); + auto request_handle = grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustNotBeCalled, this), + "foo.bar.com:1", "1", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), + ""); grpc_core::ExecCtx::Get()->Flush(); // initiate DNS requests ASSERT_TRUE(grpc_core::GetDNSResolver()->Cancel(request_handle)); Finish(); @@ -479,9 +483,10 @@ TEST_F(ResolveAddressTest, DeleteInterestedPartiesAfterCancellation) { // Create a pollset_set, destroyed immediately after cancellation std::unique_ptr pss = PollsetSetWrapper::Create(); // Run the test - auto request_handle = grpc_core::GetDNSResolver()->ResolveName( - "foo.bar.com:1", "1", pss->pollset_set(), - absl::bind_front(&ResolveAddressTest::MustNotBeCalled, this)); + auto request_handle = grpc_core::GetDNSResolver()->LookupHostname( + absl::bind_front(&ResolveAddressTest::MustNotBeCalled, this), + "foo.bar.com:1", "1", grpc_core::kDefaultDNSRequestTimeout, + pss->pollset_set(), ""); grpc_core::ExecCtx::Get()->Flush(); // initiate DNS requests ASSERT_TRUE(grpc_core::GetDNSResolver()->Cancel(request_handle)); } @@ -493,6 +498,40 @@ TEST_F(ResolveAddressTest, DeleteInterestedPartiesAfterCancellation) { PollPollsetUntilRequestDone(); } +TEST_F(ResolveAddressTest, NativeResolverCannotLookupSRVRecords) { + if (absl::string_view(g_resolver_type) == "ares") { + GTEST_SKIP() << "this test is only for native resolvers"; + } + grpc_core::ExecCtx exec_ctx; + grpc_core::GetDNSResolver()->LookupSRV( + [this](absl::StatusOr> error) { + grpc_core::ExecCtx exec_ctx; + EXPECT_EQ(error.status().code(), absl::StatusCode::kUnimplemented); + Finish(); + }, + "localhost", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), + /*name_server=*/""); + grpc_core::ExecCtx::Get()->Flush(); + PollPollsetUntilRequestDone(); +} + +TEST_F(ResolveAddressTest, NativeResolverCannotLookupTXTRecords) { + if (absl::string_view(g_resolver_type) == "ares") { + GTEST_SKIP() << "this test is only for native resolvers"; + } + grpc_core::ExecCtx exec_ctx; + grpc_core::GetDNSResolver()->LookupTXT( + [this](absl::StatusOr error) { + grpc_core::ExecCtx exec_ctx; + EXPECT_EQ(error.status().code(), absl::StatusCode::kUnimplemented); + Finish(); + }, + "localhost", grpc_core::kDefaultDNSRequestTimeout, pollset_set(), + /*name_server=*/""); + grpc_core::ExecCtx::Get()->Flush(); + PollPollsetUntilRequestDone(); +} + int main(int argc, char** argv) { // Configure the DNS resolver (c-ares vs. native) based on the // name of the binary. TODO(apolcyn): is there a way to pass command diff --git a/test/core/surface/server_test.cc b/test/core/surface/server_test.cc index 81171db79e1..538ce1d1ec6 100644 --- a/test/core/surface/server_test.cc +++ b/test/core/surface/server_test.cc @@ -127,7 +127,7 @@ void test_bind_server_to_addr(const char* host, bool secure) { } static bool external_dns_works(const char* host) { - return grpc_core::GetDNSResolver()->ResolveNameBlocking(host, "80").ok(); + return grpc_core::GetDNSResolver()->LookupHostnameBlocking(host, "80").ok(); } static void test_bind_server_to_addrs(const char** addrs, size_t n) { diff --git a/test/core/transport/chttp2/settings_timeout_test.cc b/test/core/transport/chttp2/settings_timeout_test.cc index dc062e99014..8bed037c061 100644 --- a/test/core/transport/chttp2/settings_timeout_test.cc +++ b/test/core/transport/chttp2/settings_timeout_test.cc @@ -114,7 +114,7 @@ class Client { void Connect() { ExecCtx exec_ctx; absl::StatusOr> addresses_or = - GetDNSResolver()->ResolveNameBlocking(server_address_, "80"); + GetDNSResolver()->LookupHostnameBlocking(server_address_, "80"); ASSERT_EQ(absl::OkStatus(), addresses_or.status()) << addresses_or.status().ToString(); ASSERT_GE(addresses_or->size(), 1UL); diff --git a/test/core/util/resolve_localhost_ip46.cc b/test/core/util/resolve_localhost_ip46.cc index bcfbd077671..7dc1f5d3381 100644 --- a/test/core/util/resolve_localhost_ip46.cc +++ b/test/core/util/resolve_localhost_ip46.cc @@ -33,7 +33,7 @@ gpr_once g_resolve_localhost_ipv46 = GPR_ONCE_INIT; void InitResolveLocalhost() { absl::StatusOr> addresses_or = - GetDNSResolver()->ResolveNameBlocking("localhost", "https"); + GetDNSResolver()->LookupHostnameBlocking("localhost", "https"); GPR_ASSERT(addresses_or.ok()); for (const auto& addr : *addresses_or) { const grpc_sockaddr* sock_addr =