Skip to content

feat(k8s_sat_token_authorizer): add Kubernetes service-account-token authorizer extension#3581

Draft
gouslu wants to merge 5 commits into
open-telemetry:mainfrom
gouslu:gouslu/sat-token-authz
Draft

feat(k8s_sat_token_authorizer): add Kubernetes service-account-token authorizer extension#3581
gouslu wants to merge 5 commits into
open-telemetry:mainfrom
gouslu:gouslu/sat-token-authz

Conversation

@gouslu

@gouslu gouslu commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What

Adds a new contrib extension, k8s_sat_token_authorizer, exposing the
BearerTokenAuthorizer capability (added in #3494). It authenticates and admits
inbound Kubernetes service-account tokens (SATs) presented on data-path
requests, so a receiver depends on this single capability rather than validating
tokens itself.

  • Authentication: validates the token via the Kubernetes TokenReview API
    (in-cluster kube client, lazily built on first use).
  • Admission (one of, mutually exclusive):
    • a static service-account allow-list (allowed_service_accounts), or
    • a Kubernetes RBAC check via SubjectAccessReview (resource_attributes);
    • with neither set, any authenticated account is admitted (audience-only).
  • Fail closed: an unreachable API server is an Err (undetermined), never an
    allow. A reached deny is a normal Deny decision.
  • Reached decisions are cached, keyed by the opaque token and bounded by a
    configurable TTL and entry cap, to bound TokenReview/SubjectAccessReview
    calls.

Gated behind the k8s-sat-token-authorizer-extension feature (and the aggregate
contrib-extensions feature); registered under
urn:otel:extension:k8s_sat_token_authorizer.

Execution model

Passive, with two capability variants sharing one implementation:

  • shared (Send, Arc + Mutex cache), and
  • local (!Send, Rc + lock-free RefCell cache) so thread-per-core
    consumers avoid the shared Mutex and cross-core contention (no SharedAsLocal
    adaptation).

To keep the two from drifting, the entire request flow lives once in
Core::authorize, parameterized over a DecisionStore trait; both variants sit
side by side in authorizer.rs as one-line delegations.

The Kubernetes client is built lazily on the first authorize() call (a build
failure fails closed and the next request retries), so no readiness gate is
needed. This version exposes no metrics (passive extensions receive no
CollectTelemetry; metric support for passive extensions is a follow-up).

Alignment

There is no official Collector-contrib TokenReview authenticator; this follows
the community Jamstah/k8ssaauthextension design (TokenReview + optional
SubjectAccessReview). Header/scheme extraction stays on the receiver per the
BearerTokenAuthorizer capability contract.

Testing

  • Unit tests: config validation, SA normalization, admission, cache, factory
    registration, and both variant wirings.
  • Live-cluster integration tests (#[ignore]d by default; no-op without
    K8S_SAT_TOKEN) covering TokenReview, allow-list, RBAC allow/deny, and the
    local variant. Verified against a local kind cluster; fixtures in
    testdata/integration-fixtures.yaml.
  • cargo xtask-equivalent checks (fmt, clippy -D warnings, tests),
    markdownlint, and tools/sanitycheck.py pass.

Docs: docs/k8s-sat-token-authorizer-extension.md (design + collector RBAC
requirements) and a new_component changelog entry are included.

Draft: the branch is based on an older main; I'll rebase before marking ready.

gouslu and others added 5 commits July 23, 2026 09:06
Implement the BearerTokenAuthorizer capability by validating inbound
Kubernetes service-account tokens via the TokenReview API and admitting
them against a configured audience and service-account allow-list.

Reached decisions are cached, keyed by the opaque token and bounded by a
configurable TTL and entry cap; an unreachable API server surfaces as an
error so callers fail closed. Gated behind the
k8s-sat-token-authorizer-extension feature and registered under
urn:otel:extension:k8s_sat_token_authorizer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 900b6834-24bb-44cd-9ddf-662baf1eb1eb
The authorizer has no periodic work and nothing to drain at shutdown, so it
does not need an active control loop. Make it Passive+Shared and build the
Kubernetes client lazily on first authorize() via a tokio OnceCell (a build
failure is undetermined and fails closed; the next request retries), removing
the start() loop and readiness gate.

Passive extensions receive no CollectTelemetry, so drop the metric set for now
(metric support for passive extensions is a future enhancement). Also drop the
now-unused startup_timeout config field.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 900b6834-24bb-44cd-9ddf-662baf1eb1eb
…izer

Add an alternative admission strategy that delegates authorization to
Kubernetes RBAC. After TokenReview authenticates the token, when
resource_attributes is configured the extension issues a SubjectAccessReview
asking whether the authenticated subject (user, uid, groups, extra) may perform
the configured verb on the configured resource; only an allow admits.

allowed_service_accounts (allow-list) and resource_attributes (RBAC) are
mutually exclusive, validated at config load; with neither set any
authenticated account is admitted. A failed SubjectAccessReview call is
undetermined and fails closed. This aligns with the community Go
k8ssaauthextension, which pairs TokenReview with SubjectAccessReview.

Docs note the collector ServiceAccount also needs create on
authorization.k8s.io/subjectaccessreviews. No engine/capability changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 900b6834-24bb-44cd-9ddf-662baf1eb1eb
Add #[ignore]d integration tests that exercise the real TokenReview and
SubjectAccessReview paths through authorize() against a Kubernetes cluster:
valid-token admission, bogus-token invalid-credential denial, allow-list
admit/deny, and RBAC allow (get pods) / deny (delete pods). They read the token
from K8S_SAT_TOKEN and no-op when it is unset, so default runs stay hermetic.

Add testdata/integration-fixtures.yaml (namespace, service account, Role,
RoleBinding) to reproduce the cluster setup, and document how to run them.

Verified against a local kind cluster (all four pass).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 900b6834-24bb-44cd-9ddf-662baf1eb1eb
Provide a native local (!Send) capability variant alongside the shared one so
thread-per-core consumers get a lock-free path instead of adapting the shared,
Mutex-guarded instance via SharedAsLocal. The shared variant keeps Arc + Mutex
(required by the shared instance factory's Send bound); the local variant uses
Rc + a lock-free RefCell cache.

To keep the two from drifting, the entire request flow lives once in
Core::authorize, parameterized over a DecisionStore trait (impl'd for
Mutex<DecisionCache> and RefCell<DecisionCache>). Both variants sit side by side
in authorizer.rs as one-line delegations, with common logic factored into
core.rs and cache.rs. Registered as a dual variant via extension_capabilities!.

Add live-cluster integration tests (ignored by default) covering TokenReview,
allow-list, RBAC SubjectAccessReview, and the local variant; verified against a
local kind cluster.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 900b6834-24bb-44cd-9ddf-662baf1eb1eb
@github-actions github-actions Bot added documentation Improvements or additions to documentation lang:rust Pull requests that update Rust code labels Jul 24, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 24, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-24 23:43:36 UTC.

  • Waiting on: Author
  • Next step: Move out of draft to request review.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 56.73575% with 167 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.72%. Comparing base (7987c8b) to head (d09cbeb).
⚠️ Report is 10 commits behind head on main.

❌ Your patch check has failed because the patch coverage (56.73%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3581      +/-   ##
==========================================
- Coverage   86.79%   86.72%   -0.08%     
==========================================
  Files         784      790       +6     
  Lines      319527   320253     +726     
==========================================
+ Hits       277347   277731     +384     
- Misses      41656    41998     +342     
  Partials      524      524              
Components Coverage Δ
otap-dataflow 87.75% <56.73%> (-0.10%) ⬇️
query_engine 89.57% <ø> (ø)
otel-arrow-go 52.45% <ø> (ø)
quiver 92.20% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines +26 to +42
/// Audiences the presented token must be valid for. Sent to the Kubernetes
/// `TokenReview` API as the requested audiences; the API server accepts the
/// token only if it is valid for at least one. Must be non-empty so a token
/// minted for an unrelated audience is never admitted.
pub audiences: Vec<String>,

/// Allow-list of service accounts admitted after authentication. Each entry
/// may be given as a full username
/// (`system:serviceaccount:<namespace>:<name>`), or the shorthand
/// `<namespace>:<name>` or `<namespace>/<name>`. An empty list admits any
/// service account the API server authenticates (audience-only admission).
///
/// Mutually exclusive with [`resource_attributes`](Self::resource_attributes):
/// admission is either a static allow-list or a Kubernetes RBAC check, not
/// both.
#[serde(default)]
pub allowed_service_accounts: Vec<String>,

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.

I think we might have an issue with allowing cross-tenant admission since audience and service-account allow-list are checked independently.

audiences and allowed_service_accounts are two independent flat lists. Admission is effectively: token is valid for some audience in the list AND its SA is in the global allow-list. The two conditions aren't tied together, so a service account isn't bound to a specific audience.

Concretely, with two tenants configured:

audiences: ["aud-tenant-a", "aud-tenant-b"]
allowed_service_accounts: ["ns-a:sa-a", "ns-b:sa-b"]

a token minted by sa-a for tenant B's audience (aud-tenant-b) is admitted: TokenReview authenticates it (that audience is in the list) and sa-a is in the global allow-list, even though sa-a should only be trusted for tenant A. There's nothing preventing one tenant's identity from authenticating against another tenant's audience.

For real multi-tenant isolation, could we reshape the config into a list of per-tenant bindings, each pairing an audience with its own allowed service accounts (and optionally its own resource_attributes), e.g.:

bindings:
  - audience: aud-tenant-a
    allowed_service_accounts: ["ns-a:sa-a"]
  - audience: aud-tenant-b
    allowed_service_accounts: ["ns-b:sa-b"]

and have admission check the SA against the binding for the matched audience (the one TokenReview confirms), rather than a global set? That closes the cross-tenant path. TokenReview still sends all audiences; only the admission step changes to key off the matched audience.

/// `TokenReview` round-trip. Entries expire after `ttl`; the map is capped at
/// `max_entries` to bound memory.
pub(crate) struct DecisionCache {
entries: HashMap<String, CachedDecision>,

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.

The decision cache is keyed on the plaintext token (HashMap<String, CachedDecision>). Since those keys are live, valid service-account tokens, this keeps un-zeroized credentials in memory for the whole TTL, which widens what a memory or core dump exposes, and it puts secret comparison (short-circuiting String equality) on the lookup path.

Could we key on SHA-256(token) instead (HashMap<[u8; 32], CachedDecision>)? That way:

  • no plaintext credential is retained in the cache, only a 32-byte digest, so a memory dump exposes far less,
  • lookups compare unpredictable digests, removing any token-comparison timing signal (defense in depth; not a known exploit here),
  • the key is Copy/Eq/Hash for free, and the per-miss to_owned() of the token goes away.

SHA-256 is collision-resistant, so distinct tokens never share an entry, and sha2 is already a workspace dependency. Behavior is otherwise unchanged.

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

Labels

documentation Improvements or additions to documentation lang:rust Pull requests that update Rust code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants