From da9881d5253b1304d56ca388d57fe13f331a7024 Mon Sep 17 00:00:00 2001 From: Johnny Greco Date: Thu, 6 Aug 2026 18:07:05 -0400 Subject: [PATCH 1/5] Clarify gateway and middleware timeouts --- projects/egress-gate/README.md | 10 ++- .../docs/architecture/service-boundary.md | 14 ++-- .../diagrams/component-architecture.svg | 2 +- .../assets/diagrams/processing-pipeline.svg | 2 +- projects/egress-gate/docs/evaluation.md | 2 +- projects/egress-gate/docs/operations.md | 22 +++++-- .../docs/reference/limits-and-failures.md | 4 +- .../examples/class-based-gate/README.md | 2 +- .../examples/custom-gate/README.md | 2 +- .../examples/regex-redaction/README.md | 2 +- projects/egress-gate/src/egress_gate/cli.py | 63 +++++++++++------- .../egress-gate/src/egress_gate/constants.py | 14 ++-- .../egress-gate/src/egress_gate/errors.py | 6 +- .../src/egress_gate/gateway_config.py | 39 +++++++++-- .../src/egress_gate/service/server.py | 13 ++-- .../src/egress_gate/service/servicer.py | 21 ++++-- .../egress-gate/src/egress_gate/timeout.py | 64 ++++++++++++++++--- .../egress-gate/tests/service/test_server.py | 28 +++++--- .../tests/service/test_servicer.py | 18 ++++-- projects/egress-gate/tests/test_cli.py | 57 +++++++++++++++-- .../egress-gate/tests/test_gateway_config.py | 38 +++++++++-- projects/egress-gate/tests/test_timeout.py | 46 +++++++++++-- 22 files changed, 365 insertions(+), 104 deletions(-) diff --git a/projects/egress-gate/README.md b/projects/egress-gate/README.md index 84f37e35..e85e39b5 100644 --- a/projects/egress-gate/README.md +++ b/projects/egress-gate/README.md @@ -92,12 +92,18 @@ written by a harness to disk. from egress_gate.gates import create_builtin_registry from egress_gate.service import EgressGateServer -server = EgressGateServer(create_builtin_registry()) +server = EgressGateServer( + create_builtin_registry(), + timeout_middleware_processing=10, +) server.serve_sync("127.0.0.1:50051") ``` -The service creates one `Timeout` per evaluation and passes that deadline +`timeout_middleware_processing` is the actual service setting, in seconds. The +service turns it into one `Timeout` per evaluation and passes that same deadline through slot acquisition, policy preparation, and `RequestProcessor.process`. +The OpenShell gateway enforces its separately configured +`timeout_gateway_ceiling` as an upper bound. ## Documentation and examples diff --git a/projects/egress-gate/docs/architecture/service-boundary.md b/projects/egress-gate/docs/architecture/service-boundary.md index 00f3ed4c..1a20c24b 100644 --- a/projects/egress-gate/docs/architecture/service-boundary.md +++ b/projects/egress-gate/docs/architecture/service-boundary.md @@ -19,7 +19,7 @@ supervisor applies allowed mutations to the intercepted request. | RPC | Behavior | | --- | --- | -| `Describe` | Advertise Egress Gate and the pre-credentials HTTP binding | +| `Describe` | Return Egress Gate's pre-credentials HTTP binding | | `ValidateConfig` | Validate a complete registry-backed pipeline without publishing it | | `EvaluateHttpRequest` | Adapt one request, prepare/reuse policy, execute, and serialize | @@ -29,10 +29,14 @@ encoded configuration before registry parsing. ## Shared deadline and workers -`EvaluateHttpRequest` creates one monotonic `Timeout`. That same deadline is -used for semaphore acquisition, policy preparation, replacement-lock waits, -gate execution, and final result checks. `RequestProcessor.process` accepts the -caller-owned timeout and never creates or stores one. +The service's `timeout_middleware_processing` value is returned in the +`MiddlewareBinding` from `Describe`. `EvaluateHttpRequest` converts that same +value into one monotonic `Timeout` used for semaphore acquisition, policy +preparation, replacement-lock waits, gate execution, and final result checks. +The gateway's separate `timeout_gateway_ceiling` can shorten, but never extend, +that processing time. +`RequestProcessor.process` accepts the caller-owned timeout and never creates or +stores one. Synchronous work runs in a bounded four-slot executor. The gRPC server permits sixteen concurrent RPCs. Cancellation does not stop Python code that already diff --git a/projects/egress-gate/docs/assets/diagrams/component-architecture.svg b/projects/egress-gate/docs/assets/diagrams/component-architecture.svg index 0f5db97c..dc7ca530 100644 --- a/projects/egress-gate/docs/assets/diagrams/component-architecture.svg +++ b/projects/egress-gate/docs/assets/diagrams/component-architecture.svg @@ -57,7 +57,7 @@ PIPELINE PROCESSOR RequestProcessor - gate order · one timeout · aggregation · allow or deny + gate order · one deadline · aggregation · allow or deny REQUEST GATES diff --git a/projects/egress-gate/docs/assets/diagrams/processing-pipeline.svg b/projects/egress-gate/docs/assets/diagrams/processing-pipeline.svg index f1f26e99..901f114c 100644 --- a/projects/egress-gate/docs/assets/diagrams/processing-pipeline.svg +++ b/projects/egress-gate/docs/assets/diagrams/processing-pipeline.svg @@ -44,7 +44,7 @@ 2 Pipeline processor - One shared timeout + bounds + One shared deadline + bounds diff --git a/projects/egress-gate/docs/evaluation.md b/projects/egress-gate/docs/evaluation.md index a6650a28..bd678a61 100644 --- a/projects/egress-gate/docs/evaluation.md +++ b/projects/egress-gate/docs/evaluation.md @@ -36,7 +36,7 @@ The repository includes a regex policy and two request cases. Run them from uv run egress-gate evaluate \ --policy examples/regex-redaction/egress-gate-config.yaml \ --cases examples/regex-redaction/cases.yaml \ - --timeout-seconds 1 + --timeout 1s ``` The command prepares the policy once, runs each case with a fresh timeout, and diff --git a/projects/egress-gate/docs/operations.md b/projects/egress-gate/docs/operations.md index 4ff39f8c..09420488 100644 --- a/projects/egress-gate/docs/operations.md +++ b/projects/egress-gate/docs/operations.md @@ -13,7 +13,7 @@ environment as needed. ```bash title="Start Egress Gate" uv run egress-gate gates list uv run egress-gate gates schema -uv run egress-gate serve --listen 0.0.0.0:50051 --timeout-seconds 4 +uv run egress-gate serve --listen 0.0.0.0:50051 --timeout 4s ``` Use a reachable non-loopback address only when the supervisor is outside the @@ -37,6 +37,12 @@ The command updates `OPENSHELL_GATEWAY_CONFIG`, then Start the gateways again with the same commands or service managers that you normally use. +The registration writes `timeout = "30s"` in the gateway TOML. Egress Gate calls +this setting `timeout_gateway_ceiling`: the maximum time the gateway permits +for Egress Gate. Set the actual service value with +`egress-gate serve --timeout DURATION`; Egress Gate calls that setting +`timeout_middleware_processing`. + To remove a registration, stop any running gateways that use the configuration again. List the available names with: @@ -54,9 +60,17 @@ uv run egress-gate remove-gateway-registration --name egress-gate Start the gateways again after the command completes. -The generated OpenShell middleware timeout is five seconds. Keep the Egress -Gate `--timeout-seconds` below it so queueing, preparation, and transport have -headroom. +Egress Gate uses `timeout_middleware_processing` across queueing, policy +preparation, and every configured gate. OpenShell applies +`timeout_gateway_ceiling` as an independent upper bound: + +```text +effective timeout = min(timeout_gateway_ceiling, timeout_middleware_processing) +``` + +With the registration command's 30s ceiling, any supported `--timeout` value +becomes the effective timeout. If an operator lowers the gateway ceiling by +editing the gateway configuration, that lower value wins. If the middleware RPC returns gRPC `RESOURCE_EXHAUSTED`, capacity may remain accounted for briefly while completed RPCs are torn down. The OpenShell gateway diff --git a/projects/egress-gate/docs/reference/limits-and-failures.md b/projects/egress-gate/docs/reference/limits-and-failures.md index 2bec400a..36cb7e94 100644 --- a/projects/egress-gate/docs/reference/limits-and-failures.md +++ b/projects/egress-gate/docs/reference/limits-and-failures.md @@ -20,7 +20,9 @@ limits. | Result metadata aggregate strings | 32 KiB | | Gate traces per result | 10 | | Header mutations per gate evaluation | 64 | -| Processing timeout | 30 seconds maximum | +| Offline `--timeout` | 30 seconds maximum | +| `timeout_middleware_processing` | 30 seconds maximum | +| `timeout_gateway_ceiling` | 30 seconds in registrations created by Egress Gate | | Concurrent processing slots | 4 | Request context and target aggregates, headers, replacement bodies, regex diff --git a/projects/egress-gate/examples/class-based-gate/README.md b/projects/egress-gate/examples/class-based-gate/README.md index fecbe9d5..48763139 100644 --- a/projects/egress-gate/examples/class-based-gate/README.md +++ b/projects/egress-gate/examples/class-based-gate/README.md @@ -39,7 +39,7 @@ Start Egress Gate with this example registry and content-safe debug diagnostics: uv run egress-gate \ --debug \ --registry examples.class-based-gate.keyword_gate:registry \ - serve --listen 0.0.0.0:50051 --timeout-seconds 4 + serve --listen 0.0.0.0:50051 --timeout 4s ``` Before you change the gateway configuration, stop any running OpenShell diff --git a/projects/egress-gate/examples/custom-gate/README.md b/projects/egress-gate/examples/custom-gate/README.md index 38238df1..2905f8c2 100644 --- a/projects/egress-gate/examples/custom-gate/README.md +++ b/projects/egress-gate/examples/custom-gate/README.md @@ -46,7 +46,7 @@ Start Egress Gate with this example registry and content-safe debug diagnostics: uv run egress-gate \ --debug \ --registry examples.custom-gate.keyword_gate:registry \ - serve --listen 0.0.0.0:50051 --timeout-seconds 4 + serve --listen 0.0.0.0:50051 --timeout 4s ``` Before you change the gateway configuration, stop any running OpenShell diff --git a/projects/egress-gate/examples/regex-redaction/README.md b/projects/egress-gate/examples/regex-redaction/README.md index 9190b796..7df9a3d1 100644 --- a/projects/egress-gate/examples/regex-redaction/README.md +++ b/projects/egress-gate/examples/regex-redaction/README.md @@ -26,7 +26,7 @@ working directory contains the pattern catalog referenced by `policy.yaml`. ```bash uv run egress-gate --debug serve \ --listen 0.0.0.0:50051 \ - --timeout-seconds 4 + --timeout 4s ``` Before you change the gateway configuration, stop any running OpenShell diff --git a/projects/egress-gate/src/egress_gate/cli.py b/projects/egress-gate/src/egress_gate/cli.py index fa8e9d1a..e328be8c 100644 --- a/projects/egress-gate/src/egress_gate/cli.py +++ b/projects/egress-gate/src/egress_gate/cli.py @@ -29,14 +29,14 @@ from egress_gate.base import StrictDomainModel from egress_gate.constants import ( - DEFAULT_TIMEOUT_SECONDS, + DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING, MAX_BODY_BYTES, MAX_EVALUATION_CASE_NAME_BYTES, MAX_EVALUATION_CASES, MAX_EVALUATION_FILE_BYTES, MAX_EVALUATION_TAGS, MAX_PROTO_FINDING_GROUPS, - MAX_TIMEOUT_SECONDS, + TIMEOUT_GATEWAY_CEILING, ) from egress_gate.errors import EgressGateError, GateRegistryError from egress_gate.gates.base import GateCapability @@ -61,7 +61,11 @@ from egress_gate.request import HttpHeader, HttpRequest, HttpTarget, RequestContext from egress_gate.result import EgressResult, GateDecisionSource from egress_gate.string_validators import BoundedMetadataString -from egress_gate.timeout import Timeout, validate_timeout_seconds +from egress_gate.timeout import ( + Timeout, + parse_timeout_duration, + validate_timeout_middleware_processing, +) app = typer.Typer( name="egress-gate", @@ -138,31 +142,32 @@ def serve( ), ), ] = "127.0.0.1:50051", - timeout_seconds: Annotated[ - float, + timeout: Annotated[ + str, typer.Option( + "--timeout", help=( - "Total processing time available to all gates for one request. " - f"The value must be greater than 0 and at most {MAX_TIMEOUT_SECONDS:g}." + "Middleware processing timeout for one request, for example 10s " + "or 500ms. It cannot exceed the 30s gateway ceiling." ), ), - ] = DEFAULT_TIMEOUT_SECONDS, + ] = f"{DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING:g}s", ) -> None: """Start the Egress Gate gRPC service and run until shutdown.""" options = _command_options(context) from egress_gate.service.server import EgressGateServer try: - validated_timeout_seconds = validate_timeout_seconds(timeout_seconds) + timeout_middleware_processing = parse_timeout_duration(timeout) except ValueError as error: raise typer.BadParameter( str(error), - param_hint="--timeout-seconds", + param_hint="--timeout", ) from None try: EgressGateServer( options.registry, - timeout_seconds=validated_timeout_seconds, + timeout_middleware_processing=timeout_middleware_processing, ).serve_sync(listen) except EgressGateError as error: _render_egress_error("Egress Gate could not start", error) @@ -214,7 +219,7 @@ def add_gateway_registration( ), ] = 50051, ) -> None: - """Add or update Egress Gate in an OpenShell gateway TOML file.""" + """Add or update Egress Gate with a 30s timeout gateway ceiling.""" try: address = ipaddress.IPv4Address(host_ip) except ipaddress.AddressValueError: @@ -235,7 +240,6 @@ def add_gateway_registration( str(error), param_hint="--name", ) from None - config_path = config or default_gateway_config_path() try: result = update_gateway_config( @@ -263,6 +267,7 @@ def add_gateway_registration( config_path=config_path, name=validated_name, endpoint=f"http://{address}:{port}", + timeout_gateway_ceiling=f"{TIMEOUT_GATEWAY_CEILING:g}s", change=change, next_step=( "Start Egress Gate, then restart the OpenShell gateway to load this " @@ -441,24 +446,25 @@ def evaluate( help="Path to the YAML file of saved request cases and expected results.", ), ], - timeout_seconds: Annotated[ - float, + timeout: Annotated[ + str, typer.Option( + "--timeout", help=( - "Maximum seconds for policy preparation and, separately, each case. " - f"The value must be greater than 0 and at most {MAX_TIMEOUT_SECONDS:g}." + "Timeout for policy preparation and, separately, each case, for " + f"example 1s or 500ms. Maximum {TIMEOUT_GATEWAY_CEILING:g}s." ), ), - ] = DEFAULT_TIMEOUT_SECONDS, + ] = f"{DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING:g}s", ) -> None: """Test saved requests against a policy without starting the service.""" options = _command_options(context) try: - validated_timeout_seconds = validate_timeout_seconds(timeout_seconds) + timeout_seconds = parse_timeout_duration(timeout) except ValueError as error: raise typer.BadParameter( str(error), - param_hint="--timeout-seconds", + param_hint="--timeout", ) from None try: policy_values = _load_policy(policy) @@ -485,7 +491,7 @@ def evaluate( options.registry, policy_values, corpus, - timeout_seconds=validated_timeout_seconds, + timeout_seconds=timeout_seconds, ) except _CaseExecutionError as error: if error.completed: @@ -854,7 +860,7 @@ def _run_corpus( timeout_seconds: float, ) -> _EvaluationSummary: """Prepare once, then evaluate every case with a fresh shared timeout.""" - validated_timeout = validate_timeout_seconds(timeout_seconds) + validated_timeout = validate_timeout_middleware_processing(timeout_seconds) validated_config = registry.validate_config(policy_values) processor = registry.prepare_processor( validated_config, @@ -988,6 +994,7 @@ def _render_registration( config_path: Path, name: str, endpoint: str | None = None, + timeout_gateway_ceiling: str | None = None, change: str | None = None, next_step: str | None = None, status_style: str = "bold green", @@ -1001,6 +1008,11 @@ def _render_registration( details.add_row("Registration", Text(name)) if endpoint is not None: details.add_row("Endpoint", Text(endpoint)) + if timeout_gateway_ceiling is not None: + details.add_row( + "Timeout gateway ceiling", + Text(timeout_gateway_ceiling), + ) if change is not None: details.add_row("Change", Text(change)) _CONSOLE.print(details) @@ -1022,8 +1034,13 @@ def _render_gateway_registrations( table = Table(box=None, pad_edge=False, padding=(0, 2), header_style="bold cyan") table.add_column("Name", style="bold", no_wrap=True) table.add_column("Endpoint", overflow="fold") + table.add_column("Timeout gateway ceiling", no_wrap=True) for registration in registrations: - table.add_row(registration.name, registration.endpoint or "Not set") + table.add_row( + registration.name, + registration.endpoint or "Not set", + registration.timeout_gateway_ceiling or "Not set", + ) _CONSOLE.print(table) _CONSOLE.print( Text.assemble( diff --git a/projects/egress-gate/src/egress_gate/constants.py b/projects/egress-gate/src/egress_gate/constants.py index 06716351..363a38e4 100644 --- a/projects/egress-gate/src/egress_gate/constants.py +++ b/projects/egress-gate/src/egress_gate/constants.py @@ -9,9 +9,9 @@ import re from importlib.metadata import version -# Configurable processing timeout. -DEFAULT_TIMEOUT_SECONDS = 1.0 -MAX_TIMEOUT_SECONDS = 30.0 +# Middleware processing timeout and the gateway's upper bound for it. +DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING = 1.0 +TIMEOUT_GATEWAY_CEILING = 30.0 # Middleware identity and stable response values. SERVICE_NAME = "egress-gate" @@ -21,12 +21,8 @@ LIMIT_REASON = ( "Egress Gate exceeded a processing safety limit. Check Egress Gate logs " "for the limit kind. Reduce the request or replacement size, simplify the " - "configured gates and rules, or increase the processing timeout with " - "--timeout-seconds or " - "EgressGateServer(timeout_seconds=...) to at most " - f"{MAX_TIMEOUT_SECONDS:g} seconds. If increasing it, give OpenShell's " - "middleware timeout additional headroom for queueing and configuration " - "preparation, then retry." + "configured gates and rules, or increase the timeout passed to " + "egress-gate serve --timeout, then retry." ) LIMIT_REASON_CODE = "egress_gate_limit_exceeded" # Text input limits. diff --git a/projects/egress-gate/src/egress_gate/errors.py b/projects/egress-gate/src/egress_gate/errors.py index 3d99f5cf..670b2aed 100644 --- a/projects/egress-gate/src/egress_gate/errors.py +++ b/projects/egress-gate/src/egress_gate/errors.py @@ -5,7 +5,7 @@ from dataclasses import dataclass from enum import StrEnum -from egress_gate.constants import MAX_PROTO_CONFIG_BYTES, MAX_TIMEOUT_SECONDS +from egress_gate.constants import MAX_PROTO_CONFIG_BYTES, TIMEOUT_GATEWAY_CEILING class ErrorKind(StrEnum): @@ -106,8 +106,8 @@ class TimeoutExpiredError(Exception): def __init__(self) -> None: super().__init__( "Egress Gate processing timed out. Reduce the request size or simplify " - "the configured gates and rules, or increase the processing timeout " - f"to at most {MAX_TIMEOUT_SECONDS:g} seconds, then retry." + "the configured gates and rules, or increase egress-gate serve " + f"--timeout to at most {TIMEOUT_GATEWAY_CEILING:g}s, then retry." ) diff --git a/projects/egress-gate/src/egress_gate/gateway_config.py b/projects/egress-gate/src/egress_gate/gateway_config.py index c6b0eef8..1238e819 100644 --- a/projects/egress-gate/src/egress_gate/gateway_config.py +++ b/projects/egress-gate/src/egress_gate/gateway_config.py @@ -12,6 +12,8 @@ from enum import Enum from pathlib import Path +from egress_gate.constants import TIMEOUT_GATEWAY_CEILING + class GatewayConfigUpdate(Enum): """Result of writing one Egress Gate middleware registration.""" @@ -39,6 +41,7 @@ class GatewayMiddlewareRegistration: name: str endpoint: str | None + timeout_gateway_ceiling: str | None # Mirrors OpenShell's stable-identifier byte limit for external middleware @@ -79,6 +82,7 @@ def list_gateway_registrations( for entry in _middleware_entries(_load_gateway_config(contents, path), path): name = entry.get("name") endpoint = entry.get("grpc_endpoint") + timeout_gateway_ceiling = entry.get("timeout") if not isinstance(name, str) or not name: raise GatewayConfigError( f"{path} contains a middleware registration without a valid name." @@ -88,8 +92,19 @@ def list_gateway_registrations( f"The middleware registration {name!r} in {path} has an invalid " "grpc_endpoint." ) + if timeout_gateway_ceiling is not None and not isinstance( + timeout_gateway_ceiling, str + ): + raise GatewayConfigError( + f"The middleware registration {name!r} in {path} has an invalid " + "timeout." + ) registrations.append( - GatewayMiddlewareRegistration(name=name, endpoint=endpoint) + GatewayMiddlewareRegistration( + name=name, + endpoint=endpoint, + timeout_gateway_ceiling=timeout_gateway_ceiling, + ) ) return tuple(registrations) @@ -255,7 +270,11 @@ def validate_middleware_name(name: str) -> str: return name -def _new_gateway_config(*, middleware_name: str, endpoint: str) -> str: +def _new_gateway_config( + *, + middleware_name: str, + endpoint: str, +) -> str: return "[openshell]\nversion = 1\n\n" + _middleware_block( middleware_name=middleware_name, endpoint=endpoint, @@ -330,17 +349,25 @@ def _append_middleware_block( ) -def _middleware_block(*, middleware_name: str, endpoint: str) -> str: +def _middleware_block( + *, + middleware_name: str, + endpoint: str, +) -> str: return ( "[[openshell.supervisor.middleware]]\n" f'name = "{middleware_name}"\n' f'grpc_endpoint = "{endpoint}"\n' "max_body_bytes = 4194304\n" - 'timeout = "5s"\n' + f'timeout = "{TIMEOUT_GATEWAY_CEILING:g}s"\n' ) -def _update_middleware_block(block: str, *, endpoint: str) -> str: +def _update_middleware_block( + block: str, + *, + endpoint: str, +) -> str: updated = _replace_or_append_assignment( block, key="grpc_endpoint", @@ -354,7 +381,7 @@ def _update_middleware_block(block: str, *, endpoint: str) -> str: return _replace_or_append_assignment( updated, key="timeout", - value='"5s"', + value=f'"{TIMEOUT_GATEWAY_CEILING:g}s"', ) diff --git a/projects/egress-gate/src/egress_gate/service/server.py b/projects/egress-gate/src/egress_gate/service/server.py index cd5857a4..6da9146c 100644 --- a/projects/egress-gate/src/egress_gate/service/server.py +++ b/projects/egress-gate/src/egress_gate/service/server.py @@ -11,7 +11,7 @@ from egress_gate.bindings import supervisor_middleware_pb2_grpc as pb2_grpc from egress_gate.constants import ( - DEFAULT_TIMEOUT_SECONDS, + DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING, MAX_CONCURRENT_RPCS, MAX_RECEIVE_MESSAGE_BYTES, ) @@ -30,11 +30,11 @@ def __init__( self, registry: GateRegistry, *, - timeout_seconds: float = DEFAULT_TIMEOUT_SECONDS, + timeout_middleware_processing: float = DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING, ) -> None: self._middleware = EgressGateMiddleware( registry, - timeout_seconds=timeout_seconds, + timeout_middleware_processing=timeout_middleware_processing, ) def serve_sync(self, listen: str = DEFAULT_LISTEN_ADDRESS) -> None: @@ -53,7 +53,12 @@ async def serve_async(self, listen: str = DEFAULT_LISTEN_ADDRESS) -> None: bound_port = server.add_insecure_port(listen) if bound_port != requested_port: raise EgressGateError(ErrorCode.SERVER_BIND_FAILED) - _LOGGER.info("egress_gate_server_bound listen=%r", listen) + _LOGGER.info( + "egress_gate_server_bound listen=%r " + "timeout_middleware_processing=%s", + listen, + self._middleware.timeout_middleware_processing, + ) await server.start() except RuntimeError: raise EgressGateError(ErrorCode.SERVER_BIND_FAILED) from None diff --git a/projects/egress-gate/src/egress_gate/service/servicer.py b/projects/egress-gate/src/egress_gate/service/servicer.py index 965365be..96fe46a0 100644 --- a/projects/egress-gate/src/egress_gate/service/servicer.py +++ b/projects/egress-gate/src/egress_gate/service/servicer.py @@ -20,7 +20,7 @@ from egress_gate.config import EgressGateConfig from egress_gate.constants import ( BLOCK_REASON, - DEFAULT_TIMEOUT_SECONDS, + DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING, LIMIT_REASON, LIMIT_REASON_CODE, MAX_BODY_BYTES, @@ -65,7 +65,7 @@ SourcedFinding, ) from egress_gate.string_validators import validate_bounded_metadata_string -from egress_gate.timeout import Timeout, validate_timeout_seconds +from egress_gate.timeout import Timeout, format_timeout_middleware_processing class EgressGateMiddleware(pb2_grpc.SupervisorMiddlewareServicer): @@ -75,11 +75,14 @@ def __init__( self, registry: GateRegistry, *, - timeout_seconds: float = DEFAULT_TIMEOUT_SECONDS, + timeout_middleware_processing: float = DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING, ) -> None: registry.configuration_json_schema() self._registry = registry - self._timeout_seconds = validate_timeout_seconds(timeout_seconds) + self._timeout_middleware_processing = format_timeout_middleware_processing( + timeout_middleware_processing + ) + self._timeout_middleware_processing_seconds = timeout_middleware_processing self._policy = _ActivePolicy(registry) self._processing_slots = asyncio.Semaphore(MAX_CONCURRENT_PROCESSING) self._processing_executor = ThreadPoolExecutor( @@ -87,6 +90,11 @@ def __init__( thread_name_prefix="egress-gate-processing", ) + @property + def timeout_middleware_processing(self) -> str: + """Return the configured middleware processing timeout.""" + return self._timeout_middleware_processing + async def close(self) -> None: """Wait for in-flight synchronous gates during shutdown.""" self._processing_executor.shutdown(wait=True, cancel_futures=True) @@ -97,7 +105,7 @@ async def Describe( request: object, context: grpc.aio.ServicerContext[object, pb2.MiddlewareManifest], ) -> pb2.MiddlewareManifest: - """Advertise the binding and its complete policy schema.""" + """Describe the binding and its complete policy schema.""" return pb2.MiddlewareManifest( name=SERVICE_NAME, service_version=SERVICE_VERSION, @@ -106,6 +114,7 @@ async def Describe( operation=pb2.SUPERVISOR_MIDDLEWARE_OPERATION_HTTP_REQUEST, phase=pb2.SUPERVISOR_MIDDLEWARE_PHASE_PRE_CREDENTIALS, max_body_bytes=MAX_BODY_BYTES, + timeout=self._timeout_middleware_processing, ) ], ) @@ -159,7 +168,7 @@ async def _evaluate_rpc( finding_count = 0 source_kind = "none" try: - timeout = Timeout.from_seconds(self._timeout_seconds) + timeout = Timeout.from_seconds(self._timeout_middleware_processing_seconds) response, source_kind = await self._evaluate_http_request( request, timeout, diff --git a/projects/egress-gate/src/egress_gate/timeout.py b/projects/egress-gate/src/egress_gate/timeout.py index cad58c55..7fc4bee4 100644 --- a/projects/egress-gate/src/egress_gate/timeout.py +++ b/projects/egress-gate/src/egress_gate/timeout.py @@ -3,6 +3,7 @@ from __future__ import annotations import math +import re from collections.abc import Iterator from contextlib import contextmanager from time import monotonic @@ -11,26 +12,63 @@ from pydantic import Field from egress_gate.base import StrictDomainModel -from egress_gate.constants import MAX_TIMEOUT_SECONDS +from egress_gate.constants import TIMEOUT_GATEWAY_CEILING from egress_gate.errors import TimeoutExpiredError -def validate_timeout_seconds(seconds: object) -> float: - """Return a finite supported processing timeout in seconds.""" +def validate_timeout_middleware_processing(seconds: object) -> float: + """Return a processing timeout within the gateway ceiling, in seconds.""" if ( isinstance(seconds, bool) or not isinstance(seconds, int | float) or not math.isfinite(seconds) or seconds <= 0 - or seconds > MAX_TIMEOUT_SECONDS + or seconds > TIMEOUT_GATEWAY_CEILING ): raise ValueError( - "timeout seconds must be a finite number greater than 0 and at most " - f"{MAX_TIMEOUT_SECONDS:g}" + "timeout_middleware_processing must be greater than 0 and no more " + f"than the {TIMEOUT_GATEWAY_CEILING:g}s timeout_gateway_ceiling" ) return float(seconds) +def format_timeout_middleware_processing(seconds: object) -> str: + """Format a processing timeout for the OpenShell duration contract.""" + validated_seconds = validate_timeout_middleware_processing(seconds) + milliseconds = validated_seconds * 1000 + rounded_milliseconds = round(milliseconds) + if rounded_milliseconds < 10 or not math.isclose( + milliseconds, + rounded_milliseconds, + ): + raise ValueError( + "timeout_middleware_processing must use whole milliseconds and be at " + "least 10ms" + ) + if rounded_milliseconds % 1000 == 0: + return f"{rounded_milliseconds // 1000}s" + return f"{rounded_milliseconds}ms" + + +def parse_timeout_duration(duration: object) -> float: + """Parse an integer OpenShell-style duration such as ``10s`` or ``500ms``.""" + if not isinstance(duration, str): + raise ValueError("timeout must be an integer duration such as 10s or 500ms") + match = _TIMEOUT_DURATION_PATTERN.fullmatch(duration) + if match is None: + raise ValueError("timeout must be an integer duration such as 10s or 500ms") + magnitude = int(match.group("magnitude")) + seconds = magnitude / 1000 if match.group("unit") == "ms" else magnitude + try: + format_timeout_middleware_processing(seconds) + except ValueError: + raise ValueError( + "timeout must be between 10ms and " + f"{TIMEOUT_GATEWAY_CEILING:g}s, using whole milliseconds" + ) from None + return float(seconds) + + class Timeout(StrictDomainModel): """A monotonic deadline shared across gate preparation and execution.""" @@ -39,7 +77,9 @@ class Timeout(StrictDomainModel): @classmethod def from_seconds(cls, seconds: float) -> Self: """Create a timeout from a finite, positive bounded duration.""" - return cls(deadline=monotonic() + validate_timeout_seconds(seconds)) + return cls( + deadline=monotonic() + validate_timeout_middleware_processing(seconds) + ) def remaining_seconds(self) -> float: """Return the positive duration remaining or raise ``TimeoutExpiredError``.""" @@ -63,4 +103,12 @@ def enforce(self) -> Iterator[None]: self.raise_if_expired() -__all__ = ["Timeout", "validate_timeout_seconds"] +_TIMEOUT_DURATION_PATTERN = re.compile(r"(?P[0-9]{1,9})(?Pms|s)") + + +__all__ = [ + "Timeout", + "format_timeout_middleware_processing", + "parse_timeout_duration", + "validate_timeout_middleware_processing", +] diff --git a/projects/egress-gate/tests/service/test_server.py b/projects/egress-gate/tests/service/test_server.py index 736ce38f..06b774d4 100644 --- a/projects/egress-gate/tests/service/test_server.py +++ b/projects/egress-gate/tests/service/test_server.py @@ -45,17 +45,29 @@ def test_server_rejects_a_registry_without_gates() -> None: EgressGateServer(GateRegistry()) -@pytest.mark.parametrize("seconds", [True, 0, 31, float("inf")]) -def test_server_validates_the_service_timeout(seconds: bool | int | float) -> None: - with pytest.raises(ValueError, match="finite number greater than 0 and at most 30"): - EgressGateServer(create_builtin_registry(), timeout_seconds=seconds) +@pytest.mark.parametrize("seconds", [True, 0, 0.001, 31, float("inf")]) +def test_server_validates_timeout_middleware_processing( + seconds: bool | int | float, +) -> None: + with pytest.raises(ValueError, match="timeout"): + EgressGateServer( + create_builtin_registry(), + timeout_middleware_processing=seconds, + ) -def test_server_keeps_timeout_ownership_at_the_service_boundary() -> None: - server = EgressGateServer(create_builtin_registry(), timeout_seconds=4.5) +def test_server_uses_timeout_middleware_processing_for_manifest_and_pipeline() -> None: + server = EgressGateServer( + create_builtin_registry(), + timeout_middleware_processing=4.5, + ) try: - assert server._middleware._timeout_seconds == 4.5 - assert not hasattr(server._middleware._policy, "_timeout_seconds") + assert server._middleware._timeout_middleware_processing_seconds == 4.5 + assert server._middleware.timeout_middleware_processing == "4500ms" + assert not hasattr( + server._middleware._policy, + "_timeout_middleware_processing_seconds", + ) finally: asyncio.run(server._middleware.close()) diff --git a/projects/egress-gate/tests/service/test_servicer.py b/projects/egress-gate/tests/service/test_servicer.py index 018c95df..34e9759d 100644 --- a/projects/egress-gate/tests/service/test_servicer.py +++ b/projects/egress-gate/tests/service/test_servicer.py @@ -109,6 +109,19 @@ async def abort(self, code: grpc.StatusCode, details: str) -> Never: raise AssertionError("successful evaluation unexpectedly aborted") +def test_manifest_contains_the_middleware_processing_timeout() -> None: + middleware = EgressGateMiddleware( + create_builtin_registry(), + timeout_middleware_processing=4.5, + ) + try: + manifest = asyncio.run(middleware.Describe(object(), Mock())) + finally: + asyncio.run(middleware.close()) + + assert manifest.bindings[0].timeout == "4500ms" + + def test_copied_proto_remains_the_current_five_field_finding_contract() -> None: evaluation = pb2.HttpRequestEvaluation() finding = pb2.Finding() @@ -425,10 +438,7 @@ def test_in_flight_processor_reference_survives_policy_replacement() -> None: async def test_cancelled_candidate_keeps_its_slot_until_worker_exits( monkeypatch: pytest.MonkeyPatch, ) -> None: - middleware = EgressGateMiddleware( - create_builtin_registry(), - timeout_seconds=5, - ) + middleware = EgressGateMiddleware(create_builtin_registry()) started = Event() release = Event() original_build = middleware._registry.prepare_processor diff --git a/projects/egress-gate/tests/test_cli.py b/projects/egress-gate/tests/test_cli.py index e25a19e5..5f717d0f 100644 --- a/projects/egress-gate/tests/test_cli.py +++ b/projects/egress-gate/tests/test_cli.py @@ -58,6 +58,42 @@ def test_cli_narrow_help_preserves_complete_option_names() -> None: assert "--conf…" not in result.stdout +def test_cli_serve_uses_one_concise_processing_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + calls: list[tuple[float, str]] = [] + + class FakeServer: + def __init__( + self, + registry: GateRegistry, + *, + timeout_middleware_processing: float, + ) -> None: + del registry + self.timeout_middleware_processing = timeout_middleware_processing + + def serve_sync(self, listen: str) -> None: + calls.append((self.timeout_middleware_processing, listen)) + + monkeypatch.setattr( + "egress_gate.service.server.EgressGateServer", + FakeServer, + ) + + result = CliRunner().invoke( + app, + ["serve", "--listen", "127.0.0.1:50055", "--timeout", "4500ms"], + ) + + assert result.exit_code == 0, result.output + assert calls == [(4.5, "127.0.0.1:50055")] + + help_result = CliRunner().invoke(app, ["serve", "--help"]) + assert "--timeout " in help_result.stdout + assert "--timeout-seconds" not in help_result.stdout + + def test_cli_gates_describes_the_request_level_builtin() -> None: result = CliRunner().invoke(app, ["gates", "list"]) @@ -405,8 +441,12 @@ def test_cli_add_gateway_registration_reports_the_result(tmp_path: Path) -> None assert "Gateway registration is ready" in result.stdout assert "Gateway file" in result.stdout assert str(config) in "".join(result.stdout.split()) - assert "Registration egress-gate" in result.stdout - assert "Endpoint http://192.0.2.10:50051" in result.stdout + assert "Registration" in result.stdout + assert "egress-gate" in result.stdout + assert "Endpoint" in result.stdout + assert "http://192.0.2.10:50051" in result.stdout + assert "Timeout gateway ceiling" in result.stdout + assert "30s" in result.stdout assert "Created the gateway configuration file" in result.stdout assert "Next: Start Egress Gate" in result.stdout @@ -418,7 +458,8 @@ def test_cli_lists_gateway_registration_names_for_removal(tmp_path: Path) -> Non "version = 1\n\n" "[[openshell.supervisor.middleware]]\n" 'name = "eg-regex"\n' - 'grpc_endpoint = "http://192.0.2.10:50051"\n\n' + 'grpc_endpoint = "http://192.0.2.10:50051"\n' + 'timeout = "30s"\n\n' "[[openshell.supervisor.middleware]]\n" 'name = "other-service"\n' 'grpc_endpoint = "http://192.0.2.20:9000"\n' @@ -433,6 +474,8 @@ def test_cli_lists_gateway_registration_names_for_removal(tmp_path: Path) -> Non assert "OpenShell middleware registrations" in result.stdout assert "eg-regex" in result.stdout assert "http://192.0.2.10:50051" in result.stdout + assert "Timeout gateway ceiling" in result.stdout + assert "30s" in result.stdout assert "other-service" in result.stdout assert "remove-gateway-registration --name NAME" in result.stdout @@ -522,13 +565,13 @@ def test_cli_evaluate_explains_an_invalid_timeout() -> None: str(project_dir / "examples/regex-redaction/egress-gate-config.yaml"), "--cases", str(project_dir / "examples/regex-redaction/cases.yaml"), - "--timeout-seconds", - "0", + "--timeout", + "0s", ], color=True, ) assert result.exit_code == 2 error_output = Text.from_ansi(result.stderr).plain - assert "Invalid value for --timeout-seconds" in error_output - assert "greater than 0" in error_output + assert "Invalid value for --timeout" in error_output + assert "between 10ms and 30s" in error_output diff --git a/projects/egress-gate/tests/test_gateway_config.py b/projects/egress-gate/tests/test_gateway_config.py index a064fee0..f357046e 100644 --- a/projects/egress-gate/tests/test_gateway_config.py +++ b/projects/egress-gate/tests/test_gateway_config.py @@ -96,7 +96,7 @@ def test_update_gateway_config_creates_minimal_default_config( "name": "egress-gate", "grpc_endpoint": "http://192.168.1.20:50051", "max_body_bytes": 4_194_304, - "timeout": "5s", + "timeout": "30s", } ] }, @@ -113,7 +113,8 @@ def test_list_gateway_registrations_returns_names_and_endpoints( "version = 1\n\n" "[[openshell.supervisor.middleware]]\n" 'name = "eg-regex"\n' - 'grpc_endpoint = "http://10.0.0.3:50051"\n\n' + 'grpc_endpoint = "http://10.0.0.3:50051"\n' + 'timeout = "30s"\n\n' "[[openshell.supervisor.middleware]]\n" 'name = "other-service"\n' ) @@ -122,8 +123,13 @@ def test_list_gateway_registrations_returns_names_and_endpoints( GatewayMiddlewareRegistration( name="eg-regex", endpoint="http://10.0.0.3:50051", + timeout_gateway_ceiling="30s", + ), + GatewayMiddlewareRegistration( + name="other-service", + endpoint=None, + timeout_gateway_ceiling=None, ), - GatewayMiddlewareRegistration(name="other-service", endpoint=None), ) @@ -190,7 +196,7 @@ def test_update_gateway_config_updates_only_the_named_registration( assert "# Keep this registration comment." in contents assert 'grpc_endpoint = "http://10.0.0.4:50053"' in contents assert "max_body_bytes = 4194304" in contents - assert 'timeout = "5s"' in contents + assert 'timeout = "30s"' in contents repeated = update_gateway_config( path, @@ -202,6 +208,30 @@ def test_update_gateway_config_updates_only_the_named_registration( assert repeated is GatewayConfigUpdate.UNCHANGED +def test_update_gateway_config_adds_the_operator_timeout_ceiling_when_missing( + tmp_path: Path, +) -> None: + path = tmp_path / "gateway.toml" + path.write_text( + "[openshell]\n" + "version = 1\n\n" + "[[openshell.supervisor.middleware]]\n" + 'name = "egress-gate"\n' + 'grpc_endpoint = "http://10.0.0.3:50051"\n' + "max_body_bytes = 4194304\n" + ) + + result = update_gateway_config( + path, + middleware_name="egress-gate", + host_ip="10.0.0.3", + port=50051, + ) + + assert result is GatewayConfigUpdate.UPDATED + assert 'timeout = "30s"' in path.read_text() + + @pytest.mark.parametrize( "contents", [ diff --git a/projects/egress-gate/tests/test_timeout.py b/projects/egress-gate/tests/test_timeout.py index b6c7957f..7be9e666 100644 --- a/projects/egress-gate/tests/test_timeout.py +++ b/projects/egress-gate/tests/test_timeout.py @@ -5,7 +5,11 @@ import pytest from egress_gate.errors import TimeoutExpiredError -from egress_gate.timeout import Timeout +from egress_gate.timeout import ( + Timeout, + format_timeout_middleware_processing, + parse_timeout_duration, +) @pytest.mark.parametrize("seconds", [True, 0, -1, float("inf"), 31]) @@ -14,11 +18,45 @@ def test_timeout_duration_is_strict_positive_and_bounded( ) -> None: with pytest.raises( ValueError, - match="finite number greater than 0 and at most 30", + match="greater than 0 and no more than the 30s timeout_gateway_ceiling", ): Timeout.from_seconds(seconds) +@pytest.mark.parametrize( + ("seconds", "duration"), + [(0.01, "10ms"), (1.0, "1s"), (4.5, "4500ms"), (30.0, "30s")], +) +def test_middleware_timeout_uses_the_contract_duration_format( + seconds: float, + duration: str, +) -> None: + assert format_timeout_middleware_processing(seconds) == duration + + +@pytest.mark.parametrize("seconds", [0.001, 1.0001]) +def test_middleware_timeout_rejects_unsupported_precision(seconds: float) -> None: + with pytest.raises(ValueError, match="whole milliseconds"): + format_timeout_middleware_processing(seconds) + + +@pytest.mark.parametrize( + ("duration", "seconds"), + [("10ms", 0.01), ("500ms", 0.5), ("1s", 1.0), ("30s", 30.0)], +) +def test_timeout_duration_parser_accepts_concise_cli_values( + duration: str, + seconds: float, +) -> None: + assert parse_timeout_duration(duration) == seconds + + +@pytest.mark.parametrize("duration", ["", "1", "1.5s", "0s", "31s", "1m"]) +def test_timeout_duration_parser_rejects_unsupported_values(duration: str) -> None: + with pytest.raises(ValueError, match="timeout"): + parse_timeout_duration(duration) + + def test_expired_timeout_raises_typed_signal() -> None: timeout = Timeout(deadline=monotonic() - 1) @@ -27,8 +65,8 @@ def test_expired_timeout_raises_typed_signal() -> None: assert str(captured.value) == ( "Egress Gate processing timed out. Reduce the request size or simplify " - "the configured gates and rules, or increase the processing timeout " - "to at most 30 seconds, then retry." + "the configured gates and rules, or increase egress-gate serve --timeout " + "to at most 30s, then retry." ) From d512578b38018a37f6356376bcf9fec460c2e8e4 Mon Sep 17 00:00:00 2001 From: Johnny Greco Date: Fri, 7 Aug 2026 15:32:36 +0000 Subject: [PATCH 2/5] Clarify and validate timeout settings --- projects/egress-gate/src/egress_gate/cli.py | 61 +++++++-- .../egress-gate/src/egress_gate/constants.py | 11 +- .../egress-gate/src/egress_gate/errors.py | 6 +- .../src/egress_gate/gateway_config.py | 119 +++++++++++++++++- .../src/egress_gate/service/servicer.py | 19 ++- .../egress-gate/src/egress_gate/timeout.py | 37 +++--- .../egress-gate/tests/gates/test_regex.py | 2 +- .../egress-gate/tests/service/test_server.py | 8 +- .../tests/service/test_servicer.py | 4 +- projects/egress-gate/tests/test_cli.py | 54 +++++++- .../egress-gate/tests/test_gateway_config.py | 43 +++++++ projects/egress-gate/tests/test_timeout.py | 23 ++-- 12 files changed, 319 insertions(+), 68 deletions(-) diff --git a/projects/egress-gate/src/egress_gate/cli.py b/projects/egress-gate/src/egress_gate/cli.py index e328be8c..338b3bc6 100644 --- a/projects/egress-gate/src/egress_gate/cli.py +++ b/projects/egress-gate/src/egress_gate/cli.py @@ -29,6 +29,7 @@ from egress_gate.base import StrictDomainModel from egress_gate.constants import ( + DEFAULT_GATEWAY_REGISTRATION_TIMEOUT, DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING, MAX_BODY_BYTES, MAX_EVALUATION_CASE_NAME_BYTES, @@ -36,7 +37,7 @@ MAX_EVALUATION_FILE_BYTES, MAX_EVALUATION_TAGS, MAX_PROTO_FINDING_GROUPS, - TIMEOUT_GATEWAY_CEILING, + MAX_TIMEOUT_MIDDLEWARE_PROCESSING, ) from egress_gate.errors import EgressGateError, GateRegistryError from egress_gate.gates.base import GateCapability @@ -53,11 +54,13 @@ GatewayMiddlewareRegistration, default_gateway_config_path, list_gateway_registrations, + read_remembered_gateway_timeout, + remember_gateway_registration, remove_gateway_config, update_gateway_config, validate_middleware_name, ) -from egress_gate.logging import LoggingConfig, configure_logging +from egress_gate.logging import LoggingConfig, configure_logging, get_logger from egress_gate.request import HttpHeader, HttpRequest, HttpTarget, RequestContext from egress_gate.result import EgressResult, GateDecisionSource from egress_gate.string_validators import BoundedMetadataString @@ -67,6 +70,13 @@ validate_timeout_middleware_processing, ) +_TIMEOUT_DURATION_HELP = ( + "Use an integer followed by s for seconds or ms for milliseconds, such as " + f"10s or 500ms. Accepted range: 10ms through " + f"{MAX_TIMEOUT_MIDDLEWARE_PROCESSING:g}s." +) +_LOG = get_logger(__name__) + app = typer.Typer( name="egress-gate", help=( @@ -147,8 +157,9 @@ def serve( typer.Option( "--timeout", help=( - "Middleware processing timeout for one request, for example 10s " - "or 500ms. It cannot exceed the 30s gateway ceiling." + "Internal processing budget for one request. " + f"{_TIMEOUT_DURATION_HELP} The OpenShell gateway applies its " + "separately configured RPC timeout." ), ), ] = f"{DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING:g}s", @@ -164,6 +175,32 @@ def serve( str(error), param_hint="--timeout", ) from None + try: + remembered_timeout = read_remembered_gateway_timeout() + except GatewayConfigError as error: + _render_cli_error( + "Gateway timeout could not be validated", + code="gateway_config_error", + message=str(error), + ) + raise typer.Exit(code=1) from None + if remembered_timeout is not None: + remembered, timeout_gateway_ceiling = remembered_timeout + if timeout_middleware_processing >= timeout_gateway_ceiling: + raise typer.BadParameter( + "The middleware processing timeout must be less than the " + f"{timeout_gateway_ceiling:g}s gateway timeout configured for " + f"{remembered.middleware_name!r} in {remembered.config_path}.", + param_hint="--timeout", + ) + _LOG.info( + "Validated timeout_middleware_processing=%ss against " + "timeout_gateway_ceiling=%ss registration=%s gateway_config=%s", + timeout_middleware_processing, + timeout_gateway_ceiling, + remembered.middleware_name, + remembered.config_path, + ) try: EgressGateServer( options.registry, @@ -219,7 +256,7 @@ def add_gateway_registration( ), ] = 50051, ) -> None: - """Add or update Egress Gate with a 30s timeout gateway ceiling.""" + """Add or update Egress Gate with the default gateway RPC timeout.""" try: address = ipaddress.IPv4Address(host_ip) except ipaddress.AddressValueError: @@ -248,6 +285,10 @@ def add_gateway_registration( host_ip=str(address), port=port, ) + remember_gateway_registration( + config_path, + middleware_name=validated_name, + ) except GatewayConfigError as error: _render_cli_error( "Gateway registration could not be saved", @@ -267,7 +308,7 @@ def add_gateway_registration( config_path=config_path, name=validated_name, endpoint=f"http://{address}:{port}", - timeout_gateway_ceiling=f"{TIMEOUT_GATEWAY_CEILING:g}s", + timeout_gateway_ceiling=f"{DEFAULT_GATEWAY_REGISTRATION_TIMEOUT:g}s", change=change, next_step=( "Start Egress Gate, then restart the OpenShell gateway to load this " @@ -451,8 +492,8 @@ def evaluate( typer.Option( "--timeout", help=( - "Timeout for policy preparation and, separately, each case, for " - f"example 1s or 500ms. Maximum {TIMEOUT_GATEWAY_CEILING:g}s." + "Timeout for policy preparation and, separately, each case. " + f"{_TIMEOUT_DURATION_HELP}" ), ), ] = f"{DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING:g}s", @@ -1010,7 +1051,7 @@ def _render_registration( details.add_row("Endpoint", Text(endpoint)) if timeout_gateway_ceiling is not None: details.add_row( - "Timeout gateway ceiling", + "Gateway RPC ceiling", Text(timeout_gateway_ceiling), ) if change is not None: @@ -1034,7 +1075,7 @@ def _render_gateway_registrations( table = Table(box=None, pad_edge=False, padding=(0, 2), header_style="bold cyan") table.add_column("Name", style="bold", no_wrap=True) table.add_column("Endpoint", overflow="fold") - table.add_column("Timeout gateway ceiling", no_wrap=True) + table.add_column("Gateway RPC ceiling", no_wrap=True) for registration in registrations: table.add_row( registration.name, diff --git a/projects/egress-gate/src/egress_gate/constants.py b/projects/egress-gate/src/egress_gate/constants.py index 363a38e4..aad642e7 100644 --- a/projects/egress-gate/src/egress_gate/constants.py +++ b/projects/egress-gate/src/egress_gate/constants.py @@ -9,9 +9,12 @@ import re from importlib.metadata import version -# Middleware processing timeout and the gateway's upper bound for it. +# Egress Gate's internal maximum leaves headroom under the registration helper's +# default. This is not a claim about the active gateway configuration: operators +# can change that timeout outside this service. DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING = 1.0 -TIMEOUT_GATEWAY_CEILING = 30.0 +MAX_TIMEOUT_MIDDLEWARE_PROCESSING = 29.0 +DEFAULT_GATEWAY_REGISTRATION_TIMEOUT = 30.0 # Middleware identity and stable response values. SERVICE_NAME = "egress-gate" @@ -21,8 +24,8 @@ LIMIT_REASON = ( "Egress Gate exceeded a processing safety limit. Check Egress Gate logs " "for the limit kind. Reduce the request or replacement size, simplify the " - "configured gates and rules, or increase the timeout passed to " - "egress-gate serve --timeout, then retry." + "configured gates and rules, or increase the middleware processing timeout, " + "then retry." ) LIMIT_REASON_CODE = "egress_gate_limit_exceeded" # Text input limits. diff --git a/projects/egress-gate/src/egress_gate/errors.py b/projects/egress-gate/src/egress_gate/errors.py index 670b2aed..547ce059 100644 --- a/projects/egress-gate/src/egress_gate/errors.py +++ b/projects/egress-gate/src/egress_gate/errors.py @@ -5,7 +5,7 @@ from dataclasses import dataclass from enum import StrEnum -from egress_gate.constants import MAX_PROTO_CONFIG_BYTES, TIMEOUT_GATEWAY_CEILING +from egress_gate.constants import MAX_PROTO_CONFIG_BYTES class ErrorKind(StrEnum): @@ -106,8 +106,8 @@ class TimeoutExpiredError(Exception): def __init__(self) -> None: super().__init__( "Egress Gate processing timed out. Reduce the request size or simplify " - "the configured gates and rules, or increase egress-gate serve " - f"--timeout to at most {TIMEOUT_GATEWAY_CEILING:g}s, then retry." + "the configured gates and rules, or increase the middleware processing " + "timeout, then retry." ) diff --git a/projects/egress-gate/src/egress_gate/gateway_config.py b/projects/egress-gate/src/egress_gate/gateway_config.py index 1238e819..465ccb68 100644 --- a/projects/egress-gate/src/egress_gate/gateway_config.py +++ b/projects/egress-gate/src/egress_gate/gateway_config.py @@ -11,8 +11,9 @@ from dataclasses import dataclass from enum import Enum from pathlib import Path +from typing import Literal -from egress_gate.constants import TIMEOUT_GATEWAY_CEILING +from egress_gate.constants import DEFAULT_GATEWAY_REGISTRATION_TIMEOUT class GatewayConfigUpdate(Enum): @@ -44,6 +45,14 @@ class GatewayMiddlewareRegistration: timeout_gateway_ceiling: str | None +@dataclass(frozen=True) +class RememberedGatewayRegistration: + """The gateway registration most recently managed by Egress Gate.""" + + config_path: Path + middleware_name: str + + # Mirrors OpenShell's stable-identifier byte limit for external middleware # registrations. MAX_MIDDLEWARE_REGISTRATION_NAME_BYTES = 19 @@ -62,6 +71,105 @@ def default_gateway_config_path() -> Path: return Path.home() / ".config" / "openshell" / "gateway.toml" +def default_registration_state_path() -> Path: + """Return the per-user location for the remembered gateway registration.""" + config_home = os.environ.get("XDG_CONFIG_HOME") + root = Path(config_home) if config_home else Path.home() / ".config" + return root / "openshell-egress-gate" / "registration.toml" + + +def remember_gateway_registration( + config_path: Path, + *, + middleware_name: str, +) -> None: + """Remember where the CLI most recently managed an Egress Gate registration.""" + validate_middleware_name(middleware_name) + escaped_path = ( + str(config_path.expanduser().resolve()) + .replace("\\", "\\\\") + .replace('"', '\\"') + ) + escaped_name = middleware_name.replace("\\", "\\\\").replace('"', '\\"') + _write_atomically( + default_registration_state_path(), + f'gateway_config = "{escaped_path}"\nregistration_name = "{escaped_name}"\n', + ) + + +def load_remembered_gateway_registration() -> RememberedGatewayRegistration | None: + """Load the gateway registration most recently managed by the CLI.""" + state_path = default_registration_state_path() + try: + contents = state_path.read_text(encoding="utf-8") + except FileNotFoundError: + return None + except (OSError, UnicodeError) as error: + raise GatewayConfigError( + f"Could not read {state_path}. Check that it is readable UTF-8 TOML." + ) from error + try: + values = tomllib.loads(contents) + except tomllib.TOMLDecodeError as error: + raise GatewayConfigError( + f"Could not parse {state_path}. Remove it and register Egress Gate again." + ) from error + config_path = values.get("gateway_config") + middleware_name = values.get("registration_name") + if not isinstance(config_path, str) or not config_path: + raise GatewayConfigError( + f"{state_path} does not contain a valid gateway_config path. Remove it " + "and register Egress Gate again." + ) + if not isinstance(middleware_name, str): + raise GatewayConfigError( + f"{state_path} does not contain a valid registration_name. Remove it " + "and register Egress Gate again." + ) + validate_middleware_name(middleware_name) + return RememberedGatewayRegistration( + config_path=Path(config_path), + middleware_name=middleware_name, + ) + + +def read_remembered_gateway_timeout( + unit: Literal["s", "ms"] = "s", +) -> tuple[RememberedGatewayRegistration, float] | None: + """Read the current timeout for the remembered gateway registration.""" + remembered = load_remembered_gateway_registration() + if remembered is None: + return None + matches = [ + registration + for registration in list_gateway_registrations(remembered.config_path) + if registration.name == remembered.middleware_name + ] + if not matches: + raise GatewayConfigError( + f"The remembered registration {remembered.middleware_name!r} is not in " + f"{remembered.config_path}. Register Egress Gate again." + ) + duration = matches[0].timeout_gateway_ceiling + if duration is None: + raise GatewayConfigError( + f"The remembered registration {remembered.middleware_name!r} in " + f"{remembered.config_path} has no timeout. Add one or register Egress " + "Gate again." + ) + match = re.fullmatch(r"([1-9][0-9]*)(ms|s)", duration) + if match is None: + raise GatewayConfigError( + f"The timeout for the remembered registration " + f"{remembered.middleware_name!r} in {remembered.config_path} must use " + "whole seconds or milliseconds, such as 30s or 500ms." + ) + amount = int(match.group(1)) + milliseconds = amount if match.group(2) == "ms" else amount * 1000 + timeout = milliseconds / 1000 if unit == "s" else float(milliseconds) + return remembered, timeout + + def list_gateway_registrations( path: Path, ) -> tuple[GatewayMiddlewareRegistration, ...]: @@ -359,7 +467,7 @@ def _middleware_block( f'name = "{middleware_name}"\n' f'grpc_endpoint = "{endpoint}"\n' "max_body_bytes = 4194304\n" - f'timeout = "{TIMEOUT_GATEWAY_CEILING:g}s"\n' + f'timeout = "{DEFAULT_GATEWAY_REGISTRATION_TIMEOUT:g}s"\n' ) @@ -381,7 +489,7 @@ def _update_middleware_block( return _replace_or_append_assignment( updated, key="timeout", - value=f'"{TIMEOUT_GATEWAY_CEILING:g}s"', + value=f'"{DEFAULT_GATEWAY_REGISTRATION_TIMEOUT:g}s"', ) @@ -456,11 +564,16 @@ def _write_atomically(path: Path, contents: str) -> None: __all__ = [ "GatewayConfigError", "GatewayMiddlewareRegistration", + "RememberedGatewayRegistration", "GatewayConfigRemoval", "GatewayConfigUpdate", "MAX_MIDDLEWARE_REGISTRATION_NAME_BYTES", "default_gateway_config_path", + "default_registration_state_path", "list_gateway_registrations", + "load_remembered_gateway_registration", + "remember_gateway_registration", + "read_remembered_gateway_timeout", "remove_gateway_config", "update_gateway_config", "validate_middleware_name", diff --git a/projects/egress-gate/src/egress_gate/service/servicer.py b/projects/egress-gate/src/egress_gate/service/servicer.py index 96fe46a0..78fdc3d9 100644 --- a/projects/egress-gate/src/egress_gate/service/servicer.py +++ b/projects/egress-gate/src/egress_gate/service/servicer.py @@ -65,7 +65,11 @@ SourcedFinding, ) from egress_gate.string_validators import validate_bounded_metadata_string -from egress_gate.timeout import Timeout, format_timeout_middleware_processing +from egress_gate.timeout import ( + Timeout, + format_timeout_middleware_processing, + validate_timeout_middleware_processing, +) class EgressGateMiddleware(pb2_grpc.SupervisorMiddlewareServicer): @@ -79,10 +83,9 @@ def __init__( ) -> None: registry.configuration_json_schema() self._registry = registry - self._timeout_middleware_processing = format_timeout_middleware_processing( - timeout_middleware_processing + self._timeout_middleware_processing_seconds = ( + validate_timeout_middleware_processing(timeout_middleware_processing) ) - self._timeout_middleware_processing_seconds = timeout_middleware_processing self._policy = _ActivePolicy(registry) self._processing_slots = asyncio.Semaphore(MAX_CONCURRENT_PROCESSING) self._processing_executor = ThreadPoolExecutor( @@ -93,7 +96,9 @@ def __init__( @property def timeout_middleware_processing(self) -> str: """Return the configured middleware processing timeout.""" - return self._timeout_middleware_processing + return format_timeout_middleware_processing( + self._timeout_middleware_processing_seconds + ) async def close(self) -> None: """Wait for in-flight synchronous gates during shutdown.""" @@ -106,6 +111,9 @@ async def Describe( context: grpc.aio.ServicerContext[object, pb2.MiddlewareManifest], ) -> pb2.MiddlewareManifest: """Describe the binding and its complete policy schema.""" + # The protocol does not expose the operator-configured gateway timeout + # to this service. An empty binding timeout leaves that RPC limit under + # gateway ownership instead of replacing it with the internal budget. return pb2.MiddlewareManifest( name=SERVICE_NAME, service_version=SERVICE_VERSION, @@ -114,7 +122,6 @@ async def Describe( operation=pb2.SUPERVISOR_MIDDLEWARE_OPERATION_HTTP_REQUEST, phase=pb2.SUPERVISOR_MIDDLEWARE_PHASE_PRE_CREDENTIALS, max_body_bytes=MAX_BODY_BYTES, - timeout=self._timeout_middleware_processing, ) ], ) diff --git a/projects/egress-gate/src/egress_gate/timeout.py b/projects/egress-gate/src/egress_gate/timeout.py index 7fc4bee4..2e5a0800 100644 --- a/projects/egress-gate/src/egress_gate/timeout.py +++ b/projects/egress-gate/src/egress_gate/timeout.py @@ -12,39 +12,40 @@ from pydantic import Field from egress_gate.base import StrictDomainModel -from egress_gate.constants import TIMEOUT_GATEWAY_CEILING +from egress_gate.constants import MAX_TIMEOUT_MIDDLEWARE_PROCESSING from egress_gate.errors import TimeoutExpiredError def validate_timeout_middleware_processing(seconds: object) -> float: - """Return a processing timeout within the gateway ceiling, in seconds.""" + """Return a supported internal processing timeout, in seconds.""" if ( isinstance(seconds, bool) or not isinstance(seconds, int | float) or not math.isfinite(seconds) - or seconds <= 0 - or seconds > TIMEOUT_GATEWAY_CEILING ): raise ValueError( - "timeout_middleware_processing must be greater than 0 and no more " - f"than the {TIMEOUT_GATEWAY_CEILING:g}s timeout_gateway_ceiling" + "timeout_middleware_processing must be between 10ms and " + f"{MAX_TIMEOUT_MIDDLEWARE_PROCESSING:g}s, using whole milliseconds" ) - return float(seconds) - - -def format_timeout_middleware_processing(seconds: object) -> str: - """Format a processing timeout for the OpenShell duration contract.""" - validated_seconds = validate_timeout_middleware_processing(seconds) + validated_seconds = float(seconds) milliseconds = validated_seconds * 1000 rounded_milliseconds = round(milliseconds) - if rounded_milliseconds < 10 or not math.isclose( - milliseconds, - rounded_milliseconds, + if ( + rounded_milliseconds < 10 + or validated_seconds > MAX_TIMEOUT_MIDDLEWARE_PROCESSING + or not math.isclose(milliseconds, rounded_milliseconds) ): raise ValueError( - "timeout_middleware_processing must use whole milliseconds and be at " - "least 10ms" + "timeout_middleware_processing must be between 10ms and " + f"{MAX_TIMEOUT_MIDDLEWARE_PROCESSING:g}s, using whole milliseconds" ) + return validated_seconds + + +def format_timeout_middleware_processing(seconds: object) -> str: + """Format a processing timeout for the OpenShell duration contract.""" + validated_seconds = validate_timeout_middleware_processing(seconds) + rounded_milliseconds = round(validated_seconds * 1000) if rounded_milliseconds % 1000 == 0: return f"{rounded_milliseconds // 1000}s" return f"{rounded_milliseconds}ms" @@ -64,7 +65,7 @@ def parse_timeout_duration(duration: object) -> float: except ValueError: raise ValueError( "timeout must be between 10ms and " - f"{TIMEOUT_GATEWAY_CEILING:g}s, using whole milliseconds" + f"{MAX_TIMEOUT_MIDDLEWARE_PROCESSING:g}s, using whole milliseconds" ) from None return float(seconds) diff --git a/projects/egress-gate/tests/gates/test_regex.py b/projects/egress-gate/tests/gates/test_regex.py index e002bb91..d9a19bab 100644 --- a/projects/egress-gate/tests/gates/test_regex.py +++ b/projects/egress-gate/tests/gates/test_regex.py @@ -523,7 +523,7 @@ def test_pattern_search_has_an_enforceable_timeout() -> None: with pytest.raises(TimeoutExpiredError): RegexGate(config, None).evaluate( _request((b"a" * 100_000) + b"!"), - timeout=Timeout.from_seconds(0.001), + timeout=Timeout.from_seconds(0.01), ) diff --git a/projects/egress-gate/tests/service/test_server.py b/projects/egress-gate/tests/service/test_server.py index 06b774d4..c7f6bb66 100644 --- a/projects/egress-gate/tests/service/test_server.py +++ b/projects/egress-gate/tests/service/test_server.py @@ -45,7 +45,10 @@ def test_server_rejects_a_registry_without_gates() -> None: EgressGateServer(GateRegistry()) -@pytest.mark.parametrize("seconds", [True, 0, 0.001, 31, float("inf")]) +@pytest.mark.parametrize( + "seconds", + [True, 0, 0.001, 1.0001, 30, float("inf")], +) def test_server_validates_timeout_middleware_processing( seconds: bool | int | float, ) -> None: @@ -56,7 +59,7 @@ def test_server_validates_timeout_middleware_processing( ) -def test_server_uses_timeout_middleware_processing_for_manifest_and_pipeline() -> None: +def test_server_uses_timeout_middleware_processing_for_pipeline_and_logs() -> None: server = EgressGateServer( create_builtin_registry(), timeout_middleware_processing=4.5, @@ -64,6 +67,7 @@ def test_server_uses_timeout_middleware_processing_for_manifest_and_pipeline() - try: assert server._middleware._timeout_middleware_processing_seconds == 4.5 assert server._middleware.timeout_middleware_processing == "4500ms" + assert not hasattr(server._middleware, "_timeout_middleware_processing") assert not hasattr( server._middleware._policy, "_timeout_middleware_processing_seconds", diff --git a/projects/egress-gate/tests/service/test_servicer.py b/projects/egress-gate/tests/service/test_servicer.py index 34e9759d..44392d76 100644 --- a/projects/egress-gate/tests/service/test_servicer.py +++ b/projects/egress-gate/tests/service/test_servicer.py @@ -109,7 +109,7 @@ async def abort(self, code: grpc.StatusCode, details: str) -> Never: raise AssertionError("successful evaluation unexpectedly aborted") -def test_manifest_contains_the_middleware_processing_timeout() -> None: +def test_manifest_leaves_the_gateway_rpc_timeout_to_the_operator() -> None: middleware = EgressGateMiddleware( create_builtin_registry(), timeout_middleware_processing=4.5, @@ -119,7 +119,7 @@ def test_manifest_contains_the_middleware_processing_timeout() -> None: finally: asyncio.run(middleware.close()) - assert manifest.bindings[0].timeout == "4500ms" + assert manifest.bindings[0].timeout == "" def test_copied_proto_remains_the_current_five_field_finding_contract() -> None: diff --git a/projects/egress-gate/tests/test_cli.py b/projects/egress-gate/tests/test_cli.py index 5f717d0f..0d137a6c 100644 --- a/projects/egress-gate/tests/test_cli.py +++ b/projects/egress-gate/tests/test_cli.py @@ -60,6 +60,7 @@ def test_cli_narrow_help_preserves_complete_option_names() -> None: def test_cli_serve_uses_one_concise_processing_timeout( monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, ) -> None: calls: list[tuple[float, str]] = [] @@ -80,6 +81,7 @@ def serve_sync(self, listen: str) -> None: "egress_gate.service.server.EgressGateServer", FakeServer, ) + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path)) result = CliRunner().invoke( app, @@ -90,8 +92,44 @@ def serve_sync(self, listen: str) -> None: assert calls == [(4.5, "127.0.0.1:50055")] help_result = CliRunner().invoke(app, ["serve", "--help"]) - assert "--timeout " in help_result.stdout - assert "--timeout-seconds" not in help_result.stdout + serve_help = " ".join(help_result.stdout.split()) + assert "--timeout " in serve_help + assert "--timeout-seconds" not in serve_help + assert "s for seconds or ms for milliseconds" in serve_help + assert "10ms through 29s" in serve_help + assert "RPC timeout" in serve_help + + evaluate_help = CliRunner().invoke(app, ["evaluate", "--help"]) + assert evaluate_help.exit_code == 0, evaluate_help.output + normalized_evaluate_help = " ".join(evaluate_help.stdout.split()) + assert "s for seconds or ms for milliseconds" in normalized_evaluate_help + assert "10ms through 29s" in normalized_evaluate_help + + +def test_cli_serve_rejects_timeout_at_remembered_gateway_ceiling( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "user-config")) + config = tmp_path / "gateway.toml" + registration = CliRunner().invoke( + app, + [ + "add-gateway-registration", + "--host-ip", + "192.0.2.10", + "--config", + str(config), + ], + ) + assert registration.exit_code == 0, registration.output + config.write_text(config.read_text().replace('timeout = "30s"', 'timeout = "1s"')) + + result = CliRunner().invoke(app, ["serve", "--timeout", "1s"]) + + assert result.exit_code == 2 + assert "must be less than the 1s gateway timeout" in result.stderr + assert str(config) in result.stderr def test_cli_gates_describes_the_request_level_builtin() -> None: @@ -424,7 +462,11 @@ def test_cli_evaluate_names_a_failing_case_and_keeps_completed_results( assert '"/w=="' not in result.output -def test_cli_add_gateway_registration_reports_the_result(tmp_path: Path) -> None: +def test_cli_add_gateway_registration_reports_the_result( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "user-config")) config = tmp_path / "gateway.toml" result = CliRunner().invoke( app, @@ -445,7 +487,7 @@ def test_cli_add_gateway_registration_reports_the_result(tmp_path: Path) -> None assert "egress-gate" in result.stdout assert "Endpoint" in result.stdout assert "http://192.0.2.10:50051" in result.stdout - assert "Timeout gateway ceiling" in result.stdout + assert "Gateway RPC ceiling" in result.stdout assert "30s" in result.stdout assert "Created the gateway configuration file" in result.stdout assert "Next: Start Egress Gate" in result.stdout @@ -474,7 +516,7 @@ def test_cli_lists_gateway_registration_names_for_removal(tmp_path: Path) -> Non assert "OpenShell middleware registrations" in result.stdout assert "eg-regex" in result.stdout assert "http://192.0.2.10:50051" in result.stdout - assert "Timeout gateway ceiling" in result.stdout + assert "Gateway RPC ceiling" in result.stdout assert "30s" in result.stdout assert "other-service" in result.stdout assert "remove-gateway-registration --name NAME" in result.stdout @@ -574,4 +616,4 @@ def test_cli_evaluate_explains_an_invalid_timeout() -> None: assert result.exit_code == 2 error_output = Text.from_ansi(result.stderr).plain assert "Invalid value for --timeout" in error_output - assert "between 10ms and 30s" in error_output + assert "between 10ms and 29s" in error_output diff --git a/projects/egress-gate/tests/test_gateway_config.py b/projects/egress-gate/tests/test_gateway_config.py index f357046e..ddfb327a 100644 --- a/projects/egress-gate/tests/test_gateway_config.py +++ b/projects/egress-gate/tests/test_gateway_config.py @@ -14,7 +14,11 @@ GatewayConfigUpdate, GatewayMiddlewareRegistration, default_gateway_config_path, + default_registration_state_path, list_gateway_registrations, + load_remembered_gateway_registration, + read_remembered_gateway_timeout, + remember_gateway_registration, remove_gateway_config, update_gateway_config, validate_middleware_name, @@ -56,6 +60,45 @@ def test_default_gateway_config_path_honors_openshell_override( assert default_gateway_config_path() == configured_path +def test_remembered_registration_reads_current_gateway_timeout( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "user-config")) + gateway_config = tmp_path / "gateway.toml" + gateway_config.write_text( + "[openshell]\n" + "version = 1\n\n" + "[[openshell.supervisor.middleware]]\n" + 'name = "egress-gate"\n' + 'grpc_endpoint = "http://192.0.2.10:50051"\n' + 'timeout = "2500ms"\n' + ) + + remember_gateway_registration( + gateway_config, + middleware_name="egress-gate", + ) + + remembered = load_remembered_gateway_registration() + assert remembered is not None + assert remembered.config_path == gateway_config.resolve() + assert remembered.middleware_name == "egress-gate" + assert default_registration_state_path().stat().st_mode & 0o777 == 0o600 + assert read_remembered_gateway_timeout() == (remembered, 2.5) + assert read_remembered_gateway_timeout(unit="ms") == (remembered, 2500.0) + + +def test_remembered_registration_is_optional( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path)) + + assert load_remembered_gateway_registration() is None + assert read_remembered_gateway_timeout() is None + + def test_middleware_name_validation_matches_openshell_constraints() -> None: assert MAX_MIDDLEWARE_REGISTRATION_NAME_BYTES == 19 longest_name = "a" * MAX_MIDDLEWARE_REGISTRATION_NAME_BYTES diff --git a/projects/egress-gate/tests/test_timeout.py b/projects/egress-gate/tests/test_timeout.py index 7be9e666..ceb2be93 100644 --- a/projects/egress-gate/tests/test_timeout.py +++ b/projects/egress-gate/tests/test_timeout.py @@ -12,20 +12,23 @@ ) -@pytest.mark.parametrize("seconds", [True, 0, -1, float("inf"), 31]) +@pytest.mark.parametrize( + "seconds", + [True, 0, -1, 0.001, 1.0001, float("inf"), 30], +) def test_timeout_duration_is_strict_positive_and_bounded( seconds: bool | int | float, ) -> None: with pytest.raises( ValueError, - match="greater than 0 and no more than the 30s timeout_gateway_ceiling", + match="between 10ms and 29s, using whole milliseconds", ): Timeout.from_seconds(seconds) @pytest.mark.parametrize( ("seconds", "duration"), - [(0.01, "10ms"), (1.0, "1s"), (4.5, "4500ms"), (30.0, "30s")], + [(0.01, "10ms"), (1.0, "1s"), (4.5, "4500ms"), (29.0, "29s")], ) def test_middleware_timeout_uses_the_contract_duration_format( seconds: float, @@ -34,15 +37,9 @@ def test_middleware_timeout_uses_the_contract_duration_format( assert format_timeout_middleware_processing(seconds) == duration -@pytest.mark.parametrize("seconds", [0.001, 1.0001]) -def test_middleware_timeout_rejects_unsupported_precision(seconds: float) -> None: - with pytest.raises(ValueError, match="whole milliseconds"): - format_timeout_middleware_processing(seconds) - - @pytest.mark.parametrize( ("duration", "seconds"), - [("10ms", 0.01), ("500ms", 0.5), ("1s", 1.0), ("30s", 30.0)], + [("10ms", 0.01), ("500ms", 0.5), ("1s", 1.0), ("29s", 29.0)], ) def test_timeout_duration_parser_accepts_concise_cli_values( duration: str, @@ -51,7 +48,7 @@ def test_timeout_duration_parser_accepts_concise_cli_values( assert parse_timeout_duration(duration) == seconds -@pytest.mark.parametrize("duration", ["", "1", "1.5s", "0s", "31s", "1m"]) +@pytest.mark.parametrize("duration", ["", "1", "1.5s", "0s", "30s", "1m"]) def test_timeout_duration_parser_rejects_unsupported_values(duration: str) -> None: with pytest.raises(ValueError, match="timeout"): parse_timeout_duration(duration) @@ -65,8 +62,8 @@ def test_expired_timeout_raises_typed_signal() -> None: assert str(captured.value) == ( "Egress Gate processing timed out. Reduce the request size or simplify " - "the configured gates and rules, or increase egress-gate serve --timeout " - "to at most 30s, then retry." + "the configured gates and rules, or increase the middleware processing " + "timeout, then retry." ) From f18eb922578d0c900ad0358936a549ba0ef8425e Mon Sep 17 00:00:00 2001 From: Johnny Greco Date: Fri, 7 Aug 2026 15:32:55 +0000 Subject: [PATCH 3/5] Document timeout ownership and validation --- projects/egress-gate/README.md | 12 +++++-- .../docs/architecture/request-lifecycle.md | 6 ++++ .../docs/architecture/service-boundary.md | 21 ++++++++---- projects/egress-gate/docs/operations.md | 32 +++++++++---------- .../docs/reference/limits-and-failures.md | 19 +++++++---- 5 files changed, 59 insertions(+), 31 deletions(-) diff --git a/projects/egress-gate/README.md b/projects/egress-gate/README.md index e85e39b5..0e2b08cc 100644 --- a/projects/egress-gate/README.md +++ b/projects/egress-gate/README.md @@ -102,8 +102,16 @@ server.serve_sync("127.0.0.1:50051") `timeout_middleware_processing` is the actual service setting, in seconds. The service turns it into one `Timeout` per evaluation and passes that same deadline through slot acquisition, policy preparation, and `RequestProcessor.process`. -The OpenShell gateway enforces its separately configured -`timeout_gateway_ceiling` as an upper bound. +It accepts 10 milliseconds through 29 seconds. `Describe` leaves the optional +binding RPC timeout empty, so OpenShell applies the timeout configured on the +gateway registration to the complete RPC. The registration helper uses 30 +seconds by default, and operators can change that value in the gateway TOML. +The helper remembers the gateway file and registration name. On later CLI +starts, `serve` reads the current gateway timeout and requires the processing +timeout to be lower. If no registration has been added with the CLI, `serve` +starts without this check. If the gateway timeout expires, OpenShell applies +the policy's `on_error` behavior; use `on_error: fail_closed` when middleware +timeout failures must deny. ## Documentation and examples diff --git a/projects/egress-gate/docs/architecture/request-lifecycle.md b/projects/egress-gate/docs/architecture/request-lifecycle.md index 47e42d56..ee876949 100644 --- a/projects/egress-gate/docs/architecture/request-lifecycle.md +++ b/projects/egress-gate/docs/architecture/request-lifecycle.md @@ -55,6 +55,12 @@ encoded output limits return an atomic deny with source `runtime_limit` and `egress_gate_limit_exceeded`. No partial mutations or findings are returned. Gate contract and execution failures remain gRPC failures. +The internal processing timeout can return this denial only while the RPC is +still active. The OpenShell gateway owns a separate outer RPC ceiling. If that +outer clock expires first, OpenShell applies the middleware entry's `on_error` +policy instead of receiving an Egress Gate result. Use `on_error: fail_closed` +when middleware timeout failures must deny the request. + ## 5. Serialize the result The Egress Gate service adapter maps the protobuf-free `EgressResult` to diff --git a/projects/egress-gate/docs/architecture/service-boundary.md b/projects/egress-gate/docs/architecture/service-boundary.md index 1a20c24b..686cd493 100644 --- a/projects/egress-gate/docs/architecture/service-boundary.md +++ b/projects/egress-gate/docs/architecture/service-boundary.md @@ -29,12 +29,21 @@ encoded configuration before registry parsing. ## Shared deadline and workers -The service's `timeout_middleware_processing` value is returned in the -`MiddlewareBinding` from `Describe`. `EvaluateHttpRequest` converts that same -value into one monotonic `Timeout` used for semaphore acquisition, policy -preparation, replacement-lock waits, gate execution, and final result checks. -The gateway's separate `timeout_gateway_ceiling` can shorten, but never extend, -that processing time. +`EvaluateHttpRequest` converts `timeout_middleware_processing` into one monotonic +`Timeout` used for semaphore acquisition, policy preparation, replacement-lock +waits, gate execution, and final result checks. `Describe` leaves the binding's +optional RPC timeout empty. OpenShell therefore applies the separately +configured gateway registration timeout to the complete RPC. + +The middleware protocol does not report the resolved gateway timeout back to +Egress Gate, and OpenShell does not propagate it as a gRPC deadline. For the +normal CLI-managed path, `add-gateway-registration` remembers the gateway TOML +path and registration name. `serve` reads the current timeout from that entry +at startup and requires it to be greater than +`timeout_middleware_processing`. Direct Python API use and manually managed +registrations do not have this startup check. If the gateway timeout expires +first, OpenShell applies the policy's `on_error` behavior. + `RequestProcessor.process` accepts the caller-owned timeout and never creates or stores one. diff --git a/projects/egress-gate/docs/operations.md b/projects/egress-gate/docs/operations.md index 09420488..055160c5 100644 --- a/projects/egress-gate/docs/operations.md +++ b/projects/egress-gate/docs/operations.md @@ -34,14 +34,21 @@ uv run egress-gate add-gateway-registration \ The command updates `OPENSHELL_GATEWAY_CONFIG`, then `$XDG_CONFIG_HOME/openshell/gateway.toml`, then `~/.config/openshell/gateway.toml`. Use `--config PATH` for another file. +It remembers the absolute gateway file path and registration name in +`$XDG_CONFIG_HOME/openshell-egress-gate/registration.toml`, or under +`~/.config` when `XDG_CONFIG_HOME` is unset. Start the gateways again with the same commands or service managers that you normally use. -The registration writes `timeout = "30s"` in the gateway TOML. Egress Gate calls -this setting `timeout_gateway_ceiling`: the maximum time the gateway permits -for Egress Gate. Set the actual service value with -`egress-gate serve --timeout DURATION`; Egress Gate calls that setting -`timeout_middleware_processing`. +The registration helper writes `timeout = "30s"` in the gateway TOML. This is a +default, not a fixed limit: operators can edit the registration to use another +supported timeout. Rerunning `add-gateway-registration` resets it to 30s. Set +Egress Gate's internal processing budget with +`egress-gate serve --timeout DURATION`; the Python API calls that setting +`timeout_middleware_processing` and accepts 10ms through 29s. When a remembered +registration exists, `serve` reads its current gateway timeout and refuses to +start unless the processing timeout is lower. A manually managed setup with no +remembered registration starts without this check. To remove a registration, stop any running gateways that use the configuration again. List the available names with: @@ -60,17 +67,10 @@ uv run egress-gate remove-gateway-registration --name egress-gate Start the gateways again after the command completes. -Egress Gate uses `timeout_middleware_processing` across queueing, policy -preparation, and every configured gate. OpenShell applies -`timeout_gateway_ceiling` as an independent upper bound: - -```text -effective timeout = min(timeout_gateway_ceiling, timeout_middleware_processing) -``` - -With the registration command's 30s ceiling, any supported `--timeout` value -becomes the effective timeout. If an operator lowers the gateway ceiling by -editing the gateway configuration, that lower value wins. +`serve --timeout` covers queueing, policy preparation, and every configured +gate. If the gateway timeout expires first despite the startup check, OpenShell +applies the policy's `on_error` setting: `fail_closed` denies the request, while +`fail_open` allows it to continue. If the middleware RPC returns gRPC `RESOURCE_EXHAUSTED`, capacity may remain accounted for briefly while completed RPCs are torn down. The OpenShell gateway diff --git a/projects/egress-gate/docs/reference/limits-and-failures.md b/projects/egress-gate/docs/reference/limits-and-failures.md index 36cb7e94..d44b8cc1 100644 --- a/projects/egress-gate/docs/reference/limits-and-failures.md +++ b/projects/egress-gate/docs/reference/limits-and-failures.md @@ -6,9 +6,10 @@ agent_markdown: true # Limits and failure behavior -Limits are fail-closed and content-safe. The `service/` package checks exact -encoded protobuf sizes. Domain models check scalar, aggregate, and result -limits. +Egress Gate-owned limits are fail-closed and content-safe. The `service/` +package checks exact encoded protobuf sizes. Domain models check scalar, +aggregate, and result limits. OpenShell owns the separate outer RPC ceiling and +applies its configured `on_error` behavior when that ceiling expires first. | Area | Limit | | --- | ---: | @@ -20,9 +21,9 @@ limits. | Result metadata aggregate strings | 32 KiB | | Gate traces per result | 10 | | Header mutations per gate evaluation | 64 | -| Offline `--timeout` | 30 seconds maximum | -| `timeout_middleware_processing` | 30 seconds maximum | -| `timeout_gateway_ceiling` | 30 seconds in registrations created by Egress Gate | +| Offline `--timeout` | 29 seconds maximum | +| `timeout_middleware_processing` | 29 seconds maximum | +| Gateway registration timeout | Operator-configurable; helper default 30 seconds | | Concurrent processing slots | 4 | Request context and target aggregates, headers, replacement bodies, regex @@ -36,7 +37,7 @@ rejected value. | --- | --- | | Invalid phase, envelope, policy, or input encoding | gRPC `INVALID_ARGUMENT` | | Gate contract or unexpected execution failure | gRPC `INTERNAL` | -| Deadline or pipeline processor limit | deny, source `runtime_limit`, code `egress_gate_limit_exceeded` | +| Internal processing deadline or pipeline processor limit | deny, source `runtime_limit`, code `egress_gate_limit_exceeded` | | Gate terminal deny | deny, source `gate`, gate-owned reason code | | Pipeline default deny | deny, source `pipeline_default`, code `egress_gate_default_deny` | | Pipeline default allow | allow, source `pipeline_default`, no reason code | @@ -46,6 +47,10 @@ trace details. Failed policy preparation leaves the active policy unchanged. Stable error catalogs and reason codes never include request content or arbitrary exception text. +An internal processing timeout returns the runtime-limit denial only while the +RPC remains active. If the gateway's independent outer RPC ceiling expires +first, OpenShell applies the middleware entry's `on_error` policy. + ## Finding contract The released OpenShell wire contract has five fields. The pipeline processor's From e0713e4785ab9c7198a8db51f26acd1230986f55 Mon Sep 17 00:00:00 2001 From: Johnny Greco Date: Fri, 7 Aug 2026 16:14:09 +0000 Subject: [PATCH 4/5] Make gateway timeout configurable --- projects/egress-gate/README.md | 17 +++-- projects/egress-gate/docs/operations.md | 19 ++--- .../docs/reference/limits-and-failures.md | 4 +- projects/egress-gate/src/egress_gate/cli.py | 30 ++++++-- .../egress-gate/src/egress_gate/constants.py | 8 +-- .../src/egress_gate/gateway_config.py | 33 ++++++--- .../egress-gate/src/egress_gate/timeout.py | 70 ++++++++++++------- .../egress-gate/tests/service/test_server.py | 2 +- projects/egress-gate/tests/test_cli.py | 14 ++-- .../egress-gate/tests/test_gateway_config.py | 15 +++- projects/egress-gate/tests/test_timeout.py | 21 ++++-- 11 files changed, 158 insertions(+), 75 deletions(-) diff --git a/projects/egress-gate/README.md b/projects/egress-gate/README.md index 0e2b08cc..6853d668 100644 --- a/projects/egress-gate/README.md +++ b/projects/egress-gate/README.md @@ -99,13 +99,16 @@ server = EgressGateServer( server.serve_sync("127.0.0.1:50051") ``` -`timeout_middleware_processing` is the actual service setting, in seconds. The -service turns it into one `Timeout` per evaluation and passes that same deadline -through slot acquisition, policy preparation, and `RequestProcessor.process`. -It accepts 10 milliseconds through 29 seconds. `Describe` leaves the optional -binding RPC timeout empty, so OpenShell applies the timeout configured on the -gateway registration to the complete RPC. The registration helper uses 30 -seconds by default, and operators can change that value in the gateway TOML. +In this example, `timeout_middleware_processing` gives each evaluation 10 +seconds. Omitting it uses the one-second service default. The value is expressed +in seconds, must be at least 10 milliseconds, and must resolve to whole +milliseconds. The service passes one resulting `Timeout` through slot +acquisition, policy preparation, and `RequestProcessor.process`. + +`Describe` leaves the optional binding RPC timeout empty, so OpenShell applies +the timeout configured on the gateway registration to the complete RPC. The +registration CLI defaults to 30 seconds and accepts `--timeout` to write a +different value. The helper remembers the gateway file and registration name. On later CLI starts, `serve` reads the current gateway timeout and requires the processing timeout to be lower. If no registration has been added with the CLI, `serve` diff --git a/projects/egress-gate/docs/operations.md b/projects/egress-gate/docs/operations.md index 055160c5..3fb94919 100644 --- a/projects/egress-gate/docs/operations.md +++ b/projects/egress-gate/docs/operations.md @@ -28,7 +28,7 @@ registrations. ```bash title="Register Egress Gate" uv run egress-gate add-gateway-registration \ - --host-ip YOUR_HOST_IPV4 --name egress-gate --port 50051 + --host-ip YOUR_HOST_IPV4 --name egress-gate --port 50051 --timeout 30s ``` The command updates `OPENSHELL_GATEWAY_CONFIG`, then @@ -40,15 +40,16 @@ It remembers the absolute gateway file path and registration name in Start the gateways again with the same commands or service managers that you normally use. -The registration helper writes `timeout = "30s"` in the gateway TOML. This is a -default, not a fixed limit: operators can edit the registration to use another -supported timeout. Rerunning `add-gateway-registration` resets it to 30s. Set -Egress Gate's internal processing budget with +The optional registration `--timeout` sets the gateway RPC timeout written to +the TOML file and defaults to 30 seconds. It accepts whole seconds or +milliseconds, such as `45s` or `500ms`. Rerunning the command writes the value +passed on that invocation. Set Egress Gate's internal processing budget with `egress-gate serve --timeout DURATION`; the Python API calls that setting -`timeout_middleware_processing` and accepts 10ms through 29s. When a remembered -registration exists, `serve` reads its current gateway timeout and refuses to -start unless the processing timeout is lower. A manually managed setup with no -remembered registration starts without this check. +`timeout_middleware_processing`. It must be at least 10ms and resolve to whole +milliseconds. When a remembered registration exists, `serve` reads its current +gateway timeout and refuses to start unless the processing timeout is lower. A +manually managed setup with no remembered registration starts without this +check. To remove a registration, stop any running gateways that use the configuration again. List the available names with: diff --git a/projects/egress-gate/docs/reference/limits-and-failures.md b/projects/egress-gate/docs/reference/limits-and-failures.md index d44b8cc1..ea5cdc0a 100644 --- a/projects/egress-gate/docs/reference/limits-and-failures.md +++ b/projects/egress-gate/docs/reference/limits-and-failures.md @@ -21,8 +21,8 @@ applies its configured `on_error` behavior when that ceiling expires first. | Result metadata aggregate strings | 32 KiB | | Gate traces per result | 10 | | Header mutations per gate evaluation | 64 | -| Offline `--timeout` | 29 seconds maximum | -| `timeout_middleware_processing` | 29 seconds maximum | +| Offline `--timeout` | 10 milliseconds minimum; whole milliseconds | +| `timeout_middleware_processing` | 10 milliseconds minimum; whole milliseconds | | Gateway registration timeout | Operator-configurable; helper default 30 seconds | | Concurrent processing slots | 4 | diff --git a/projects/egress-gate/src/egress_gate/cli.py b/projects/egress-gate/src/egress_gate/cli.py index 338b3bc6..5277c021 100644 --- a/projects/egress-gate/src/egress_gate/cli.py +++ b/projects/egress-gate/src/egress_gate/cli.py @@ -37,7 +37,6 @@ MAX_EVALUATION_FILE_BYTES, MAX_EVALUATION_TAGS, MAX_PROTO_FINDING_GROUPS, - MAX_TIMEOUT_MIDDLEWARE_PROCESSING, ) from egress_gate.errors import EgressGateError, GateRegistryError from egress_gate.gates.base import GateCapability @@ -66,15 +65,16 @@ from egress_gate.string_validators import BoundedMetadataString from egress_gate.timeout import ( Timeout, + parse_duration, parse_timeout_duration, validate_timeout_middleware_processing, ) -_TIMEOUT_DURATION_HELP = ( +_DURATION_FORMAT_HELP = ( "Use an integer followed by s for seconds or ms for milliseconds, such as " - f"10s or 500ms. Accepted range: 10ms through " - f"{MAX_TIMEOUT_MIDDLEWARE_PROCESSING:g}s." + "10s or 500ms." ) +_TIMEOUT_DURATION_HELP = f"{_DURATION_FORMAT_HELP} Minimum 10ms." _LOG = get_logger(__name__) app = typer.Typer( @@ -255,8 +255,18 @@ def add_gateway_registration( ), ), ] = 50051, + timeout: Annotated[ + str, + typer.Option( + "--timeout", + help=( + "Gateway RPC timeout to write in the registration. " + f"{_DURATION_FORMAT_HELP}" + ), + ), + ] = DEFAULT_GATEWAY_REGISTRATION_TIMEOUT, ) -> None: - """Add or update Egress Gate with the default gateway RPC timeout.""" + """Add or update Egress Gate with a configurable gateway RPC timeout.""" try: address = ipaddress.IPv4Address(host_ip) except ipaddress.AddressValueError: @@ -277,6 +287,13 @@ def add_gateway_registration( str(error), param_hint="--name", ) from None + try: + parse_duration(timeout) + except ValueError as error: + raise typer.BadParameter( + str(error), + param_hint="--timeout", + ) from None config_path = config or default_gateway_config_path() try: result = update_gateway_config( @@ -284,6 +301,7 @@ def add_gateway_registration( middleware_name=validated_name, host_ip=str(address), port=port, + timeout_gateway_ceiling=timeout, ) remember_gateway_registration( config_path, @@ -308,7 +326,7 @@ def add_gateway_registration( config_path=config_path, name=validated_name, endpoint=f"http://{address}:{port}", - timeout_gateway_ceiling=f"{DEFAULT_GATEWAY_REGISTRATION_TIMEOUT:g}s", + timeout_gateway_ceiling=timeout, change=change, next_step=( "Start Egress Gate, then restart the OpenShell gateway to load this " diff --git a/projects/egress-gate/src/egress_gate/constants.py b/projects/egress-gate/src/egress_gate/constants.py index aad642e7..d9de72ce 100644 --- a/projects/egress-gate/src/egress_gate/constants.py +++ b/projects/egress-gate/src/egress_gate/constants.py @@ -9,12 +9,10 @@ import re from importlib.metadata import version -# Egress Gate's internal maximum leaves headroom under the registration helper's -# default. This is not a claim about the active gateway configuration: operators -# can change that timeout outside this service. +# Timeout defaults. The gateway registration value remains independently +# configurable by the operator. DEFAULT_TIMEOUT_MIDDLEWARE_PROCESSING = 1.0 -MAX_TIMEOUT_MIDDLEWARE_PROCESSING = 29.0 -DEFAULT_GATEWAY_REGISTRATION_TIMEOUT = 30.0 +DEFAULT_GATEWAY_REGISTRATION_TIMEOUT = "30s" # Middleware identity and stable response values. SERVICE_NAME = "egress-gate" diff --git a/projects/egress-gate/src/egress_gate/gateway_config.py b/projects/egress-gate/src/egress_gate/gateway_config.py index 465ccb68..bd8399bc 100644 --- a/projects/egress-gate/src/egress_gate/gateway_config.py +++ b/projects/egress-gate/src/egress_gate/gateway_config.py @@ -14,6 +14,7 @@ from typing import Literal from egress_gate.constants import DEFAULT_GATEWAY_REGISTRATION_TIMEOUT +from egress_gate.timeout import parse_duration class GatewayConfigUpdate(Enum): @@ -157,16 +158,14 @@ def read_remembered_gateway_timeout( f"{remembered.config_path} has no timeout. Add one or register Egress " "Gate again." ) - match = re.fullmatch(r"([1-9][0-9]*)(ms|s)", duration) - if match is None: + try: + timeout = parse_duration(duration, unit=unit) + except ValueError: raise GatewayConfigError( f"The timeout for the remembered registration " f"{remembered.middleware_name!r} in {remembered.config_path} must use " "whole seconds or milliseconds, such as 30s or 500ms." - ) - amount = int(match.group(1)) - milliseconds = amount if match.group(2) == "ms" else amount * 1000 - timeout = milliseconds / 1000 if unit == "s" else float(milliseconds) + ) from None return remembered, timeout @@ -223,9 +222,17 @@ def update_gateway_config( middleware_name: str, host_ip: str, port: int, + timeout_gateway_ceiling: str = DEFAULT_GATEWAY_REGISTRATION_TIMEOUT, ) -> GatewayConfigUpdate: """Add or update one named Egress Gate middleware registration.""" validate_middleware_name(middleware_name) + try: + parse_duration(timeout_gateway_ceiling) + except ValueError: + raise GatewayConfigError( + "The gateway timeout must use whole seconds or milliseconds, such as " + "30s or 500ms." + ) from None endpoint = f"http://{host_ip}:{port}" try: original = path.read_text(encoding="utf-8") @@ -233,6 +240,7 @@ def update_gateway_config( updated = _new_gateway_config( middleware_name=middleware_name, endpoint=endpoint, + timeout_gateway_ceiling=timeout_gateway_ceiling, ) _write_atomically(path, updated) return GatewayConfigUpdate.CREATED @@ -245,6 +253,7 @@ def update_gateway_config( updated = _new_gateway_config( middleware_name=middleware_name, endpoint=endpoint, + timeout_gateway_ceiling=timeout_gateway_ceiling, ) _write_atomically(path, updated) return GatewayConfigUpdate.CREATED @@ -274,6 +283,7 @@ def update_gateway_config( replacement = _update_middleware_block( block.group(0), endpoint=endpoint, + timeout_gateway_ceiling=timeout_gateway_ceiling, ) updated = original[: block.start()] + replacement + original[block.end() :] result = GatewayConfigUpdate.UPDATED @@ -282,6 +292,7 @@ def update_gateway_config( original, middleware_name=middleware_name, endpoint=endpoint, + timeout_gateway_ceiling=timeout_gateway_ceiling, ) result = GatewayConfigUpdate.ADDED @@ -382,10 +393,12 @@ def _new_gateway_config( *, middleware_name: str, endpoint: str, + timeout_gateway_ceiling: str, ) -> str: return "[openshell]\nversion = 1\n\n" + _middleware_block( middleware_name=middleware_name, endpoint=endpoint, + timeout_gateway_ceiling=timeout_gateway_ceiling, ) @@ -446,6 +459,7 @@ def _append_middleware_block( *, middleware_name: str, endpoint: str, + timeout_gateway_ceiling: str, ) -> str: return ( contents.rstrip() @@ -453,6 +467,7 @@ def _append_middleware_block( + _middleware_block( middleware_name=middleware_name, endpoint=endpoint, + timeout_gateway_ceiling=timeout_gateway_ceiling, ) ) @@ -461,13 +476,14 @@ def _middleware_block( *, middleware_name: str, endpoint: str, + timeout_gateway_ceiling: str, ) -> str: return ( "[[openshell.supervisor.middleware]]\n" f'name = "{middleware_name}"\n' f'grpc_endpoint = "{endpoint}"\n' "max_body_bytes = 4194304\n" - f'timeout = "{DEFAULT_GATEWAY_REGISTRATION_TIMEOUT:g}s"\n' + f'timeout = "{timeout_gateway_ceiling}"\n' ) @@ -475,6 +491,7 @@ def _update_middleware_block( block: str, *, endpoint: str, + timeout_gateway_ceiling: str, ) -> str: updated = _replace_or_append_assignment( block, @@ -489,7 +506,7 @@ def _update_middleware_block( return _replace_or_append_assignment( updated, key="timeout", - value=f'"{DEFAULT_GATEWAY_REGISTRATION_TIMEOUT:g}s"', + value=f'"{timeout_gateway_ceiling}"', ) diff --git a/projects/egress-gate/src/egress_gate/timeout.py b/projects/egress-gate/src/egress_gate/timeout.py index 2e5a0800..9d108c36 100644 --- a/projects/egress-gate/src/egress_gate/timeout.py +++ b/projects/egress-gate/src/egress_gate/timeout.py @@ -7,16 +7,24 @@ from collections.abc import Iterator from contextlib import contextmanager from time import monotonic -from typing import Self +from typing import Literal, Self -from pydantic import Field +from pydantic import Field, ValidationError from egress_gate.base import StrictDomainModel -from egress_gate.constants import MAX_TIMEOUT_MIDDLEWARE_PROCESSING from egress_gate.errors import TimeoutExpiredError +TIMEOUT_DURATION_PATTERN = r"^(?P[1-9][0-9]{0,8})(?Pms|s)$" +_TIMEOUT_DURATION_PATTERN = re.compile(TIMEOUT_DURATION_PATTERN) -def validate_timeout_middleware_processing(seconds: object) -> float: + +class _DurationValue(StrictDomainModel): + """A duration string validated with the shared OpenShell-style pattern.""" + + value: str = Field(pattern=TIMEOUT_DURATION_PATTERN) + + +def validate_timeout_middleware_processing(seconds: float) -> float: """Return a supported internal processing timeout, in seconds.""" if ( isinstance(seconds, bool) @@ -24,25 +32,28 @@ def validate_timeout_middleware_processing(seconds: object) -> float: or not math.isfinite(seconds) ): raise ValueError( - "timeout_middleware_processing must be between 10ms and " - f"{MAX_TIMEOUT_MIDDLEWARE_PROCESSING:g}s, using whole milliseconds" + "timeout_middleware_processing must be at least 10ms, using whole " + "milliseconds" ) validated_seconds = float(seconds) milliseconds = validated_seconds * 1000 + if not math.isfinite(milliseconds): + raise ValueError( + "timeout_middleware_processing must be at least 10ms, using whole " + "milliseconds" + ) rounded_milliseconds = round(milliseconds) - if ( - rounded_milliseconds < 10 - or validated_seconds > MAX_TIMEOUT_MIDDLEWARE_PROCESSING - or not math.isclose(milliseconds, rounded_milliseconds) + if rounded_milliseconds < 10 or not math.isclose( + milliseconds, rounded_milliseconds ): raise ValueError( - "timeout_middleware_processing must be between 10ms and " - f"{MAX_TIMEOUT_MIDDLEWARE_PROCESSING:g}s, using whole milliseconds" + "timeout_middleware_processing must be at least 10ms, using whole " + "milliseconds" ) return validated_seconds -def format_timeout_middleware_processing(seconds: object) -> str: +def format_timeout_middleware_processing(seconds: float) -> str: """Format a processing timeout for the OpenShell duration contract.""" validated_seconds = validate_timeout_middleware_processing(seconds) rounded_milliseconds = round(validated_seconds * 1000) @@ -51,21 +62,31 @@ def format_timeout_middleware_processing(seconds: object) -> str: return f"{rounded_milliseconds}ms" -def parse_timeout_duration(duration: object) -> float: - """Parse an integer OpenShell-style duration such as ``10s`` or ``500ms``.""" - if not isinstance(duration, str): +def parse_duration( + duration: str, + unit: Literal["s", "ms"] = "s", +) -> float: + """Parse a validated OpenShell-style duration into the requested unit.""" + try: + validated_duration = _DurationValue(value=duration).value + except ValidationError: raise ValueError("timeout must be an integer duration such as 10s or 500ms") - match = _TIMEOUT_DURATION_PATTERN.fullmatch(duration) + match = _TIMEOUT_DURATION_PATTERN.fullmatch(validated_duration) if match is None: - raise ValueError("timeout must be an integer duration such as 10s or 500ms") + raise AssertionError("Pydantic accepted an unmatched timeout duration") magnitude = int(match.group("magnitude")) - seconds = magnitude / 1000 if match.group("unit") == "ms" else magnitude + milliseconds = magnitude if match.group("unit") == "ms" else magnitude * 1000 + return milliseconds / 1000 if unit == "s" else float(milliseconds) + + +def parse_timeout_duration(duration: str) -> float: + """Parse and validate an internal middleware processing duration.""" + seconds = parse_duration(duration) try: format_timeout_middleware_processing(seconds) except ValueError: raise ValueError( - "timeout must be between 10ms and " - f"{MAX_TIMEOUT_MIDDLEWARE_PROCESSING:g}s, using whole milliseconds" + "timeout must be at least 10ms, using whole milliseconds" ) from None return float(seconds) @@ -77,7 +98,7 @@ class Timeout(StrictDomainModel): @classmethod def from_seconds(cls, seconds: float) -> Self: - """Create a timeout from a finite, positive bounded duration.""" + """Create a timeout from a finite positive duration.""" return cls( deadline=monotonic() + validate_timeout_middleware_processing(seconds) ) @@ -104,12 +125,11 @@ def enforce(self) -> Iterator[None]: self.raise_if_expired() -_TIMEOUT_DURATION_PATTERN = re.compile(r"(?P[0-9]{1,9})(?Pms|s)") - - __all__ = [ + "TIMEOUT_DURATION_PATTERN", "Timeout", "format_timeout_middleware_processing", + "parse_duration", "parse_timeout_duration", "validate_timeout_middleware_processing", ] diff --git a/projects/egress-gate/tests/service/test_server.py b/projects/egress-gate/tests/service/test_server.py index c7f6bb66..4cdea0ef 100644 --- a/projects/egress-gate/tests/service/test_server.py +++ b/projects/egress-gate/tests/service/test_server.py @@ -47,7 +47,7 @@ def test_server_rejects_a_registry_without_gates() -> None: @pytest.mark.parametrize( "seconds", - [True, 0, 0.001, 1.0001, 30, float("inf")], + [True, 0, 0.001, 1.0001, float("inf")], ) def test_server_validates_timeout_middleware_processing( seconds: bool | int | float, diff --git a/projects/egress-gate/tests/test_cli.py b/projects/egress-gate/tests/test_cli.py index 0d137a6c..60773d8d 100644 --- a/projects/egress-gate/tests/test_cli.py +++ b/projects/egress-gate/tests/test_cli.py @@ -54,6 +54,7 @@ def test_cli_narrow_help_preserves_complete_option_names() -> None: assert result.exit_code == 0 assert "--host-ip" in result.stdout assert "--config" in result.stdout + assert "--timeout" in result.stdout assert "--host…" not in result.stdout assert "--conf…" not in result.stdout @@ -96,14 +97,14 @@ def serve_sync(self, listen: str) -> None: assert "--timeout " in serve_help assert "--timeout-seconds" not in serve_help assert "s for seconds or ms for milliseconds" in serve_help - assert "10ms through 29s" in serve_help + assert "Minimum 10ms" in serve_help assert "RPC timeout" in serve_help evaluate_help = CliRunner().invoke(app, ["evaluate", "--help"]) assert evaluate_help.exit_code == 0, evaluate_help.output normalized_evaluate_help = " ".join(evaluate_help.stdout.split()) assert "s for seconds or ms for milliseconds" in normalized_evaluate_help - assert "10ms through 29s" in normalized_evaluate_help + assert "Minimum 10ms" in normalized_evaluate_help def test_cli_serve_rejects_timeout_at_remembered_gateway_ceiling( @@ -476,6 +477,8 @@ def test_cli_add_gateway_registration_reports_the_result( "192.0.2.10", "--config", str(config), + "--timeout", + "45s", ], ) @@ -488,7 +491,8 @@ def test_cli_add_gateway_registration_reports_the_result( assert "Endpoint" in result.stdout assert "http://192.0.2.10:50051" in result.stdout assert "Gateway RPC ceiling" in result.stdout - assert "30s" in result.stdout + assert "45s" in result.stdout + assert 'timeout = "45s"' in config.read_text() assert "Created the gateway configuration file" in result.stdout assert "Next: Start Egress Gate" in result.stdout @@ -608,7 +612,7 @@ def test_cli_evaluate_explains_an_invalid_timeout() -> None: "--cases", str(project_dir / "examples/regex-redaction/cases.yaml"), "--timeout", - "0s", + "9ms", ], color=True, ) @@ -616,4 +620,4 @@ def test_cli_evaluate_explains_an_invalid_timeout() -> None: assert result.exit_code == 2 error_output = Text.from_ansi(result.stderr).plain assert "Invalid value for --timeout" in error_output - assert "between 10ms and 29s" in error_output + assert "at least 10ms" in error_output diff --git a/projects/egress-gate/tests/test_gateway_config.py b/projects/egress-gate/tests/test_gateway_config.py index ddfb327a..ebb3c63c 100644 --- a/projects/egress-gate/tests/test_gateway_config.py +++ b/projects/egress-gate/tests/test_gateway_config.py @@ -231,6 +231,7 @@ def test_update_gateway_config_updates_only_the_named_registration( middleware_name="egress-gate", host_ip="10.0.0.4", port=50053, + timeout_gateway_ceiling="45s", ) assert result is GatewayConfigUpdate.UPDATED @@ -239,18 +240,30 @@ def test_update_gateway_config_updates_only_the_named_registration( assert "# Keep this registration comment." in contents assert 'grpc_endpoint = "http://10.0.0.4:50053"' in contents assert "max_body_bytes = 4194304" in contents - assert 'timeout = "30s"' in contents + assert 'timeout = "45s"' in contents repeated = update_gateway_config( path, middleware_name="egress-gate", host_ip="10.0.0.4", port=50053, + timeout_gateway_ceiling="45s", ) assert repeated is GatewayConfigUpdate.UNCHANGED +def test_update_gateway_config_rejects_invalid_timeout(tmp_path: Path) -> None: + with pytest.raises(GatewayConfigError, match="gateway timeout"): + update_gateway_config( + tmp_path / "gateway.toml", + middleware_name="egress-gate", + host_ip="10.0.0.4", + port=50053, + timeout_gateway_ceiling="1m", + ) + + def test_update_gateway_config_adds_the_operator_timeout_ceiling_when_missing( tmp_path: Path, ) -> None: diff --git a/projects/egress-gate/tests/test_timeout.py b/projects/egress-gate/tests/test_timeout.py index ceb2be93..3c248507 100644 --- a/projects/egress-gate/tests/test_timeout.py +++ b/projects/egress-gate/tests/test_timeout.py @@ -8,27 +8,28 @@ from egress_gate.timeout import ( Timeout, format_timeout_middleware_processing, + parse_duration, parse_timeout_duration, ) @pytest.mark.parametrize( "seconds", - [True, 0, -1, 0.001, 1.0001, float("inf"), 30], + [True, 0, -1, 0.001, 0.0101, 1.0001, float("inf")], ) -def test_timeout_duration_is_strict_positive_and_bounded( +def test_timeout_duration_has_minimum_and_whole_milliseconds( seconds: bool | int | float, ) -> None: with pytest.raises( ValueError, - match="between 10ms and 29s, using whole milliseconds", + match="at least 10ms, using whole milliseconds", ): Timeout.from_seconds(seconds) @pytest.mark.parametrize( ("seconds", "duration"), - [(0.01, "10ms"), (1.0, "1s"), (4.5, "4500ms"), (29.0, "29s")], + [(0.01, "10ms"), (1.0, "1s"), (4.5, "4500ms"), (45.0, "45s")], ) def test_middleware_timeout_uses_the_contract_duration_format( seconds: float, @@ -39,7 +40,7 @@ def test_middleware_timeout_uses_the_contract_duration_format( @pytest.mark.parametrize( ("duration", "seconds"), - [("10ms", 0.01), ("500ms", 0.5), ("1s", 1.0), ("29s", 29.0)], + [("10ms", 0.01), ("500ms", 0.5), ("1s", 1.0), ("45s", 45.0)], ) def test_timeout_duration_parser_accepts_concise_cli_values( duration: str, @@ -48,12 +49,20 @@ def test_timeout_duration_parser_accepts_concise_cli_values( assert parse_timeout_duration(duration) == seconds -@pytest.mark.parametrize("duration", ["", "1", "1.5s", "0s", "30s", "1m"]) +@pytest.mark.parametrize( + "duration", + ["", "1", "1.5s", "0s", "9ms", "1m", "1000000000s"], +) def test_timeout_duration_parser_rejects_unsupported_values(duration: str) -> None: with pytest.raises(ValueError, match="timeout"): parse_timeout_duration(duration) +def test_shared_duration_parser_supports_gateway_values_and_units() -> None: + assert parse_duration("1ms") == 0.001 + assert parse_duration("45s", unit="ms") == 45_000.0 + + def test_expired_timeout_raises_typed_signal() -> None: timeout = Timeout(deadline=monotonic() - 1) From 6fbcb36813b209dd30518fc9ed436f58ff3f7183 Mon Sep 17 00:00:00 2001 From: Johnny Greco Date: Fri, 7 Aug 2026 16:22:30 +0000 Subject: [PATCH 5/5] Fix registration timeout lifecycle --- projects/egress-gate/docs/operations.md | 5 +- projects/egress-gate/src/egress_gate/cli.py | 11 ++- .../src/egress_gate/gateway_config.py | 46 ++++++++++-- projects/egress-gate/tests/test_cli.py | 74 +++++++++++++++++++ .../egress-gate/tests/test_gateway_config.py | 8 +- 5 files changed, 130 insertions(+), 14 deletions(-) diff --git a/projects/egress-gate/docs/operations.md b/projects/egress-gate/docs/operations.md index 3fb94919..82d9fbd8 100644 --- a/projects/egress-gate/docs/operations.md +++ b/projects/egress-gate/docs/operations.md @@ -42,8 +42,9 @@ normally use. The optional registration `--timeout` sets the gateway RPC timeout written to the TOML file and defaults to 30 seconds. It accepts whole seconds or -milliseconds, such as `45s` or `500ms`. Rerunning the command writes the value -passed on that invocation. Set Egress Gate's internal processing budget with +milliseconds, such as `45s` or `500ms`, and must be greater than 10ms so the +internal processing budget can remain lower. Rerunning the command writes the +value passed on that invocation. Set Egress Gate's internal processing budget with `egress-gate serve --timeout DURATION`; the Python API calls that setting `timeout_middleware_processing`. It must be at least 10ms and resolve to whole milliseconds. When a remembered registration exists, `serve` reads its current diff --git a/projects/egress-gate/src/egress_gate/cli.py b/projects/egress-gate/src/egress_gate/cli.py index 5277c021..e919052f 100644 --- a/projects/egress-gate/src/egress_gate/cli.py +++ b/projects/egress-gate/src/egress_gate/cli.py @@ -52,11 +52,13 @@ GatewayConfigUpdate, GatewayMiddlewareRegistration, default_gateway_config_path, + forget_gateway_registration, list_gateway_registrations, read_remembered_gateway_timeout, remember_gateway_registration, remove_gateway_config, update_gateway_config, + validate_gateway_timeout, validate_middleware_name, ) from egress_gate.logging import LoggingConfig, configure_logging, get_logger @@ -65,7 +67,6 @@ from egress_gate.string_validators import BoundedMetadataString from egress_gate.timeout import ( Timeout, - parse_duration, parse_timeout_duration, validate_timeout_middleware_processing, ) @@ -288,8 +289,8 @@ def add_gateway_registration( param_hint="--name", ) from None try: - parse_duration(timeout) - except ValueError as error: + validate_gateway_timeout(timeout) + except GatewayConfigError as error: raise typer.BadParameter( str(error), param_hint="--timeout", @@ -393,6 +394,10 @@ def remove_gateway_registration( config_path, middleware_name=name, ) + forget_gateway_registration( + config_path, + middleware_name=name, + ) except GatewayConfigError as error: _render_cli_error( "Gateway registration could not be removed", diff --git a/projects/egress-gate/src/egress_gate/gateway_config.py b/projects/egress-gate/src/egress_gate/gateway_config.py index bd8399bc..fdbefdd6 100644 --- a/projects/egress-gate/src/egress_gate/gateway_config.py +++ b/projects/egress-gate/src/egress_gate/gateway_config.py @@ -98,6 +98,27 @@ def remember_gateway_registration( ) +def forget_gateway_registration( + config_path: Path, + *, + middleware_name: str, +) -> None: + """Forget the registration when it matches the CLI-managed registration.""" + remembered = load_remembered_gateway_registration() + if remembered is None or ( + remembered.config_path != config_path.expanduser().resolve() + or remembered.middleware_name != middleware_name + ): + return + state_path = default_registration_state_path() + try: + state_path.unlink(missing_ok=True) + except OSError as error: + raise GatewayConfigError( + f"Could not remove {state_path}. Check that its directory is writable." + ) from error + + def load_remembered_gateway_registration() -> RememberedGatewayRegistration | None: """Load the gateway registration most recently managed by the CLI.""" state_path = default_registration_state_path() @@ -226,13 +247,7 @@ def update_gateway_config( ) -> GatewayConfigUpdate: """Add or update one named Egress Gate middleware registration.""" validate_middleware_name(middleware_name) - try: - parse_duration(timeout_gateway_ceiling) - except ValueError: - raise GatewayConfigError( - "The gateway timeout must use whole seconds or milliseconds, such as " - "30s or 500ms." - ) from None + validate_gateway_timeout(timeout_gateway_ceiling) endpoint = f"http://{host_ip}:{port}" try: original = path.read_text(encoding="utf-8") @@ -389,6 +404,21 @@ def validate_middleware_name(name: str) -> str: return name +def validate_gateway_timeout(duration: str) -> float: + """Validate a gateway timeout that can exceed the processing minimum.""" + message = ( + "The gateway timeout must be greater than 10ms and use whole seconds or " + "milliseconds, such as 30s or 500ms." + ) + try: + seconds = parse_duration(duration) + except ValueError: + raise GatewayConfigError(message) from None + if seconds <= 0.01: + raise GatewayConfigError(message) + return seconds + + def _new_gateway_config( *, middleware_name: str, @@ -587,11 +617,13 @@ def _write_atomically(path: Path, contents: str) -> None: "MAX_MIDDLEWARE_REGISTRATION_NAME_BYTES", "default_gateway_config_path", "default_registration_state_path", + "forget_gateway_registration", "list_gateway_registrations", "load_remembered_gateway_registration", "remember_gateway_registration", "read_remembered_gateway_timeout", "remove_gateway_config", "update_gateway_config", + "validate_gateway_timeout", "validate_middleware_name", ] diff --git a/projects/egress-gate/tests/test_cli.py b/projects/egress-gate/tests/test_cli.py index 60773d8d..aaeafad3 100644 --- a/projects/egress-gate/tests/test_cli.py +++ b/projects/egress-gate/tests/test_cli.py @@ -497,6 +497,80 @@ def test_cli_add_gateway_registration_reports_the_result( assert "Next: Start Egress Gate" in result.stdout +def test_cli_rejects_gateway_timeout_without_processing_headroom( + tmp_path: Path, +) -> None: + config = tmp_path / "gateway.toml" + + result = CliRunner().invoke( + app, + [ + "add-gateway-registration", + "--host-ip", + "192.0.2.10", + "--config", + str(config), + "--timeout", + "10ms", + ], + ) + + assert result.exit_code == 2 + assert "gateway timeout must be greater than 10ms" in result.stderr + assert not config.exists() + + +def test_cli_removal_forgets_registration_before_later_serve( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + calls: list[str] = [] + + class FakeServer: + def __init__( + self, + registry: GateRegistry, + *, + timeout_middleware_processing: float, + ) -> None: + del registry, timeout_middleware_processing + + def serve_sync(self, listen: str) -> None: + calls.append(listen) + + monkeypatch.setattr("egress_gate.service.server.EgressGateServer", FakeServer) + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "user-config")) + config = tmp_path / "gateway.toml" + add_result = CliRunner().invoke( + app, + [ + "add-gateway-registration", + "--host-ip", + "192.0.2.10", + "--config", + str(config), + ], + ) + assert add_result.exit_code == 0, add_result.output + + remove_result = CliRunner().invoke( + app, + [ + "remove-gateway-registration", + "--name", + "egress-gate", + "--config", + str(config), + ], + ) + assert remove_result.exit_code == 0, remove_result.output + + serve_result = CliRunner().invoke(app, ["serve"]) + + assert serve_result.exit_code == 0, serve_result.output + assert calls == ["127.0.0.1:50051"] + + def test_cli_lists_gateway_registration_names_for_removal(tmp_path: Path) -> None: config = tmp_path / "gateway.toml" config.write_text( diff --git a/projects/egress-gate/tests/test_gateway_config.py b/projects/egress-gate/tests/test_gateway_config.py index ebb3c63c..eed27f22 100644 --- a/projects/egress-gate/tests/test_gateway_config.py +++ b/projects/egress-gate/tests/test_gateway_config.py @@ -253,14 +253,18 @@ def test_update_gateway_config_updates_only_the_named_registration( assert repeated is GatewayConfigUpdate.UNCHANGED -def test_update_gateway_config_rejects_invalid_timeout(tmp_path: Path) -> None: +@pytest.mark.parametrize("timeout", ["1m", "10ms"]) +def test_update_gateway_config_rejects_invalid_timeout( + tmp_path: Path, + timeout: str, +) -> None: with pytest.raises(GatewayConfigError, match="gateway timeout"): update_gateway_config( tmp_path / "gateway.toml", middleware_name="egress-gate", host_ip="10.0.0.4", port=50053, - timeout_gateway_ceiling="1m", + timeout_gateway_ceiling=timeout, )