test: improved coverage and handling of deprecated config (#8057)

Making ENVOY_DISABLE_DEPRECATED_FEATURES work for unit tests without runtime configured. Fixing up a handful of unit tests to remove legacy code or use the handy DEPRECATED_FEATURE_TEST macro Adding back coverage of cors.enabled() and redis.catch_all_route() Risk Level: Low (test only) Testing: new unit tests Docs Changes: n/a Release Notes: n/a Fixes #8013 Fixes #7548 Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
envoyproxy · Aug 28, 2019 · 64243c9 · 64243c9
1 parent b020b63
commit 64243c9
Show file tree

Hide file tree

Showing 14 changed files with 153 additions and 46 deletions.
diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc
@@ -192,7 +192,11 @@ void MessageUtil::checkForDeprecation(const Protobuf::Message& message, Runtime:
       continue;
     }
 
+#ifdef ENVOY_DISABLE_DEPRECATED_FEATURES
+    bool warn_only = false;
+#else
     bool warn_only = true;
+#endif
     absl::string_view filename = filenameFromPath(field->file()->name());
     // Allow runtime to be null both to not crash if this is called before server initialization,
     // and so proto validation works in context where runtime singleton is not set up (e.g.

diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc
@@ -116,7 +116,8 @@ class ConfigImplTestBase {
 
 class RouteMatcherTest : public testing::Test, public ConfigImplTestBase {};
 
-TEST_F(RouteMatcherTest, TestRoutes) {
+// TODO(alyssawilk) go through all these tests and update or duplicate.
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(TestRoutes)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
 - name: www2
@@ -617,7 +618,7 @@ TEST_F(RouteMatcherTest, TestRoutesWithWildcardAndDefaultOnly) {
             config.route(genHeaders("example.com", "/", "GET"), 0)->routeEntry()->clusterName());
 }
 
-TEST_F(RouteMatcherTest, TestRoutesWithInvalidRegex) {
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(TestRoutesWithInvalidRegex)) {
   std::string invalid_route = R"EOF(
 virtual_hosts:
   - name: regex
@@ -1077,7 +1078,7 @@ name: foo
   }
 }
 
-TEST_F(RouteMatcherTest, Priority) {
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(Priority)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
 - name: local_service
@@ -1164,7 +1165,7 @@ TEST_F(RouteMatcherTest, NoAutoRewriteAndAutoRewriteHeader) {
                EnvoyException);
 }
 
-TEST_F(RouteMatcherTest, HeaderMatchedRouting) {
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(HeaderMatchedRouting)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
 - name: local_service
@@ -1288,7 +1289,7 @@ TEST_F(RouteMatcherTest, HeaderMatchedRouting) {
 }
 
 // Verify the fixes for https://github.com/envoyproxy/envoy/issues/2406
-TEST_F(RouteMatcherTest, InvalidHeaderMatchedRoutingConfig) {
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(InvalidHeaderMatchedRoutingConfig)) {
   std::string value_with_regex_chars = R"EOF(
 virtual_hosts:
   - name: local_service
@@ -1323,7 +1324,7 @@ TEST_F(RouteMatcherTest, InvalidHeaderMatchedRoutingConfig) {
       EnvoyException, "Invalid regex");
 }
 
-TEST_F(RouteMatcherTest, QueryParamMatchedRouting) {
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(QueryParamMatchedRouting)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
 - name: local_service
@@ -1438,7 +1439,7 @@ TEST_F(RouteMatcherTest, QueryParamMatchedRouting) {
 }
 
 // Verify the fixes for https://github.com/envoyproxy/envoy/issues/2406
-TEST_F(RouteMatcherTest, InvalidQueryParamMatchedRoutingConfig) {
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(InvalidQueryParamMatchedRoutingConfig)) {
   std::string value_with_regex_chars = R"EOF(
 virtual_hosts:
   - name: local_service
@@ -2249,7 +2250,7 @@ TEST_F(RouteMatcherTest, ClusterNotFoundResponseCodeConfig404) {
             config.route(headers, 0)->routeEntry()->clusterNotFoundResponseCode());
 }
 
-TEST_F(RouteMatcherTest, Shadow) {
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(Shadow)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
 - name: www2
@@ -2307,7 +2308,7 @@ TEST_F(RouteMatcherTest, Shadow) {
 
 class RouteConfigurationV2 : public testing::Test, public ConfigImplTestBase {};
 
-TEST_F(RouteConfigurationV2, RequestMirrorPolicy) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(RequestMirrorPolicy)) {
   const std::string yaml = R"EOF(
 name: foo
 virtual_hosts:
@@ -4264,7 +4265,7 @@ TEST_F(RoutePropertyTest, excludeVHRateLimits) {
   EXPECT_TRUE(config_ptr->route(headers, 0)->routeEntry()->includeVirtualHostRateLimits());
 }
 
-TEST_F(RoutePropertyTest, TestVHostCorsConfig) {
+TEST_F(RoutePropertyTest, DEPRECATED_FEATURE_TEST(TestVHostCorsConfig)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
   - name: "default"
@@ -4326,7 +4327,7 @@ TEST_F(RoutePropertyTest, TestVHostCorsConfig) {
   EXPECT_EQ(cors_policy->allowCredentials(), true);
 }
 
-TEST_F(RoutePropertyTest, TestRouteCorsConfig) {
+TEST_F(RoutePropertyTest, DEPRECATED_FEATURE_TEST(TestRouteCorsConfig)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
   - name: "default"
@@ -4379,7 +4380,7 @@ TEST_F(RoutePropertyTest, TestRouteCorsConfig) {
   EXPECT_EQ(cors_policy->allowCredentials(), true);
 }
 
-TEST_F(RoutePropertyTest, TestVHostCorsLegacyConfig) {
+TEST_F(RoutePropertyTest, DEPRECATED_FEATURE_TEST(TestVHostCorsLegacyConfig)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
 - name: default
@@ -4418,7 +4419,7 @@ TEST_F(RoutePropertyTest, TestVHostCorsLegacyConfig) {
   EXPECT_EQ(cors_policy->allowCredentials(), true);
 }
 
-TEST_F(RoutePropertyTest, TestRouteCorsLegacyConfig) {
+TEST_F(RoutePropertyTest, DEPRECATED_FEATURE_TEST(TestRouteCorsLegacyConfig)) {
   const std::string yaml = R"EOF(
 virtual_hosts:
 - name: default
@@ -4909,7 +4910,7 @@ name: foo
       Envoy::EnvoyException, "Cannot create a Baz when metadata is empty.");
 }
 
-TEST_F(RouteConfigurationV2, RouteConfigGetters) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(RouteConfigGetters)) {
   const std::string yaml = R"EOF(
 name: foo
 virtual_hosts:
@@ -4948,7 +4949,7 @@ name: foo
   EXPECT_EQ("foo", route_entry->virtualHost().routeConfig().name());
 }
 
-TEST_F(RouteConfigurationV2, RouteTracingConfig) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(RouteTracingConfig)) {
   const std::string yaml = R"EOF(
 name: foo
 virtual_hosts:
@@ -5003,7 +5004,7 @@ name: foo
 }
 
 // Test to check Prefix Rewrite for redirects
-TEST_F(RouteConfigurationV2, RedirectPrefixRewrite) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(RedirectPrefixRewrite)) {
   std::string RedirectPrefixRewrite = R"EOF(
 name: AllRedirects
 virtual_hosts:
@@ -5190,7 +5191,7 @@ name: AllRedirects
   }
 }
 
-TEST_F(RouteMatcherTest, HeaderMatchedRoutingV2) {
+TEST_F(RouteMatcherTest, DEPRECATED_FEATURE_TEST(HeaderMatchedRoutingV2)) {
   const std::string yaml = R"EOF(
 name: foo
 virtual_hosts:
@@ -5365,7 +5366,8 @@ name: foo
   }
 }
 
-TEST_F(RouteConfigurationV2, RegexPrefixWithNoRewriteWorksWhenPathChanged) {
+TEST_F(RouteConfigurationV2,
+       DEPRECATED_FEATURE_TEST(RegexPrefixWithNoRewriteWorksWhenPathChanged)) {
 
   // Setup regex route entry. the regex is trivial, that's ok as we only want to test that
   // path change works.
@@ -5396,7 +5398,7 @@ name: RegexNoMatch
   }
 }
 
-TEST_F(RouteConfigurationV2, NoIdleTimeout) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(NoIdleTimeout)) {
   const std::string NoIdleTimeout = R"EOF(
 name: NoIdleTimeout
 virtual_hosts:
@@ -5414,7 +5416,7 @@ name: NoIdleTimeout
   EXPECT_EQ(absl::nullopt, route_entry->idleTimeout());
 }
 
-TEST_F(RouteConfigurationV2, ZeroIdleTimeout) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(ZeroIdleTimeout)) {
   const std::string ZeroIdleTimeout = R"EOF(
 name: ZeroIdleTimeout
 virtual_hosts:
@@ -5433,7 +5435,7 @@ name: ZeroIdleTimeout
   EXPECT_EQ(0, route_entry->idleTimeout().value().count());
 }
 
-TEST_F(RouteConfigurationV2, ExplicitIdleTimeout) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(ExplicitIdleTimeout)) {
   const std::string ExplicitIdleTimeout = R"EOF(
 name: ExplicitIdleTimeout
 virtual_hosts:
@@ -5453,7 +5455,7 @@ name: ExplicitIdleTimeout
   EXPECT_EQ(7 * 1000, route_entry->idleTimeout().value().count());
 }
 
-TEST_F(RouteConfigurationV2, RetriableStatusCodes) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(RetriableStatusCodes)) {
   const std::string ExplicitIdleTimeout = R"EOF(
 name: RetriableStatusCodes
 virtual_hosts:
@@ -5475,7 +5477,7 @@ name: RetriableStatusCodes
   EXPECT_EQ(expected_codes, retry_policy.retriableStatusCodes());
 }
 
-TEST_F(RouteConfigurationV2, UpgradeConfigs) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(UpgradeConfigs)) {
   const std::string UpgradeYaml = R"EOF(
 name: RetriableStatusCodes
 virtual_hosts:
@@ -5499,7 +5501,7 @@ name: RetriableStatusCodes
   EXPECT_FALSE(upgrade_map.find("disabled")->second);
 }
 
-TEST_F(RouteConfigurationV2, DuplicateUpgradeConfigs) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(DuplicateUpgradeConfigs)) {
   const std::string yaml = R"EOF(
 name: RetriableStatusCodes
 virtual_hosts:
@@ -5522,7 +5524,7 @@ name: RetriableStatusCodes
 
 // Verifies that we're creating a new instance of the retry plugins on each call instead of always
 // returning the same one.
-TEST_F(RouteConfigurationV2, RetryPluginsAreNotReused) {
+TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(RetryPluginsAreNotReused)) {
   const std::string ExplicitIdleTimeout = R"EOF(
 name: RetriableStatusCodes
 virtual_hosts:

diff --git a/test/config/integration/server.yaml b/test/config/integration/server.yaml
@@ -168,10 +168,19 @@ stats_sinks:
     "@type": type.googleapis.com/envoy.config.metrics.v2.StatsdSink
     tcp_cluster_name: statsd
 watchdog: {}
-runtime:
-  symlink_root: "{{ test_tmpdir }}/test/common/runtime/test_data/current"
-  subdirectory: envoy
-  override_subdirectory: envoy_override
+layered_runtime:
+  layers:
+    - name: root
+      disk_layer:
+        symlink_root: "{{ test_tmpdir }}/test/common/runtime/test_data/current"
+        subdirectory: envoy
+    - name: override
+      disk_layer:
+        symlink_root: "{{ test_tmpdir }}/test/common/runtime/test_data/current"
+        subdirectory: envoy_override
+        append_service_cluster: true
+    - name: admin
+      admin_layer: {}
 admin:
   access_log_path: "/dev/null"
   profile_path: "{{ test_tmpdir }}/envoy.prof"

diff --git a/test/config_test/example_configs_test.cc b/test/config_test/example_configs_test.cc
@@ -5,7 +5,7 @@
 #include "gtest/gtest.h"
 
 namespace Envoy {
-TEST(ExampleConfigsTest, All) {
+TEST(ExampleConfigsTest, DEPRECATED_FEATURE_TEST(All)) {
   TestEnvironment::exec(
       {TestEnvironment::runfilesPath("test/config_test/example_configs_test_setup.sh")});
 

diff --git a/test/extensions/filters/http/cors/cors_filter_integration_test.cc b/test/extensions/filters/http/cors/cors_filter_integration_test.cc
@@ -195,6 +195,33 @@ TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestCorsDisabled)) {
       });
 }
 
+TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestLegacyCorsDisabled)) {
+  config_helper_.addConfigModifier(
+      [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm)
+          -> void {
+        auto* route_config = hcm.mutable_route_config();
+        auto* virtual_host = route_config->mutable_virtual_hosts(0);
+        auto* route = virtual_host->add_routes();
+        route->mutable_match()->set_prefix("/legacy-no-cors");
+        route->mutable_route()->set_cluster("cluster_0");
+        route->mutable_route()->mutable_cors()->mutable_enabled()->set_value(false);
+      });
+  testNormalRequest(
+      Http::TestHeaderMapImpl{
+          {":method", "OPTIONS"},
+          {":path", "/legacy-no-cors/test"},
+          {":scheme", "http"},
+          {":authority", "test-host"},
+          {"access-control-request-method", "GET"},
+          {"origin", "test-origin"},
+      },
+      Http::TestHeaderMapImpl{
+          {"server", "envoy"},
+          {"content-length", "0"},
+          {":status", "200"},
+      });
+}
+
 TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestEncodeHeaders)) {
   testNormalRequest(
       Http::TestHeaderMapImpl{

diff --git a/test/extensions/filters/network/dubbo_proxy/conn_manager_test.cc b/test/extensions/filters/network/dubbo_proxy/conn_manager_test.cc
@@ -1156,7 +1156,8 @@ TEST_F(ConnectionManagerTest, PendingMessageEnd) {
   EXPECT_EQ(1U, store_.gauge("test.request_active", Stats::Gauge::ImportMode::Accumulate).value());
 }
 
-TEST_F(ConnectionManagerTest, Routing) {
+// TODO(alyssawilk) update.
+TEST_F(ConnectionManagerTest, DEPRECATED_FEATURE_TEST(Routing)) {
   const std::string yaml = R"EOF(
 stat_prefix: test
 protocol_type: Dubbo

diff --git a/test/extensions/filters/network/dubbo_proxy/route_matcher_test.cc b/test/extensions/filters/network/dubbo_proxy/route_matcher_test.cc
@@ -38,7 +38,8 @@ parseDubboProxyFromV2Yaml(const std::string& yaml) {
 
 } // namespace
 
-TEST(DubboRouteMatcherTest, RouteByServiceNameWithAnyMethod) {
+// TODO(alyssawilk) update.
+TEST(DubboRouteMatcherTest, DEPRECATED_FEATURE_TEST(RouteByServiceNameWithAnyMethod)) {
   {
     const std::string yaml = R"EOF(
 name: local_route
@@ -291,7 +292,8 @@ interface: org.apache.dubbo.demo.DemoService
   EXPECT_EQ("user_service_dubbo_server", matcher.route(metadata, 0)->routeEntry()->clusterName());
 }
 
-TEST(DubboRouteMatcherTest, RouteByMethodWithRegexMatch) {
+// TODO(alyssawilk) update.
+TEST(DubboRouteMatcherTest, DEPRECATED_FEATURE_TEST(RouteByMethodWithRegexMatch)) {
   const std::string yaml = R"EOF(
 name: local_route
 interface: org.apache.dubbo.demo.DemoService