HYPERFLEET-1303 - feat: add JWT authentication E2E test suite - #151
Open
kuudori wants to merge 2 commits into
Open
HYPERFLEET-1303 - feat: add JWT authentication E2E test suite#151kuudori wants to merge 2 commits into
kuudori wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds JWT authentication helpers and E2E suites covering enforcement, issuer validation, identity propagation, token renewal, adapter reporting, and upgrade-path configuration. Adds an RFC9457 error matcher with unit tests, registers the auth suite, adds the JWT dependency and Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AuthE2E
participant HyperfleetAPI
participant Sentinel
participant Adapter
AuthE2E->>HyPerfleetAPI: Create or patch cluster with JWT
HyperfleetAPI->>Sentinel: Publish authenticated event
Sentinel->>Adapter: Request reconciliation
Adapter-->>HyPerfleetAPI: Report status with JWT
HyperfleetAPI-->>AuthE2E: Return reconciliation and status results
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@e2e/auth/helpers.go`:
- Around line 48-57: Update craftExpiredJWT and the corresponding
unconfigured-issuer token helper to sign with the configured trusted test
issuer/private key or dedicated test JWKS key, rather than a random key. Keep
the signing setup identical to the valid-token path and vary only the claim
being tested, preserving the expired exp and alternate iss values so middleware
reaches those validations.
In `@e2e/auth/multi_issuer.go`:
- Around line 123-135: The adapter status verification currently checks only
CreatedTime, which does not validate JWT identity propagation. In the status
loop within the multi-issuer authentication scenario, assert that the relevant
audit identity field populated from the adapter’s JWT is non-empty for every
status; only rename the scenario if authenticated reporting, rather than
identity propagation, is the intended contract.
- Around line 89-93: Update the alternate-service-account setup around
CreateToken in the multi-issuer test so the account is provisioned during test
setup and capability-disabled environments are the only reason to skip. Replace
the broad ginkgo.Skip handling for token acquisition errors with a test failure
for unexpected TokenRequest, RBAC, or authentication errors, while preserving
the existing successful altToken path.
In `@e2e/auth/token_renewal.go`:
- Around line 23-27: Update the token-renewal test flow around the initial and
second reconciliation cycles to force a refresh boundary before triggering the
second cycle. Configure a bounded, short-lived credential, then wait until it
expires or observe projected-volume rotation before proceeding, so the second
cycle must authenticate with a renewed token; apply the same behavior to the
additional renewal scenarios.
In `@e2e/auth/upgrade_path.go`:
- Around line 45-64: Update the step 4 validation in the upgrade-path test
around CreateClusterFromPayload so it explicitly verifies that jwk_cert_ca_file
is loaded and used for JWKS certificate validation, rather than treating the
successful authenticated request as sufficient. Exercise a CA-dependent JWKS
endpoint whose certificate is trusted only by the configured CA, or directly
assert the deployed configuration, while preserving the existing authentication
and reconciliation checks.
In `@pkg/helper/matchers.go`:
- Around line 248-255: Update rfc9457ErrorMatcher.Match to explicitly reject a
typed-nil *http.Response after the type assertion and before accessing
resp.Body, returning the existing matcher-error style instead of panicking. Add
a test case covering (*http.Response)(nil) and verify it returns an error.
🪄 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: ebe34f40-dc5c-4c03-a945-121d8a1d9dac
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum,!**/go.sum
📒 Files selected for processing (12)
README.mddocs/runbook.mde2e/auth/helpers.goe2e/auth/jwt_enforcement.goe2e/auth/multi_issuer.goe2e/auth/token_renewal.goe2e/auth/upgrade_path.goe2e/e2e.gogo.modpkg/helper/matchers.gopkg/helper/matchers_test.gopkg/labels/labels.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)
Add new e2e/auth/ test suite validating JWT authentication enforcement, multi-issuer identity, token renewal, and upgrade path configuration. New files: - e2e/auth/helpers.go: RequestEditorFn helpers (withoutAuth, withBearerToken) and JWT crafting utilities for negative auth tests - e2e/auth/jwt_enforcement.go: Tier0 tests for unauthenticated, invalid signature, expired, and malformed token rejection - e2e/auth/multi_issuer.go: Tier0 tests for audit field population, unconfigured issuer rejection, multi-SA acceptance, adapter identity - e2e/auth/token_renewal.go: Tier1 tests for Sentinel and Adapter token refresh across reconciliation cycles - e2e/auth/upgrade_path.go: Tier0 test validating all 4 JWT upgrade config items in a single end-to-end flow Supporting changes: - pkg/helper/matchers.go: HaveRFC9457Error matcher for RFC 9457 Problem Details validation with mime.ParseMediaType content-type checking - pkg/helper/matchers_test.go: unit tests for HaveRFC9457Error - pkg/labels/labels.go: Auth label for test filtering - e2e/e2e.go: register auth suite via blank import - go.mod/go.sum: add golang-jwt/jwt/v5 dependency
… to pkg/client - Replace openapi.RequestEditorFn with rawRequest() for auth enforcement - Replace openapi.ResourceConditionStatusTrue with client.ResourceConditionStatusTrue - Replace openapi.ProblemDetails with inline struct in HaveRFC9457Error matcher - Use HaveValue(Equal()) for *string field assertions - Remove unused strPtr from matchers_test.go - Update README and runbook to remove stale openapi references
kuudori
force-pushed
the
feat/HYPERFLEET-1303-jwt-e2e-validation
branch
from
July 30, 2026 16:03
0aaacb5 to
1b5ab2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@pkg/helper/k8s.go`:
- Around line 194-196: Wrap the errors returned by EnsureServiceAccount and
DeleteServiceAccount in their respective helper methods instead of using bare
return err, preserving the underlying error as the cause and adding operation
context per ERR-04.
🪄 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: 1ffcd0bd-25c5-4681-b3d8-1a6cf3fb0216
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum,!**/go.sum
📒 Files selected for processing (14)
README.mddocs/runbook.mde2e/auth/helpers.goe2e/auth/jwt_enforcement.goe2e/auth/multi_issuer.goe2e/auth/token_renewal.goe2e/auth/upgrade_path.goe2e/e2e.gogo.modpkg/client/kubernetes/client.gopkg/helper/k8s.gopkg/helper/matchers.gopkg/helper/matchers_test.gopkg/labels/labels.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)
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/labels/labels.go
- docs/runbook.md
- README.md
Comment on lines
+194
to
+196
| if err := h.K8sClient.EnsureServiceAccount(ctx, namespace, name); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Bare return err violates the Error Model Standard (ERR-04).
Both EnsureServiceAccount and DeleteServiceAccount propagate the underlying error unwrapped.
🔧 Proposed fix
if err := h.K8sClient.EnsureServiceAccount(ctx, namespace, name); err != nil {
- return err
+ return fmt.Errorf("ensure service account %s/%s: %w", namespace, name, err)
} if err := h.K8sClient.DeleteServiceAccount(ctx, namespace, name); err != nil {
- return err
+ return fmt.Errorf("delete service account %s/%s: %w", namespace, name, err)
}As per path instructions, "Wrap errors per Error Model Standard — no bare return err."
Also applies to: 206-208
🤖 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 `@pkg/helper/k8s.go` around lines 194 - 196, Wrap the errors returned by
EnsureServiceAccount and DeleteServiceAccount in their respective helper methods
instead of using bare return err, preserving the underlying error as the cause
and adding operation context per ERR-04.
Source: Path instructions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)