-
Notifications
You must be signed in to change notification settings - Fork 35
fix(backend): re-use tls contexts where possible #7327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| from infrahub.core.constants import GLOBAL_BRANCH_NAME | ||
| from infrahub.core.registry import registry | ||
| from infrahub.tasks.registry import refresh_branches | ||
| from infrahub.workers.dependencies import get_http | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainAvoid using FastAPI Depends-based get_http() here; also use an async context for get_client
As per coding guidelines Apply this diff here: - from infrahub.workers.dependencies import get_http
+ from infrahub.services.adapters.http.httpx import HttpxAdapter- client = get_client(httpx_settings={"verify": get_http().verify_tls()}, sync_client=False)
+ async with get_client(
+ httpx_settings={"verify": _HTTP_ADAPTER.verify_tls()}, sync_client=False
+ ) as client:And add this at module scope: # at top-level
_HTTP_ADAPTER = HttpxAdapter() # caches SSLContext internallyAnd adjust the call site: - await client.update_flow_run(current_flow_run_id, tags=list(new_tags))
+ await client.update_flow_run(current_flow_run_id, tags=list(new_tags))Verification script to confirm DI usage of get_http: Also applies to: 30-30 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Definition of get_http:"
rg -n -C2 'def get_http\(' backend/infrahub/workers/dependencies.py || true
echo
echo "Imports of Depends in dependencies module:"
rg -n -C2 '\bDepends\(' backend/infrahub/workers/dependencies.py || true
echo
echo "Call sites of get_http() outside FastAPI routes:"
rg -n -C3 '\bget_http\s*\(\s*\)' -g '!**/tests/**' backend | sed -n '1,200p'Length of output: 4978 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Inspect HttpxAdapter definition:"
rg -n "class HttpxAdapter" -C3 backend/infrahub/services/adapters/http/httpx.py || true
echo
echo "Inspect verify_tls in HttpxAdapter:"
rg -n "def verify_tls" -C3 backend/infrahub/services/adapters/http/httpx.py || true
echo
echo "Context around imports and usage in utils.py:"
rg -n -C5 "get_http" -C5 backend/infrahub/workflows/utils.py || trueLength of output: 1387 Replace
🤖 Prompt for AI Agents |
||
|
|
||
| from .constants import TAG_NAMESPACE, WorkflowTag | ||
|
|
||
|
|
@@ -26,7 +27,7 @@ async def add_tags( | |
| namespace: bool = True, | ||
| db_change: bool = False, | ||
| ) -> None: | ||
| client = get_client(sync_client=False) | ||
| client = get_client(httpx_settings={"verify": get_http().verify_tls()}, sync_client=False) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bit confusing in the codebase that we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Exactly, |
||
| current_flow_run_id = flow_run.id | ||
| current_tags: list[str] = flow_run.tags | ||
| branch_tags = ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Avoid DI-dependent get_http() and hardening around private attribute
Apply within this block:
Add these supporting changes outside the snippet:
If you prefer to keep get_http(), ensure it returns a concrete InfrahubHTTP in this runtime; otherwise this will fail.
🏁 Script executed:
Length of output: 3418
🏁 Script executed:
Length of output: 8203
🏁 Script executed:
Length of output: 3706
Replace DI-dependent get_http() with class-level HttpxAdapter and guard private attribute
get_http() uses FastAPI Depends and will return a Depends object outside DI, breaking .verify_tls(). Use a dedicated HttpxAdapter instance and only mutate the private attribute if it exists:
Add at the top of the file:
And inside class InfrahubWorkerAsync:
🤖 Prompt for AI Agents