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:30Z

[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 vanderchen 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:

pkg/router/OWNERS

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

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 78.12500% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.71%. Comparing base (5903457) to head (165d36d).
⚠️ Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/router/jwt.go	77.41%	4 Missing and 3 partials ⚠️
❗ 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%   60.71%   +0.73%     
==========================================
  Files          36       35       -1     
  Lines        3589     3643      +54     
==========================================
+ Hits         2153     2212      +59     
+ Misses       1216     1206      -10     
- Partials      220      225       +5

Flag	Coverage Δ
unittests	`60.71% <78.12%> (+0.73%)`	⬆️

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.

acsoto · 2026-10-20T01:38:50Z

+
+	now := time.Now()
+
+	jm.tokenMu.Lock()


Is the outer mutex needed here? The LRU is already thread-safe, and initializing it once and calling Clear() when loading the key would avoid replacing the cache pointer.

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

Navigation Menu

Repository navigation

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

router: reuse signed sandbox JWTs for a session#479
manaspros wants to merge 3 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!

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!

acsoto Oct 20, 2026

Uh oh!

This branch has not been deployed

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!

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!

acsoto Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Oct 18, 2026 •

edited

Loading