Skip to content

Commit 143b105

Browse files
committed
LCORE-511: Fix type checking issues in k8s authentication module
Fix type checking errors in `src/authentication/k8s.py` caused by incomplete type annotations in the Kubernetes Python client library. The Kubernetes Python client library (`kubernetes-client`) has incomplete type annotations for many of its model classes and API responses. This PR adds targeted type ignore comments to suppress false positives while maintaining type safety for the rest of the codebase. Changes: - Added explicit type conversions for `k8s_config.host` and `k8s_config.ssl_ca_cert` to ensure proper string types - Added `# type: ignore` directives for Kubernetes client objects that lack proper type stubs: - `V1TokenReview.status` and `.user` attributes - `V1SubjectAccessReview.status.allowed` attribute - Custom object API responses (dict-like objects with dynamic structure) - Added type assertions to help Pyright understand singleton initialization pattern Note: There is no functional changes to authentication logic being introduced in this PR, code behaviour remains identical. Signed-off-by: Anik Bhattacharjee <anbhatta@redhat.com>
1 parent 3350016 commit 143b105

2 files changed

Lines changed: 31 additions & 25 deletions

File tree

pyproject.toml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,6 @@ dependencies = [
7474

7575
[tool.pyright]
7676
exclude = [
77-
# TODO(lucasagomes): This module was copied from road-core
78-
# service/ols/src/auth/k8s.py and currently has 58 Pyright issues. It
79-
# might need to be rewritten down the line.
80-
"src/authentication/k8s.py",
8177
# Agent API v1 endpoints - deprecated API but still supported
8278
# Type errors due to llama-stack-client not exposing Agent API types
8379
"src/app/endpoints/conversations.py",

src/authentication/k8s.py

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,19 @@ def __new__(cls: type[Self]) -> Self:
8080
ce,
8181
)
8282

83-
k8s_config.host = (
84-
configuration.authentication_configuration.k8s_cluster_api
85-
or k8s_config.host
86-
)
83+
k8s_api_url = configuration.authentication_configuration.k8s_cluster_api
84+
if k8s_api_url:
85+
k8s_config.host = str(k8s_api_url)
8786
k8s_config.verify_ssl = (
8887
not configuration.authentication_configuration.skip_tls_verification
8988
)
90-
k8s_config.ssl_ca_cert = (
89+
ca_cert_path = (
9190
configuration.authentication_configuration.k8s_ca_cert_path
92-
if configuration.authentication_configuration.k8s_ca_cert_path
93-
not in {None, Path()}
94-
else k8s_config.ssl_ca_cert
9591
)
92+
if ca_cert_path and ca_cert_path != Path():
93+
# Kubernetes client library has incomplete type stubs for ssl_ca_cert
94+
k8s_config.ssl_ca_cert = str(ca_cert_path) # type: ignore[assignment]
95+
# else keep the default k8s_config.ssl_ca_cert
9696
api_client = kubernetes.client.ApiClient(k8s_config)
9797
cls._api_client = api_client
9898
cls._custom_objects_api = kubernetes.client.CustomObjectsApi(api_client)
@@ -101,7 +101,10 @@ def __new__(cls: type[Self]) -> Self:
101101
except Exception as e:
102102
logger.info("Failed to initialize Kubernetes client: %s", e)
103103
raise
104-
return cls._instance
104+
# At this point _instance is guaranteed to be initialized
105+
# but we need this anyway for type checker
106+
assert cls._instance is not None
107+
return cls._instance # type: ignore[return-value]
105108

106109
@classmethod
107110
def get_authn_api(cls) -> kubernetes.client.AuthenticationV1Api:
@@ -159,20 +162,24 @@ def _get_cluster_id(cls) -> str:
159162
ClusterIDUnavailableError: If the cluster ID cannot be obtained due
160163
to missing keys, an API error, or any unexpected error.
161164
"""
165+
version_data = None
162166
try:
163167
custom_objects_api = cls.get_custom_objects_api()
164168
version_data = custom_objects_api.get_cluster_custom_object(
165169
"config.openshift.io", "v1", "clusterversions", "version"
166170
)
167-
cluster_id = version_data["spec"]["clusterID"]
171+
# Type assertion: we know this returns a dict-like object
172+
assert isinstance(version_data, dict), "version_data must be a dict"
173+
# Kubernetes client library returns untyped dict-like objects
174+
cluster_id = str(version_data["spec"]["clusterID"]) # type: ignore[index]
168175
cls._cluster_id = cluster_id
169176
return cluster_id
170177
except KeyError as e:
171178
logger.error(
172179
"Failed to get cluster_id from cluster, missing keys in version object"
173180
)
174181
raise ClusterIDUnavailableError("Failed to get cluster ID") from e
175-
except TypeError as e:
182+
except (TypeError, AssertionError) as e:
176183
logger.error(
177184
"Failed to get cluster_id, version object is: %s", version_data
178185
)
@@ -239,8 +246,9 @@ def get_user_info(token: str) -> Optional[kubernetes.client.V1TokenReview]:
239246
)
240247
try:
241248
response = auth_api.create_token_review(token_review)
242-
if response.status.authenticated:
243-
return response.status
249+
# Kubernetes client library has incomplete type stubs
250+
if response.status.authenticated: # type: ignore[union-attr]
251+
return response.status # type: ignore[union-attr]
244252
return None
245253
except Exception as e: # pylint: disable=broad-exception-caught
246254
logger.error("API exception during TokenReview: %s", e)
@@ -307,9 +315,10 @@ async def __call__(self, request: Request) -> tuple[str, str, bool, str]:
307315
response = UnauthorizedResponse(cause="Invalid or expired Kubernetes token")
308316
raise HTTPException(**response.model_dump())
309317

310-
if user_info.user.username == "kube:admin":
318+
# Kubernetes client library has incomplete type stubs
319+
if user_info.user.username == "kube:admin": # type: ignore[union-attr]
311320
try:
312-
user_info.user.uid = K8sClientSingleton.get_cluster_id()
321+
user_info.user.uid = K8sClientSingleton.get_cluster_id() # type: ignore[union-attr]
313322
except ClusterIDUnavailableError as e:
314323
logger.error("Failed to get cluster ID: %s", e)
315324
response = InternalServerErrorResponse(
@@ -322,8 +331,8 @@ async def __call__(self, request: Request) -> tuple[str, str, bool, str]:
322331
authorization_api = K8sClientSingleton.get_authz_api()
323332
sar = kubernetes.client.V1SubjectAccessReview(
324333
spec=kubernetes.client.V1SubjectAccessReviewSpec(
325-
user=user_info.user.username,
326-
groups=user_info.user.groups,
334+
user=user_info.user.username, # type: ignore[union-attr]
335+
groups=user_info.user.groups, # type: ignore[union-attr]
327336
non_resource_attributes=kubernetes.client.V1NonResourceAttributes(
328337
path=self.virtual_path, verb="get"
329338
),
@@ -339,13 +348,14 @@ async def __call__(self, request: Request) -> tuple[str, str, bool, str]:
339348
)
340349
raise HTTPException(**response.model_dump()) from e
341350

342-
if not response.status.allowed:
343-
response = ForbiddenResponse.endpoint(user_id=user_info.user.uid)
351+
# Kubernetes client library has incomplete type stubs
352+
if not response.status.allowed: # type: ignore[union-attr]
353+
response = ForbiddenResponse.endpoint(user_id=user_info.user.uid) # type: ignore[union-attr]
344354
raise HTTPException(**response.model_dump())
345355

346356
return (
347-
user_info.user.uid,
348-
user_info.user.username,
357+
user_info.user.uid, # type: ignore[union-attr]
358+
user_info.user.username, # type: ignore[union-attr]
349359
self.skip_userid_check,
350360
token,
351361
)

0 commit comments

Comments
 (0)