feat(kernel): support federated token providers - #444
Conversation
|
For Go, we should also reuse-existing FederationProvider to avoid confusion as much as possible. Can we thread that through? Long term is support custom token provider in kernel which is out of scope for now |
vuanhphung
changed the title
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 2 Low
Looks good — the federated-provider kernel wiring is correct: the wrapper preserves the Thrift path, GetToken is snapshotted exactly once (telemetry short-circuit verified by test), and the federation client ID is correctly gated to the PAT branch. Two minor low-severity items: one stale retry-range comment the kernel bump left behind, and missing coverage for the two new federated error paths.
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — the kernel federated-token path is implemented correctly: the base provider is snapshotted once (kernel does its own exchange via set_identity_federation_client_id), the telemetry classifier short-circuits to avoid a second GetToken, and the new federatedTokenAuthenticator wrapper is safe on the Thrift path (nothing type-asserts the concrete provider, and the embedded authenticator implements neither M2M/U2M interface). One low doc-consistency gap: CONNECTION_PARAMETERS.md's kernel notes weren't updated with the snapshot/expiry caveat that README got.
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — clean reuse of WithFederatedTokenProvider* on the kernel path with correct type-switch ordering, context-threaded token resolution, and a telemetry short-circuit that avoids a second provider snapshot. One low-severity question about the account-wide (no client-id) federation path.
|
@eric-wang-1990 PTAL! |
2 participants
Reuses
WithFederatedTokenProvider*on the kernel backend. The driver snapshots one base-provider token for kernel-side PAT federation;WithFederatedTokenProviderAndClientIDalso forwards the SP-wide client ID. Because the kernel cannot refresh the provider, an expired token requires a new connection.Pins
KERNEL_REVto the federation-capable kernel. The kernel bump also requires aligning one retry-setter assertion with its clamp behavior.Validation:
go test ./...,make test-kernel, andmake lint.