router: reuse signed sandbox JWTs for a session by manaspros · Pull Request #479 · volcano-sh/agentcube

manaspros · 2026-10-18T07:49:22Z

What type of PR is this?

/kind enhancement

What this PR does / why we need it:

The router signed a new RS256 JWT on every request to a Sandbox or SandboxClaim session. With RSA-2048 that's about 0.55 ms of CPU per request, roughly ten times the rest of the invoke path.

This PR caches the signed token in JWTManager.GenerateTokenCached:

The key is (session ID, user sub, user email), which are exactly the claims generateSandboxJWT puts in the token.
A token is reused for 2.5 minutes, half of its 5 minute lifetime, then signed again. picod always gets a token with at least 2.5 minutes left, and no token lives longer than before.
The cache is a k8s.io/utils/lru with 4096 entries (already a dependency), behind one mutex.
loadPrivateKeyPEM clears it, so a token signed with an old key is never handed out.

There are two commits. The first only adds the benchmark, so you can check it out to reproduce the "before" numbers.

go test ./pkg/router -run xxx -bench BenchmarkInvoke -benchmem -benchtime=1000x -count=10
benchstat before.txt after.txt

                          │    before      │                after                 │
                          │     sec/op     │    sec/op     vs base                │
Invoke/SandboxClaim-24      617.28µ ±   1%   52.66µ ± 11%  -91.47% (p=0.000 n=10)
Invoke/CodeInterpreter-24   103.58µ ± 101%   55.44µ ± 10%        ~ (p=0.123 n=10)

                          │    before    │                after                │
                          │     B/op     │     B/op      vs base               │
Invoke/SandboxClaim-24      55.67Ki ± 0%   52.31Ki ± 0%  -6.03% (p=0.000 n=10)
Invoke/CodeInterpreter-24   50.97Ki ± 1%   51.00Ki ± 0%       ~ (p=0.699 n=10)

                          │   before   │              after                 │
                          │ allocs/op  │ allocs/op   vs base                │
Invoke/SandboxClaim-24      241.0 ± 0%   200.0 ± 0%  -17.01% (p=0.000 n=10)
Invoke/CodeInterpreter-24   194.0 ± 0%   194.0 ± 0%        ~ (p=0.474 n=10)

CodeInterpreter doesn't sign, so it's the control and doesn't change. Its "before" run had one slow outlier. The same code measured 48.10µ ± 4% in a separate 10-run sample. Machine: AMD Ryzen 9 9900X, Go 1.27.1, Linux.

I use -benchtime=1000x because of the per-request preflight dial: with the default benchtime the run exhausts ephemeral ports (TIME_WAIT) and requests start failing. That's a separate thing, not touched here.

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

Special notes for your reviewer:

The behaviour change to look at: a sandbox can now get the same token on several requests within 2.5 minutes. picod checks signature, exp and iat (pkg/picod/auth.go) and has no jti or nonce check, so I don't think anything relies on a fresh token.

Tests: TestGenerateTokenCached covers reuse, a different key, expiry, key swap, a zero-value manager and concurrent use. There's also a handler-level test. go test -race ./pkg/... passes.

Lint: make lint doesn't run as is. The Makefile pins golangci-lint v1.64.1 but .golangci.yml is a v2 config. I ran v2.15 with an equivalent config. There are no new findings in non-test code. The new tests only add goconst repeats, same as the existing tests.

Not tested: a real cluster, real network latency, Redis, concurrent load.

AI use: I used Claude Code (Claude Sonnet 5.5 and Claude Opus 5.5) to find this and draft the code, benchmark and tests. I've read and run all of it and can explain each line.

Does this PR introduce a user-facing change?:

NONE

Benchmark handleInvoke end to end (session lookup, preflight dial, JWT signing, reverse proxy) against an in-process upstream, for the SandboxClaim kind that signs a JWT and the CodeInterpreter kind that does not. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: manaspros <manasnandchoudhary@gmail.com>

generateSandboxJWT signed a new RS256 token on every request. With RSA-2048 that is about 0.55 ms of CPU, roughly ten times the rest of the invoke path. Cache the token per (session, user sub, user email) and sign again once it is older than half its 5 minute lifetime. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: manaspros <manasnandchoudhary@gmail.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

volcano-sh-bot · 2026-10-18T07:49:33Z

Welcome @manaspros! It looks like this is your first PR to volcano-sh/agentcube 🎉

codecov-commenter · 2026-10-18T07:56:19Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 76.92308% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.46%. Comparing base (5903457) to head (5c0b32e).
⚠️ Report is 37 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/router/jwt.go	80.00%	3 Missing and 2 partials ⚠️
pkg/router/handlers.go	0.00%	1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
+ Coverage   59.98%   61.46%   +1.47%     
==========================================
  Files          36       35       -1     
  Lines        3589     3659      +70     
==========================================
+ Hits         2153     2249      +96     
+ Misses       1216     1185      -31     
- Partials      220      225       +5

Flag	Coverage Δ
unittests	`61.46% <76.92%> (+1.47%)`	⬆️

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 · 2026-10-19T08:24:05Z

a good reuse practice

Replace GenerateTokenCached(key, claims) with GenerateSandboxToken(sessionID, caller). It builds the cache key and the claims together, so they cannot get out of sync, and the handler no longer has to know about the cache key. RS256 is deterministic, so comparing two tokens signed in the same second did not prove that the cache was used. The tests now swap the signing key between calls and check which key the token verifies with. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: manaspros <manasnandchoudhary@gmail.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

manaspros · 2026-10-19T15:04:38Z

pushed the fix for both of acsoto's review comments (commit 165d36d). The reuse tests now fail when the cache is off, and the key and claims are built in one place.

k8s.io/utils/lru already locks on every call. The cache is now created once in NewJWTManager and emptied with Clear() when a private key is loaded, instead of being replaced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: manaspros <manasnandchoudhary@gmail.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

acsoto · 2026-10-27T07:04:53Z

/lgtm
/approve

volcano-sh-bot · 2026-10-27T07:04:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acsoto

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/router/OWNERS~~ [acsoto]

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

manaspros · 2026-10-27T08:16:39Z

You're right, the LRU already locks internally. Removed the mutex in 5c0b32e: the cache is now created once in NewJWTManager, and loading a key calls Clear() instead of replacing it. go test -race ./pkg/router passes.

manaspros and others added 2 commits October 18, 2026 11:41

Copilot AI balanced review requested due to automatic review settings October 18, 2026 07:49

volcano-sh-bot added the kind/enhancement New feature or request label Oct 18, 2026

volcano-sh-bot requested review from YaoZengzeng and acsoto October 18, 2026 07:49

Copilot AI reviewed Oct 18, 2026

volcano-sh-bot added the size/L label Oct 18, 2026

acsoto reviewed Oct 19, 2026

View reviewed changes

Comment thread pkg/router/jwt_cache_test.go

acsoto reviewed Oct 19, 2026

View reviewed changes

Comment thread pkg/router/jwt.go Outdated

Copilot AI balanced review requested due to automatic review settings October 19, 2026 15:00

Copilot AI reviewed Oct 19, 2026

acsoto reviewed Oct 20, 2026

View reviewed changes

Comment thread pkg/router/jwt.go Outdated

Copilot AI balanced review requested due to automatic review settings October 27, 2026 05:11

Copilot AI reviewed Oct 27, 2026

volcano-sh-bot assigned acsoto Oct 27, 2026

volcano-sh-bot added the lgtm label Oct 27, 2026

volcano-sh-bot added the approved label Oct 27, 2026

volcano-sh-bot merged commit 1017f4d into volcano-sh:main Oct 27, 2026
15 checks passed

Navigation Menu

Repository navigation

router: reuse signed sandbox JWTs for a session - #479

router: reuse signed sandbox JWTs for a session#479
volcano-sh-bot merged 4 commits into
volcano-sh:mainfrom
manaspros:perf/router-jwt-cache

manaspros commented Oct 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

volcano-sh-bot commented Oct 18, 2026

Uh oh!

codecov-commenter commented Oct 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

acsoto commented Oct 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

manaspros commented Oct 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

acsoto commented Oct 27, 2026

Uh oh!

volcano-sh-bot commented Oct 27, 2026

Uh oh!

Uh oh!

manaspros commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

manaspros commented Oct 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

volcano-sh-bot commented Oct 18, 2026

Uh oh!

codecov-commenter commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

acsoto commented Oct 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

manaspros commented Oct 19, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

acsoto commented Oct 27, 2026

Uh oh!

volcano-sh-bot commented Oct 27, 2026

Uh oh!

Uh oh!

manaspros commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Oct 18, 2026 •

edited

Loading