fix: address review comments

Signed-off-by: Swati Mukund Bagal <[email protected]>

Author: SB159

tests/model_serving/maas_billing/maas_subscription/conftest.py

@@ -49,49 +49,6 @@
 CHAT_COMPLETIONS = OpenAIEnpoints.CHAT_COMPLETIONS
 
 
-# # TEMPORARY: patches maas-controller to pr-498 to fix /v1/models 500 response.
-# # Remove once the upstream maas-controller image is fixed (Konflux onboarding complete).
-# @pytest.fixture(scope="session")
-# def maas_controller_pr498_image(
-#     admin_client: DynamicClient,
-#     maas_subscription_controller_enabled_latest: DataScienceCluster,
-# ) -> Generator[None, Any, Any]:
-#     """Patch maas-controller image to pr-498 after MaaS is enabled.
-
-#     The maas-controller:latest image has a broken /v1/models endpoint (returns 500).
-#     This fixture temporarily overrides the image after the operator creates the deployment.
-#     """
-#     deployment = Deployment(
-#         client=admin_client,
-#         name="maas-controller",
-#         namespace=MAAS_DB_NAMESPACE,
-#     )
-#     deployment.wait_for_condition(condition="Available", status="True", timeout=120)
-
-#     with ResourceEditor(
-#         patches={
-#             deployment: {
-#                 "metadata": {"annotations": {"opendatahub.io/managed": "false"}},
-#                 "spec": {
-#                     "template": {
-#                         "spec": {
-#                             "containers": [
-#                                 {
-#                                     "name": "manager",
-#                                     "image": "quay.io/opendatahub/maas-controller:pr-498",
-#                                 }
-#                             ]
-#                         }
-#                     }
-#                 },
-#             }
-#         }
-#     ):
-#         deployment.wait_for_condition(condition="Available", status="True", timeout=180)
-#         LOGGER.info("[TEMPORARY] maas-controller patched to pr-498 image")
-#         yield
-
-
 @pytest.fixture(scope="session")
 def maas_subscription_controller_enabled_latest(
     dsc_resource: DataScienceCluster,

tests/model_serving/maas_billing/maas_subscription/test_api_key_expiration.py

@@ -5,8 +5,10 @@
 
 from tests.model_serving.maas_billing.maas_subscription.utils import (
     assert_api_key_created_ok,
+    assert_api_key_get_ok,
     create_api_key,
     get_api_key,
+    revoke_api_key,
 )
 from utilities.general import generate_random_name
 from utilities.opendatahub_logger import get_logger
@@ -44,11 +46,17 @@ def test_create_key_within_expiration_limit(
             expires_in=f"{expires_in_hours}h",
             raise_on_error=False,
         )
-        assert_api_key_created_ok(create_resp, create_body, required_fields=("key", "expiresAt"))
+        assert_api_key_created_ok(resp=create_resp, body=create_body, required_fields=("key", "expiresAt"))
         LOGGER.info(
             f"[expiration] Created key within limit: expires_in={expires_in_hours}h, "
             f"expiresAt={create_body.get('expiresAt')}"
         )
+        revoke_api_key(
+            request_session_http=request_session_http,
+            base_url=base_url,
+            key_id=create_body["id"],
+            ocp_user_token=ocp_token_for_actor,
+        )
 
     @pytest.mark.tier1
     def test_create_key_at_expiration_limit(
@@ -69,11 +77,17 @@ def test_create_key_at_expiration_limit(
             expires_in=f"{expires_in_hours}h",
             raise_on_error=False,
         )
-        assert_api_key_created_ok(create_resp, create_body, required_fields=("key", "expiresAt"))
+        assert_api_key_created_ok(resp=create_resp, body=create_body, required_fields=("key", "expiresAt"))
         LOGGER.info(
             f"[expiration] Created key at limit: expires_in={expires_in_hours}h "
             f"({MAAS_API_KEY_MAX_EXPIRATION_DAYS} days)"
         )
+        revoke_api_key(
+            request_session_http=request_session_http,
+            base_url=base_url,
+            key_id=create_body["id"],
+            ocp_user_token=ocp_token_for_actor,
+        )
 
     @pytest.mark.tier1
     def test_create_key_exceeds_expiration_limit(
@@ -122,10 +136,9 @@ def test_create_key_without_expiration(
             key_id=active_api_key_id,
             ocp_user_token=ocp_token_for_actor,
         )
-        assert get_resp.status_code == 200, (
-            f"Expected 200 on GET /v1/api-keys/{active_api_key_id}, got {get_resp.status_code}: {get_resp.text[:200]}"
-        )
+        assert_api_key_get_ok(resp=get_resp, body=get_body, key_id=active_api_key_id)
         expires_at = get_body.get("expiresAt")
+        assert expires_at is None, f"Expected no 'expiresAt' for key created without expiration, got: {expires_at!r}"
         LOGGER.info(f"[expiration] Key without expiration field: expiresAt={expires_at!r}")
 
     @pytest.mark.tier2
@@ -136,17 +149,14 @@ def test_create_key_with_short_expiration(
         ocp_token_for_actor: str,
         short_expiration_api_key_id: str,
     ) -> None:
-        """Verify a key created with a 1-hour expiration has a non-null expiresAt value."""
+        """Verify a key created with a 1-hour expiration has a non-null expirationDate value."""
         get_resp, get_body = get_api_key(
             request_session_http=request_session_http,
             base_url=base_url,
             key_id=short_expiration_api_key_id,
             ocp_user_token=ocp_token_for_actor,
         )
-        assert get_resp.status_code == 200, (
-            f"Expected 200 on GET /v1/api-keys/{short_expiration_api_key_id}, "
-            f"got {get_resp.status_code}: {get_resp.text[:200]}"
-        )
+        assert_api_key_get_ok(resp=get_resp, body=get_body, key_id=short_expiration_api_key_id)
         assert get_body.get("expirationDate"), (
             f"Expected non-null 'expirationDate' for 1h key, got: {get_body.get('expirationDate')!r}"
         )

tests/model_serving/maas_billing/maas_subscription/utils.py

@@ -460,6 +460,13 @@ def assert_api_key_created_ok(
         assert field in body, f"Response must contain '{field}'"
 
 
+def assert_api_key_get_ok(resp: Response, body: dict[str, Any], key_id: str) -> None:
+    """Assert a GET /v1/api-keys/{id} response has status 200."""
+    assert resp.status_code == 200, (
+        f"Expected 200 on GET /v1/api-keys/{key_id}, got {resp.status_code}: {resp.text[:200]}"
+    )
+
+
 def get_maas_postgres_labels() -> dict[str, str]:
     return {
         "app": "postgres",