The ORIGINAL_DST_LB load balancing policy is deprecated, use CLUSTER_…

…PROVIDED policy instead when configuring an original destination cluster. (#17230) Risk Level: low Testing: bazel test //test/... Signed-off-by: Tianyu Xia <[email protected]>
envoyproxy · Jul 20, 2021 · e0380ad · e0380ad
1 parent 6acfb40
commit e0380ad
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 107 deletions.
diff --git a/source/common/upstream/original_dst_cluster.cc b/source/common/upstream/original_dst_cluster.cc
@@ -183,14 +183,12 @@ OriginalDstClusterFactory::createClusterImpl(
     const envoy::config::cluster::v3::Cluster& cluster, ClusterFactoryContext& context,
     Server::Configuration::TransportSocketFactoryContextImpl& socket_factory_context,
     Stats::ScopePtr&& stats_scope) {
-  if (cluster.lb_policy() !=
-          envoy::config::cluster::v3::Cluster::hidden_envoy_deprecated_ORIGINAL_DST_LB &&
-      cluster.lb_policy() != envoy::config::cluster::v3::Cluster::CLUSTER_PROVIDED) {
-    throw EnvoyException(fmt::format(
-        "cluster: LB policy {} is not valid for Cluster type {}. Only 'CLUSTER_PROVIDED' or "
-        "'ORIGINAL_DST_LB' is allowed with cluster type 'ORIGINAL_DST'",
-        envoy::config::cluster::v3::Cluster::LbPolicy_Name(cluster.lb_policy()),
-        envoy::config::cluster::v3::Cluster::DiscoveryType_Name(cluster.type())));
+  if (cluster.lb_policy() != envoy::config::cluster::v3::Cluster::CLUSTER_PROVIDED) {
+    throw EnvoyException(
+        fmt::format("cluster: LB policy {} is not valid for Cluster type {}. Only "
+                    "'CLUSTER_PROVIDED' is allowed with cluster type 'ORIGINAL_DST'",
+                    envoy::config::cluster::v3::Cluster::LbPolicy_Name(cluster.lb_policy()),
+                    envoy::config::cluster::v3::Cluster::DiscoveryType_Name(cluster.type())));
   }
 
   // TODO(mattklein123): The original DST load balancer type should be deprecated and instead

diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc
@@ -787,22 +787,6 @@ ClusterInfoImpl::ClusterInfoImpl(
   case envoy::config::cluster::v3::Cluster::RING_HASH:
     lb_type_ = LoadBalancerType::RingHash;
     break;
-  case envoy::config::cluster::v3::Cluster::hidden_envoy_deprecated_ORIGINAL_DST_LB:
-    if (config.type() != envoy::config::cluster::v3::Cluster::ORIGINAL_DST) {
-      throw EnvoyException(
-          fmt::format("cluster: LB policy {} is not valid for Cluster type {}. 'ORIGINAL_DST_LB' "
-                      "is allowed only with cluster type 'ORIGINAL_DST'",
-                      envoy::config::cluster::v3::Cluster::LbPolicy_Name(config.lb_policy()),
-                      envoy::config::cluster::v3::Cluster::DiscoveryType_Name(config.type())));
-    }
-    if (config.has_lb_subset_config()) {
-      throw EnvoyException(
-          fmt::format("cluster: LB policy {} cannot be combined with lb_subset_config",
-                      envoy::config::cluster::v3::Cluster::LbPolicy_Name(config.lb_policy())));
-    }
-
-    lb_type_ = LoadBalancerType::ClusterProvided;
-    break;
   case envoy::config::cluster::v3::Cluster::MAGLEV:
     lb_type_ = LoadBalancerType::Maglev;
     break;

diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc
@@ -502,32 +502,7 @@ TEST_F(ClusterManagerImplTest, OriginalDstLbRestriction) {
   EXPECT_THROW_WITH_MESSAGE(
       create(parseBootstrapFromV3Yaml(yaml)), EnvoyException,
       "cluster: LB policy ROUND_ROBIN is not valid for Cluster type ORIGINAL_DST. Only "
-      "'CLUSTER_PROVIDED' or 'ORIGINAL_DST_LB' is allowed with cluster type 'ORIGINAL_DST'");
-}
-
-TEST_F(ClusterManagerImplTest, DEPRECATED_FEATURE_TEST(OriginalDstLbRestriction2)) {
-  TestDeprecatedV2Api _deprecated_v2_api;
-  const std::string yaml = R"EOF(
- static_resources:
-  clusters:
-  - name: cluster_1
-    connect_timeout: 0.250s
-    type: static
-    lb_policy: original_dst_lb
-    load_assignment:
-      endpoints:
-      - lb_endpoints:
-        - endpoint:
-            address:
-              socket_address:
-                address: 127.0.0.1
-                port_value: 11001
-  )EOF";
-
-  EXPECT_THROW_WITH_MESSAGE(create(parseBootstrapFromV3Yaml(yaml, false)), EnvoyException,
-                            "cluster: LB policy hidden_envoy_deprecated_ORIGINAL_DST_LB is not "
-                            "valid for Cluster type STATIC. "
-                            "'ORIGINAL_DST_LB' is allowed only with cluster type 'ORIGINAL_DST'");
+      "'CLUSTER_PROVIDED' is allowed with cluster type 'ORIGINAL_DST'");
 }
 
 class ClusterManagerSubsetInitializationTest
@@ -545,7 +520,9 @@ class ClusterManagerSubsetInitializationTest
     for (int i = first; i <= last; i++) {
       if (envoy::config::cluster::v3::Cluster::LbPolicy_IsValid(i)) {
         auto policy = static_cast<envoy::config::cluster::v3::Cluster::LbPolicy>(i);
-        if (policy != envoy::config::cluster::v3::Cluster::LOAD_BALANCING_POLICY_CONFIG) {
+        if (policy !=
+                envoy::config::cluster::v3::Cluster::hidden_envoy_deprecated_ORIGINAL_DST_LB &&
+            policy != envoy::config::cluster::v3::Cluster::LOAD_BALANCING_POLICY_CONFIG) {
           policies.push_back(policy);
         }
       }
@@ -591,16 +568,14 @@ TEST_P(ClusterManagerSubsetInitializationTest, SubsetLoadBalancerInitialization)
   const std::string& policy_name = envoy::config::cluster::v3::Cluster::LbPolicy_Name(GetParam());
 
   std::string cluster_type = "type: STATIC";
-  if (GetParam() == envoy::config::cluster::v3::Cluster::hidden_envoy_deprecated_ORIGINAL_DST_LB) {
-    cluster_type = "type: ORIGINAL_DST";
-  } else if (GetParam() == envoy::config::cluster::v3::Cluster::CLUSTER_PROVIDED) {
+
+  if (GetParam() == envoy::config::cluster::v3::Cluster::CLUSTER_PROVIDED) {
     // This custom cluster type is registered by linking test/integration/custom/static_cluster.cc.
     cluster_type = "cluster_type: { name: envoy.clusters.custom_static_with_lb }";
   }
   const std::string yaml = fmt::format(yamlPattern, cluster_type, policy_name);
 
-  if (GetParam() == envoy::config::cluster::v3::Cluster::hidden_envoy_deprecated_ORIGINAL_DST_LB ||
-      GetParam() == envoy::config::cluster::v3::Cluster::CLUSTER_PROVIDED) {
+  if (GetParam() == envoy::config::cluster::v3::Cluster::CLUSTER_PROVIDED) {
     EXPECT_THROW_WITH_MESSAGE(
         create(parseBootstrapFromV3Yaml(yaml)), EnvoyException,
         fmt::format("cluster: LB policy {} cannot be combined with lb_subset_config",
@@ -627,26 +602,6 @@ INSTANTIATE_TEST_SUITE_P(ClusterManagerSubsetInitializationTest,
                          testing::ValuesIn(ClusterManagerSubsetInitializationTest::lbPolicies()),
                          ClusterManagerSubsetInitializationTest::paramName);
 
-TEST_F(ClusterManagerImplTest, DEPRECATED_FEATURE_TEST(SubsetLoadBalancerOriginalDstRestriction)) {
-  TestDeprecatedV2Api _deprecated_v2_api;
-  const std::string yaml = R"EOF(
- static_resources:
-  clusters:
-  - name: cluster_1
-    connect_timeout: 0.250s
-    type: original_dst
-    lb_policy: original_dst_lb
-    lb_subset_config:
-      fallback_policy: ANY_ENDPOINT
-      subset_selectors:
-        - keys: [ "x" ]
-  )EOF";
-
-  EXPECT_THROW_WITH_MESSAGE(create(parseBootstrapFromV3Yaml(yaml, false)), EnvoyException,
-                            "cluster: LB policy hidden_envoy_deprecated_ORIGINAL_DST_LB cannot be "
-                            "combined with lb_subset_config");
-}
-
 TEST_F(ClusterManagerImplTest, SubsetLoadBalancerClusterProvidedLbRestriction) {
   const std::string yaml = R"EOF(
  static_resources:

diff --git a/test/integration/proxy_proto_integration_test.cc b/test/integration/proxy_proto_integration_test.cc
@@ -160,37 +160,6 @@ TEST_P(ProxyProtoIntegrationTest, AccessLog) {
   EXPECT_EQ(tokens[1], "1.2.3.4:12345");
 }
 
-TEST_P(ProxyProtoIntegrationTest, DEPRECATED_FEATURE_TEST(OriginalDst)) {
-  // Change the cluster to an original destination cluster. An original destination cluster
-  // ignores the configured hosts, and instead uses the restored destination address from the
-  // incoming (server) connection as the destination address for the outgoing (client) connection.
-  config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
-    auto* cluster = bootstrap.mutable_static_resources()->mutable_clusters(0);
-    cluster->clear_load_assignment();
-    cluster->set_type(envoy::config::cluster::v3::Cluster::ORIGINAL_DST);
-    cluster->set_lb_policy(
-        envoy::config::cluster::v3::Cluster::hidden_envoy_deprecated_ORIGINAL_DST_LB);
-  });
-
-  ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr {
-    Network::ClientConnectionPtr conn = makeClientConnection(lookupPort("http"));
-    // Create proxy protocol line that has the fake upstream address as the destination address.
-    // This address will become the "restored" address for the server connection and will
-    // be used as the destination address by the original destination cluster.
-    std::string proxyLine = fmt::format(
-        "PROXY {} {} 65535 {}\r\n",
-        GetParam() == Network::Address::IpVersion::v4 ? "TCP4 1.2.3.4" : "TCP6 1:2:3::4",
-        Network::Test::getLoopbackAddressString(GetParam()),
-        fake_upstreams_[0]->localAddress()->ip()->port());
-
-    Buffer::OwnedImpl buf(proxyLine);
-    conn->write(buf, false);
-    return conn;
-  };
-
-  testRouterRequestAndResponseWithBody(1024, 512, false, false, &creator);
-}
-
 TEST_P(ProxyProtoIntegrationTest, ClusterProvided) {
   // Change the cluster to an original destination cluster. An original destination cluster
   // ignores the configured hosts, and instead uses the restored destination address from the