Skip to content

CWFHEALTH-4934: add trusted-proxy authentication module#1887

Merged
tisnik merged 1 commit into
lightspeed-core:mainfrom
willianrampazzo:trusted_proxy_auth_module
Jun 10, 2026
Merged

CWFHEALTH-4934: add trusted-proxy authentication module#1887
tisnik merged 1 commit into
lightspeed-core:mainfrom
willianrampazzo:trusted_proxy_auth_module

Conversation

@willianrampazzo

@willianrampazzo willianrampazzo commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Implements the trusted-proxy authentication module for lightspeed-stack, enabling the Konflux UI proxy to authenticate requests on behalf of end users without forwarding their JWT tokens between services.

Description

In the Konflux architecture, the UI proxy sits between the user's browser and lightspeed-stack. The original approach would have required passing the user's JWT token from the proxy to lightspeed-stack, which is undesirable as it exposes user credentials to internal services and couples lightspeed-stack to the UI's identity provider. This was raised during the architecture design review of konflux-lightspeed (section 4.2 - Authentication Flow).

How it works

The trusted-proxy module separates proxy authentication from user identification:

  1. Proxy authentication: The UI proxy authenticates itself by sending its Kubernetes ServiceAccount token in the Authorization: Bearer header. lightspeed-stack validates this token via the Kubernetes TokenReview API.
  2. User identification: The end user's identity is forwarded in a configurable HTTP header (default: X-Forwarded-User). No user JWT or credentials are passed or stored.
  3. SA allowlist (optional): An allowlist of ServiceAccount identities can restrict which proxies are permitted to forward requests, preventing unauthorized services from impersonating the proxy.
  4. Health probe and metrics skip: When configured, unauthenticated requests to /readiness, /liveness, and /metrics are allowed through, matching the behavior of existing auth modules.

Configuration

authentication:
  module: trusted-proxy
  skip_for_health_probes: true
  skip_for_metrics: true
  trusted_proxy_config:
    user_header: "X-Forwarded-User"
    allowed_service_accounts:
      - namespace: "konflux-lightspeed"
        name: "ui-proxy"

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
  • Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  1. Deploy lightspeed-stack with trusted-proxy auth on a kind cluster running Konflux:
authentication:
  module: trusted-proxy
  skip_for_health_probes: true
  skip_for_metrics: true
  trusted_proxy_config:
    user_header: "X-Forwarded-User"
    allowed_service_accounts:
      - namespace: "konflux-lightspeed"
        name: "test-proxy"
  1. Create a test-proxy ServiceAccount and grant tokenreview-creator ClusterRole to the lightspeed-stack SA.

  2. Create a test pod using the test-proxy SA and run the following tests against the lightspeed-stack pod:

  • Health probe without auth: curl http://<pod-ip>:8080/readiness = 200

  • Liveness probe without auth: curl http://<pod-ip>:8080/liveness = 200

  • Valid proxy token + user header on readiness: curl -H "Authorization: Bearer $TOKEN" -H "X-Forwarded-User: user@test" http://<pod-ip>:8080/readiness = 200

  • Valid proxy token + user header on query: curl -X POST -H "Authorization: Bearer $TOKEN" -H "X-Forwarded-User: user@test" -H "Content-Type: application/json" -d '{"query":"hello"}' http://<pod-ip>:8080/v1/query = 200

  • No auth header on query: curl -X POST -H "Content-Type: application/json" -d '{"query":"hello"}' http://<pod-ip>:8080/v1/query = 401

  • Valid token, missing user header: curl -X POST -H "Authorization: Bearer $TOKEN" -H "Content-Type: application/json" -d '{"query":"hello"}' http://<pod-ip>:8080/v1/query = 401

  • Invalid token: curl -X POST -H "Authorization: Bearer faketoken" -H "X-Forwarded-User: user@test" -H "Content-Type: application/json" -d '{"query":"hello"}' http://<pod-ip>:8080/v1/query = 401

  • SA not in allowlist: Use a token from a different SA (e.g., lightspeed-stack) = 403

  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • New Features
    • Added trusted-proxy authentication for Kubernetes proxy scenarios: service-account validation, optional allowlisting, and configurable forwarded-user header.
    • Optional bypass of auth for health probes and metrics when enabled.
  • Authorization
    • Trusted-proxy flows reuse existing authorization resolver path (access-rule enforcement).
  • Documentation
    • OpenAPI updated with trusted-proxy configuration schemas.
  • Tests
    • Added comprehensive unit tests for trusted-proxy behavior and resolver selection.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@willianrampazzo, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 13 minutes and 19 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c45fe86d-83d5-4343-bece-a55521bda903

📥 Commits

Reviewing files that changed from the base of the PR and between 7f2a7df and 091a76d.

📒 Files selected for processing (10)
  • docs/openapi.json
  • src/authentication/__init__.py
  • src/authentication/trusted_proxy.py
  • src/authorization/middleware.py
  • src/constants.py
  • src/models/config.py
  • tests/unit/authentication/test_auth.py
  • tests/unit/authentication/test_trusted_proxy.py
  • tests/unit/authorization/test_middleware.py
  • tests/unit/models/config/test_dump_configuration.py

Walkthrough

This PR introduces a new trusted-proxy authentication module that enables authentication for requests forwarded by Kubernetes proxies. The implementation validates proxy service accounts, optionally enforces an allowlist, extracts forwarded user identity from headers, and bypasses authentication for health-probe and metrics endpoints. The feature integrates into the existing authentication dispatch and authorization systems with comprehensive test coverage.

Changes

Trusted-Proxy Authentication Module

Layer / File(s) Summary
Constants and configuration models
src/constants.py, src/models/config.py
New AUTH_MOD_TRUSTED_PROXY constant and TrustedProxyConfiguration, TrustedProxyServiceAccount models added to AuthenticationConfiguration with validation and accessor property.
Trusted-proxy authentication dependency
src/authentication/trusted_proxy.py
TrustedProxyAuthDependency performs token validation, optional service-account allowlist enforcement, forwarded-user header extraction, and bypass for health probes/metrics endpoints.
Module dispatch and wiring
src/authentication/__init__.py
Integrated into get_auth_dependency() by importing the module and adding a match case for AUTH_MOD_TRUSTED_PROXY that constructs the dependency from configuration.
Authorization resolver integration
src/authorization/middleware.py
Updated resolver selection to treat trusted-proxy the same as rh-identity, using access rules with noop roles resolver.
OpenAPI schema
docs/openapi.json
Added nullable trusted_proxy_config property and component schemas TrustedProxyConfiguration and TrustedProxyServiceAccount.
Comprehensive test coverage
tests/unit/authentication/test_auth.py, tests/unit/authentication/test_trusted_proxy.py, tests/unit/authorization/test_middleware.py, tests/unit/models/config/test_dump_configuration.py
Unit tests covering token validation, allowlist enforcement, header extraction, endpoint bypass paths, custom headers, dispatch integration, authorization resolver behavior, and updated configuration dump expectations.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers:

  • are-ces
  • tisnik
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a new trusted-proxy authentication module, which is the primary objective of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🤖 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 `@src/authentication/trusted_proxy.py`:
- Around line 80-82: Validate the Kubernetes TokenReview response before blind
casting: check that user_info.user is not None and is an instance of
kubernetes.client.V1UserInfo, and that user.username exists and is a non-empty
str before assigning sa_username; if any check fails, handle it (return an
unauthorized error/raise a clear exception or log and abort) instead of
proceeding with the cast. Use the existing variables user_info, user and
sa_username to locate the spot to add these guards and ensure the subsequent
logic only runs when the validated types/values are present.
- Around line 63-69: Restructure the Authorization-bypass logic so
health/metrics bypasses are checked explicitly before attempting to extract a
token: check request.url.path against the health paths
("/readiness","/liveness") and the metrics path ("/metrics") when
configuration.authentication_configuration.skip_for_health_probes or
skip_for_metrics are enabled and return NO_AUTH_TUPLE for those matches; if the
Authorization header is missing and no bypass matched, return an explicit
auth-failure (instead of falling through to extract_user_token); keep references
to request.headers.get("Authorization"),
configuration.authentication_configuration.skip_for_health_probes,
configuration.authentication_configuration.skip_for_metrics, request.url.path,
extract_user_token and NO_AUTH_TUPLE while harmonizing the metrics check to use
the single string "/metrics".
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e620ce8a-64f5-463e-ba2e-d9bf9084b0f4

📥 Commits

Reviewing files that changed from the base of the PR and between 82f6ca4 and 18a3025.

📒 Files selected for processing (9)
  • src/authentication/__init__.py
  • src/authentication/trusted_proxy.py
  • src/authorization/middleware.py
  • src/constants.py
  • src/models/config.py
  • tests/unit/authentication/test_auth.py
  • tests/unit/authentication/test_trusted_proxy.py
  • tests/unit/authorization/test_middleware.py
  • tests/unit/models/config/test_dump_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: E2E: library mode / ci / group 2
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Llama Stack imports: Use from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Use async def for I/O operations and external API calls
Use standard log levels with clear purposes: debug() for diagnostic info, info() for program execution, warning() for unexpected events, error() for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes: Configuration, Error/Exception, Resolver, Interface
Abstract classes must use ABC with @abstractmethod decorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes

Files:

  • src/authentication/trusted_proxy.py
  • src/constants.py
  • src/authorization/middleware.py
  • src/models/config.py
  • src/authentication/__init__.py
src/constants.py

📄 CodeRabbit inference engine (AGENTS.md)

Use constants.py for shared constants with descriptive comments and type hints using Final[type]

Files:

  • src/constants.py
tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Use pytest.mark.asyncio marker for async tests

Files:

  • tests/unit/authentication/test_auth.py
  • tests/unit/authorization/test_middleware.py
  • tests/unit/models/config/test_dump_configuration.py
  • tests/unit/authentication/test_trusted_proxy.py
src/models/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Pydantic models must use @model_validator and @field_validator for validation and complete type annotations for all attributes, avoiding Any type

Files:

  • src/models/config.py
src/**/__init__.py

📄 CodeRabbit inference engine (AGENTS.md)

Package __init__.py files must contain brief package descriptions

Files:

  • src/authentication/__init__.py
🧠 Learnings (2)
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.

Applied to files:

  • src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.

Applied to files:

  • src/models/config.py
🔇 Additional comments (12)
src/constants.py (1)

122-122: LGTM!

Also applies to: 132-132

src/models/config.py (4)

1268-1297: LGTM!


1321-1321: LGTM!


1370-1376: LGTM!


1430-1448: LGTM!

tests/unit/authentication/test_trusted_proxy.py (1)

1-424: LGTM!

tests/unit/models/config/test_dump_configuration.py (1)

157-157: LGTM!

Also applies to: 367-367, 721-721, 974-974, 1207-1207, 1430-1430, 1803-1803, 2022-2022, 2241-2241, 2467-2467

src/authorization/middleware.py (1)

82-91: LGTM!

tests/unit/authorization/test_middleware.py (2)

102-105: LGTM!


188-207: LGTM!

src/authentication/__init__.py (1)

86-90: Update: None access is already prevented for trusted-proxy auth.

AuthenticationConfiguration has a @model_validator(mode="after") (check_authentication_model) that raises ValueError when module == constants.AUTH_MOD_TRUSTED_PROXY and trusted_proxy_config is None; additionally, the trusted_proxy_configuration property raises if trusted_proxy_config is None. The trusted_proxy_configuration access in the AUTH_MOD_TRUSTED_PROXY branch is therefore guarded.

tests/unit/authentication/test_auth.py (1)

44-52: Reassess attribute-name mismatch: trusted_proxy_configuration is a property alias for trusted_proxy_config.

AuthenticationConfiguration defines the underlying field trusted_proxy_config, and its trusted_proxy_configuration property returns self.trusted_proxy_config (and errors only if that field is None). get_auth_dependency() uses trusted_proxy_configuration, while the test sets trusted_proxy_config, so the attribute naming is consistent and no AttributeError should occur.

			> Likely an incorrect or invalid review comment.

Comment thread src/authentication/trusted_proxy.py
Comment thread src/authentication/trusted_proxy.py
@willianrampazzo willianrampazzo force-pushed the trusted_proxy_auth_module branch 2 times, most recently from 17375d7 to 7f2a7df Compare June 10, 2026 17:17

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/unit/authorization/test_middleware.py (1)

93-123: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clear resolver cache per parameterized iteration.

get_authorization_resolvers() is cached, so this parameterized test can reuse the first computed resolvers and skip exercising later auth-module branches (including trusted-proxy). Add a cache clear at test start.

Suggested fix
 def test_noop_auth_modules(
     self,
     mocker: MockerFixture,
     mock_configuration: MockType,
     auth_module: str,
     expected_types: tuple[AccessResolver, AccessResolver],
 ) -> None:
     """Test resolver selection for noop-style authentication modules."""
+    get_authorization_resolvers.cache_clear()
     mock_configuration.authentication_configuration.module = auth_module
     mocker.patch("authorization.middleware.configuration", mock_configuration)
🤖 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 `@tests/unit/authorization/test_middleware.py` around lines 93 - 123, The
parameterized test reuses cached results from get_authorization_resolvers(), so
add an explicit cache clear at the start of test_noop_auth_modules to force
recomputation per iteration: call get_authorization_resolvers.cache_clear() (or
the appropriate cache-clear method if a different caching decorator is used)
right after setting up mock_configuration and before invoking
get_authorization_resolvers() in the test to ensure each auth_module branch
(including trusted-proxy) is exercised.
🤖 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 `@docs/openapi.json`:
- Around line 11617-11626: The OpenAPI artifact is stale; regenerate and commit
the updated schema so docs/openapi.json matches the code. Run the provided
generation script (uv run scripts/generate_openapi_schema.py docs/openapi.json)
to regenerate the file, verify that the "trusted_proxy_config" schema and
surrounding ranges (around lines 19770-19820) are updated, and commit the
regenerated docs/openapi.json to the PR.

In `@src/authentication/trusted_proxy.py`:
- Around line 109-114: The header value read into forwarded_user can be
only-whitespace and bypass the falsy check; update the handler that reads
request.headers.get(self.config.user_header) to strip() the value, re-check
emptiness, and raise the same UnauthorizedResponse/HTTPException if the stripped
value is empty, then use the stripped value when creating the AuthTuple (or
wherever forwarded_user is used as user id/username) so downstream services
never receive a whitespace-only principal.
- Around line 116-120: The debug log currently writes the forwarded end-user
identifier (forwarded_user) in logger.debug inside trusted_proxy.py; change it
to avoid persisting PII by logging only the proxy service account (sa_username)
and whether a forwarded header was present (e.g., boolean or redacted/hash), or
replace forwarded_user with a non-identifying placeholder or hashed value;
update the logger.debug call that references sa_username and forwarded_user to
remove the raw forwarded_user and instead log sa_username and a redacted/hashed
indicator or header_present flag.
- Around line 73-74: In trusted_proxy.py inside the async __call__ flow, avoid
calling the synchronous get_user_info(token) directly; offload the TokenReview
work by running get_user_info in a threadpool (e.g., use run_in_threadpool or an
equivalent executor) so the AuthenticationV1Api.create_token_review call inside
src/authentication/k8s.get_user_info does not block the event loop. Also tighten
the forwarded_user empty check by treating whitespace-only strings as missing
(use .strip() semantics) and stop logging the raw forwarded end-user identifier
in debug logs—either omit it or log a masked/hashed version instead when
referencing forwarded_user. Ensure you reference
extract_user_token(request.headers) for token extraction and replace direct
get_user_info(token) calls with the threadpool-wrapped call.

In `@src/models/config.py`:
- Around line 1291-1296: The allowed_service_accounts field currently treats an
explicit empty list the same as None, so add a Pydantic validation to reject []
(or convert it to an explicit deny-all behavior) by implementing a validator on
allowed_service_accounts in the config model using `@field_validator` or a
`@model_validator`; specifically, in the class that defines
allowed_service_accounts, create a validator that raises a ValueError when the
value is an empty list (or alternately maps [] to a distinct sentinel that
TrustedProxyAuthDependency recognizes as deny-all), and ensure
TrustedProxyAuthDependency then enforces the allowlist only when the value is a
non-empty list (or treats the deny-all sentinel as denying all) so explicit []
does not silently open access.

In `@tests/unit/authentication/test_trusted_proxy.py`:
- Around line 124-127: The test creates a token-review mock via
mock_get_user_info and sets status.user = None but doesn't set
status.authenticated; update the fixture setup used in the test (the mocked
return value from authentication.trusted_proxy.get_user_info, i.e., the status
mock) to explicitly set status.authenticated = True so downstream code that
checks "is True" sees an authenticated token while still having user=None;
modify the mock_get_user_info.return_value (status) to include
authenticated=True alongside user=None.

---

Outside diff comments:
In `@tests/unit/authorization/test_middleware.py`:
- Around line 93-123: The parameterized test reuses cached results from
get_authorization_resolvers(), so add an explicit cache clear at the start of
test_noop_auth_modules to force recomputation per iteration: call
get_authorization_resolvers.cache_clear() (or the appropriate cache-clear method
if a different caching decorator is used) right after setting up
mock_configuration and before invoking get_authorization_resolvers() in the test
to ensure each auth_module branch (including trusted-proxy) is exercised.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d68942d4-23f4-4cde-9db9-1c7d7e176ade

📥 Commits

Reviewing files that changed from the base of the PR and between 18a3025 and 7f2a7df.

📒 Files selected for processing (10)
  • docs/openapi.json
  • src/authentication/__init__.py
  • src/authentication/trusted_proxy.py
  • src/authorization/middleware.py
  • src/constants.py
  • src/models/config.py
  • tests/unit/authentication/test_auth.py
  • tests/unit/authentication/test_trusted_proxy.py
  • tests/unit/authorization/test_middleware.py
  • tests/unit/models/config/test_dump_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Pylinter
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: build-pr
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Llama Stack imports: Use from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Use async def for I/O operations and external API calls
Use standard log levels with clear purposes: debug() for diagnostic info, info() for program execution, warning() for unexpected events, error() for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes: Configuration, Error/Exception, Resolver, Interface
Abstract classes must use ABC with @abstractmethod decorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes

Files:

  • src/authorization/middleware.py
  • src/constants.py
  • src/authentication/__init__.py
  • src/authentication/trusted_proxy.py
  • src/models/config.py
src/constants.py

📄 CodeRabbit inference engine (AGENTS.md)

Use constants.py for shared constants with descriptive comments and type hints using Final[type]

Files:

  • src/constants.py
tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Use pytest.mark.asyncio marker for async tests

Files:

  • tests/unit/authentication/test_auth.py
  • tests/unit/models/config/test_dump_configuration.py
  • tests/unit/authorization/test_middleware.py
  • tests/unit/authentication/test_trusted_proxy.py
src/**/__init__.py

📄 CodeRabbit inference engine (AGENTS.md)

Package __init__.py files must contain brief package descriptions

Files:

  • src/authentication/__init__.py
src/models/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Pydantic models must use @model_validator and @field_validator for validation and complete type annotations for all attributes, avoiding Any type

Files:

  • src/models/config.py
🧠 Learnings (2)
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.

Applied to files:

  • src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.

Applied to files:

  • src/models/config.py
🪛 GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt
docs/openapi.json

[error] 1-1: OpenAPI schema mismatch. Step failed because 'docs/openapi.json is out of date' (diff -u between docs/openapi.json and regenerated schema). Regenerate with: 'uv run scripts/generate_openapi_schema.py docs/openapi.json'.

🪛 GitHub Actions: OpenAPI (Spectral) / spectral
docs/openapi.json

[error] 1-1: CI check failed: docs/openapi.json is out of date versus the generated schema. Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json

🔇 Additional comments (6)
tests/unit/authentication/test_trusted_proxy.py (1)

1-123: LGTM!

Also applies to: 129-479

src/authentication/__init__.py (1)

13-13: LGTM!

Also applies to: 22-23, 86-90

src/authorization/middleware.py (1)

82-91: LGTM!

tests/unit/authentication/test_auth.py (1)

3-9: LGTM!

Also applies to: 11-17, 44-52

tests/unit/authorization/test_middleware.py (1)

188-208: LGTM!

tests/unit/models/config/test_dump_configuration.py (1)

157-157: LGTM!

Also applies to: 367-367, 721-721, 974-974, 1207-1207, 1430-1430, 1803-1803, 2022-2022, 2241-2241, 2467-2467

Comment thread docs/openapi.json
Comment thread src/authentication/trusted_proxy.py
Comment thread src/authentication/trusted_proxy.py Outdated
Comment thread src/authentication/trusted_proxy.py
Comment thread src/models/config.py
Comment thread tests/unit/authentication/test_trusted_proxy.py
@willianrampazzo willianrampazzo force-pushed the trusted_proxy_auth_module branch from 7f2a7df to 2dbaf0d Compare June 10, 2026 17:55
Add a new auth module that validates requests forwarded by a trusted
Kubernetes proxy. The proxy authenticates itself via a ServiceAccount
token (validated through TokenReview), then the end user's identity is
extracted from a configurable HTTP header (default: X-Forwarded-User).

Key behaviors:
- Optional SA allowlist restricts which proxies may forward requests
- Health probe and metrics endpoints can skip auth when configured
- Authorization resolvers use the same pattern as rh-identity (NoopRoles
  + GenericAccess when access rules are defined)

Signed-off-by: Willian Rampazzo <willianr@redhat.com>
@willianrampazzo willianrampazzo force-pushed the trusted_proxy_auth_module branch from 2dbaf0d to 091a76d Compare June 10, 2026 18:03

@tisnik tisnik left a comment

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.

LGTM

@tisnik tisnik merged commit ad85ff7 into lightspeed-core:main Jun 10, 2026
26 of 28 checks passed
@willianrampazzo willianrampazzo deleted the trusted_proxy_auth_module branch June 10, 2026 18:48
@coderabbitai coderabbitai Bot mentioned this pull request Jun 11, 2026
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants