Repository navigation
test(e2e): cover Azure code_interpreter container files by native id with a service-account key - #43122
Conversation
…with a service-account key
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
| api_base: Final = os.environ.get("AZURE_API_BASE") | ||
| api_key: Final = os.environ.get("AZURE_API_KEY") | ||
| if not api_base or not api_key: | ||
| pytest.skip("set AZURE_API_BASE and AZURE_API_KEY in the pytest env; the deployments are registered with them") |
There was a problem hiding this comment.
Proxy credentials mask routing failures The changed-E2E runner exports these Azure credentials to the proxy too. If deployment-ID recovery breaks, container calls can still succeed with global credentials, so this test misses the regression it claims to catch. Run it with credentials available to pytest but absent from the proxy.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Seven e2e suites register Azure deployments through os.environ/AZURE_API_BASE on the proxy, so CI keeps that env; the no-env walk in the proof section covers routing
There was a problem hiding this comment.
That context makes sense. Keeping Azure credentials in CI is necessary for the existing suites, and the no-env walk in the proof section validates the deployment-ID routing path. I’ll treat this as an intentional CI limitation rather than request a test change; the remaining risk is only that a future routing regression could be masked in the automated run.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… scope the container call timeout
|
bugbot run |
…ssing instead of skipping
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9caa4b3. Configure here.
TLDR
Problem this solves:
code_interpretercontainer, nativecntr_<hex>id, service-account key, two Azure deploymentsHow it solves it:
tests/e2e/llm_translation/test_containers_e2e.pythat walks that exact shape/model/new, the decoy first, so routing by container id is what makes it passUser Flow
Before: a developer whose app runs on a team service-account key cannot read back the file it just wrote into the container
code_interpretercreatedtools: [{"type": "code_interpreter", "container": {"type": "auto"}}], and get acode_interpreter_calloutput naming a containercntr_<hex>id from that output, the way their store already does, and send POST https://litellm-domain/v1/containers/cntr_/files?custom_llm_provider=azure with a small file403 {"detail": "Forbidden"}(on a proxy with no Azure credentials in its env,500 api_base is required for Azure AI Studio ... Passed api_base=Noneinstead)After: the same key writes the file and reads it back by the native container id
code_interpreter_calloutput naming a container200with a file id shaped likecfile_<hex>200with the file's bytesLinear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Shared setup for every run below: a local proxy booted with
unset AZURE_API_BASE AZURE_API_KEYin front of it, Postgres andstore_model_in_db: true, and a config whose only deployment is an unrelatedplaceholder-model. The two Azure deployments come in throughPOST /model/new(realazure/gpt-5.4-nano,api_version: v1), the decoy with a bogus key registered first. Before was captured on a proxy built at each fix's parent commit, After on this PR's tip, both with the suite file from this PR. Keys are redactedBefore (183092d, parent of #27921, and 9cac0471ae6d, parent of #28990)
Service-account key writes and reads a container file by its native id
cd tests/e2e && python -m pytest llm_translation/test_containers_e2e.py -p no:cacheprovider -rA --timeout 600POST /containers/cntr_.../files?custom_llm_provider=azure HTTP/1.1" 403 Forbidden1 failed in 36.38s): the ownership bug fires before the routing bug gets a chance toMaster key writes a container file by its native id with no Azure credentials in the proxy env
POST /model/new(200,probe-azure-27921), thenPOST /v1/responseswith{"model": "probe-azure-27921", "input": "Use python to print 1+1", "tools": [{"type": "code_interpreter", "container": {"type": "auto"}}]}container_idin thecode_interpreter_calloutput to its native id:cntr_6ab5e7186f748190841fae3f037cf6630783f99814852b83curl -F file=@probe.txt "$P/v1/containers/$NATIVE/files?custom_llm_provider=azure"with the master keyHTTP 500:After (9caa4b3)
Service-account key writes and reads a container file by its native id
env | grep -c '^AZURE_API_BASE='printed0first):Master key writes a container file by its native id with no Azure credentials in the proxy env
/model/new,POST /v1/responseswith the master key, native id decodedcurl -F file=@master.txt "$P/v1/containers/$NATIVE/files?custom_llm_provider=azure"with the master keyMissing credentials fail the suite instead of skipping it
AZURE_API_BASEandAZURE_API_KEYblank in the pytest envCI runs at 9caa4b3
e2e-changed, run https://github.com/BerriAI/litellm/actions/runs/36093833771) selected this file and ran it three times against the stage-mirror stack:AZURE_API_BASEandAZURE_API_KEY):mainbefore documentation about replicate is not correct #279 merged, failed the same test withFailed: set AZURE_API_BASE and AZURE_API_KEY in the pytest env; the deployments are registered with them(1 failed, 13 passed), which was the redbuildkite/e2e-testscheck on this PR until documentation about replicate is not correct #279 merged; documentation about replicate is not correct #279 merged at 131e4eaa, and rebuilding the gate (https://buildkite.com/berriai-1/e2e-tests/builds/18638) triggered https://buildkite.com/berriai-1/litellm-e2e-pr/builds/1005 on the releaser'smain, which passed with the suite actually running, sobuildkite/e2e-testsis green on this PR now:Type
✅ Test
Caveats (if any)
Low
tests/e2e/gateway/stage_mirror_ci_config.yml) feeds Azure credentials into the proxy env throughfiles_settings, so a fix(router): use forwarded model_id for native Azure container IDs #27921-style regression (routing lost, global-credential fallback) stays green there, while the fix(containers): record ownership for service-account keys + fix Prisma Json serialization #28990 half (ownership lost, the customer's 403) turns red anywhere. Both halves turned red locally with the proxy booted without Azure env credentials, which is the runbook below and how the Before above was captured. Fixing it in CI is net negative here: the container endpoints return no header naming the deployment that served a call, so the suite cannot assert routing from the response; the other routes are a second Azure resource (new secrets in both CI runners) or a per-suite proxy redeploy without those env credentials, an e2e infra change other Azure suites constrain, tracked as a follow-up/v1/responsesvariant is not covered here: the container ownership row is written after[DONE]and the OpenAI SDK closes the stream at[DONE], so every follow-up container call 403s (LIT-8612). That cell lands with its fixGET /v1/containers?custom_llm_provider=azurewith no model to route by and no Azure env credentials answers 500 with the sameapi_base=Nonemessage; not covered here, since the customer's flow names a container every timetests/e2e/): themcp-integrationjob flaked on its first run and passed on rerun (job https://github.com/BerriAI/litellm/actions/runs/36093833771/job/107944542296); CircleCIlocal_testing_part1failstest_get_model_info_bedrock_cross_region_capability_parityexactly as main's own pipeline does (main job https://app.circleci.com/pipelines/github/BerriAI/litellm/90381/workflows/717108e9-cccb-41cc-a28b-782677eb61b9/jobs/2215854), and the rest of the CircleCI reds at pipeline 90383 are also not from this PR, each given its one from-failed rerun (https://app.circleci.com/pipelines/github/BerriAI/litellm/90383/workflows/084b9924-4786-4875-9ab4-4ee3dbf6b5ab):local_testing_part1also failstest_get_model_info_bedrock_regionon the rerun, a CircleCI flaky-insight test with 5 flakes;local_testing_part2failstest_openai_stream_options_call_text_completionbefore and after the rerun, a live OpenAI text-completion stream with 12 flakes in the flaky insight and green on main's own pipeline;unitshares 19 failures with main's own unit job (https://app.circleci.com/pipelines/github/BerriAI/litellm/90381/workflows/717108e9-cccb-41cc-a28b-782677eb61b9/jobs/2215836) plus the two prompt-caching event-loop tests intest_prompt_caching_deployment_check.py(flaky insight 3 and 1 flakes), both of which passed on the rerun, where the only failure not on main's list istest_stream_completion_counts_tokens_off_the_event_loopintest_a2a_streaming_iterator.py, another entry in the same flaky insight (1 flake);litellm_router_unit_testingfailed one counter test on the first run and passed on the rerun;build_and_testfails the twotest_rust_python_harnessocr imports exactly as main's own job (https://app.circleci.com/pipelines/github/BerriAI/litellm/90381/workflows/717108e9-cccb-41cc-a28b-782677eb61b9/jobs/2215842) plustest_key_model_list[/v1/models-key-gpt-3.5-turbo]and[/v1/models-team-gpt-3.5-turbo], whoseassert 2 == 1expectation main rewrote in fix(proxy): authorize key model aliases the same way as team aliases #43049 (1d51a8d, after this branch's merge base), unchanged on the rerun;using_litellm_on_windowstimes out in itsInstall Dependenciesstep on both runs with zero test failures, as main's own windows job did (https://app.circleci.com/pipelines/github/BerriAI/litellm/90381/workflows/717108e9-cccb-41cc-a28b-782677eb61b9/jobs/2215860). None of these checks is required onmain, and a test-only PR undertests/e2e/cannot introduce themQA runbook
code_interpreter, writes a file into it by Azure's native id, and reads the bytes back, on a proxy that routes by container id rather than by env credentialsAZURE_API_BASEandAZURE_API_KEYunset in its env, Postgres, andstore_model_in_db: true; export both values only in the shell that runs the testazure/gpt-5.4-nanowithapi_key: decoy-key, then the same model with the real key (api_version: v1), both carryingapi_basemodelsnaming the real deployment; POST /user/new withuser_role: internal_user; POST /team/member_add; POST /key/service-account/generate with that team and model; use only that key from here ontools: [{"type": "code_interpreter", "container": {"type": "auto"}}]and expect acode_interpreter_calloutput whosecontainer_iddecodes to acntr_<hex>idcfile_id500 api_base is required for Azure AI Studioon it with the proxy at 183092d and the master keyFinal Attestation
The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR
9caa4b3 passes /live-pr-risk