Skip to content

Commit

Permalink
forward proxy: introduce DnsCacheImpl::getHost() (envoyproxy#13530)
Browse files Browse the repository at this point in the history
Make DynamicForwardProxy::DnsCache support retrieving the DnsHostInfo of a
given host name from the cached host map.

Risk Level: Low
Testing: //test/extensions/common/dynamic_forward_proxy:dns_cache_impl_test
Docs Changes: N/A
Release Notes: N/A
Fixes envoyproxy#13505

Signed-off-by: Tal Nordan <[email protected]>
  • Loading branch information
talnordan authored Oct 22, 2020
1 parent 1010982 commit 0dd4b61
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 0 deletions.
8 changes: 8 additions & 0 deletions source/extensions/common/dynamic_forward_proxy/dns_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ class DnsCache {
*/
virtual absl::flat_hash_map<std::string, DnsHostInfoSharedPtr> hosts() PURE;

/**
* Retrieve the DNS host info of a given host currently stored in the cache.
* @param host_name supplies the host name.
* @return the DNS host info associated with the given host name if the host's address is cached,
* otherwise `absl::nullopt`.
*/
virtual absl::optional<const DnsHostInfoSharedPtr> getHost(absl::string_view host_name) PURE;

/**
* Check if a DNS request is allowed given resource limits.
* @param pending_request optional pending request resource limit. If no resource limit is
Expand Down
19 changes: 19 additions & 0 deletions source/extensions/common/dynamic_forward_proxy/dns_cache_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,25 @@ absl::flat_hash_map<std::string, DnsHostInfoSharedPtr> DnsCacheImpl::hosts() {
return ret;
}

absl::optional<const DnsHostInfoSharedPtr> DnsCacheImpl::getHost(absl::string_view host_name) {
// Find a host with the given name.
auto it = primary_hosts_.find(host_name);
if (it == primary_hosts_.end()) {
return {};
}

// Extract host info.
auto&& host_info = it->second->host_info_;

// Only include hosts that have ever resolved to an address.
if (host_info->address_ == nullptr) {
return {};
}

// Return host info.
return host_info;
}

DnsCacheImpl::AddUpdateCallbacksHandlePtr
DnsCacheImpl::addUpdateCallbacks(UpdateCallbacks& callbacks) {
return std::make_unique<AddUpdateCallbacksHandleImpl>(update_callbacks_, callbacks);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class DnsCacheImpl : public DnsCache, Logger::Loggable<Logger::Id::forward_proxy
LoadDnsCacheEntryCallbacks& callbacks) override;
AddUpdateCallbacksHandlePtr addUpdateCallbacks(UpdateCallbacks& callbacks) override;
absl::flat_hash_map<std::string, DnsHostInfoSharedPtr> hosts() override;
absl::optional<const DnsHostInfoSharedPtr> getHost(absl::string_view host_name) override;
Upstream::ResourceAutoIncDecPtr
canCreateDnsRequest(ResourceLimitOptRef pending_requests) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ TEST_F(DnsCacheImplTest, MultipleResolveDifferentHost) {
auto result1 = dns_cache_->loadDnsCacheEntry("foo.com", 80, callbacks1);
EXPECT_EQ(DnsCache::LoadDnsCacheEntryStatus::Loading, result1.status_);
EXPECT_NE(result1.handle_, nullptr);
EXPECT_EQ(dns_cache_->getHost("foo.com"), absl::nullopt);

MockLoadDnsCacheEntryCallbacks callbacks2;
Network::DnsResolver::ResolveCb resolve_cb2;
Expand All @@ -554,6 +555,7 @@ TEST_F(DnsCacheImplTest, MultipleResolveDifferentHost) {
auto result2 = dns_cache_->loadDnsCacheEntry("bar.com", 443, callbacks2);
EXPECT_EQ(DnsCache::LoadDnsCacheEntryStatus::Loading, result2.status_);
EXPECT_NE(result2.handle_, nullptr);
EXPECT_EQ(dns_cache_->getHost("bar.com"), absl::nullopt);

EXPECT_CALL(update_callbacks_,
onDnsHostAddOrUpdate("bar.com", DnsHostInfoEquals("10.0.0.1:443", "bar.com", false)));
Expand All @@ -571,6 +573,14 @@ TEST_F(DnsCacheImplTest, MultipleResolveDifferentHost) {
EXPECT_EQ(2, hosts.size());
EXPECT_THAT(hosts["bar.com"], DnsHostInfoEquals("10.0.0.1:443", "bar.com", false));
EXPECT_THAT(hosts["foo.com"], DnsHostInfoEquals("10.0.0.2:80", "foo.com", false));

EXPECT_TRUE(dns_cache_->getHost("bar.com").has_value());
EXPECT_THAT(dns_cache_->getHost("bar.com").value(),
DnsHostInfoEquals("10.0.0.1:443", "bar.com", false));
EXPECT_TRUE(dns_cache_->getHost("foo.com").has_value());
EXPECT_THAT(dns_cache_->getHost("foo.com").value(),
DnsHostInfoEquals("10.0.0.2:80", "foo.com", false));
EXPECT_EQ(dns_cache_->getHost("baz.com"), absl::nullopt);
}

// A successful resolve followed by a cache hit.
Expand Down
1 change: 1 addition & 0 deletions test/extensions/common/dynamic_forward_proxy/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class MockDnsCache : public DnsCache {
(UpdateCallbacks & callbacks));

MOCK_METHOD((absl::flat_hash_map<std::string, DnsHostInfoSharedPtr>), hosts, ());
MOCK_METHOD((absl::optional<const DnsHostInfoSharedPtr>), getHost, (absl::string_view));
MOCK_METHOD(Upstream::ResourceAutoIncDec*, canCreateDnsRequest_, (ResourceLimitOptRef));
};

Expand Down

0 comments on commit 0dd4b61

Please sign in to comment.