envoyproxy · zuercher · Jun 16, 2021 · Jun 10, 2021 · Jun 11, 2021 · Jun 12, 2021
diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc
@@ -254,22 +254,6 @@ Grpc::AsyncClientFactoryPtr Utility::factoryForGrpcApiConfigSource(
   return async_client_manager.factoryForGrpcService(grpc_service, scope, skip_cluster_check);
 }
 
-envoy::config::endpoint::v3::ClusterLoadAssignment Utility::translateClusterHosts(
-    const Protobuf::RepeatedPtrField<envoy::config::core::v3::Address>& hosts) {
-  envoy::config::endpoint::v3::ClusterLoadAssignment load_assignment;
-  envoy::config::endpoint::v3::LocalityLbEndpoints* locality_lb_endpoints =
-      load_assignment.add_endpoints();
-  // Since this LocalityLbEndpoints is built from hosts list, set the default weight to 1.
-  locality_lb_endpoints->mutable_load_balancing_weight()->set_value(1);
-  for (const envoy::config::core::v3::Address& host : hosts) {
-    envoy::config::endpoint::v3::LbEndpoint* lb_endpoint =
-        locality_lb_endpoints->add_lb_endpoints();
-    lb_endpoint->mutable_endpoint()->mutable_address()->MergeFrom(host);
-    lb_endpoint->mutable_load_balancing_weight()->set_value(1);
-  }
-  return load_assignment;
-}
-
 void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config,
                                     const ProtobufWkt::Struct& config,
                                     ProtobufMessage::ValidationVisitor& validation_visitor,

diff --git a/source/common/config/utility.h b/source/common/config/utility.h
@@ -451,14 +451,6 @@ class Utility {
                                 const envoy::config::core::v3::ApiConfigSource& api_config_source,
                                 Stats::Scope& scope, bool skip_cluster_check);
 
-  /**
-   * Translate a set of cluster's hosts into a load assignment configuration.
-   * @param hosts cluster's list of hosts.
-   * @return envoy::config::endpoint::v3::ClusterLoadAssignment a load assignment configuration.
-   */
-  static envoy::config::endpoint::v3::ClusterLoadAssignment
-  translateClusterHosts(const Protobuf::RepeatedPtrField<envoy::config::core::v3::Address>& hosts);
-
   /**
    * Translate opaque config from google.protobuf.Any or google.protobuf.Struct to defined proto
    * message.

diff --git a/source/common/config/version_converter.cc b/source/common/config/version_converter.cc
@@ -110,7 +110,7 @@ void VersionConverter::annotateWithOriginalType(const Protobuf::Descriptor& prev
       // ClusterManagerImpl with type erasure, but protobuf doesn't free up memory
       // as expected, we probably need some arena level trick to address this.
       if (prev_descriptor->full_name() == "envoy.api.v2.Cluster" &&
-          (field.name() == "hidden_envoy_deprecated_hosts" || field.name() == "load_assignment")) {
+          field.name() == "load_assignment") {
         // This will cause the sub-message visit to abort early.
         return field.message_type();
       }

diff --git a/source/common/upstream/logical_dns_cluster.cc b/source/common/upstream/logical_dns_cluster.cc
@@ -57,10 +57,7 @@ LogicalDnsCluster::LogicalDnsCluster(
       resolve_timer_(
           factory_context.dispatcher().createTimer([this]() -> void { startResolve(); })),
       local_info_(factory_context.localInfo()),
-      load_assignment_(
-          cluster.has_load_assignment()
-              ? convertPriority(cluster.load_assignment())
-              : Config::Utility::translateClusterHosts(cluster.hidden_envoy_deprecated_hosts())) {
+      load_assignment_(convertPriority(cluster.load_assignment())) {
   failure_backoff_strategy_ =
       Config::Utility::prepareDnsRefreshStrategy<envoy::config::cluster::v3::Cluster>(
           cluster, dns_refresh_rate_ms_.count(), factory_context.api().randomGenerator());

diff --git a/source/common/upstream/original_dst_cluster.cc b/source/common/upstream/original_dst_cluster.cc
@@ -119,9 +119,8 @@ OriginalDstCluster::OriginalDstCluster(
                            ? info_->lbOriginalDstConfig().value().use_http_header()
                            : false),
       host_map_(std::make_shared<HostMap>()) {
-  // TODO(dio): Remove hosts check once the hosts field is removed.
-  if (config.has_load_assignment() || !config.hidden_envoy_deprecated_hosts().empty()) {
-    throw EnvoyException("ORIGINAL_DST clusters must have no load assignment or hosts configured");
+  if (config.has_load_assignment()) {
+    throw EnvoyException("ORIGINAL_DST clusters must have no load assignment configured");
   }
   cleanup_timer_->enableTimer(cleanup_interval_ms_);
 }

diff --git a/source/common/upstream/static_cluster.cc b/source/common/upstream/static_cluster.cc
@@ -15,12 +15,8 @@ StaticClusterImpl::StaticClusterImpl(
                       factory_context.dispatcher().timeSource()),
       priority_state_manager_(
           new PriorityStateManager(*this, factory_context.localInfo(), nullptr)) {
-  // TODO(dio): Use by-reference when cluster.hosts() is removed.
-  const envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment(
-      cluster.has_load_assignment()
-          ? cluster.load_assignment()
-          : Config::Utility::translateClusterHosts(cluster.hidden_envoy_deprecated_hosts()));
-
+  const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment =
+      cluster.load_assignment();
   overprovisioning_factor_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
       cluster_load_assignment.policy(), overprovisioning_factor, kDefaultOverProvisioningFactor);
 

diff --git a/source/common/upstream/strict_dns_cluster.cc b/source/common/upstream/strict_dns_cluster.cc
@@ -15,11 +15,8 @@ StrictDnsClusterImpl::StrictDnsClusterImpl(
     Stats::ScopePtr&& stats_scope, bool added_via_api)
     : BaseDynamicClusterImpl(cluster, runtime, factory_context, std::move(stats_scope),
                              added_via_api, factory_context.dispatcher().timeSource()),
-      load_assignment_{
-          cluster.has_load_assignment()
-              ? cluster.load_assignment()
-              : Config::Utility::translateClusterHosts(cluster.hidden_envoy_deprecated_hosts())},
-      local_info_(factory_context.localInfo()), dns_resolver_(dns_resolver),
+      load_assignment_(cluster.load_assignment()), local_info_(factory_context.localInfo()),
+      dns_resolver_(dns_resolver),
       dns_refresh_rate_ms_(
           std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(cluster, dns_refresh_rate, 5000))),
       respect_dns_ttl_(cluster.respect_dns_ttl()) {

@@ -39,12 +39,9 @@ RedisCluster::RedisCluster(
       host_degraded_refresh_threshold_(redis_cluster.host_degraded_refresh_threshold()),
       dispatcher_(factory_context.dispatcher()), dns_resolver_(std::move(dns_resolver)),
       dns_lookup_family_(Upstream::getDnsLookupFamilyFromCluster(cluster)),
-      load_assignment_(
-          cluster.has_load_assignment()
-              ? cluster.load_assignment()
-              : Config::Utility::translateClusterHosts(cluster.hidden_envoy_deprecated_hosts())),
-      local_info_(factory_context.localInfo()), random_(api.randomGenerator()),
-      redis_discovery_session_(*this, redis_client_factory), lb_factory_(std::move(lb_factory)),
+      load_assignment_(cluster.load_assignment()), local_info_(factory_context.localInfo()),
+      random_(api.randomGenerator()), redis_discovery_session_(*this, redis_client_factory),
+      lb_factory_(std::move(lb_factory)),
       auth_username_(
           NetworkFilters::RedisProxy::ProtocolOptionsConfigImpl::authUsername(info(), api)),
       auth_password_(

diff --git a/test/common/config/version_converter_test.cc b/test/common/config/version_converter_test.cc
@@ -32,7 +32,6 @@ bool hasOriginalTypeInformation(const Protobuf::Message& message) {
 TEST(VersionConverterTest, Upgrade) {
   // Create a v2 Cluster message with some fields set.
   API_NO_BOOST(envoy::api::v2::Cluster) source;
-  source.add_hosts();
   source.mutable_load_assignment()->set_cluster_name("bar");
   source.mutable_eds_cluster_config()->set_service_name("foo");
   source.set_drain_connections_on_host_removal(true);
@@ -41,8 +40,6 @@ TEST(VersionConverterTest, Upgrade) {
   VersionConverter::upgrade(source, dst);
   // Verify fields in v3 Cluster.
   EXPECT_TRUE(hasOriginalTypeInformation(dst));
-  EXPECT_FALSE(dst.hidden_envoy_deprecated_hosts().empty());
-  EXPECT_FALSE(hasOriginalTypeInformation(dst.hidden_envoy_deprecated_hosts(0)));
   EXPECT_EQ("bar", dst.load_assignment().cluster_name());
   EXPECT_FALSE(hasOriginalTypeInformation(dst.load_assignment()));
   EXPECT_EQ("foo", dst.eds_cluster_config().service_name());

diff --git a/test/common/upstream/original_dst_cluster_test.cc b/test/common/upstream/original_dst_cluster_test.cc
@@ -141,28 +141,8 @@ TEST_F(OriginalDstClusterTest, BadConfigWithLoadAssignment) {
                 port_value: 8000
   )EOF";
 
-  EXPECT_THROW_WITH_MESSAGE(
-      setupFromYaml(yaml), EnvoyException,
-      "ORIGINAL_DST clusters must have no load assignment or hosts configured");
-}
-
-TEST_F(OriginalDstClusterTest, DEPRECATED_FEATURE_TEST(BadConfigWithDeprecatedHosts)) {
-  TestDeprecatedV2Api _deprecated_v2_api;
-  const std::string yaml = R"EOF(
-    name: name
-    connect_timeout: 0.25s
-    type: ORIGINAL_DST
-    lb_policy: ORIGINAL_DST_LB
-    cleanup_interval: 1s
-    hosts:
-      - socket_address:
-          address: 127.0.0.1
-          port_value: 8000
-  )EOF";
-
-  EXPECT_THROW_WITH_MESSAGE(
-      setupFromYaml(yaml, false), EnvoyException,
-      "ORIGINAL_DST clusters must have no load assignment or hosts configured");
+  EXPECT_THROW_WITH_MESSAGE(setupFromYaml(yaml), EnvoyException,
+                            "ORIGINAL_DST clusters must have no load assignment configured");
 }
 
 TEST_F(OriginalDstClusterTest, CleanupInterval) {

diff --git a/test/config/utility.cc b/test/config/utility.cc
@@ -610,9 +610,6 @@ ConfigHelper::ConfigHelper(const Network::Address::IpVersion version, Api::Api&
 
   for (int i = 0; i < static_resources->clusters_size(); ++i) {
     auto* cluster = static_resources->mutable_clusters(i);
-    RELEASE_ASSERT(
-        cluster->hidden_envoy_deprecated_hosts().empty(),
-        "Hosts should be specified via load_assignment() in the integration test framework.");
     for (int j = 0; j < cluster->load_assignment().endpoints_size(); ++j) {
       auto* locality_lb = cluster->mutable_load_assignment()->mutable_endpoints(j);
       for (int k = 0; k < locality_lb->lb_endpoints_size(); ++k) {
@@ -831,9 +828,6 @@ void ConfigHelper::finalize(const std::vector<uint32_t>& ports) {
       custom_cluster = true;
     } else {
       // Assign ports to statically defined load_assignment hosts.
-      RELEASE_ASSERT(
-          cluster->hidden_envoy_deprecated_hosts().empty(),
-          "Hosts should be specified via load_assignment() in the integration test framework.");
       for (int j = 0; j < cluster->load_assignment().endpoints_size(); ++j) {
         auto locality_lb = cluster->mutable_load_assignment()->mutable_endpoints(j);
         for (int k = 0; k < locality_lb->lb_endpoints_size(); ++k) {

diff --git a/test/server/server_fuzz_test.cc b/test/server/server_fuzz_test.cc
@@ -63,10 +63,6 @@ makeHermeticPathsAndPorts(Fuzz::PerTestEnvironment& test_env,
         health_check.mutable_http_health_check()->clear_codec_client_type();
       }
     }
-    // We may have both deprecated hosts() or load_assignment().
-    for (auto& host : *cluster.mutable_hidden_envoy_deprecated_hosts()) {
-      makePortHermetic(test_env, host);
-    }
     for (int j = 0; j < cluster.load_assignment().endpoints_size(); ++j) {
       auto* locality_lb = cluster.mutable_load_assignment()->mutable_endpoints(j);
       for (int k = 0; k < locality_lb->lb_endpoints_size(); ++k) {