listener: fix ipv6 error

[ramaraochavali]

Signed-off-by: Rama rama.rao@salesforce.com
Description:
Fixes the bug #3911
Risk Level: Low
Testing: Automated
Docs Changes: N/A
Release Notes: N/A

Fixes #3911

[ramaraochavali]

@PiotrSikora I am not 100% sure if this is the right fix because I am fully familiar with this part. PTAL and suggest. I can change based on your feedback.

[PiotrSikora]

I think we need support IPv6-only setups as well:

diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc
index 393fdfe1e..e69bebe4b 100644
--- a/source/server/listener_manager_impl.cc
+++ b/source/server/listener_manager_impl.cc
@@ -380,17 +380,19 @@ void ListenerImpl::convertDestinationIPsMapToTrie() {
     for (const auto& entry : destination_ips_map) {
       std::vector<Network::Address::CidrRange> subnets;
       if (entry.first == EMPTY_STRING) {
-        list.push_back(
-            std::make_pair<ServerNamesMapSharedPtr, std::vector<Network::Address::CidrRange>>(
-                std::make_shared<ServerNamesMap>(entry.second),
-                {Network::Address::CidrRange::create("0.0.0.0/0"),
-                 Network::Address::CidrRange::create("::/0")}));
+        if (Network::Address::ipFamilySupported(AF_INET)) {
+          subnets.push_back(Network::Address::CidrRange::create("0.0.0.0/0"));
+        }
+        if (Network::Address::ipFamilySupported(AF_INET6)) {
+          subnets.push_back(Network::Address::CidrRange::create("::/0"));
+        }
       } else {
-        list.push_back(
-            std::make_pair<ServerNamesMapSharedPtr, std::vector<Network::Address::CidrRange>>(
-                std::make_shared<ServerNamesMap>(entry.second),
-                {Network::Address::CidrRange::create(entry.first)}));
+        subnets.push_back(Network::Address::CidrRange::create(entry.first));
       }
+      list.push_back(
+          std::make_pair<ServerNamesMapSharedPtr, std::vector<Network::Address::CidrRange>>(
+              std::make_shared<ServerNamesMap>(entry.second),
+              std::vector<Network::Address::CidrRange>(subnets)));
     }
     destination_ips_pair.second = std::make_unique<DestinationIPsTrie>(list, true);
   }

(diff against master branch, since it's more readable)

[PiotrSikora]

cc @ggreenway

[ggreenway]

I agree with @PiotrSikora

[PiotrSikora]

Could you use the patch I provided (using unused subnets and sharing more of the code with the != EMPTY_STRING path)? It's a bit more readable, IMHO.

[ramaraochavali]

Updated. Please check

[PiotrSikora]

Thanks!

[ggreenway]

Ideally I'd like to have some test coverage (which would require mocking ipFamilySupported() or it's underlying syscalls). But I'm ok with committing this as-is if it's urgent for you.

[ramaraochavali]

@ggreenway Thanks. I was thinking about such a test and checked to see if we can write a mock but looks like for functions like this writing mock is not possible with gmocks. If there is any possible way to do it, do let me know. I will look in to it.

[ggreenway]

@ramaraochavali The way to mock this is to add socket() to OsSysCalls (see envoy/include/envoy/api/os_sys_calls.h). Then use OsSysCalls::socket() in ipFamilySupported(). You can then mock the behavior of ::socket().