Skip to content

Commit

Permalink
apple_dns: Add error code in the DNS resolution details on completion (…
Browse files Browse the repository at this point in the history 
…envoyproxy#36617)

Commit Message: Renames the resolution details from `apple_dns_success`
to `apple_dns_completed` with error code since `completed` is more
correct that `success` given that the callback will still be called when
the error code is `kDNSServiceErr_NoSuchRecord`.
Additional Description: This behavior is also consistent with the
implementations of other resolvers.
-
[getaddrinfo](https://github.com/envoyproxy/envoy/blob/7847268600ed62e2485fc0c4ab853e7fd79ddf63/source/extensions/network/dns_resolver/getaddrinfo/getaddrinfo.cc#L216)
-
[c-ares](https://github.com/envoyproxy/envoy/blob/7847268600ed62e2485fc0c4ab853e7fd79ddf63/source/extensions/network/dns_resolver/cares/dns_impl.cc#L219)

Risk Level: low
Testing: unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: apple_dns

---------

Signed-off-by: Fredy Wijaya <[email protected]>
  • Loading branch information
@fredyw
fredyw authored Oct 16, 2024
1 parent c811a88 commit 4804af4
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 15 deletions.
2 changes: 1 addition & 1 deletion source/extensions/network/dns_resolver/apple/apple_dns_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ void AppleDnsResolverImpl::PendingResolution::onDNSServiceGetAddrInfoReply(
isAddressFamilyProcessed(kDNSServiceProtocol_IPv6)) {
ENVOY_LOG(debug, "DNS Resolver flushing queries pending callback");
pending_response_.status_ = ResolutionStatus::Completed;
pending_response_.details_ = "apple_dns_success";
pending_response_.details_ = absl::StrCat("apple_dns_completed_", error_code);
finishResolve();
// Note: Nothing can follow this call to finishResolve due to deletion of this
// object upon resolution.
Expand Down
28 changes: 14 additions & 14 deletions test/extensions/network/dns_resolver/apple/apple_dns_impl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ TEST_F(AppleDnsImplTest, DnsIpAddressVersionAllSupportsV6Only) {
[=](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& results) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
// On v4 only networks, there will be no results.
for (const auto& result : results) {
const auto& addrinfo = result.addrInfo();
Expand Down Expand Up @@ -368,7 +368,7 @@ TEST_F(AppleDnsImplTest, DnsIpAddressVersionV6OnlySupportsV4Only) {
[=](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& results) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
for (const auto& result : results) {
const auto& addrinfo = result.addrInfo();
EXPECT_THAT(addrinfo.address_->ip()->ipv6(), NotNull());
Expand All @@ -386,7 +386,7 @@ TEST_F(AppleDnsImplTest, DnsIpAddressVersionV6OnlySupportsV6Only) {
[=](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& results) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_FALSE(results.empty());
for (const auto& result : results) {
const auto& addrinfo = result.addrInfo();
Expand All @@ -411,7 +411,7 @@ TEST_F(AppleDnsImplTest, DnsIpAddressVersionAutoSupportsV6Only) {
[=](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& results) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
// On v4 only networks, there will be no results.
for (const auto& result : results) {
const auto& addrinfo = result.addrInfo();
Expand All @@ -436,7 +436,7 @@ TEST_F(AppleDnsImplTest, DnsIpAddressVersionV4PreferredSupportsV6Only) {
[=](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& results) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
// On v4 only networks, there will be no results.
for (const auto& result : results) {
const auto& addrinfo = result.addrInfo();
Expand Down Expand Up @@ -660,7 +660,7 @@ class AppleDnsImplFakeApiTest : public testing::Test {
expected_address_size](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& response) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_EQ(expected_address_size, response.size());

if (dns_lookup_family == DnsLookupFamily::Auto) {
Expand Down Expand Up @@ -918,7 +918,7 @@ TEST_F(AppleDnsImplFakeApiTest, QuerySynchronousCompletionUnroutableFamilies) {
absl::string_view details,
std::list<DnsResponse>&& response) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_EQ(1, response.size());
EXPECT_EQ("1.2.3.4:0", response.front().addrInfo().address_->asString());
EXPECT_EQ(std::chrono::seconds(30), response.front().addrInfo().ttl_);
Expand Down Expand Up @@ -958,7 +958,7 @@ TEST_F(AppleDnsImplFakeApiTest, QuerySynchronousCompletion) {
absl::string_view details,
std::list<DnsResponse>&& response) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_EQ(1, response.size());
EXPECT_EQ("1.2.3.4:0", response.front().addrInfo().address_->asString());
EXPECT_EQ(std::chrono::seconds(30), response.front().addrInfo().ttl_);
Expand Down Expand Up @@ -1015,7 +1015,7 @@ TEST_F(AppleDnsImplFakeApiTest, MultipleAddresses) {
[&dns_callback_executed](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& response) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_EQ(2, response.size());
dns_callback_executed.Notify();
});
Expand Down Expand Up @@ -1148,7 +1148,7 @@ TEST_F(AppleDnsImplFakeApiTest, MultipleQueries) {
[&dns_callback_executed](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& response) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_EQ(1, response.size());
EXPECT_EQ("1.2.3.4:0", response.front().addrInfo().address_->asString());
EXPECT_EQ(std::chrono::seconds(30), response.front().addrInfo().ttl_);
Expand All @@ -1170,7 +1170,7 @@ TEST_F(AppleDnsImplFakeApiTest, MultipleQueries) {
[&dns_callback_executed2](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& response) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_EQ(1, response.size());
EXPECT_EQ("5.6.7.8:0", response.front().addrInfo().address_->asString());
EXPECT_EQ(std::chrono::seconds(30), response.front().addrInfo().ttl_);
Expand Down Expand Up @@ -1225,7 +1225,7 @@ TEST_F(AppleDnsImplFakeApiTest, MultipleQueriesOneFails) {
// Even though the second query will fail, this one will flush with the
// state it had.
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_EQ(1, response.size());
EXPECT_EQ("1.2.3.4:0", response.front().addrInfo().address_->asString());
EXPECT_EQ(std::chrono::seconds(30), response.front().addrInfo().ttl_);
Expand Down Expand Up @@ -1289,7 +1289,7 @@ TEST_F(AppleDnsImplFakeApiTest, ResultWithOnlyNonAdditiveReplies) {
[&dns_callback_executed](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& response) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_TRUE(response.empty());
dns_callback_executed.Notify();
});
Expand Down Expand Up @@ -1367,7 +1367,7 @@ TEST_F(AppleDnsImplFakeApiTest, DeallocateOnDestruction) {
[&dns_callback_executed](DnsResolver::ResolutionStatus status, absl::string_view details,
std::list<DnsResponse>&& response) -> void {
EXPECT_EQ(DnsResolver::ResolutionStatus::Completed, status);
EXPECT_EQ(details, "apple_dns_success");
EXPECT_THAT(details, StartsWith("apple_dns_completed"));
EXPECT_EQ(1, response.size());
dns_callback_executed.Notify();
});
Expand Down

0 comments on commit 4804af4

Please sign in to comment.