From e69102e3cd70d3b4ac12cb668ba7c6d6aaf8a0e5 Mon Sep 17 00:00:00 2001 From: Thibault Deutsch Date: Mon, 26 Feb 2024 12:06:12 +0000 Subject: [PATCH] module: add listener_max_connection_lifetime_ms option Add listener_max_connection_lifetime_ms, similar to how listener_idle_timeout_ms is implemented. This limit the maximum age of a downstream connections by adding max_connection_duration to the listener's common_http_protocol_options. This option could already be set on the clusters (upstream connections), but setting it on the downstream connections could be useful in multiple scenarios: - to periodically rebalance the load between multiple emissary-ingress, especially when most of the clients are using HTTP2/gRPC and connection stick for a really long time. - when using mTLS (client certificate), to ensure that clients reconnect with their renewed certificate, as Envoy doesn't close the connection when the certificate expires. Fix #2900 Signed-off-by: Thibault Deutsch --- python/ambassador/envoy/v3/v3listener.py | 9 ++ python/ambassador/ir/ir.py | 3 + python/ambassador/ir/irambassador.py | 2 + .../kat/t_listenermaxconnectionlifetime.py | 83 +++++++++++++++++++ ...t_listener_common_http_protocol_options.py | 21 ++++- 5 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 python/tests/kat/t_listenermaxconnectionlifetime.py diff --git a/python/ambassador/envoy/v3/v3listener.py b/python/ambassador/envoy/v3/v3listener.py index b2eda1f279..8b3bb3eabf 100644 --- a/python/ambassador/envoy/v3/v3listener.py +++ b/python/ambassador/envoy/v3/v3listener.py @@ -523,6 +523,15 @@ def base_http_config(self) -> Dict[str, Any]: "idle_timeout": "%0.3fs" % (float(listener_idle_timeout_ms) / 1000.0) } + listener_max_connection_lifetime_ms = self.config.ir.ambassador_module.get( + "listener_max_connection_lifetime_ms", None + ) + if listener_max_connection_lifetime_ms: + common_http_options = base_http_config.setdefault("common_http_protocol_options", {}) + common_http_options["max_connection_duration"] = "%0.3fs" % ( + float(listener_max_connection_lifetime_ms) / 1000.0 + ) + if "headers_with_underscores_action" in self.config.ir.ambassador_module: if "common_http_protocol_options" in base_http_config: base_http_config["common_http_protocol_options"][ diff --git a/python/ambassador/ir/ir.py b/python/ambassador/ir/ir.py index 5676e56be9..b1cc75b288 100644 --- a/python/ambassador/ir/ir.py +++ b/python/ambassador/ir/ir.py @@ -1160,6 +1160,9 @@ def features(self) -> Dict[str, Any]: od["listener_idle_timeout_ms"] = self.ambassador_module.get( "listener_idle_timeout_ms", None ) + od["listener_max_connection_lifetime_ms"] = self.ambassador_module.get( + "listener_max_connection_lifetime_ms", None + ) od["headers_with_underscores_action"] = self.ambassador_module.get( "headers_with_underscores_action", None ) diff --git a/python/ambassador/ir/irambassador.py b/python/ambassador/ir/irambassador.py index fd10cd4a7f..7362a4bdbf 100644 --- a/python/ambassador/ir/irambassador.py +++ b/python/ambassador/ir/irambassador.py @@ -49,6 +49,7 @@ class IRAmbassador(IRResource): "headers_with_underscores_action", "keepalive", "listener_idle_timeout_ms", + "listener_max_connection_lifetime_ms", "liveness_probe", "load_balancer", "max_request_headers_kb", @@ -132,6 +133,7 @@ def __init__( envoy_validation_timeout=IRAmbassador.default_validation_timeout, enable_ipv4=True, listener_idle_timeout_ms=None, + listener_max_connection_lifetime_ms=None, liveness_probe={"enabled": True}, readiness_probe={"enabled": True}, diagnostics={"enabled": True}, # TODO(lukeshu): In getambassador.io/v3alpha2, change diff --git a/python/tests/kat/t_listenermaxconnectionlifetime.py b/python/tests/kat/t_listenermaxconnectionlifetime.py new file mode 100644 index 0000000000..035a146d7d --- /dev/null +++ b/python/tests/kat/t_listenermaxconnectionlifetime.py @@ -0,0 +1,83 @@ +import json +from typing import Generator, Tuple, Union + +from abstract_tests import HTTP, AmbassadorTest, Node, ServiceType +from kat.harness import Query + + +class ListenerMaxConnectionLifetime(AmbassadorTest): + target: ServiceType + + def init(self): + self.target = HTTP() + + def config(self) -> Generator[Union[str, Tuple[Node, str]], None, None]: + yield self, self.format( + """ +--- +apiVersion: getambassador.io/v3alpha1 +kind: Module +name: ambassador +ambassador_id: [{self.ambassador_id}] +config: + listener_max_connection_lifetime_ms: 3600000 +""" + ) + yield self, self.format( + """ +--- +apiVersion: getambassador.io/v3alpha1 +kind: Mapping +name: config__dump +hostname: "*" +prefix: /config_dump +rewrite: /config_dump +service: http://127.0.0.1:8001 +""" + ) + + def queries(self): + yield Query(self.url("config_dump"), phase=2) + + def check(self): + expected_val = "3600s" + actual_val = "" + assert self.results[0].body + body = json.loads(self.results[0].body) + for config_obj in body.get("configs"): + if config_obj.get("@type") == "type.googleapis.com/envoy.admin.v3.ListenersConfigDump": + listeners = config_obj.get("dynamic_listeners") + found_max_conn_duration = False + for listener_obj in listeners: + listener = listener_obj.get("active_state").get("listener") + filter_chains = listener.get("filter_chains") + for filters in filter_chains: + for filter in filters.get("filters"): + if ( + filter.get("name") + == "envoy.filters.network.http_connection_manager" + ): + filter_config = filter.get("typed_config") + common_http_protocol_options = filter_config.get( + "common_http_protocol_options" + ) + if common_http_protocol_options: + actual_val = common_http_protocol_options.get( + "max_connection_duration", "" + ) + if actual_val != "": + if actual_val == expected_val: + found_max_conn_duration = True + else: + assert ( + False + ), "Expected to find common_http_protocol_options.max_connection_duration property on listener" + else: + assert ( + False + ), "Expected to find common_http_protocol_options property on listener" + assert ( + found_max_conn_duration + ), "Expected common_http_protocol_options.max_connection_duration = {}, Got common_http_protocol_options.max_connection_duration = {}".format( + expected_val, actual_val + ) diff --git a/python/tests/unit/test_listener_common_http_protocol_options.py b/python/tests/unit/test_listener_common_http_protocol_options.py index d64c580c51..c3d1da05e8 100644 --- a/python/tests/unit/test_listener_common_http_protocol_options.py +++ b/python/tests/unit/test_listener_common_http_protocol_options.py @@ -40,12 +40,29 @@ def test_listener_idle_timeout_ms(): _test_listener_common_http_protocol_options(yaml, expectations={"idle_timeout": "150.000s"}) +@pytest.mark.compilertest +def test_listener_max_connection_lifetime_ms(): + yaml = module_and_mapping_manifests(["listener_max_connection_lifetime_ms: 3600000"], []) + _test_listener_common_http_protocol_options( + yaml, expectations={"max_connection_duration": "3600.000s"} + ) + + @pytest.mark.compilertest def test_all_listener_common_http_protocol_options(): yaml = module_and_mapping_manifests( - ["headers_with_underscores_action: DROP_HEADER", "listener_idle_timeout_ms: 4005"], [] + [ + "headers_with_underscores_action: DROP_HEADER", + "listener_idle_timeout_ms: 4005", + "listener_max_connection_lifetime_ms: 3600005", + ], + [], ) _test_listener_common_http_protocol_options( yaml, - expectations={"headers_with_underscores_action": "DROP_HEADER", "idle_timeout": "4.005s"}, + expectations={ + "headers_with_underscores_action": "DROP_HEADER", + "idle_timeout": "4.005s", + "max_connection_duration": "3600.005s", + }, )