Skip to content

workloadmanager: honor default code interpreter auth mode#398

Open
avinxshKD wants to merge 1 commit into
volcano-sh:mainfrom
avinxshKD:fix/codeinterpreter-authmode-default
Open

workloadmanager: honor default code interpreter auth mode#398
avinxshKD wants to merge 1 commit into
volcano-sh:mainfrom
avinxshKD:fix/codeinterpreter-authmode-default

Conversation

@avinxshKD

Copy link
Copy Markdown

/kind bug

What this PR does / why we need it:

Keeps CodeInterpreter auth handling consistent between warm-pool and non-warm sandbox creation.

The non-warm path only treated explicit picod as PicoD auth. This makes the default/empty value behave the same way, matching the warm-pool path: anything except none gets the PicoD public key handling.

Which issue(s) this PR fixes:
Fixes #397

Special notes for your reviewer:

Added coverage for empty/default authMode in env injection and missing-key handling.

Does this PR introduce a user-facing change?:

NONE

Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>
@volcano-sh-bot volcano-sh-bot added the kind/bug Something isn't working label Jun 22, 2026
@volcano-sh-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yaozengzeng for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the workload builder to inject and validate the public key for all authentication modes except when authMode is explicitly set to none (previously, this was only done when authMode was set to picod). It also adds corresponding unit tests to verify this behavior, including a test for empty/default authentication modes. There are no review comments, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@avinxshKD

Copy link
Copy Markdown
Author

Hey @RainbowMango @hezhangjian Pls take a look,

Found this while checking the CodeInterpreter auth paths.

Warm-pool already treats empty authMode as default PicoD auth, but the non-warm builder only handled explicit picod.
This keeps both paths consistent.

cc @acsoto @YaoZengzeng

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.57%. Comparing base (524e55e) to head (2728c71).
⚠️ Report is 146 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #398       +/-   ##
===========================================
+ Coverage   47.57%   58.57%   +11.00%     
===========================================
  Files          30       37        +7     
  Lines        2819     3491      +672     
===========================================
+ Hits         1341     2045      +704     
+ Misses       1338     1237      -101     
- Partials      140      209       +69     
Flag Coverage Δ
unittests 58.57% <100.00%> (+11.00%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@acsoto

acsoto commented Jun 26, 2026

Copy link
Copy Markdown
Member

/lgtm

@avinxshKD

avinxshKD commented Jun 26, 2026

Copy link
Copy Markdown
Author

@acsoto thankyou for reviewing, pls take a look at this too #389 when you get chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working lgtm size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CodeInterpreter authMode default is skipped in non-warm sandbox creation

4 participants