Fix unexpected output of endpoint additional_addresses dump (#35120)

Commit Message: Fix unexpected output of endpoint additional_addresses
dump  

- Fix static clusters not setting addresses_list for hosts which led to
incomplete configdumps of lb endpoints
- Fix a mistake in #34755 which led to adding the default_address
in the additional_addresses for dynamic clusters.

Signed-off-by: Leonardo Sarra <[email protected]>

source/extensions/clusters/static/static_cluster.cc

@@ -25,11 +25,22 @@ StaticClusterImpl::StaticClusterImpl(const envoy::config::cluster::v3::Cluster&
     THROW_IF_NOT_OK(validateEndpointsForZoneAwareRouting(locality_lb_endpoint));
     priority_state_manager_->initializePriorityFor(locality_lb_endpoint);
     for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) {
+      std::vector<Network::Address::InstanceConstSharedPtr> address_list;
+      if (!lb_endpoint.endpoint().additional_addresses().empty()) {
+        address_list.emplace_back(
+            THROW_OR_RETURN_VALUE(resolveProtoAddress(lb_endpoint.endpoint().address()),
+                                  const Network::Address::InstanceConstSharedPtr));
+        for (const auto& additional_address : lb_endpoint.endpoint().additional_addresses()) {
+          address_list.emplace_back(
+              THROW_OR_RETURN_VALUE(resolveProtoAddress(additional_address.address()),
+                                    const Network::Address::InstanceConstSharedPtr));
+        }
+      }
       priority_state_manager_->registerHostForPriority(
           lb_endpoint.endpoint().hostname(),
           THROW_OR_RETURN_VALUE(resolveProtoAddress(lb_endpoint.endpoint().address()),
                                 const Network::Address::InstanceConstSharedPtr),
-          {}, locality_lb_endpoint, lb_endpoint, dispatcher.timeSource());
+          address_list, locality_lb_endpoint, lb_endpoint, dispatcher.timeSource());
     }
   }
 }

source/server/admin/config_dump_handler.cc

@@ -384,10 +384,13 @@ void ConfigDumpHandler::addLbEndpoint(
   endpoint.set_hostname(host->hostname());
   Network::Utility::addressToProtobufAddress(*host->address(), *endpoint.mutable_address());
   if (host->addressListOrNull() != nullptr) {
-    for (auto& additional_address : *host->addressListOrNull()) {
-      auto& new_address = *endpoint.mutable_additional_addresses()->Add();
-      Network::Utility::addressToProtobufAddress(*additional_address,
-                                                 *new_address.mutable_address());
+    const auto& address_list = *host->addressListOrNull();
+    if (address_list.size() > 1) {
+      // skip first address of the list as the default address is not an additional one.
+      for (auto it = std::next(address_list.begin()); it != address_list.end(); ++it) {
+        auto& new_address = *endpoint.mutable_additional_addresses()->Add();
+        Network::Utility::addressToProtobufAddress(**it, *new_address.mutable_address());
+      }
     }
   }
   auto& health_check_config = *endpoint.mutable_health_check_config();

test/common/upstream/cluster_manager_impl_test.cc

@@ -6330,6 +6330,85 @@ TEST_F(ClusterManagerImplTest, ConnectionPoolPerDownstreamConnection) {
                                                          Http::Protocol::Http11, &lb_context)));
 }
 
+TEST_F(ClusterManagerImplTest, CheckAddressesList) {
+  const std::string bootstrap = R"EOF(
+  static_resources:
+    clusters:
+    - name: cluster_0
+      connect_timeout: 0.250s
+      type: STATIC
+      lb_policy: ROUND_ROBIN
+      load_assignment:
+        cluster_name: cluster_0
+        endpoints:
+        - lb_endpoints:
+          - endpoint:
+              additionalAddresses:
+              - address:
+                  socketAddress:
+                    address: ::1
+                    portValue: 11001
+              address:
+                socket_address:
+                  address: 127.0.0.1
+                  port_value: 11001
+  )EOF";
+  create(parseBootstrapFromV3Yaml(bootstrap));
+  // Verify address list for static cluster in bootstrap.
+  auto cluster = cluster_manager_->getThreadLocalCluster("cluster_0");
+  auto hosts = cluster->prioritySet().hostSetsPerPriority()[0]->hosts();
+  ASSERT_NE(hosts[0]->addressListOrNull(), nullptr);
+  ASSERT_EQ(hosts[0]->addressListOrNull()->size(), 2);
+
+  const std::string cluster_api = R"EOF(
+    name: added_via_api
+    connect_timeout: 0.250s
+    type: STATIC
+    lb_policy: ROUND_ROBIN
+    load_assignment:
+      cluster_name: added_via_api
+      endpoints:
+      - lb_endpoints:
+        - endpoint:
+            address:
+              socket_address:
+                address: 127.0.0.1
+                port_value: 11001
+  )EOF";
+  const std::string cluster_api_update = R"EOF(
+    name: added_via_api
+    connect_timeout: 0.250s
+    type: STATIC
+    lb_policy: ROUND_ROBIN
+    load_assignment:
+      cluster_name: added_via_api
+      endpoints:
+      - lb_endpoints:
+        - endpoint:
+            additionalAddresses:
+              - address:
+                  socketAddress:
+                    address: ::1
+                    portValue: 11001
+            address:
+              socket_address:
+                address: 127.0.0.1
+                port_value: 11001
+  )EOF";
+  // Add static cluster via api and check that addresses list is empty.
+  EXPECT_TRUE(cluster_manager_->addOrUpdateCluster(parseClusterFromV3Yaml(cluster_api), "v1"));
+  cluster = cluster_manager_->getThreadLocalCluster("added_via_api");
+  hosts = cluster->prioritySet().hostSetsPerPriority()[0]->hosts();
+  ASSERT_EQ(hosts[0]->addressListOrNull(), nullptr);
+  // Update cluster to have additional addresses and check that address list is not empty anymore.
+  EXPECT_TRUE(
+      cluster_manager_->addOrUpdateCluster(parseClusterFromV3Yaml(cluster_api_update), "v2"));
+  cluster = cluster_manager_->getThreadLocalCluster("added_via_api");
+  hosts = cluster->prioritySet().hostSetsPerPriority()[0]->hosts();
+  ASSERT_NE(hosts[0]->addressListOrNull(), nullptr);
+  ASSERT_EQ(hosts[0]->addressListOrNull()->size(), 2);
+}
+
 TEST_F(ClusterManagerImplTest, CheckActiveStaticCluster) {
   const std::string yaml = R"EOF(
   static_resources:

test/server/admin/config_dump_handler_test.cc

@@ -27,6 +27,7 @@ void addHostInfo(NiceMock<Upstream::MockHost>& host, const std::string& hostname
   ON_CALL(host, address()).WillByDefault(Return(address));
   std::shared_ptr<Upstream::HostImplBase::AddressVector> address_list =
       std::make_shared<Upstream::HostImplBase::AddressVector>();
+  address_list->push_back(*Network::Utility::resolveUrl(address_url));
   for (auto& new_addr : additional_addresses_url) {
     address_list->push_back(*Network::Utility::resolveUrl(new_addr));
   }