diff --git a/app/config.py b/app/config.py index ba3f108..790e4ef 100644 --- a/app/config.py +++ b/app/config.py @@ -1,8 +1,9 @@ """Configuration and secrets supplied by the environment.""" from typing import Annotated +from urllib.parse import urlparse -from pydantic import Field +from pydantic import Field, field_validator from pydantic_settings import BaseSettings, SettingsConfigDict @@ -67,5 +68,37 @@ class Settings(BaseSettings): portkey_api_key: str = "" portkey_virtual_key: str = "" + @field_validator("llm_guardrail_base_url") + @classmethod + def _remote_detector_must_be_https(cls, url: str) -> str: + """Refuse a remote detector address that is not https. + + A detector behind ingress that refuses plain HTTP answers it with a + redirect. Readiness still passes, because the probe follows the + redirect and receives a 200, so the worker wakes the GPU and only then + fails every screening: following a redirect turns the guardrail's POST + into a GET, which the endpoint rejects. Refusing the address at startup + costs a clear error instead of a cold start. + + A local address is exempt, having no ingress in front of it. + + Args: + url: The configured detector address. + + Returns: + The address, unchanged. + + Raises: + ValueError: The address is remote and does not use https. + """ + host = urlparse(url).hostname or "" + if host in ("localhost", "127.0.0.1", "::1"): + return url + if not url.startswith("https://"): + raise ValueError( + f"llm_guardrail_base_url must be https for a remote host, got {url!r}" + ) + return url + settings = Settings() # type: ignore[call-arg] diff --git a/infra/gemma/README.md b/infra/gemma/README.md index 32c53bc..6799679 100644 --- a/infra/gemma/README.md +++ b/infra/gemma/README.md @@ -341,9 +341,15 @@ patient. Both fixes were needed for different reasons and neither solves this on **Calling by app name instead of FQDN does not help.** The docs say calls by app name go "directly to app B" while FQDN calls route via the edge proxy, so this looked like a free fix. -Tested: `http://screening-gemma/v1/models` resolves, routes, and *does* trigger the cold start +Tested: `https://screening-gemma/v1/models` resolves, routes, and *does* trigger the cold start — then dies at exactly 240s with the same 504. The timeout applies either way. +The scheme has to be `https`. The original test used `urllib.request.urlopen`, which follows +redirects, so an `http://` address in that command reached the detector over HTTPS anyway and +the cold start it triggered was credited to the wrong scheme. A client that does *not* follow +redirects gets only the redirect, which starts nothing. See "The detector's address must be +https" below. + **Premium ingress can raise it** (idle request timeout, 4–30 min) but requires a non-Consumption workload profile, D4–D32, minimum two node instances, billed continuously. That removes the scale-to-zero economics this whole architecture exists to preserve. @@ -409,6 +415,10 @@ Use `https://` in `SCREENING_LLM_GUARDRAIL_BASE_URL` and neither arises. Interna ingress terminates TLS inside the environment, so this costs nothing and the traffic still never leaves it. +`Settings` refuses a remote address that is not `https`, so a wrong scheme is a +startup error rather than a cold start followed by a 405. Localhost is exempt, +having no ingress in front of it. + ### Cost scales with wake-ups, not with screenings The GPU bills for its cold start whether or not the caller survives it, so the unit of cost is @@ -437,7 +447,7 @@ az containerapp update -n screening-app -g screening-rg --min-replicas 1 # rem az containerapp exec -n screening-app -g screening-rg --command /bin/sh ``` ```sh -date; python -c "import urllib.request,time; t=time.time(); r=urllib.request.urlopen('http://screening-gemma/v1/models',timeout=1800).read().decode(); print(round(time.time()-t),'s')"; date +date; python -c "import urllib.request,time; t=time.time(); r=urllib.request.urlopen('https://screening-gemma/v1/models',timeout=1800).read().decode(); print(round(time.time()-t),'s')"; date ``` The two `date` stamps bracket the failure. Anything at ~240s is the proxy, not the app. diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 75d6f4f..99c02b7 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -34,3 +34,28 @@ def test_jobs_storage_defaults_point_at_the_general_purpose_account(): assert "screeningjobs" in Settings.model_fields["jobs_account_url"].default assert Settings.model_fields["jobs_table_name"].default assert Settings.model_fields["jobs_queue_name"].default + + +def test_a_plain_http_detector_address_is_refused(): + """The detector's ingress answers plain HTTP with a redirect rather than + serving it, and a client following that redirect turns the POST into a GET, + which the endpoint rejects with 405. Readiness still passes -- the probe + follows the redirect and gets a 200 -- so the failure arrives only after a + cold start has been paid for.""" + import pytest + from pydantic import ValidationError + + from app.config import Settings + + with pytest.raises(ValidationError): + Settings(llm_guardrail_base_url="http://screening-gemma.internal.example/v1") + + +def test_a_local_http_detector_address_is_allowed(): + """A detector served on localhost has no ingress in front of it, so the + scheme carries none of the same risk.""" + from app.config import Settings + + settings = Settings(llm_guardrail_base_url="http://localhost:8001/v1") + + assert settings.llm_guardrail_base_url.startswith("http://")