Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apple dns: add fake api test suite #13780

Merged
merged 4 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions source/common/network/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ envoy_cc_library(
"//include/envoy/network:dns_interface",
"//source/common/common:assert_lib",
"//source/common/common:linked_object",
"//source/common/singleton:threadsafe_singleton",
],
)

Expand Down
48 changes: 39 additions & 9 deletions source/common/network/apple_dns_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <string>

#include "envoy/common/platform.h"
#include "envoy/event/file_event.h"

#include "common/common/assert.h"
#include "common/common/fmt.h"
Expand All @@ -20,6 +21,30 @@
namespace Envoy {
namespace Network {

void DnsService::dnsServiceRefDeallocate(DNSServiceRef sdRef) { DNSServiceRefDeallocate(sdRef); }

DNSServiceErrorType DnsService::dnsServiceCreateConnection(DNSServiceRef* sdRef) {
ENVOY_LOG_MISC(debug, "In real implementation");
junr03 marked this conversation as resolved.
Show resolved Hide resolved
return DNSServiceCreateConnection(sdRef);
}

dnssd_sock_t DnsService::dnsServiceRefSockFD(DNSServiceRef sdRef) {
return DNSServiceRefSockFD(sdRef);
}

DNSServiceErrorType DnsService::dnsServiceProcessResult(DNSServiceRef sdRef) {
return DNSServiceProcessResult(sdRef);
}

DNSServiceErrorType DnsService::dnsServiceGetAddrInfo(DNSServiceRef* sdRef, DNSServiceFlags flags,
uint32_t interfaceIndex,
DNSServiceProtocol protocol,
const char* hostname,
DNSServiceGetAddrInfoReply callBack,
void* context) {
return DNSServiceGetAddrInfo(sdRef, flags, interfaceIndex, protocol, hostname, callBack, context);
}

AppleDnsResolverImpl::AppleDnsResolverImpl(Event::Dispatcher& dispatcher)
: dispatcher_(dispatcher) {
ENVOY_LOG(debug, "Constructing DNS resolver");
Expand All @@ -38,7 +63,7 @@ void AppleDnsResolverImpl::deallocateMainSdRef() {
// be removed BEFORE DNSServiceRefDeallocate() is called, as this function closes the
// reference's socket.
sd_ref_event_.reset();
DNSServiceRefDeallocate(main_sd_ref_);
DnsServiceSingleton::get().dnsServiceRefDeallocate(main_sd_ref_);
}

void AppleDnsResolverImpl::initializeMainSdRef() {
Expand All @@ -54,10 +79,11 @@ void AppleDnsResolverImpl::initializeMainSdRef() {
// However, using a shared connection brings some complexities detailed in the inline comments
// for kDNSServiceFlagsShareConnection in dns_sd.h, and copied (and edited) in this implementation
// where relevant.
auto error = DNSServiceCreateConnection(&main_sd_ref_);
RELEASE_ASSERT(!error, fmt::format("error ({}) in DNSServiceCreateConnection", error));
auto error = DnsServiceSingleton::get().dnsServiceCreateConnection(&main_sd_ref_);
RELEASE_ASSERT(error == kDNSServiceErr_NoError,
fmt::format("error={} in DNSServiceCreateConnection", error));

auto fd = DNSServiceRefSockFD(main_sd_ref_);
auto fd = DnsServiceSingleton::get().dnsServiceRefSockFD(main_sd_ref_);
RELEASE_ASSERT(fd != -1, "error in DNSServiceRefSockFD");
ENVOY_LOG(debug, "DNS resolver has fd={}", fd);

Expand All @@ -72,8 +98,9 @@ void AppleDnsResolverImpl::initializeMainSdRef() {

void AppleDnsResolverImpl::onEventCallback(uint32_t events) {
ENVOY_LOG(debug, "DNS resolver file event ({})", events);
ASSERT(events & Event::FileReadyType::Read);
DNSServiceErrorType error = DNSServiceProcessResult(main_sd_ref_);
RELEASE_ASSERT(events & Event::FileReadyType::Read,
fmt::format("invalid FileReadyType event={}", events));
DNSServiceErrorType error = DnsServiceSingleton::get().dnsServiceProcessResult(main_sd_ref_);
if (error != kDNSServiceErr_NoError) {
ENVOY_LOG(warn, "DNS resolver error ({}) in DNSServiceProcessResult", error);
// Similar to receiving an error in onDNSServiceGetAddrInfoReply, an error while processing fd
Expand Down Expand Up @@ -163,7 +190,7 @@ AppleDnsResolverImpl::PendingResolution::~PendingResolution() {
// thus the DNSServiceRef is null.
// Therefore, only deallocate if the ref is not null.
if (individual_sd_ref_) {
DNSServiceRefDeallocate(individual_sd_ref_);
DnsServiceSingleton::get().dnsServiceRefDeallocate(individual_sd_ref_);
}
}

Expand Down Expand Up @@ -204,7 +231,8 @@ void AppleDnsResolverImpl::PendingResolution::onDNSServiceGetAddrInfoReply(
"error_code={}, hostname={}",
dns_name_, flags, flags & kDNSServiceFlagsMoreComing ? "yes" : "no",
flags & kDNSServiceFlagsAdd ? "yes" : "no", interface_index, error_code, hostname);
ASSERT(interface_index == 0);
RELEASE_ASSERT(interface_index == 0,
fmt::format("unexpected interface_index={}", interface_index));

// Generic error handling.
if (error_code != kDNSServiceErr_NoError) {
Expand Down Expand Up @@ -236,6 +264,8 @@ void AppleDnsResolverImpl::PendingResolution::onDNSServiceGetAddrInfoReply(

if (!pending_cb_) {
ENVOY_LOG(debug, "Adding to queries pending callback");
// DISCUSSION: should this be added on creation so even if a query returns success, but no new
// addresses the callbakc target is called?
junr03 marked this conversation as resolved.
Show resolved Hide resolved
pending_cb_ = {ResolutionStatus::Success, {dns_response}};
parent_.addPendingQuery(this);
} else {
Expand Down Expand Up @@ -287,7 +317,7 @@ AppleDnsResolverImpl::PendingResolution::dnsServiceGetAddrInfo(DnsLookupFamily d
// TODO: explore caching: there are caching flags in the dns_sd.h flags, allow expired answers
// from the cache?
// TODO: explore validation via DNSSEC?
return DNSServiceGetAddrInfo(
return DnsServiceSingleton::get().dnsServiceGetAddrInfo(
&individual_sd_ref_, kDNSServiceFlagsShareConnection | kDNSServiceFlagsTimeout, 0, protocol,
dns_name_.c_str(),
/*
Expand Down
18 changes: 18 additions & 0 deletions source/common/network/apple_dns_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,30 @@
#include "common/common/linked_object.h"
#include "common/common/logger.h"
#include "common/common/utility.h"
#include "common/singleton/threadsafe_singleton.h"

#include "absl/container/node_hash_map.h"

namespace Envoy {
namespace Network {

// This abstraction allows for finer control in tests by using a mocked API. Production code simply
// forwards the function calls to Apple's API.
class DnsService {
public:
virtual ~DnsService() = default;
virtual void dnsServiceRefDeallocate(DNSServiceRef sdRef);
virtual DNSServiceErrorType dnsServiceCreateConnection(DNSServiceRef* sdRef);
virtual dnssd_sock_t dnsServiceRefSockFD(DNSServiceRef sdRef);
virtual DNSServiceErrorType dnsServiceProcessResult(DNSServiceRef sdRef);
virtual DNSServiceErrorType
dnsServiceGetAddrInfo(DNSServiceRef* sdRef, DNSServiceFlags flags, uint32_t interfaceIndex,
DNSServiceProtocol protocol, const char* hostname,
DNSServiceGetAddrInfoReply callBack, void* context);
};

using DnsServiceSingleton = ThreadSafeSingleton<DnsService>;

/**
* Implementation of DnsResolver that uses Apple dns_sd.h APIs. All calls and callbacks are assumed
* to happen on the thread that owns the creating dispatcher.
Expand Down
20 changes: 8 additions & 12 deletions test/common/network/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -103,29 +103,25 @@ envoy_cc_test(
"//bazel:apple": ["apple_dns_impl_test.cc"],
"//conditions:default": [],
}),
external_deps = ["abseil_synchronization"],
deps = [
"//include/envoy/event:dispatcher_interface",
"//include/envoy/network:address_interface",
"//include/envoy/network:dns_interface",
"//source/common/buffer:buffer_lib",
"//source/common/event:dispatcher_includes",
"//include/envoy/event:file_event_interface",
"//source/common/event:dispatcher_lib",
"//source/common/network:address_lib",
"//source/common/network:filter_lib",
"//source/common/network:listen_socket_lib",
"//source/common/stats:stats_lib",
"//source/common/stream_info:stream_info_lib",
"//test/mocks/network:network_mocks",
"//test/test_common:environment_lib",
"//test/test_common:network_utility_lib",
"//test/test_common:utility_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
"//test/mocks/local_info:local_info_mocks",
"//test/mocks/protobuf:protobuf_mocks",
"//test/mocks/runtime:runtime_mocks",
"//test/mocks/thread_local:thread_local_mocks",
"//test/test_common:threadsafe_singleton_injector_lib",
"//test/mocks/event:event_mocks",
] + select({
"//bazel:apple": ["//source/common/network:dns_lib"],
"//bazel:apple": [
"//source/common/network:dns_lib",
"//test/mocks/network:dns_service_mocks",
],
"//conditions:default": [],
}),
)
Expand Down
Loading