Skip to content

HYPERFLEET-1237 - feat: add JWT bearer token auth to HyperFleet API client#234

Open
rh-amarin wants to merge 3 commits into
openshift-hyperfleet:mainfrom
rh-amarin:jwt
Open

HYPERFLEET-1237 - feat: add JWT bearer token auth to HyperFleet API client#234
rh-amarin wants to merge 3 commits into
openshift-hyperfleet:mainfrom
rh-amarin:jwt

Conversation

@rh-amarin

Copy link
Copy Markdown
Contributor

Summary

  • Adds optional JWT bearer token authentication to the HyperFleet API HTTP client using Kubernetes projected ServiceAccount tokens
  • Auth is disabled by default — existing deployments are unaffected
  • Follows the same pattern as the sentinel implementation (HYPERFLEET-1237)

Changes

Go — internal/hyperfleetapi/

  • New fileTokenSource (token.go): reads bearer token from disk with optional TTL-based in-memory cache and double-checked locking for safe concurrent use
  • AuthConfig struct added to ClientConfig (token_path, token_cache_ttl)
  • WithAuth client option; tokenSource field on httpClient; Authorization: Bearer <token> header injected in doRequest

Go — internal/configloader/

  • HyperfleetAPIAuthConfig type alias
  • Env var mappings: HYPERFLEET_API_AUTH_TOKEN_PATH, HYPERFLEET_API_AUTH_TOKEN_CACHE_TTL
  • validateHyperfleetAuth in AdapterConfigValidator: enforces non-empty absolute path and non-negative TTL

Go — cmd/adapter/main.go

  • createAPIClient wires WithAuth from config

Helm chart

  • values.yaml: auth block under adapterConfig.hyperfleetApi (disabled by default, audience, tokenPath, expirationSeconds, tokenCacheTtl)
  • deployment.yaml: conditional projected serviceAccountToken volume + volume mount + env vars when auth.enabled
  • _helpers.tpl: Helm-time validation that tokenPath is an absolute path

Docs

  • docs/configuration.md: new auth.* fields and env vars documented
  • docs/deployment.md: new HyperFleet API Authentication section with usage example
  • configs/adapter-config-template.yaml: commented-out auth example

Test plan

  • make test — unit tests pass (token_test.go, client_test.go auth tests, configloader validator auth tests)
  • make build — binary builds cleanly
  • make lint — no lint errors
  • Deploy with auth.enabled: false (default) — no behavior change
  • Deploy with auth.enabled: true — confirm Authorization: Bearer header sent to HyperFleet API

🤖 Generated with Claude Code

@openshift-ci openshift-ci Bot requested review from ldornele and rafabene July 8, 2026 17:27
@openshift-ci

openshift-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

[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 jsell-rh for approval. For more information see the 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:

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

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds JWT bearer-token auth for the HyperFleet API client, with file-backed token loading, TTL caching, and request header injection. Helm chart values/templates now mount a projected ServiceAccount token, set auth env vars, and validate tokenPath. Config loading adds auth schema mapping and structure checks, while task validation now emits deprecation warnings for precondition.api_call instead of failing.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Helm as Helm chart
  participant Adapter as adapter main
  participant Client as HyperFleet API client
  participant TokenFile as projected token file
  participant API as HyperFleet API

  Helm->>Adapter: auth values + env vars
  Adapter->>Client: WithAuth(AuthConfig)
  Client->>TokenFile: read token
  TokenFile-->>Client: JWT bearer token
  Client->>API: request with Authorization header
  API-->>Client: response
Loading
🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.58% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (10 passed)
Check name Status Explanation
Title check ✅ Passed Clearly matches the JWT bearer-token auth change; no CWE-20/CWE-73 mismatch.
Description check ✅ Passed On-topic: it covers auth, Helm wiring, validation, and docs; no CWE-20/CWE-73 issue.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output ✅ Passed No non-test/example log or fmt.Print* call site in the PR emits token/password/credential/secret fields or interpolated strings.
No Hardcoded Secrets ✅ Passed No CWE-798 hit: only placeholder docs URLs, test fixture tokens, and a mount-path literal; no real API keys/passwords/private keys or base64 blobs.
No Weak Cryptography ✅ Passed PASS: Changed files add token-file auth only; no md5/des/rc4/sha1/ECB, no custom crypto, and no non-constant-time secret comparison. CWE-327 not present.
No Injection Vectors ✅ Passed No new CWE-89/78/79/502 sinks in the touched hunks; auth code only reads a token file, validates absolute path, and sets a header.
No Privileged Containers ✅ Passed No PR-added privileged flags or root runtime settings; chart keeps allowPrivilegeEscalation:false/runAsNonRoot:true, and Dockerfile root use is documented build/test only (CWE-269 not triggered).
No Pii Or Sensitive Data In Logs ✅ Passed No new logs expose bodies, tokens, session IDs, or credentials; added warnings log only deprecation text and config paths. CWE-532 not introduced.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
internal/hyperfleetapi/token_test.go (1)

84-111: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Direct field manipulation ts.expiresAt = time.Now().Add(-time.Second).UnixNano() (Line 98) will need updating to time.Now().Add(-time.Second) if expiresAt is switched to time.Time per the token.go comment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/hyperfleetapi/token_test.go` around lines 84 - 111, The test is
manually setting the cache expiration using a raw Unix nano value, but the token
cache is being changed to store expiration as a time.Time. Update
TestFileTokenSource_RefreshesAfterTTL to expire the cache by assigning a
time.Time value to ts.expiresAt instead of using UnixNano, and keep the refresh
assertion against FileTokenSource.get so the test matches the new expiresAt
type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@charts/templates/deployment.yaml`:
- Around line 163-167: The Hyperfleet auth volumeMount in the deployment
template can resolve to a reserved parent directory, causing duplicate mountPath
conflicts in the pod spec. Update the chart logic around the
hyperfleet-api-token mount to validate
.Values.adapterConfig.hyperfleetApi.auth.tokenPath via a new _helpers.tpl guard,
and reject values whose dir(tokenPath) collapses to /etc/adapter or /etc/broker
before rendering the mount.

In `@internal/hyperfleetapi/token.go`:
- Around line 14-20: The token cache expiry logic in fileTokenSource uses
wall-clock UnixNano comparisons, which can keep serving stale tokens if the
system clock moves backward. Update fileTokenSource to store and compare
time.Time values instead of raw UnixNano, and use the existing time-based
comparisons so monotonic clock behavior is preserved when checking expiration in
the token fetch/cache path. Adjust the cached expiry handling around
fileTokenSource, expiresAt, and the read/refresh logic so the TTL is enforced
relative to time.Time rather than wall-clock integers.

---

Nitpick comments:
In `@internal/hyperfleetapi/token_test.go`:
- Around line 84-111: The test is manually setting the cache expiration using a
raw Unix nano value, but the token cache is being changed to store expiration as
a time.Time. Update TestFileTokenSource_RefreshesAfterTTL to expire the cache by
assigning a time.Time value to ts.expiresAt instead of using UnixNano, and keep
the refresh assertion against FileTokenSource.get so the test matches the new
expiresAt type.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: b6fd24ce-e126-43dc-9502-3a76956cab7d

📥 Commits

Reviewing files that changed from the base of the PR and between 2a8b444 and 851de92.

📒 Files selected for processing (16)
  • charts/templates/_helpers.tpl
  • charts/templates/deployment.yaml
  • charts/values.yaml
  • cmd/adapter/main.go
  • configs/adapter-config-template.yaml
  • docs/configuration.md
  • docs/deployment.md
  • internal/configloader/types.go
  • internal/configloader/validator.go
  • internal/configloader/validator_test.go
  • internal/configloader/viper_loader.go
  • internal/hyperfleetapi/client.go
  • internal/hyperfleetapi/client_test.go
  • internal/hyperfleetapi/token.go
  • internal/hyperfleetapi/token_test.go
  • internal/hyperfleetapi/types.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • openshift-hyperfleet/architecture (manual)
  • openshift-hyperfleet/hyperfleet-api (manual)
  • openshift-hyperfleet/hyperfleet-sentinel (manual)
  • openshift-hyperfleet/hyperfleet-adapter (manual)
  • openshift-hyperfleet/hyperfleet-broker (manual)

Comment thread charts/templates/deployment.yaml
Comment thread internal/hyperfleetapi/token.go
@hyperfleet-ci-bot

hyperfleet-ci-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown

Risk Score: 4 — risk/high

Signal Detail Points
PR size 500 lines (>200) +1
Sensitive paths cmd/ +2
Test coverage Missing tests for: cmd/adapter +1

Computed by hyperfleet-risk-scorer

rh-amarin and others added 2 commits July 9, 2026 09:29
…lient

Add optional JWT bearer token authentication to the HyperFleet API HTTP
client using Kubernetes projected ServiceAccount tokens. Auth is disabled
by default so existing deployments are unaffected.

- internal/hyperfleetapi: new fileTokenSource (TTL cache, double-checked
  locking), AuthConfig in ClientConfig, tokenSource on httpClient, WithAuth
  option, Authorization header injection in doRequest
- internal/configloader: HyperfleetAPIAuthConfig alias, env var mappings
  for HYPERFLEET_API_AUTH_TOKEN_PATH / TOKEN_CACHE_TTL, auth validation in
  AdapterConfigValidator (absolute path, non-negative TTL)
- cmd/adapter/main.go: wire WithAuth in createAPIClient
- Helm chart: auth block in values.yaml (disabled by default), conditional
  projected ServiceAccountToken volume + env vars in deployment.yaml,
  tokenPath absolute-path validation in _helpers.tpl
- docs: configuration reference, deployment guide, config template

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Downgrade the hard validation error introduced in HYPERFLEET-1290 to a
deprecation warning so existing adapter configs using api_call directly
on preconditions continue to work without changes.

The precondition executor already handles api_call at runtime — the
breakage was purely in the semantic validator. Callers are now warned
at startup to migrate to the params source.api_call style.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant