Skip to content

Commit 7b5dc66

Browse files
committed
fix: add skip_for_metrics option to bypass auth on /metrics endpoint
The /metrics endpoint returns 401 when using rh-identity or k8s auth modules because only /readiness and /liveness are covered by skip_for_health_probes. Prometheus and other scrapers hit /metrics without auth headers, causing noisy logs and failed scrapes. Add a new skip_for_metrics config option (default: false) to AuthenticationConfiguration that allows skipping auth on /metrics, following the same pattern as skip_for_health_probes. RSPEED-2934 Signed-off-by: Major Hayden <major@redhat.com>
1 parent 094c904 commit 7b5dc66

6 files changed

Lines changed: 229 additions & 2 deletions

File tree

src/authentication/k8s.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,10 @@ async def __call__(self, request: Request) -> tuple[str, str, bool, str]:
441441
if configuration.authentication_configuration.skip_for_health_probes:
442442
if request.url.path in ("/readiness", "/liveness"):
443443
return NO_AUTH_TUPLE
444+
# Skip auth for metrics endpoint when configured
445+
if configuration.authentication_configuration.skip_for_metrics:
446+
if request.url.path in ("/metrics",):
447+
return NO_AUTH_TUPLE
444448

445449
token = extract_user_token(request.headers)
446450
user_info = get_user_info(token)

src/authentication/rh_identity.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,10 @@ async def __call__(self, request: Request) -> AuthTuple:
307307
if request.url.path.endswith(("/readiness", "/liveness")):
308308
if configuration.authentication_configuration.skip_for_health_probes:
309309
return NO_AUTH_TUPLE
310+
# Skip auth for metrics endpoint when configured
311+
if request.url.path.endswith("/metrics"):
312+
if configuration.authentication_configuration.skip_for_metrics:
313+
return NO_AUTH_TUPLE
310314
logger.warning("Missing x-rh-identity header")
311315
raise HTTPException(status_code=401, detail="Missing x-rh-identity header")
312316

src/models/config.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1181,6 +1181,11 @@ class AuthenticationConfiguration(ConfigurationBase):
11811181
title="Skip authorization for probes",
11821182
description="Skip authorization for readiness and liveness probes",
11831183
)
1184+
skip_for_metrics: bool = Field(
1185+
False,
1186+
title="Skip authorization for metrics",
1187+
description="Skip authorization for the /metrics endpoint",
1188+
)
11841189
k8s_cluster_api: Optional[AnyHttpUrl] = None
11851190
k8s_ca_cert_path: Optional[FilePath] = None
11861191
jwk_config: Optional[JwkConfiguration] = None
@@ -1873,7 +1878,8 @@ class Configuration(ConfigurationBase):
18731878

18741879
authentication: AuthenticationConfiguration = Field(
18751880
default_factory=lambda: AuthenticationConfiguration(
1876-
skip_for_health_probes=False
1881+
skip_for_health_probes=False,
1882+
skip_for_metrics=False,
18771883
),
18781884
title="Authentication configuration",
18791885
description="Authentication configuration",

tests/unit/authentication/test_k8s.py

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""Unit tests for authentication/k8s module."""
22

3-
# pylint: disable=too-many-arguments,too-many-positional-arguments,too-few-public-methods,protected-access
3+
# pylint: disable=too-many-arguments,too-many-positional-arguments,too-few-public-methods,protected-access,too-many-lines
44

55
import os
66
from http import HTTPStatus
@@ -481,6 +481,143 @@ async def test_auth_dependency_no_token_normal_endpoints(
481481
assert detail["cause"] == "No Authorization header found"
482482

483483

484+
@pytest.mark.asyncio
485+
async def test_auth_dependency_no_token_metrics_endpoint_skip_enabled(
486+
mocker: MockerFixture,
487+
) -> None:
488+
"""Test the auth dependency without a token for the metrics endpoint.
489+
490+
When skip_for_metrics is True, the /metrics endpoint should bypass auth.
491+
"""
492+
config_dict = {
493+
"name": "test",
494+
"service": {
495+
"host": "localhost",
496+
"port": 8080,
497+
"auth_enabled": False,
498+
"workers": 1,
499+
"color_log": True,
500+
"access_log": True,
501+
},
502+
"llama_stack": {
503+
"api_key": "test-key",
504+
"url": "http://test.com:1234",
505+
"use_as_library_client": False,
506+
},
507+
"authentication": {
508+
"module": "k8s",
509+
"skip_for_metrics": True,
510+
},
511+
"user_data_collection": {
512+
"feedback_enabled": False,
513+
"feedback_storage": ".",
514+
"transcripts_enabled": False,
515+
"transcripts_storage": ".",
516+
},
517+
}
518+
cfg = AppConfig()
519+
cfg.init_from_dict(config_dict)
520+
mocker.patch("authentication.k8s.configuration", cfg)
521+
522+
dependency = K8SAuthDependency()
523+
524+
# Mock the Kubernetes API calls
525+
mock_authn_api = mocker.patch("authentication.k8s.K8sClientSingleton.get_authn_api")
526+
mock_authz_api = mocker.patch("authentication.k8s.K8sClientSingleton.get_authz_api")
527+
528+
# Setup mock responses for invalid token
529+
mock_authn_api.return_value.create_token_review.return_value = MockK8sResponse(
530+
authenticated=False
531+
)
532+
mock_authz_api.return_value.create_subject_access_review.return_value = (
533+
MockK8sResponse(allowed=False)
534+
)
535+
536+
request = Request(
537+
scope={
538+
"type": "http",
539+
"headers": [],
540+
"path": "/metrics",
541+
}
542+
)
543+
544+
user_uid, username, skip_userid_check, token = await dependency(request)
545+
546+
assert user_uid == "00000000-0000-0000-0000-000"
547+
assert username == "lightspeed-user"
548+
assert skip_userid_check is True
549+
assert token == ""
550+
551+
552+
@pytest.mark.asyncio
553+
async def test_auth_dependency_no_token_metrics_endpoint_skip_disabled(
554+
mocker: MockerFixture,
555+
) -> None:
556+
"""Test the auth dependency without a token for the metrics endpoint.
557+
558+
When skip_for_metrics is False, the /metrics endpoint should require auth.
559+
"""
560+
config_dict = {
561+
"name": "test",
562+
"service": {
563+
"host": "localhost",
564+
"port": 8080,
565+
"auth_enabled": False,
566+
"workers": 1,
567+
"color_log": True,
568+
"access_log": True,
569+
},
570+
"llama_stack": {
571+
"api_key": "test-key",
572+
"url": "http://test.com:1234",
573+
"use_as_library_client": False,
574+
},
575+
"authentication": {
576+
"module": "k8s",
577+
"skip_for_metrics": False,
578+
},
579+
"user_data_collection": {
580+
"feedback_enabled": False,
581+
"feedback_storage": ".",
582+
"transcripts_enabled": False,
583+
"transcripts_storage": ".",
584+
},
585+
}
586+
cfg = AppConfig()
587+
cfg.init_from_dict(config_dict)
588+
mocker.patch("authentication.k8s.configuration", cfg)
589+
590+
dependency = K8SAuthDependency()
591+
592+
# Mock the Kubernetes API calls
593+
mock_authn_api = mocker.patch("authentication.k8s.K8sClientSingleton.get_authn_api")
594+
mock_authz_api = mocker.patch("authentication.k8s.K8sClientSingleton.get_authz_api")
595+
596+
# Setup mock responses for invalid token
597+
mock_authn_api.return_value.create_token_review.return_value = MockK8sResponse(
598+
authenticated=False
599+
)
600+
mock_authz_api.return_value.create_subject_access_review.return_value = (
601+
MockK8sResponse(allowed=False)
602+
)
603+
604+
request = Request(
605+
scope={
606+
"type": "http",
607+
"headers": [],
608+
"path": "/metrics",
609+
}
610+
)
611+
612+
with pytest.raises(HTTPException) as exc_info:
613+
await dependency(request)
614+
615+
assert exc_info.value.status_code == 401
616+
detail = cast(dict[str, str], exc_info.value.detail)
617+
assert detail["response"] == ("Missing or invalid credentials provided by client")
618+
assert detail["cause"] == "No Authorization header found"
619+
620+
484621
@pytest.mark.asyncio
485622
async def test_cluster_id_is_used_for_kube_admin(mocker: MockerFixture) -> None:
486623
"""Test the cluster id is used as user_id when user is kube:admin."""

tests/unit/authentication/test_rh_identity.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,80 @@ async def test_non_probe_paths_require_auth_when_skip_enabled(
565565
assert exc_info.value.status_code == 401
566566

567567

568+
class TestRHIdentityMetricsSkip:
569+
"""Test suite for metrics endpoint skip functionality in RH Identity auth."""
570+
571+
@staticmethod
572+
def _mock_configuration(mocker: MockerFixture, skip_for_metrics: bool) -> None:
573+
"""Patch the configuration singleton with a mock for metrics skip tests.
574+
575+
Parameters:
576+
skip_for_metrics (bool): Value to assign to the mocked
577+
configuration's authentication_configuration.skip_for_metrics flag.
578+
"""
579+
mock_config = mocker.MagicMock()
580+
mock_config.authentication_configuration.skip_for_metrics = skip_for_metrics
581+
# Ensure health probe skip is disabled so it doesn't interfere
582+
mock_config.authentication_configuration.skip_for_health_probes = False
583+
mocker.patch("authentication.rh_identity.configuration", mock_config)
584+
585+
@pytest.mark.asyncio
586+
@pytest.mark.parametrize(
587+
"path",
588+
[
589+
"/metrics",
590+
"/api/lightspeed/metrics",
591+
],
592+
)
593+
async def test_metrics_path_skips_auth_when_enabled(
594+
self, mocker: MockerFixture, path: str
595+
) -> None:
596+
"""Test metrics paths bypass auth when skip_for_metrics is True."""
597+
self._mock_configuration(mocker, skip_for_metrics=True)
598+
599+
auth_dep = RHIdentityAuthDependency()
600+
request = Request(scope={"type": "http", "headers": [], "path": path})
601+
602+
result = await auth_dep(request)
603+
assert result == NO_AUTH_TUPLE
604+
605+
@pytest.mark.asyncio
606+
@pytest.mark.parametrize(
607+
"path",
608+
[
609+
"/metrics",
610+
"/api/lightspeed/metrics",
611+
],
612+
)
613+
async def test_metrics_path_requires_auth_when_disabled(
614+
self, mocker: MockerFixture, path: str
615+
) -> None:
616+
"""Test metrics paths still require auth when skip_for_metrics is False."""
617+
self._mock_configuration(mocker, skip_for_metrics=False)
618+
619+
auth_dep = RHIdentityAuthDependency()
620+
request = Request(scope={"type": "http", "headers": [], "path": path})
621+
622+
with pytest.raises(HTTPException) as exc_info:
623+
await auth_dep(request)
624+
assert exc_info.value.status_code == 401
625+
626+
@pytest.mark.asyncio
627+
@pytest.mark.parametrize("path", ["/", "/v1/query"])
628+
async def test_non_metrics_paths_require_auth_when_skip_enabled(
629+
self, mocker: MockerFixture, path: str
630+
) -> None:
631+
"""Test non-metrics paths still require auth even when skip_for_metrics is True."""
632+
self._mock_configuration(mocker, skip_for_metrics=True)
633+
634+
auth_dep = RHIdentityAuthDependency()
635+
request = Request(scope={"type": "http", "headers": [], "path": path})
636+
637+
with pytest.raises(HTTPException) as exc_info:
638+
await auth_dep(request)
639+
assert exc_info.value.status_code == 401
640+
641+
568642
class TestRHIdentityHeaderSizeLimit:
569643
"""Test suite for x-rh-identity header size limit enforcement."""
570644

tests/unit/models/config/test_authentication_configuration.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,15 @@ def test_authentication_configuration() -> None:
3232
module=AUTH_MOD_NOOP,
3333
skip_tls_verification=False,
3434
skip_for_health_probes=False,
35+
skip_for_metrics=False,
3536
k8s_ca_cert_path=None,
3637
k8s_cluster_api=None,
3738
)
3839
assert auth_config is not None
3940
assert auth_config.module == AUTH_MOD_NOOP
4041
assert auth_config.skip_tls_verification is False
4142
assert auth_config.skip_for_health_probes is False
43+
assert auth_config.skip_for_metrics is False
4244
assert auth_config.k8s_ca_cert_path is None
4345
assert auth_config.k8s_cluster_api is None
4446
assert auth_config.rh_identity_config is None

0 commit comments

Comments
 (0)