hcm: forbid use of detection extensions with use_remote_addr/xff_num_trusted_hops

api/envoy/extensions/filters/network/http_connection_manager/v3/BUILD

@@ -6,7 +6,6 @@ licenses(["notice"])  # Apache 2
 
 api_proto_package(
     deps = [
-        "//envoy/annotations:pkg",
         "//envoy/config/accesslog/v3:pkg",
         "//envoy/config/core/v3:pkg",
         "//envoy/config/filter/network/http_connection_manager/v2:pkg",

api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -19,7 +19,6 @@ import "google/protobuf/any.proto";
 import "google/protobuf/duration.proto";
 import "google/protobuf/wrappers.proto";
 
-import "envoy/annotations/deprecation.proto";
 import "udpa/annotations/migrate.proto";
 import "udpa/annotations/security.proto";
 import "udpa/annotations/status.proto";
@@ -501,24 +500,7 @@ message HttpConnectionManager {
   // determining the origin client's IP address. The default is zero if this option
   // is not specified. See the documentation for
   // :ref:`config_http_conn_man_headers_x-forwarded-for` for more information.
-  //
-  // .. note::
-  //    This field is deprecated and instead :ref:`original_ip_detection_extensions
-  //    <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.original_ip_detection_extensions>`
-  //    should be used to configure the :ref:`xff extension <envoy_v3_api_msg_extensions.http.original_ip_detection.xff.v3.XffConfig>`
-  //    to configure IP detection using the :ref:`config_http_conn_man_headers_x-forwarded-for` header. To replace
-  //    this field use a config like the following:
-  //
-  // .. code-block:: yaml
-  //
-  //    original_ip_detection_extensions:
-  //    - name: envoy.http.original_ip_detection.xff
-  //      typed_config:
-  //        "@type": type.googleapis.com/envoy.extensions.http.original_ip_detection.xff.v3.XffConfig
-  //        xff_num_trusted_hops: 1
-  //
-  uint32 xff_num_trusted_hops = 19
-      [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"];
+  uint32 xff_num_trusted_hops = 19;
 
   // The configuration for the original IP detection extensions.
   //

docs/root/version_history/current.rst

@@ -40,6 +40,7 @@ Bug Fixes
 
 * access log: fix `%UPSTREAM_CLUSTER%` when used in http upstream access logs. Previously, it was always logging as an unset value.
 * cluster: delete pools when they're idle to fix unbounded memory use when using PROXY protocol upstream with tcp_proxy. This behavior can be temporarily reverted by setting the ``envoy.reloadable_features.conn_pool_delete_when_idle`` runtime guard to false.
+* hcm: remove deprecation for xff_num_trusted_hops and forbid mixing ip detection extensions with old related knobs.
 * xray: fix the AWS X-Ray tracer bug where span's error, fault and throttle information was not reported properly as per the `AWS X-Ray documentation <https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html>`_. Before this fix, server error was reported under 'annotations' section of the segment data.
 
 Removed Config or Runtime

source/extensions/filters/network/http_connection_manager/config.cc

@@ -373,6 +373,16 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
     auto* extension = ip_detection_extensions.Add();
     extension->set_name("envoy.http.original_ip_detection.xff");
     extension->mutable_typed_config()->PackFrom(xff_config);
+  } else {
+    if (use_remote_address_) {
+      throw EnvoyException(
+          "Original IP detection extensions and use_remote_address may not be mixed");
+    }
+
+    if (xff_num_trusted_hops_ > 0) {
+      throw EnvoyException(
+          "Original IP detection extensions and xff_num_trusted_hops may not be mixed");
+    }
   }
 
   original_ip_detection_extensions_.reserve(ip_detection_extensions.size());

test/extensions/filters/network/http_connection_manager/config_test.cc

@@ -2075,6 +2075,46 @@ TEST_F(HttpConnectionManagerConfigTest, OriginalIPDetectionExtension) {
   EXPECT_EQ(1, original_ip_detection_extensions.size());
 }
 
+TEST_F(HttpConnectionManagerConfigTest, OriginalIPDetectionExtensionMixedWithUseRemoteAddress) {
+  const std::string yaml_string = R"EOF(
+  stat_prefix: ingress_http
+  route_config:
+    name: local_route
+  use_remote_address: true
+  original_ip_detection_extensions:
+  - name: envoy.http.original_ip_detection.custom_header
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.http.original_ip_detection.custom_header.v3.CustomHeaderConfig
+      header_name: x-ip-header
+  http_filters:
+  - name: envoy.filters.http.router
+  )EOF";
+
+  EXPECT_THROW_WITH_REGEX(
+      createHttpConnectionManagerConfig(yaml_string), EnvoyException,
+      "Original IP detection extensions and use_remote_address may not be mixed");
+}
+
+TEST_F(HttpConnectionManagerConfigTest, OriginalIPDetectionExtensionMixedWithNumTrustedHops) {
+  const std::string yaml_string = R"EOF(
+  stat_prefix: ingress_http
+  route_config:
+    name: local_route
+  xff_num_trusted_hops: 1
+  original_ip_detection_extensions:
+  - name: envoy.http.original_ip_detection.custom_header
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.http.original_ip_detection.custom_header.v3.CustomHeaderConfig
+      header_name: x-ip-header
+  http_filters:
+  - name: envoy.filters.http.router
+  )EOF";
+
+  EXPECT_THROW_WITH_REGEX(
+      createHttpConnectionManagerConfig(yaml_string), EnvoyException,
+      "Original IP detection extensions and xff_num_trusted_hops may not be mixed");
+}
+
 TEST_F(HttpConnectionManagerConfigTest, DynamicFilterWarmingNoDefault) {
   const std::string yaml_string = R"EOF(
 codec_type: http1