-
Notifications
You must be signed in to change notification settings - Fork 23
HYPERFLEET-1480 - feat: use ServiceAccount auth scheme for API requests #264
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 |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |
| - Dashboard JSON moved from `deployments/dashboards/` to `charts/dashboards/` | ||
|
|
||
| ### Changed | ||
| - HyperFleet API authentication now uses the `ServiceAccount` Authorization scheme instead of `Bearer` | ||
|
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. If the scheme stays hard-coded this is a BREAKING CHANGE for anyone on in-app JWT, so it should be marked as one. With the knob it's an additive entry and this line is fine as is. |
||
| - BREAKING CHANGE: `tracing` moved from top-level to `monitoring.tracing` | ||
| - BREAKING CHANGE: `monitoring.serviceMonitor.additionalLabels` renamed to `monitoring.serviceMonitor.labels` | ||
| - Default tracing sampler changed from `parentbased_traceidratio` to `parentbased_always_on` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,7 +128,7 @@ config: | |
| # -- HTTP client timeout | ||
| timeout: 10s | ||
| # -- Optional JWT authentication via a Kubernetes projected service account token. | ||
| # When enabled, a projected volume is mounted and the token is sent as a Bearer | ||
| # When enabled, a projected volume is mounted and the token is sent using the ServiceAccount | ||
|
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 | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- chart values ---'
sed -n '115,145p' charts/values.yaml
printf '%s\n' '--- related chart references ---'
rg -n -C 3 'ServiceAccount|Bearer|projected|JWT|authorization' charts/values.yaml charts/README.md charts 2>/dev/null || true
printf '%s\n' '--- repository status and chart metadata ---'
sed -n '1,80p' charts/Chart.yaml
git diff -- charts/values.yaml charts/README.md charts/Chart.yamlRepository: openshift-hyperfleet/hyperfleet-sentinel Length of output: 14492 🏁 Script executed: #!/bin/bash
set -e
sed -n '100,175p' pkg/auth/jwt_handler.go
rg -n -C 3 'Authorization: Bearer|ServiceAccount|Bearer scheme|authorization header' pkg test 2>/dev/null | head -160Repository: openshift-hyperfleet/hyperfleet-api Length of output: 7248 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- auth scheme implementation ---'
rg -n -C 5 'ServiceAccount|Bearer|Authorization|tokenPath|TokenCache|auth' --glob '*.go' --glob '*.yaml' --glob '*.tpl' . | head -240
printf '%s\n' '--- deployment token wiring ---'
sed -n '118,145p' charts/templates/deployment.yamlRepository: openshift-hyperfleet/hyperfleet-sentinel Length of output: 17677 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- client files ---'
fd -t f . internal/client
printf '%s\n' '--- exact authorization construction ---'
rg -n -C 6 'ServiceAccount|SetBasicAuth|Authorization|Bearer|tokenPath|tokenPath' internal/client cmd/sentinel
printf '%s\n' '--- client implementation outline ---'
ast-grep outline internal/client --lang go 2>/dev/null | head -160 || trueRepository: openshift-hyperfleet/hyperfleet-sentinel Length of output: 13984 Keep the chart authentication contract compatible with When authentication is enabled, Sentinel sends 🤖 Prompt for AI AgentsSource: Linked repositories |
||
| # Authorization header on every API request. | ||
| 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. Following on from the client comment, this wants a |
||
| # -- Enable JWT authentication | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,9 +48,10 @@ type HyperFleetClient struct { | |
|
|
||
| // NewHyperFleetClient creates a new HyperFleet API client. | ||
| // sentinelName and version are used to build the User-Agent header sent with every request. | ||
| // tokenPath is optional; when non-empty the client reads a bearer token from that file and | ||
| // injects it as an Authorization header on every request. tokenCacheTTL controls how long | ||
| // the token is cached before the file is re-read; 0 disables caching and re-reads the file on every request. | ||
| // tokenPath is optional; when non-empty the client reads a service account token | ||
| // from that file and injects it using the ServiceAccount authorization scheme on | ||
| // every request. tokenCacheTTL controls how long the token is cached before the | ||
| // file is re-read; 0 disables caching and re-reads the file on every request. | ||
| func NewHyperFleetClient( | ||
| endpoint string, timeout time.Duration, sentinelName, version string, pageSize int32, | ||
| tokenPath string, tokenCacheTTL time.Duration, | ||
|
|
@@ -305,7 +306,7 @@ func (c *HyperFleetClient) setAuthHeader(req *http.Request) error { | |
| if err != nil { | ||
| return &TokenError{cause: err} | ||
| } | ||
| req.Header.Set("Authorization", "Bearer "+tok) | ||
| req.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 | 🏗️ Heavy lift Keep the client compatible with the current API.
🤖 Prompt for AI AgentsSource: Linked repositories
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. Heads up, hard-coding Prob better to make it a config field on scheme := c.authScheme
if scheme == "" {
scheme = "Bearer"
}
req.Header.Set("Authorization", scheme+" "+tok)Keeps the released chart backwards compatible and means the client doesn't need to know which auth boundary it's talking to. |
||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -327,7 +328,7 @@ func (c *HyperFleetClient) VerifyConnectivity(ctx context.Context, resourceType | |
| } | ||
| req.Header.Set("User-Agent", c.userAgent) | ||
| if authErr := c.setAuthHeader(req); authErr != nil { | ||
| return fmt.Errorf("bearer token unavailable: %w", authErr) | ||
| return fmt.Errorf("service account token unavailable: %w", authErr) | ||
| } | ||
|
|
||
| resp, err := c.httpClient.Do(req) | ||
|
|
||
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.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Document the authentication compatibility window.
setAuthHeadersendsServiceAccount, buthyperfleet-api/pkg/auth/jwt_handler.goaccepts onlyBearerand returns HTTP 401 for other schemes. Until the API accepts both schemes, prefix this entry withBREAKING CHANGE:and state that the API must be upgraded before Sentinel. If the rollout accepts both schemes, document that compatibility window instead; a simultaneous upgrade is not required. This changelog warning does not fix the wire-contract mismatch.🤖 Prompt for AI Agents