fix VHDS not work when vhds config set in first rds update both (envo…

…yproxy#12746) fix envoyproxy#12683 Risk Level: Medium Testing: unit test Signed-off-by: ouyangxu <[email protected]> Signed-off-by: Clara Andrew-Wani <[email protected]>
clarakosi · Sep 3, 2020 · fdf6ff1 · fdf6ff1
1 parent d289859
commit fdf6ff1
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 11 deletions.
diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc
@@ -137,18 +137,19 @@ void RdsRouteConfigSubscription::onConfigUpdate(
           config_update_info_->routeConfiguration().vhds().config_source().resource_api_version());
       vhds_subscription_->registerInitTargetWithInitManager(
           noop_init_manager == nullptr ? local_init_manager_ : *noop_init_manager);
-    } else {
-      ENVOY_LOG(debug, "rds: loading new configuration: config_name={} hash={}", route_config_name_,
-                config_update_info_->configHash());
-
-      for (auto* provider : route_config_providers_) {
-        provider->onConfigUpdate();
-      }
-      // RDS update removed VHDS configuration
-      if (!config_update_info_->routeConfiguration().has_vhds()) {
-        vhds_subscription_.release();
-      }
     }
+
+    ENVOY_LOG(debug, "rds: loading new configuration: config_name={} hash={}", route_config_name_,
+              config_update_info_->configHash());
+
+    for (auto* provider : route_config_providers_) {
+      provider->onConfigUpdate();
+    }
+    // RDS update removed VHDS configuration
+    if (!config_update_info_->routeConfiguration().has_vhds()) {
+      vhds_subscription_.release();
+    }
+
     update_callback_manager_.runCallbacks();
   }
 

diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc
@@ -267,6 +267,65 @@ TEST_F(RdsImplTest, FailureInvalidConfig) {
       "Unexpected RDS configuration (expecting foo_route_config): INVALID_NAME_FOR_route_config");
 }
 
+// rds and vhds configurations change together
+TEST_F(RdsImplTest, VHDSandRDSupdateTogether) {
+  setup();
+
+  const std::string response1_json = R"EOF(
+{
+  "version_info": "1",
+  "resources": [
+    {
+      "@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
+      "name": "foo_route_config",
+      "virtual_hosts": [
+        {
+          "name": "foo",
+          "domains": [
+            "foo"
+          ],
+          "routes": [
+            {
+              "match": {
+                "prefix": "/foo"
+              },
+              "route": {
+                "cluster": "foo"
+              }
+            }
+          ]
+        }
+      ],
+      "vhds": {
+        "config_source": {
+          "api_config_source": {
+            "api_type": "DELTA_GRPC",
+            "grpc_services": {
+              "envoy_grpc": {
+                "cluster_name": "xds_cluster"
+              }
+            }
+          }
+        }
+      }
+    }
+  ]
+}
+)EOF";
+  auto response1 =
+      TestUtility::parseYaml<envoy::service::discovery::v3::DiscoveryResponse>(response1_json);
+  const auto decoded_resources =
+      TestUtility::decodeResources<envoy::config::route::v3::RouteConfiguration>(response1);
+
+  EXPECT_CALL(init_watcher_, ready());
+  rds_callbacks_->onConfigUpdate(decoded_resources.refvec_, response1.version_info());
+  EXPECT_TRUE(rds_->config()->usesVhds());
+
+  EXPECT_EQ("foo", route(Http::TestRequestHeaderMapImpl{{":authority", "foo"}, {":path", "/foo"}})
+                       ->routeEntry()
+                       ->clusterName());
+}
+
 // Validate behavior when the config fails delivery at the subscription level.
 TEST_F(RdsImplTest, FailureSubscription) {
   InSequence s;