HCM: add compile option of path normalization

Motivation: alternative approach to enable path normalization add config "--define path_normalization_by_default=true" to bazel With this bazel config, the path normalization in HCM will - ignore the runtime - turn on the normalization if the xDS is not aware of this config Signed-off-by: Yuchen Dai <silentdai@gmail.com>
envoyproxy · Apr 9, 2019 · e668e66 · e668e66
1 parent fa69fad
commit e668e66
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 8 deletions.
diff --git a/bazel/BUILD b/bazel/BUILD
@@ -105,6 +105,11 @@ config_setting(
     values = {"define": "google_grpc=disabled"},
 )
 
+config_setting(
+    name = "enable_path_normalization_by_default",
+    values = {"define": "path_normalization_by_default=true"},
+)
+
 cc_proto_library(
     name = "grpc_health_proto",
     deps = ["@com_github_grpc_grpc//src/proto/grpc/health/v1:_health_proto_only"],

diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl
@@ -83,7 +83,8 @@ def envoy_copts(repository, test = False):
                "//conditions:default": [],
            }) + envoy_select_hot_restart(["-DENVOY_HOT_RESTART"], repository) + \
            envoy_select_perf_annotation(["-DENVOY_PERF_ANNOTATION"]) + \
-           envoy_select_google_grpc(["-DENVOY_GOOGLE_GRPC"], repository)
+           envoy_select_google_grpc(["-DENVOY_GOOGLE_GRPC"], repository) + \
+           envoy_select_path_normalization_by_default(["-DENVOY_NORMALIZE_PATH_BY_DEFAULT"], repository)
 
 def envoy_static_link_libstdcpp_linkopts():
     return envoy_select_force_libcpp(
@@ -648,6 +649,13 @@ def envoy_select_hot_restart(xs, repository = ""):
         "//conditions:default": xs,
     })
 
+# Select the given values if default path normalization is on in the current build.
+def envoy_select_path_normalization_by_default(xs, repository = ""):
+    return select({
+        repository + "//bazel:enable_path_normalization_by_default": xs,
+        "//conditions:default": [],
+    })
+
 def envoy_select_perf_annotation(xs):
     return select({
         "@envoy//bazel:enable_perf_annotation": xs,

diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc
@@ -151,13 +151,18 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
                                                                          context_.listenerScope())),
       proxy_100_continue_(config.proxy_100_continue()),
       delayed_close_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, delayed_close_timeout, 1000)),
-      normalize_path_(
-          PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, normalize_path,
-                                          // TODO(htuch): we should have a
-                                          // boolean variant of featureEnabled()
-                                          // here.
-                                          context.runtime().snapshot().featureEnabled(
-                                              "http_connection_manager.normalize_path", 0))) {
+      normalize_path_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, normalize_path,
+#ifdef ENVOY_NORMALIZE_PATH_BY_DEFAULT
+                                                      true
+#else
+                                                      // TODO(htuch): we should have a
+                                                      // boolean variant of featureEnabled()
+                                                      // here.
+                                                      context.runtime().snapshot().featureEnabled(
+                                                          "http_connection_manager.normalize_path",
+                                                          0)
+#endif
+                                                      )) {
 
   route_config_provider_ = Router::RouteConfigProviderUtil::create(config, context_, stats_prefix_,
                                                                    route_config_provider_manager_);

diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc
@@ -186,6 +186,25 @@ TEST_F(HttpConnectionManagerConfigTest, DisabledStreamIdleTimeout) {
   EXPECT_EQ(0, config.streamIdleTimeout().count());
 }
 
+#ifdef ENVOY_NORMALIZE_PATH_BY_DEFAULT
+// Validated that we normalize paths compiled with enable-by-default
+TEST_F(HttpConnectionManagerConfigTest, NormalizePathCompileConfig) {
+  const std::string yaml_string = R"EOF(
+  stat_prefix: ingress_http
+  route_config:
+    name: local_route
+  http_filters:
+  - name: envoy.router
+  )EOF";
+
+  EXPECT_CALL(context_.runtime_loader_.snapshot_,
+              featureEnabled("http_connection_manager.normalize_path", 0))
+      .Times(0);
+  HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_,
+                                     date_provider_, route_config_provider_manager_);
+  EXPECT_TRUE(config.shouldNormalizePath());
+}
+#else
 // Validated that by default we don't normalize paths
 TEST_F(HttpConnectionManagerConfigTest, NormalizePathDefault) {
   const std::string yaml_string = R"EOF(
@@ -218,6 +237,7 @@ TEST_F(HttpConnectionManagerConfigTest, NormalizePathRuntime) {
                                      date_provider_, route_config_provider_manager_);
   EXPECT_TRUE(config.shouldNormalizePath());
 }
+#endif
 
 // Validated that when configured, we normalize paths, ignoring runtime.
 TEST_F(HttpConnectionManagerConfigTest, NormalizePathTrue) {