-
Notifications
You must be signed in to change notification settings - Fork 23
HYPERFLEET-1480 - feat: use ServiceAccount auth scheme for API requests #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,9 +27,9 @@ adapterConfig: | |
| baseUrl: http://hyperfleet-api:8000 | ||
| # -- API version (`HYPERFLEET_API_VERSION`) | ||
| version: v1 | ||
| # -- JWT bearer token authentication via Kubernetes projected ServiceAccount token | ||
| # -- ServiceAccount authentication via Kubernetes projected ServiceAccount token | ||
| auth: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs a |
||
| # -- Enable bearer token auth (`HYPERFLEET_API_AUTH_TOKEN_PATH`) | ||
| # -- Enable ServiceAccount token auth (`HYPERFLEET_API_AUTH_TOKEN_PATH`) | ||
| enabled: false | ||
| # -- ServiceAccount token audience (used for the projected volume) | ||
| audience: hyperfleet-api | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,8 +114,8 @@ clients: | |
| timeout: 2s | ||
| retry_attempts: 3 | ||
| retry_backoff: exponential | ||
| # Optional JWT bearer token authentication via a file (e.g. Kubernetes projected ServiceAccount token). | ||
| # When configured, the token is read from token_path and attached as Authorization: Bearer <token>. | ||
| # Optional ServiceAccount authentication via a token file (e.g. Kubernetes projected ServiceAccount token). | ||
| # When configured, the token is read from token_path and attached as Authorization: ServiceAccount <token>. | ||
|
Comment on lines
+117
to
+118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- config defaults and auth documentation ---'
sed -n '1,150p' configs/adapter-config-template.yaml
sed -n '1,70p' charts/values.yaml
sed -n '1,55p' charts/README.md
printf '%s\n' '--- token/API client definitions and callers ---'
rg -n -C 4 'token_path|tokenPath|ServiceAccount|Authorization|hyperfleetApi|http://hyperfleet-api|redirect|TLS|tls|InsecureSkipVerify' --glob '!vendor/**' --glob '!**/*_test.go' .Repository: openshift-hyperfleet/hyperfleet-adapter Length of output: 50398 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- HTTP client construction and request path ---'
sed -n '120,185p' internal/hyperfleetapi/client.go
sed -n '285,365p' internal/hyperfleetapi/client.go
printf '%s\n' '--- client types and config wiring ---'
sed -n '1,100p' internal/hyperfleetapi/types.go
rg -n -C 5 'hyperfleetapi\.New|NewClient|WithAuth|AuthConfig|HYPERFLEET_API_AUTH_TOKEN_PATH|BaseURL' --glob '*.go' --glob '!**/*_test.go' .
printf '%s\n' '--- redirect/TLS-specific transport configuration ---'
rg -n -C 3 'CheckRedirect|Transport:|http\.Transport|TLSClientConfig|InsecureSkipVerify|url\.Parse|https?://' internal cmd pkg --glob '*.go' || trueRepository: openshift-hyperfleet/hyperfleet-adapter Length of output: 50396 Sensitive Data Exposure Reachability: Internal Require encrypted transport before sending the ServiceAccount token. When 📍 Affects 3 files
🤖 Prompt for AI Agents |
||
| # token_path must be an absolute path. | ||
| # Environment variables: HYPERFLEET_API_AUTH_TOKEN_PATH, HYPERFLEET_API_AUTH_TOKEN_CACHE_TTL | ||
| # auth: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -115,7 +115,7 @@ clients: | |
| - `base_delay` (duration string): Initial retry delay. Default: `1s`. | ||
| - `max_delay` (duration string): Maximum retry delay. Default: `30s`. | ||
| - `default_headers` (map[string]string): Headers added to all API requests. | ||
| - `auth.token_path` (string): Absolute path to a file containing a JWT bearer token. When set, the token is read from this file and attached as `Authorization: Bearer <token>` on every request. Typically a Kubernetes projected ServiceAccount token. Must be an absolute path. | ||
| - `auth.token_path` (string): Absolute path to a file containing a ServiceAccount token. When set, the token is read from this file and attached as `Authorization: ServiceAccount <token>` on every request. Typically a Kubernetes projected ServiceAccount token. Must be an absolute path. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- documentation references ---'
rg -n -C 2 'Authorization: (ServiceAccount|Bearer)|token_path|TokenPath' docs internal pkg config .github 2>/dev/null || true
printf '%s\n' '--- authentication header construction ---'
rg -n -C 4 'ServiceAccount|Bearer|Authorization' internal pkg 2>/dev/null || trueRepository: openshift-hyperfleet/hyperfleet-adapter Length of output: 29051 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- JWT scheme handling ---'
rg -n -C 4 'Bearer|Authorization|invalid.credentials|scheme' pkg/auth docs/authentication.md 2>/dev/null || trueRepository: openshift-hyperfleet/hyperfleet-api Length of output: 29281 Use the The adapter sends 🧰 Tools🪛 LanguageTool[grammar] ~118-~118: Use a hyphen to join words. (QB_NEW_EN_HYPHEN) 🤖 Prompt for AI Agents |
||
| - `auth.token_cache_ttl` (duration string): How long the token is cached in memory before re-reading the file. Zero (default) means re-read on every request. | ||
|
|
||
| ### Broker (`clients.broker`) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,7 +111,7 @@ func WithBaseURL(baseURL string) ClientOption { | |
| } | ||
| } | ||
|
|
||
| // WithAuth configures JWT bearer token authentication from a file. | ||
| // WithAuth configures ServiceAccount authentication from a token file. | ||
| func WithAuth(auth *AuthConfig) ClientOption { | ||
| return func(c *httpClient) { | ||
| c.config.Auth = auth | ||
|
|
@@ -157,7 +157,7 @@ func NewClient(opts ...ClientOption) (Client, error) { | |
| } | ||
| } | ||
|
|
||
| // Initialize token source for bearer token auth if configured | ||
| // Initialize the token source for ServiceAccount auth if configured | ||
| if c.config.Auth != nil && c.config.Auth.TokenPath != "" { | ||
| c.tokenSource = newFileTokenSource(c.config.Auth.TokenPath, c.config.Auth.TokenCacheTTL) | ||
| } | ||
|
|
@@ -334,13 +334,13 @@ func (c *httpClient) doRequest(ctx context.Context, req *Request) (*Response, er | |
| httpReq.Header.Set(k, v) | ||
| } | ||
|
|
||
| // Inject bearer token auth header | ||
| // Inject the ServiceAccount auth header | ||
| if c.tokenSource != nil { | ||
| tok, authErr := c.tokenSource.get() | ||
| if authErr != nil { | ||
| return nil, fmt.Errorf("getting auth token: %w", authErr) | ||
| } | ||
| httpReq.Header.Set("Authorization", "Bearer "+tok) | ||
| httpReq.Header.Set("Authorization", "ServiceAccount "+tok) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Preserve the API authentication contract. When 🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as openshift-hyperfleet/hyperfleet-sentinel#264. The API's JWT middleware only takes scheme := c.config.Auth.Scheme
if scheme == "" {
scheme = "Bearer"
}
httpReq.Header.Set("Authorization", scheme+" "+tok)Infra sets |
||
| } | ||
|
|
||
| // Set default Content-Type for requests with body | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sits under Fixed but it's a behaviour change, and a breaking one for in-app JWT users if the scheme stays hard-coded. Move it to Changed, and mark BREAKING unless the default stays
Bearer.