Deduplicate OIDC getToken() across token providers via base-class implementation#3881
Conversation
getToken() across token providers via base-class implementation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
Deduplicates the identical getToken() implementations across Azure, GCP, and Anthropic OIDC token providers by hoisting the logic into BaseOidcTokenProvider, and adds focused base-class tests.
Changes:
- Add
getToken()toBaseOidcTokenProviderusing the existing_getCachedValue()/_expiresAtabstraction. - Remove the duplicated
getToken()overrides from Azure, GCP, and Anthropic providers. - Add
oidc-token-provider-base.test.jscovering cached, expired, and in-flight refresh cases.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/oidc-token-provider-base.js | New shared getToken() implementation on the base class. |
| containers/api-proxy/oidc-token-provider.js | Removes Azure subclass getToken() override. |
| containers/api-proxy/gcp-oidc-token-provider.js | Removes GCP subclass getToken() override. |
| containers/api-proxy/anthropic-oidc-token-provider.js | Removes Anthropic subclass getToken() override. |
| containers/api-proxy/oidc-token-provider-base.test.js | Adds base-class tests for cached/expired/in-flight behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 0
Smoke Test: Claude Engine
Result: PASS
|
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( PR author: Overall: PARTIAL — BYOK inference ✅, pre-step smoke data vars unexpanded (workflow template issue)
|
🔬 Smoke Test Results
PR: Deduplicate OIDC Overall: PARTIAL — MCP ✅, but
|
Smoke Test: API Proxy OpenTelemetry Tracing
All 5 scenarios pass. OTEL tracing integration is fully operational.
|
|
#3852 [awf] ARC/DinD chroot: auto-stage runner binary and critical /etc files for split-filesystem Docker hosts | #3851 api-proxy: add Anthropic Workload Identity Federation provider
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
|
Smoke test result: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Chroot Runtime Version Comparison
Result: ❌ Not all runtimes match — Python and Node.js versions differ between host and chroot.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results: FAIL
Overall: FAIL —
|
Three
BaseOidcTokenProvidersubclasses carried identicalgetToken()logic in the token-serving path, creating drift risk for refresh/cache behavior changes. This PR moves that logic into the base class and removes subclass overrides.Shared token retrieval in base class
getToken()tocontainers/api-proxy/oidc-token-provider-base.js._getCachedValue()+_expiresAt, and triggers immediate refresh when stale and no refresh is in flight.Removed duplicated subclass methods
getToken()overrides from:containers/api-proxy/anthropic-oidc-token-provider.jscontainers/api-proxy/gcp-oidc-token-provider.jscontainers/api-proxy/oidc-token-provider.js(Azure)Focused base behavior coverage
containers/api-proxy/oidc-token-provider-base.test.jsto validate:nullreturned + refresh scheduled when expired