test(proxy/db): scrub DATABASE_URL across test_routing_prisma_wrapper to stop env leak - #30342
Draft
cursor[bot] wants to merge 1 commit into
Draft
test(proxy/db): scrub DATABASE_URL across test_routing_prisma_wrapper to stop env leak#30342cursor[bot] wants to merge 1 commit into
cursor[bot] wants to merge 1 commit into
Conversation
…stop env leak
PrismaWrapper.get_rds_iam_token writes DATABASE_URL directly into
os.environ. monkeypatch.delenv tracks nothing when the variable was
unset to begin with, so the synthesized writer URL (e.g. postgresql://
litellm:WRITER-TOKEN@writer.aurora.local:5432/litellm) survives the
test and leaks across the xdist worker.
Downstream effect: tests/test_litellm/proxy/common_utils/
test_key_rotation_e2e.py::TestDeprecatedKeyLookupDbE2E::
test_deprecated_key_grace_period_cache_hit_path stops skipping
(os.getenv("DATABASE_URL") is now truthy) and tries to open a real
Prisma connection against the bogus host, failing with
httpx.ConnectError under proxy-infra CI.
Add an autouse snapshot/restore fixture for the managed DATABASE_*
env vars, mirroring the pattern already used in test_db_url_settings.py
where the same docstring already documents that monkeypatch cannot
undo direct os.environ mutations.
Co-authored-by: Krrish Dholakia <krrish-berri-2@users.noreply.github.com>
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant issues
n/a (CI autofix). Fixes the recurring
proxy-infra / Run testsfailure on PRs againstlitellm_internal_stagingwhose changes do not touch any DB code, e.g. PR #30333 (CLAUDE.md only) run.Linear ticket
n/a
Pre-Submission checklist
Screenshots / Proof of Fix
Reproduced the leak locally on this branch (without the fixture) and confirmed the fix.
The failing CI job before this change: https://github.com/BerriAI/litellm/actions/runs/27451742107/job/81148306753
Type
Test
Bug Fix
Changes
PrismaWrapper.get_rds_iam_tokenwritesDATABASE_URL(andDATABASE_URL_READ_REPLICA) straight intoos.environ. The twotest_writer_get_rds_iam_token_*tests intest_routing_prisma_wrapper.pyusemonkeypatch.delenv("DATABASE_URL", raising=False)to clear the variable before exercising that path; but because pytest'smonkeypatch.delitemonly records history when the key was already present, an originally-unsetDATABASE_URLis never tracked. The synthesized writer URL (postgresql://litellm:WRITER-TOKEN@writer.aurora.local:5432/litellm?schema=public) therefore survives teardown and leaks into the rest of the xdist worker.test_key_rotation_e2e.py::TestDeprecatedKeyLookupDbE2E::test_deprecated_key_grace_period_cache_hit_pathskips whenDATABASE_URLis unset and tries toprisma_client.connect()otherwise. Once the leak is in place the skip check passes, the connect attempt aborts withhttpx.ConnectError: All connection attempts failed, the test reruns 2x (reruns=2on the proxy-infra workflow) and the whole job fails. That is what shipped on every recent staging-targeted PR whose diff did not touch any DB code (e.g. #30333).Add an autouse snapshot/restore fixture for the managed
DATABASE_*env vars at the top oftest_routing_prisma_wrapper.py. This mirrors the existing_scrub_db_envfixture intest_db_url_settings.py, whose docstring already calls out exactly this monkeypatch limitation. No production code changes; the fix is contained to the leaking test module.