listener: undeprecate bind_to_port (#14480)

See #5355 (comment) and above for context
Risk Level: Low
Testing: Listener manager unit tests, grep
Docs Changes: Generated documentation for the proto field
Release Notes:
#5355
Deprecated: envoy_v3_api_field_config.listener.v3.Listener.deprecated_v1 (which was already hidden) was deprecated in favor of the new field envoy_v3_api_field_config.listener.v3.Listener.bind_to_port

Signed-off-by: Taylor Barrella <tabarr@google.com>

api/envoy/config/listener/v3/listener.proto

@@ -36,7 +36,7 @@ message ListenerCollection {
   repeated xds.core.v3.CollectionEntry entries = 1;
 }
 
-// [#next-free-field: 26]
+// [#next-free-field: 27]
 message Listener {
   option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Listener";
 
@@ -60,12 +60,8 @@ message Listener {
     // bind can only receive connections redirected from other listeners that
     // set use_original_dst parameter to true. Default is true.
     //
-    // This is deprecated in v2, all Listeners will bind to their port. An
-    // additional filter chain must be created for every original destination
-    // port this listener may redirect to in v2, with the original port
-    // specified in the FilterChainMatch destination_port field.
-    //
-    // [#comment:TODO(PiotrSikora): Remove this once verified that we no longer need it.]
+    // This is deprecated. Use :ref:`Listener.bind_to_port
+    // <envoy_api_field_config.listener.v3.Listener.bind_to_port>`
     google.protobuf.BoolValue bind_to_port = 1;
   }
 
@@ -134,7 +130,7 @@ message Listener {
   core.v3.Metadata metadata = 6;
 
   // [#not-implemented-hide:]
-  DeprecatedV1 deprecated_v1 = 7;
+  DeprecatedV1 deprecated_v1 = 7 [deprecated = true];
 
   // The type of draining to perform at a listener-wide level.
   DrainType drain_type = 8;
@@ -272,4 +268,10 @@ message Listener {
   // The maximum length a tcp listener's pending connections queue can grow to. If no value is
   // provided net.core.somaxconn will be used on Linux and 128 otherwise.
   google.protobuf.UInt32Value tcp_backlog_size = 24;
+
+  // Whether the listener should bind to the port. A listener that doesn't
+  // bind can only receive connections redirected from other listeners that set
+  // :ref:`use_original_dst <envoy_api_field_config.listener.v3.Listener.use_original_dst>`
+  // to true. Default is true.
+  google.protobuf.BoolValue bind_to_port = 26;
 }

docs/root/version_history/current.rst

@@ -84,6 +84,7 @@ New Features
 * kill_request: added new :ref:`HTTP kill request filter <config_http_filters_kill_request>`.
 * listener: added an optional :ref:`default filter chain <envoy_v3_api_field_config.listener.v3.Listener.default_filter_chain>`. If this field is supplied, and none of the :ref:`filter_chains <envoy_v3_api_field_config.listener.v3.Listener.filter_chains>` matches, this default filter chain is used to serve the connection.
 * listener: added back the :ref:`use_original_dst field <envoy_v3_api_field_config.listener.v3.Listener.use_original_dst>`.
+* listener: added the :ref:`Listener.bind_to_port field <envoy_v3_api_field_config.listener.v3.Listener.bind_to_port>`.
 * log: added a new custom flag ``%_`` to the log pattern to print the actual message to log, but with escaped newlines.
 * lua: added `downstreamDirectRemoteAddress()` and `downstreamLocalAddress()` APIs to :ref:`streamInfo() <config_http_filters_lua_stream_info_wrapper>`.
 * mongo_proxy: the list of commands to produce metrics for is now :ref:`configurable <envoy_v3_api_field_extensions.filters.network.mongo_proxy.v3.MongoProxy.commands>`.

source/server/listener_impl.cc

@@ -73,6 +73,11 @@ bool usesProxyProto(const envoy::config::listener::v3::Listener& config) {
       config.filter_chains().empty() ? config.default_filter_chain() : config.filter_chains()[0],
       use_proxy_proto, false);
 }
+
+bool shouldBindToPort(const envoy::config::listener::v3::Listener& config) {
+  return PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, bind_to_port, true) &&
+         PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.deprecated_v1(), bind_to_port, true);
+}
 } // namespace
 
 ListenSocketFactoryImpl::ListenSocketFactoryImpl(ListenerComponentFactory& factory,
@@ -246,7 +251,7 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
                            const std::string& name, bool added_via_api, bool workers_started,
                            uint64_t hash, uint32_t concurrency)
     : parent_(parent), address_(Network::Address::resolveProtoAddress(config.address())),
-      bind_to_port_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.deprecated_v1(), bind_to_port, true)),
+      bind_to_port_(shouldBindToPort(config)),
       hand_off_restored_destination_connections_(
           PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_original_dst, false)),
       per_connection_buffer_limit_bytes_(
@@ -324,8 +329,7 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin,
                            const std::string& version_info, ListenerManagerImpl& parent,
                            const std::string& name, bool added_via_api, bool workers_started,
                            uint64_t hash, uint32_t concurrency)
-    : parent_(parent), address_(origin.address_),
-      bind_to_port_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.deprecated_v1(), bind_to_port, true)),
+    : parent_(parent), address_(origin.address_), bind_to_port_(shouldBindToPort(config)),
       hand_off_restored_destination_connections_(
           PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_original_dst, false)),
       per_connection_buffer_limit_bytes_(

test/server/listener_manager_impl_test.cc

@@ -556,8 +556,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, StatsScopeTest) {
   socket_address:
     address: 127.0.0.1
     port_value: 1234
-deprecated_v1:
-  bind_to_port: false
+bind_to_port: false
 filter_chains:
 - filters:
   - name: stats_test
@@ -1484,6 +1483,43 @@ TEST_F(ListenerManagerImplTest, BindToPortEqualToFalse) {
   manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
   const std::string listener_foo_yaml = R"EOF(
 name: foo
+address:
+  socket_address:
+    address: 127.0.0.1
+    port_value: 1234
+bind_to_port: false
+filter_chains:
+- filters: []
+  )EOF";
+
+  auto syscall_result = os_sys_calls_actual_.socket(AF_INET, SOCK_STREAM, 0);
+  ASSERT_TRUE(SOCKET_VALID(syscall_result.rc_));
+
+  ListenerHandle* listener_foo = expectListenerCreate(true, true);
+  EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, ListenSocketCreationParams(false)))
+      .WillOnce(Invoke([this, &syscall_result, &real_listener_factory](
+                           const Network::Address::InstanceConstSharedPtr& address,
+                           Network::Socket::Type socket_type,
+                           const Network::Socket::OptionsSharedPtr& options,
+                           const ListenSocketCreationParams& params) -> Network::SocketSharedPtr {
+        EXPECT_CALL(server_, hotRestart).Times(0);
+        // When bind_to_port is equal to false, create socket fd directly, and do not get socket
+        // fd through hot restart.
+        ON_CALL(os_sys_calls_, socket(AF_INET, _, 0)).WillByDefault(Return(syscall_result));
+        return real_listener_factory.createListenSocket(address, socket_type, options, params);
+      }));
+  EXPECT_CALL(listener_foo->target_, initialize());
+  EXPECT_CALL(*listener_foo, onDestroy());
+  EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV3Yaml(listener_foo_yaml), "", true));
+}
+
+TEST_F(ListenerManagerImplTest, DEPRECATED_FEATURE_TEST(DeprecatedBindToPortEqualToFalse)) {
+  InSequence s;
+  ProdListenerComponentFactory real_listener_factory(server_);
+  EXPECT_CALL(*worker_, start(_));
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
+  const std::string listener_foo_yaml = R"EOF(
+name: foo
 address:
   socket_address:
     address: 127.0.0.1
@@ -2105,8 +2141,7 @@ name: foo
   socket_address:
     address: 0.0.0.0
     port_value: 1234
-deprecated_v1:
-  bind_to_port: false
+bind_to_port: false
 filter_chains:
 - filters: []
   )EOF";
@@ -2123,8 +2158,7 @@ name: bar
   socket_address:
     address: 0.0.0.0
     port_value: 1234
-deprecated_v1:
-  bind_to_port: false
+bind_to_port: false
 filter_chains:
 - filters: []
   )EOF";