Skip to content

LCORE-1253: Wire proxy and TLS into all outgoing connections#1

Open
max-svistunov wants to merge 4 commits into
lcore-1253-networking-config-tlsfrom
lcore-1253-networking-client-wiring
Open

LCORE-1253: Wire proxy and TLS into all outgoing connections#1
max-svistunov wants to merge 4 commits into
lcore-1253-networking-config-tlsfrom
lcore-1253-networking-client-wiring

Conversation

@max-svistunov

Copy link
Copy Markdown
Owner

Description

Wire proxy and TLS networking into all outgoing HTTP connections. This is PR 2 of 3 for LCORE-1253. Depends on PR 1.

What: Connects the networking configuration (from PR 1) to all four outgoing HTTP client paths: Llama Stack client (httpx), Splunk HEC (aiohttp), JWK token fetching (aiohttp), and MCP OAuth probing (aiohttp). Adds docs/networking.md as the user-facing guide.

Key design decisions:

  • Uses DefaultAsyncHttpxClient from llama-stack-client SDK to preserve SDK defaults (60s timeout, 100 max connections, follow_redirects=True). PR LCORE-754: Add TLS profiles lightspeed-core/lightspeed-stack#869 used raw httpx.AsyncClient which would lose these.
  • no_proxy implemented via httpx mounts for URL-pattern bypass; for aiohttp via per-request target_url check in get_aiohttp_proxy().
  • extra_ca merges additional CAs into the certifi system bundle. Both httpx and aiohttp paths use the merged bundle when extra_ca is configured, even without a TLS security profile.
  • Splunk uses the networking connector whenever networking config is present (not None), falling back to verify_ssl only when no networking config exists.

Reviewer guidance:

  • The central file is src/utils/networking.py — it orchestrates TLS, proxy, no_proxy, and CA bundle logic for both httpx and aiohttp.
  • When networking is None (no config), all paths return None/defaults — zero behavioral change. Trace through client.py:75 and splunk.py:76 to verify.
  • get_aiohttp_proxy() accepts target_url to check against no_proxy. All three aiohttp callers pass the target URL.
  • https_proxy takes precedence when both proxy URLs are set. http_proxy is effectively unused in the httpx path (httpx takes a single proxy URL). This is documented in networking.md.

Type of change

  • New feature
  • Documentation Update
  • Unit tests improvement

Tools used to create PR

  • Assisted-by: Claude Opus 4.6
  • Generated by: Claude Opus 4.6

Related Tickets & Documents

  • Related Issue # LCORE-1178
  • Closes # LCORE-1253 (partially — 2 of 3 PRs)

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

Manual verification:

1. Start full stack with TLS profile config:

Add networking.tls_security_profile.type: IntermediateType to the config YAML. Verify /liveness returns {"alive": true} and /v1/models returns model list (confirming Llama Stack connection works through TLS-configured httpx client). Result: Both endpoints returned expected responses.

2. Start full stack WITHOUT networking config:

Verify identical behavior to pre-change baseline. Result: All endpoints functional, no behavioral difference.

Connect the networking configuration infrastructure (from the previous
commit) to all HTTP client paths in the Lightspeed Stack.

New module src/utils/networking.py provides factory functions:
- build_httpx_client(): Constructs an httpx.AsyncClient using
  DefaultAsyncHttpxClient from llama-stack-client SDK, preserving SDK
  defaults (60s timeout, 100 max connections, follow_redirects) while
  adding proxy and TLS settings from NetworkingConfiguration.
- build_aiohttp_connector(): Constructs an aiohttp.TCPConnector with
  ssl.SSLContext from the TLS security profile.
- get_aiohttp_proxy(): Extracts the proxy URL for aiohttp sessions.

Modified client paths:
- src/client.py: Service-mode Llama Stack client now receives a custom
  httpx.AsyncClient via the http_client parameter when networking config
  is present. No behavior change when networking config is absent.
- src/observability/splunk.py: Splunk HEC client uses networking config
  for TLS/proxy when available, falling back to the existing verify_ssl
  setting for backward compatibility.
- src/authentication/jwk_token.py: JWK endpoint fetching uses networking
  config for proxy and TLS.
- src/utils/mcp_oauth_probe.py: MCP OAuth probing uses networking config
  for proxy and TLS.

Added networking property to AppConfig singleton in configuration.py.

Documentation:
- docs/networking.md: Comprehensive guide covering proxy configuration
  (tunnel and interception), TLS security profiles (all 4 types with
  examples), custom CA certificates, and troubleshooting.
- docs/index.md: Added networking link to navigation.
- docs/ARCHITECTURE.md: Added networking to configuration overview.

Unit tests: 13 new tests for networking helper functions.
Complete the no_proxy and extra_ca implementation that was previously
defined in config models but not wired into HTTP clients.

no_proxy support:
- For httpx: Uses httpx mounts to bypass proxy for matching hosts.
  Parses comma-separated no_proxy string into mount patterns
  (exact match, leading-dot subdomain match, wildcard).
- For aiohttp: get_aiohttp_proxy() now accepts target_url parameter
  and checks it against no_proxy patterns before returning the proxy.
  All callers (Splunk, JWK, MCP OAuth) updated to pass target_url.

extra_ca support:
- _resolve_ca_cert_path() merges extra CA certificates with the system
  trust store via generate_ca_bundle() when extra_ca paths are
  configured. Falls back to TLS profile's ca_cert_path if no extra CAs.
- build_httpx_client() and build_aiohttp_connector() use the resolved
  CA path when building SSL contexts.

New helper functions:
- _parse_no_proxy(): Parse comma-separated no_proxy string.
- _host_matches_no_proxy(): Match hostname against no_proxy patterns
  (exact, leading-dot, fnmatch glob, wildcard).
- _build_no_proxy_mounts(): Convert no_proxy patterns to httpx mounts.
- _resolve_ca_cert_path(): Merge extra CAs or fall back to profile CA.

Unit tests: 10 new tests for no_proxy parsing, matching, and
aiohttp proxy bypass behavior.
…g docs

Splunk connector now uses networking config when either
tls_security_profile or extra_ca is configured, not only when
tls_security_profile is set. Previously a user who configured only
networking.proxy would not get the networking connector for Splunk.

Add /tmp default warning when certificate_directory is not configured.

Update docs/networking.md:
- Add no_proxy bypass patterns section (exact, IP, domain suffix, wildcard)
- Expand extra_ca documentation (merging behavior, precedence, defaults)
…logic

build_aiohttp_connector now handles extra_ca without a TLS profile
by creating an SSLContext with the merged CA bundle. Previously aiohttp
consumers (JWK, MCP OAuth, Splunk) ignored extra CAs when no TLS
security profile was configured.

Simplify Splunk connector condition: use networking config whenever
it is present, ensuring proxy-only and skip_tls_verification configs
are respected for all aiohttp consumers.

Add 3 unit tests for Splunk networking integration.

Fix docs/networking.md: caCertPath adds to the trust store, not
replaces it. Clarify https_proxy precedence.
Comment thread src/utils/networking.py
if networking.extra_ca:
cert_dir = networking.certificate_directory
if cert_dir is None:
cert_dir = Path("/tmp")

Check warning

Code scanning / Bandit

Probable insecure usage of temp file/directory. Warning

Probable insecure usage of temp file/directory.
max-svistunov added a commit that referenced this pull request Apr 30, 2026
Apply CodeRabbit's actionable comments and the per-comment nits:

1. PoC results section in spike doc previously listed paths under
   poc-results/ that are deleted before merge per howto-run-a-spike.md
   step 10, leaving broken links in the merged document. Replace the
   file list with a self-contained summary of what the PoC proved
   plus the heading-degradation finding, and a note pointing future
   readers at the PR diff if the raw artifacts are ever needed.

2. Drop the reference to docs/local-stack-testing.md (a local-only
   file, never committed to the repo).

3. Replace fragile line-numbered references (document_processor.py:75,
   :87, byok_guide.md ~106-118) with stable symbol anchors:
   _BaseDB.__init__, _LlamaStackDB.__init__, "Knowledge Sources"
   subsection, "Step 1" subsection. Line numbers rot; section names
   and symbol names rot less.

4. Spec doc now instructs the implementation ticket to extract the
   ("markdown", "html", "pdf") predicate to a single
   MARKDOWN_COMPATIBLE_DOC_TYPES: Final[tuple[str, ...]] constant in
   document_processor.py and reference it from both call sites,
   instead of duplicating the tuple. JIRA #1 scope updated to match.

5. Add R7: PDFReader.load_data emits a logger.warning when its docling
   output is empty / under a small threshold (a likely indicator of a
   scanned PDF given R5's no-OCR scope). Threshold is a module-level
   Final[int] constant. JIRA #1 scope and JIRA #2 test patterns
   updated to require coverage via caplog. Surfacing the silent-
   degradation case in custom_processor.py logs costs nothing and
   makes the OCR-needed signal visible.

Plus the two reviewer nits worth carrying into JIRA #1:

- Use docling's TableFormerMode.ACCURATE enum, not the string literal
  "accurate"; both work via Pydantic coercion but the enum is
  type-checked.
- Mirror HTMLReader's choice on whether to call super().__init__();
  llama-index's BaseReader does not require it but symmetry between
  the two readers is preferred.

The spec doc changelog records this revision and its trigger (the
PR lightspeed-core#1598 CodeRabbit review).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants