listener filters: use new style names (#10193)

Modifies the well-known-names of the built-in listener filters to use the same names as the extension build system. Risk Level: low, previous name is still accepted Testing: existing tests + deprecated tests for old names Docs Changes: updated names Release Notes: updated Deprecated: old names are logged as deprecated Signed-off-by: Stephan Zuercher <[email protected]>
envoyproxy · Mar 2, 2020 · 6c21374 · 6c21374
1 parent 56cfb63
commit 6c21374
Show file tree

Hide file tree

Showing 31 changed files with 179 additions and 71 deletions.
diff --git a/api/envoy/api/v2/listener/listener_components.proto b/api/envoy/api/v2/listener/listener_components.proto
@@ -136,7 +136,7 @@ message FilterChainMatch {
   // Suggested values include:
   //
   // * ``raw_buffer`` - default, used when no transport protocol is detected,
-  // * ``tls`` - set by :ref:`envoy.listener.tls_inspector <config_listener_filters_tls_inspector>`
+  // * ``tls`` - set by :ref:`envoy.filters.listener.tls_inspector <config_listener_filters_tls_inspector>`
   //   when TLS protocol is detected.
   string transport_protocol = 9;
 
@@ -146,9 +146,9 @@ message FilterChainMatch {
   //
   // Suggested values include:
   //
-  // * ``http/1.1`` - set by :ref:`envoy.listener.tls_inspector
+  // * ``http/1.1`` - set by :ref:`envoy.filters.listener.tls_inspector
   //   <config_listener_filters_tls_inspector>`,
-  // * ``h2`` - set by :ref:`envoy.listener.tls_inspector <config_listener_filters_tls_inspector>`
+  // * ``h2`` - set by :ref:`envoy.filters.listener.tls_inspector <config_listener_filters_tls_inspector>`
   //
   // .. attention::
   //

diff --git a/api/envoy/config/listener/v3/listener_components.proto b/api/envoy/config/listener/v3/listener_components.proto
@@ -138,7 +138,7 @@ message FilterChainMatch {
   // Suggested values include:
   //
   // * ``raw_buffer`` - default, used when no transport protocol is detected,
-  // * ``tls`` - set by :ref:`envoy.listener.tls_inspector <config_listener_filters_tls_inspector>`
+  // * ``tls`` - set by :ref:`envoy.filters.listener.tls_inspector <config_listener_filters_tls_inspector>`
   //   when TLS protocol is detected.
   string transport_protocol = 9;
 
@@ -148,9 +148,9 @@ message FilterChainMatch {
   //
   // Suggested values include:
   //
-  // * ``http/1.1`` - set by :ref:`envoy.listener.tls_inspector
+  // * ``http/1.1`` - set by :ref:`envoy.filters.listener.tls_inspector
   //   <config_listener_filters_tls_inspector>`,
-  // * ``h2`` - set by :ref:`envoy.listener.tls_inspector <config_listener_filters_tls_inspector>`
+  // * ``h2`` - set by :ref:`envoy.filters.listener.tls_inspector <config_listener_filters_tls_inspector>`
   //
   // .. attention::
   //

diff --git a/configs/original-dst-cluster/proxy_config.yaml b/configs/original-dst-cluster/proxy_config.yaml
@@ -26,7 +26,7 @@ static_resources:
             typed_config: {}
           codec_type: auto
     listener_filters:
-    - name: envoy.listener.original_dst
+    - name: envoy.filters.listener.original_dst
       typed_config: {}
   clusters:
   - name: cluster1

diff --git a/docs/root/configuration/best_practices/edge.rst b/docs/root/configuration/best_practices/edge.rst
@@ -62,7 +62,7 @@ The following is a YAML example of the above recommendation.
           address: 0.0.0.0
           port_value: 443
       listener_filters:
-      - name: "envoy.listener.tls_inspector"
+      - name: "envoy.filters.listener.tls_inspector"
         typed_config: {}
       per_connection_buffer_limit_bytes: 32768 # 32 KiB
       filter_chains:

diff --git a/docs/root/configuration/listeners/listener_filters/http_inspector.rst b/docs/root/configuration/listeners/listener_filters/http_inspector.rst
@@ -9,7 +9,7 @@ and if it is HTTP, it detects the HTTP protocol (HTTP/1.x or HTTP/2) further. Th
 of a :ref:`FilterChainMatch <envoy_api_msg_listener.FilterChainMatch>`.
 
 * :ref:`Listener filter v2 API reference <envoy_api_msg_config.filter.listener.http_inspector.v2.HttpInspector>`
-* This filter should be configured with the name *envoy.listener.http_inspector*.
+* This filter should be configured with the name *envoy.filters.listener.http_inspector*.
 
 Example
 -------
@@ -19,7 +19,7 @@ A sample filter configuration could be:
 .. code-block:: yaml
 
   listener_filters:
-    - name: "envoy.listener.http_inspector"
+    - name: "envoy.filters.listener.http_inspector"
       typed_config: {}
 
 Statistics

diff --git a/docs/root/configuration/listeners/listener_filters/original_dst_filter.rst b/docs/root/configuration/listeners/listener_filters/original_dst_filter.rst
@@ -12,4 +12,4 @@ destination cluster <arch_overview_service_discovery_types_original_destination>
 forward HTTP requests or TCP connections to the restored destination address.
 
 * :ref:`v2 API reference <envoy_api_field_listener.ListenerFilter.name>`
-* This filter should be configured with the name *envoy.listener.original_dst*.
+* This filter should be configured with the name *envoy.filters.listener.original_dst*.
diff --git a/docs/root/configuration/listeners/listener_filters/original_src_filter.rst b/docs/root/configuration/listeners/listener_filters/original_src_filter.rst
@@ -4,7 +4,7 @@ Original Source
 ===============
 
 * :ref:`Listener filter v2 API reference <envoy_api_msg_config.filter.listener.original_src.v2alpha1.OriginalSrc>`
-* This filter should be configured with the name *envoy.listener.original_src*.
+* This filter should be configured with the name *envoy.filters.listener.original_src*.
 
 The original source listener filter replicates the downstream remote address of the connection on
 the upstream side of Envoy. For example, if a downstream connection connects to Envoy with IP
@@ -71,8 +71,8 @@ marked with 123.
         address: 0.0.0.0
         port_value: 8888
     listener_filters:
-      - name: envoy.listener.proxy_protocol
-      - name: envoy.listener.original_src
+      - name: envoy.filters.listener.proxy_protocol
+      - name: envoy.filters.listener.original_src
         typed_config:
           "@type": type.googleapis.com/envoy.config.filter.listener.original_src.v2alpha1.OriginalSrc
           mark: 123
diff --git a/docs/root/configuration/listeners/listener_filters/proxy_protocol.rst b/docs/root/configuration/listeners/listener_filters/proxy_protocol.rst
@@ -24,7 +24,7 @@ If there is a protocol error or an unsupported address family
 (e.g. AF_UNIX) the connection will be closed and an error thrown.
 
 * :ref:`v2 API reference <envoy_api_field_listener.Filter.name>`
-* This filter should be configured with the name *envoy.listener.proxy_protocol*.
+* This filter should be configured with the name *envoy.filters.listener.proxy_protocol*.
 
 Statistics
 ----------

diff --git a/docs/root/configuration/listeners/listener_filters/tls_inspector.rst b/docs/root/configuration/listeners/listener_filters/tls_inspector.rst
@@ -16,7 +16,7 @@ of a :ref:`FilterChainMatch <envoy_api_msg_listener.FilterChainMatch>`.
 
 * :ref:`SNI <faq_how_to_setup_sni>`
 * :ref:`v2 API reference <envoy_api_field_listener.ListenerFilter.name>`
-* This filter should be configured with the name *envoy.listener.tls_inspector*.
+* This filter should be configured with the name *envoy.filters.listener.tls_inspector*.
 
 Example
 -------
@@ -26,7 +26,7 @@ A sample filter configuration could be:
 .. code-block:: yaml
 
   listener_filters:
-    - name: "envoy.listener.tls_inspector"
+    - name: "envoy.filters.listener.tls_inspector"
       typed_config: {}
 
 Statistics

diff --git a/docs/root/faq/configuration/sni.rst b/docs/root/faq/configuration/sni.rst
@@ -18,7 +18,7 @@ The following is a YAML example of the above requirement.
   address:
     socket_address: { address: 127.0.0.1, port_value: 1234 }
   listener_filters:
-  - name: "envoy.listener.tls_inspector"
+  - name: "envoy.filters.listener.tls_inspector"
     typed_config: {}
   filter_chains:
   - filter_chain_match:

diff --git a/docs/root/intro/deprecated.rst b/docs/root/intro/deprecated.rst
@@ -15,11 +15,11 @@ Deprecated items below are listed in chronological order.
 * The previous behavior for upstream connection pool circuit breaking described
   `here <https://www.envoyproxy.io/docs/envoy/v1.13.0/intro/arch_overview/upstream/circuit_breaking>`_ has
   been deprecated in favor of the new behavior described :ref:`here <arch_overview_circuit_break>`.
-* Access Logger, HTTP Filter, Network Filter, Stats Sink, and Tracer names have been deprecated in
-  favor of the extension name from the envoy build system. Disable the runtime feature
-  "envoy.deprecated_features.allow_deprecated_extension_names" to disallow the deprecated names.
-  Use of these extension names generates a log message and increments the "deprecated_feature_use"
-  metric in stats.
+* Access Logger, Listener Filter, HTTP Filter, Network Filter, Stats Sink, and Tracer names have
+  been deprecated in favor of the extension name from the envoy build system. Disable the runtime
+  feature "envoy.deprecated_features.allow_deprecated_extension_names" to disallow the deprecated
+  names. Use of these extension names generates a log message and increments the
+  "deprecated_feature_use" metric in stats.
 
   .. csv-table::
     :header: Canonical Names, Deprecated Names
@@ -44,6 +44,11 @@ Deprecated items below are listed in chronological order.
     envoy.filters.http.ratelimit, envoy.rate_limit
     envoy.filters.http.router, envoy.router
     envoy.filters.http.squash, envoy.squash
+    envoy.filters.listener.http_inspector, envoy.listener.http_inspector
+    envoy.filters.listener.original_dst, envoy.listener.original_dst
+    envoy.filters.listener.original_src, envoy.listener.original_src
+    envoy.filters.listener.proxy_protocol, envoy.listener.proxy_protocol
+    envoy.filters.listener.tls_inspector, envoy.listener.tls_inspector
     envoy.filters.network.client_ssl_auth, envoy.client_ssl_auth
     envoy.filters.network.echo, envoy.echo
     envoy.filters.network.ext_authz, envoy.ext_authz

diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -15,6 +15,8 @@ Version history
 * ext_authz: disabled the use of lowercase string matcher for headers matching in HTTP-based `ext_authz`.
   Can be reverted temporarily by setting runtime feature `envoy.reloadable_features.ext_authz_http_service_enable_case_sensitive_string_matcher` to false.
 * http: fixing a bug in HTTP/1.0 responses where Connection: keep-alive was not appended for connections which were kept alive.
+* listener filters: listener filter extensions use the "envoy.filters.listener" name space. A
+  mapping of extension names is available in the :ref:`deprecated <deprecated>` documentation.
 * mongo: the stat emitted for queries without a max time set in the :ref:`MongoDB filter<config_network_filters_mongo_proxy>` was modified to emit correctly for Mongo v3.2+.
 * network filters: network filter extensions use the "envoy.filters.network" name space. A mapping
   of extension names is available in the :ref:`deprecated <deprecated>` documentation.

diff --git a/generated_api_shadow/envoy/api/v2/listener/listener_components.proto b/generated_api_shadow/envoy/api/v2/listener/listener_components.proto
diff --git a/generated_api_shadow/envoy/config/listener/v3/listener_components.proto b/generated_api_shadow/envoy/config/listener/v3/listener_components.proto
diff --git a/source/extensions/filters/listener/http_inspector/config.cc b/source/extensions/filters/listener/http_inspector/config.cc
@@ -38,7 +38,8 @@ class HttpInspectorConfigFactory : public Server::Configuration::NamedListenerFi
  * Static registration for the http inspector filter. @see RegisterFactory.
  */
 REGISTER_FACTORY(HttpInspectorConfigFactory,
-                 Server::Configuration::NamedListenerFilterConfigFactory);
+                 Server::Configuration::NamedListenerFilterConfigFactory){
+    "envoy.listener.http_inspector"};
 
 } // namespace HttpInspector
 } // namespace ListenerFilters

diff --git a/source/extensions/filters/listener/original_dst/config.cc b/source/extensions/filters/listener/original_dst/config.cc
@@ -37,7 +37,8 @@ class OriginalDstConfigFactory : public Server::Configuration::NamedListenerFilt
 /**
  * Static registration for the original dst filter. @see RegisterFactory.
  */
-REGISTER_FACTORY(OriginalDstConfigFactory, Server::Configuration::NamedListenerFilterConfigFactory);
+REGISTER_FACTORY(OriginalDstConfigFactory, Server::Configuration::NamedListenerFilterConfigFactory){
+    "envoy.listener.original_dst"};
 
 } // namespace OriginalDst
 } // namespace ListenerFilters

diff --git a/source/extensions/filters/listener/original_src/original_src_config_factory.cc b/source/extensions/filters/listener/original_src/original_src_config_factory.cc
@@ -30,7 +30,8 @@ ProtobufTypes::MessagePtr OriginalSrcConfigFactory::createEmptyConfigProto() {
 /**
  * Static registration for the original_src filter. @see RegisterFactory.
  */
-REGISTER_FACTORY(OriginalSrcConfigFactory, Server::Configuration::NamedListenerFilterConfigFactory);
+REGISTER_FACTORY(OriginalSrcConfigFactory, Server::Configuration::NamedListenerFilterConfigFactory){
+    "envoy.listener.original_src"};
 
 } // namespace OriginalSrc
 } // namespace ListenerFilters

diff --git a/source/extensions/filters/listener/proxy_protocol/config.cc b/source/extensions/filters/listener/proxy_protocol/config.cc
@@ -38,7 +38,8 @@ class ProxyProtocolConfigFactory : public Server::Configuration::NamedListenerFi
  * Static registration for the proxy protocol filter. @see RegisterFactory.
  */
 REGISTER_FACTORY(ProxyProtocolConfigFactory,
-                 Server::Configuration::NamedListenerFilterConfigFactory);
+                 Server::Configuration::NamedListenerFilterConfigFactory){
+    "envoy.listener.proxy_protocol"};
 
 } // namespace ProxyProtocol
 } // namespace ListenerFilters

diff --git a/source/extensions/filters/listener/tls_inspector/config.cc b/source/extensions/filters/listener/tls_inspector/config.cc
@@ -40,7 +40,8 @@ class TlsInspectorConfigFactory : public Server::Configuration::NamedListenerFil
  * Static registration for the TLS inspector filter. @see RegisterFactory.
  */
 REGISTER_FACTORY(TlsInspectorConfigFactory,
-                 Server::Configuration::NamedListenerFilterConfigFactory);
+                 Server::Configuration::NamedListenerFilterConfigFactory){
+    "envoy.listener.tls_inspector"};
 
 } // namespace TlsInspector
 } // namespace ListenerFilters

diff --git a/source/extensions/filters/listener/well_known_names.h b/source/extensions/filters/listener/well_known_names.h
@@ -15,15 +15,15 @@ namespace ListenerFilters {
 class ListenerFilterNameValues {
 public:
   // HTTP Inspector listener filter
-  const std::string HttpInspector = "envoy.listener.http_inspector";
+  const std::string HttpInspector = "envoy.filters.listener.http_inspector";
   // Original destination listener filter
-  const std::string OriginalDst = "envoy.listener.original_dst";
+  const std::string OriginalDst = "envoy.filters.listener.original_dst";
   // Original source listener filter
-  const std::string OriginalSrc = "envoy.listener.original_src";
+  const std::string OriginalSrc = "envoy.filters.listener.original_src";
   // Proxy Protocol listener filter
-  const std::string ProxyProtocol = "envoy.listener.proxy_protocol";
+  const std::string ProxyProtocol = "envoy.filters.listener.proxy_protocol";
   // TLS Inspector listener filter
-  const std::string TlsInspector = "envoy.listener.tls_inspector";
+  const std::string TlsInspector = "envoy.filters.listener.tls_inspector";
 };
 
 using ListenerFilterNames = ConstSingleton<ListenerFilterNameValues>;

diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc
@@ -272,6 +272,7 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
         factory.createFilterFactoryFromProto(Envoy::ProtobufWkt::Empty(), *this));
   }
 
+  // TODO(zuercher) remove the deprecated TLS inspector name when the deprecated names are removed.
   const bool need_tls_inspector =
       std::any_of(
           config.filter_chains().begin(), config.filter_chains().end(),
@@ -281,11 +282,13 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
                    (matcher.transport_protocol().empty() &&
                     (!matcher.server_names().empty() || !matcher.application_protocols().empty()));
           }) &&
-      !std::any_of(config.listener_filters().begin(), config.listener_filters().end(),
-                   [](const auto& filter) {
-                     return filter.name() ==
-                            Extensions::ListenerFilters::ListenerFilterNames::get().TlsInspector;
-                   });
+      !std::any_of(
+          config.listener_filters().begin(), config.listener_filters().end(),
+          [](const auto& filter) {
+            return filter.name() ==
+                       Extensions::ListenerFilters::ListenerFilterNames::get().TlsInspector ||
+                   filter.name() == "envoy.listeners.tls_inspector";
+          });
   // Automatically inject TLS Inspector if it wasn't configured explicitly and it's needed.
   if (need_tls_inspector) {
     const std::string message =

diff --git a/test/extensions/filters/listener/http_inspector/http_inspector_config_test.cc b/test/extensions/filters/listener/http_inspector/http_inspector_config_test.cc
@@ -45,6 +45,16 @@ TEST(HttpInspectorConfigFactoryTest, TestCreateFactory) {
   EXPECT_NE(dynamic_cast<HttpInspector::Filter*>(added_filter.get()), nullptr);
 }
 
+// Test that the deprecated extension name still functions.
+TEST(HttpInspectorConfigFactoryTest, DEPRECATED_FEATURE_TEST(DeprecatedExtensionFilterName)) {
+  const std::string deprecated_name = "envoy.listener.http_inspector";
+
+  ASSERT_NE(
+      nullptr,
+      Registry::FactoryRegistry<
+          Server::Configuration::NamedListenerFilterConfigFactory>::getFactory(deprecated_name));
+}
+
 } // namespace
 } // namespace HttpInspector
 } // namespace ListenerFilters

diff --git a/test/extensions/filters/listener/original_dst/BUILD b/test/extensions/filters/listener/original_dst/BUILD
@@ -0,0 +1,22 @@
+licenses(["notice"])  # Apache 2
+
+load(
+    "//bazel:envoy_build_system.bzl",
+    "envoy_package",
+)
+load(
+    "//test/extensions:extensions_build_system.bzl",
+    "envoy_extension_cc_test",
+)
+
+envoy_package()
+
+envoy_extension_cc_test(
+    name = "config_test",
+    srcs = ["config_test.cc"],
+    extension_name = "envoy.filters.listener.original_dst",
+    deps = [
+        "//source/extensions/filters/listener/original_dst:config",
+        "//test/test_common:utility_lib",
+    ],
+)
diff --git a/test/extensions/filters/listener/original_dst/config_test.cc b/test/extensions/filters/listener/original_dst/config_test.cc
@@ -0,0 +1,28 @@
+#include "envoy/registry/registry.h"
+#include "envoy/server/filter_config.h"
+
+#include "test/test_common/utility.h"
+
+#include "gtest/gtest.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace ListenerFilters {
+namespace OriginalDst {
+namespace {
+
+// Test that the deprecated extension name still functions.
+TEST(OriginalDstConfigFactoryTest, DEPRECATED_FEATURE_TEST(DeprecatedExtensionFilterName)) {
+  const std::string deprecated_name = "envoy.listener.original_dst";
+
+  ASSERT_NE(
+      nullptr,
+      Registry::FactoryRegistry<
+          Server::Configuration::NamedListenerFilterConfigFactory>::getFactory(deprecated_name));
+}
+
+} // namespace
+} // namespace OriginalDst
+} // namespace ListenerFilters
+} // namespace Extensions
+} // namespace Envoy