Merge branch 'main' into guardrails-healthcheck

Author: sheltoncyril

.pre-commit-config.yaml

@@ -36,7 +36,7 @@ repos:
         exclude: .*/__snapshots__/.*|.*-input\.json$
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.14.10
+    rev: v0.14.11
     hooks:
       - id: ruff
       - id: ruff-format

OWNERS

@@ -1,4 +1,8 @@
 approvers:
-  - <>
+  - dbasunag
+  - jgarciao
+  - lugi0
 reviewers:
-  - <>
+  - dbasunag
+  - jgarciao
+  - lugi0

tests/model_registry/model_catalog/catalog_config/test_catalog_source_merge.py

@@ -0,0 +1,78 @@
+import pytest
+from simple_logger.logger import get_logger
+
+from tests.model_registry.model_catalog.constants import REDHAT_AI_CATALOG_ID
+from tests.model_registry.utils import execute_get_command
+
+LOGGER = get_logger(name=__name__)
+
+pytestmark = [pytest.mark.usefixtures("updated_dsc_component_state_scope_session", "model_registry_namespace")]
+
+
+class TestCatalogSourceMerge:
+    """
+    Test catalog source merging behavior when the same source ID appears in both
+    default and custom ConfigMaps.
+    """
+
+    @pytest.mark.parametrize(
+        "sparse_override_catalog_source",
+        [
+            {"id": REDHAT_AI_CATALOG_ID, "field_name": "name", "field_value": "Custom Override Name"},
+            {"id": REDHAT_AI_CATALOG_ID, "field_name": "labels", "field_value": ["custom-label", "override-label"]},
+            {"id": REDHAT_AI_CATALOG_ID, "field_name": "enabled", "field_value": False},
+        ],
+        indirect=True,
+    )
+    def test_catalog_source_merge(
+        self,
+        sparse_override_catalog_source: dict,
+        model_catalog_rest_url: list[str],
+        model_registry_rest_headers: dict[str, str],
+    ):
+        """
+        RHOAIENG-41738: Test that a sparse override in custom ConfigMap successfully overrides
+        specific fields while preserving unspecified fields.
+        """
+        catalog_id = sparse_override_catalog_source["catalog_id"]
+        field_name = sparse_override_catalog_source["field_name"]
+        field_value = sparse_override_catalog_source["field_value"]
+        original_catalog = sparse_override_catalog_source["original_catalog"]
+
+        # Query sources endpoint to get the merged result
+        response = execute_get_command(url=f"{model_catalog_rest_url[0]}sources", headers=model_registry_rest_headers)
+        items = response.get("items", [])
+        LOGGER.info(f"API response items: {items}")
+
+        # Find the catalog we're testing
+        merged_catalog = next((item for item in items if item.get("id") == catalog_id), None)
+        assert merged_catalog is not None, f"Catalog '{catalog_id}' not found in sources"
+
+        # Validate the overridden field has the new value
+        assert merged_catalog.get(field_name) == field_value, (
+            f"Field '{field_name}' should have value {field_value}, got: {merged_catalog.get(field_name)}"
+        )
+
+        # Determine fields to check for preservation (exclude the overridden field)
+        fields_to_check = set(original_catalog.keys()) - {field_name}
+
+        # Special case: when enabled=False, status automatically changes to "disabled"
+        if field_name == "enabled" and field_value is False:
+            assert merged_catalog.get("status") == "disabled", (
+                f"Status should be 'disabled' when enabled=False, got: {merged_catalog.get('status')}"
+            )
+            fields_to_check.discard("status")
+
+        # Validate all other fields preserve original values
+        for orig_field in fields_to_check:
+            orig_value = original_catalog[orig_field]
+            assert merged_catalog.get(orig_field) == orig_value, (
+                f"Field '{orig_field}' should preserve original value {orig_value}, "
+                f"got: {merged_catalog.get(orig_field)}"
+            )
+
+        LOGGER.info(
+            f"Sparse override merge validated for '{catalog_id}' - "
+            f"Overridden field '{field_name}': {field_value} | "
+            f"Preserved {len(original_catalog) - 1} other fields"
+        )

tests/model_registry/model_catalog/catalog_config/test_default_model_catalog.py

@@ -15,7 +15,7 @@
 from ocp_resources.route import Route
 from ocp_resources.service import Service
 
-from tests.model_registry.constants import DEFAULT_CUSTOM_MODEL_CATALOG, DEFAULT_MODEL_CATALOG_CM
+from tests.model_registry.constants import DEFAULT_MODEL_CATALOG_CM, DEFAULT_CUSTOM_MODEL_CATALOG
 from tests.model_registry.model_catalog.constants import REDHAT_AI_CATALOG_ID, CATALOG_CONTAINER, DEFAULT_CATALOGS
 from tests.model_registry.model_catalog.catalog_config.utils import (
     validate_model_catalog_enabled,
@@ -35,7 +35,6 @@
         "model_registry_namespace",
         "original_user",
         "test_idp_user",
-        "enabled_model_catalog_config_map",
     )
 ]
 
@@ -47,7 +46,7 @@ class TestModelCatalogGeneral:
         [
             pytest.param(
                 {"configmap_name": DEFAULT_CUSTOM_MODEL_CATALOG},
-                2,
+                0,
                 False,
                 id="test_model_catalog_sources_configmap_install",
                 marks=pytest.mark.install,
@@ -168,7 +167,6 @@ class TestModelCatalogDefault:
     def test_model_catalog_default_catalog_sources(
         self,
         pytestconfig: pytest.Config,
-        enabled_model_catalog_config_map: ConfigMap,
         test_idp_user: UserTestSession,
         model_catalog_rest_url: list[str],
         user_token_for_api_calls: str,
@@ -204,7 +202,6 @@ def test_model_catalog_default_catalog_sources(
 
     def test_model_default_catalog_get_models_by_source(
         self: Self,
-        enabled_model_catalog_config_map: ConfigMap,
         model_catalog_rest_url: list[str],
         randomly_picked_model_from_catalog_api_by_source: tuple[dict[Any, Any], str, str],
     ):
@@ -217,7 +214,6 @@ def test_model_default_catalog_get_models_by_source(
 
     def test_model_default_catalog_get_model_by_name(
         self: Self,
-        enabled_model_catalog_config_map: ConfigMap,
         model_catalog_rest_url: list[str],
         user_token_for_api_calls: str,
         randomly_picked_model_from_catalog_api_by_source: tuple[dict[Any, Any], str, str],
@@ -235,7 +231,6 @@ def test_model_default_catalog_get_model_by_name(
 
     def test_model_default_catalog_get_model_artifact(
         self: Self,
-        enabled_model_catalog_config_map: ConfigMap,
         model_catalog_rest_url: list[str],
         user_token_for_api_calls: str,
         randomly_picked_model_from_catalog_api_by_source: tuple[dict[Any, Any], str, str],
@@ -261,7 +256,6 @@ class TestModelCatalogDefaultData:
 
     def test_model_default_catalog_number_of_models(
         self: Self,
-        enabled_model_catalog_config_map: ConfigMap,
         default_catalog_api_response: dict[Any, Any],
         default_model_catalog_yaml_content: dict[Any, Any],
     ):
@@ -278,7 +272,6 @@ def test_model_default_catalog_number_of_models(
 
     def test_model_default_catalog_correspondence_of_model_name(
         self: Self,
-        enabled_model_catalog_config_map: ConfigMap,
         default_catalog_api_response: dict[Any, Any],
         default_model_catalog_yaml_content: dict[Any, Any],
         catalog_openapi_schema: dict[Any, Any],
@@ -343,7 +336,6 @@ def test_model_default_catalog_correspondence_of_model_name(
 
     def test_model_default_catalog_random_artifact(
         self: Self,
-        enabled_model_catalog_config_map: ConfigMap,
         default_model_catalog_yaml_content: dict[Any, Any],
         model_catalog_rest_url: list[str],
         model_registry_rest_headers: dict[str, str],

tests/model_registry/model_catalog/catalog_config/test_model_catalog_negative.py

@@ -83,7 +83,7 @@ def test_modify_default_catalog_configmap_reconciles(
     type: hf
     enabled: true
 """,
-                "includedModels cannot be empty for HuggingFace catalog",
+                "includedModels cannot be empty for Hugging Face catalog",
                 id="test_hf_source_no_include_model",
             ),
             pytest.param(
@@ -94,11 +94,67 @@ def test_modify_default_catalog_configmap_reconciles(
     type: hf
     enabled: true
     includedModels:
-    - ibm-granite/*
+    - abc-random*
 """,
-                "HuggingFace API returned status 401",
-                id="test_hf_source_unauthorized",
-                marks=pytest.mark.xfail(reason="RHOAIENG-42213 is causing this failure"),
+                'failed to expand model patterns: wildcard pattern "abc-random*" is not supported - '
+                "Hugging Face requires a specific organization",
+                id="test_hf_source_invalid_organization",
+            ),
+            pytest.param(
+                """
+catalogs:
+  - name: HuggingFace Hub
+    id: error_catalog
+    type: hf
+    enabled: true
+    includedModels:
+    - '*'
+""",
+                'failed to expand model patterns: wildcard pattern "*" is not supported - '
+                "Hugging Face requires a specific organization",
+                id="test_hf_source_global_wildcard",
+            ),
+            pytest.param(
+                """
+catalogs:
+  - name: HuggingFace Hub
+    id: error_catalog
+    type: hf
+    enabled: true
+    includedModels:
+    - '*/*'
+""",
+                'failed to expand model patterns: wildcard pattern "*/*" is not supported - '
+                "Hugging Face requires a specific organization",
+                id="test_hf_source_global_org_wildcard",
+            ),
+            pytest.param(
+                """
+catalogs:
+  - name: HuggingFace Hub
+    id: error_catalog
+    type: hf
+    enabled: true
+    includedModels:
+    - 'RedHatAI/'
+""",
+                'failed to expand model patterns: wildcard pattern "RedHatAI/" is not supported - '
+                "Hugging Face requires a specific organization",
+                id="test_hf_source_empty_model_name",
+            ),
+            pytest.param(
+                """
+catalogs:
+  - name: HuggingFace Hub
+    id: error_catalog
+    type: hf
+    enabled: true
+    includedModels:
+    - '*RedHatAI*'
+""",
+                'failed to expand model patterns: wildcard pattern "*RedHatAI*" is not supported - '
+                "Hugging Face requires a specific organization",
+                id="test_hf_source_multiple_wildcards",
             ),
         ],
         indirect=["updated_catalog_config_map_scope_function"],

tests/model_registry/model_catalog/conftest.py

@@ -18,10 +18,9 @@
     REDHAT_AI_CATALOG_ID,
     DEFAULT_CATALOGS,
 )
-from tests.model_registry.model_catalog.utils import get_models_from_catalog_api, get_catalog_url_and_headers
+from tests.model_registry.model_catalog.utils import get_models_from_catalog_api
 from tests.model_registry.constants import (
     CUSTOM_CATALOG_ID1,
-    DEFAULT_MODEL_CATALOG_CM,
     DEFAULT_CUSTOM_MODEL_CATALOG,
 )
 from tests.model_registry.utils import (
@@ -39,55 +38,63 @@
 LOGGER = get_logger(name=__name__)
 
 
-@pytest.fixture(scope="session")
-def enabled_model_catalog_config_map(
-    admin_client: DynamicClient,
+@pytest.fixture()
+def sparse_override_catalog_source(
+    request: pytest.FixtureRequest,
+    admin_client,
     model_registry_namespace: str,
-    current_client_token: str,
-) -> Generator[ConfigMap, None, None]:
+    model_catalog_rest_url: list[str],
+    model_registry_rest_headers: dict[str, str],
+) -> Generator[dict, None, None]:
     """
-    Enable all catalogs in the default model catalog configmap
+    Creates a sparse override for an existing default catalog source.
+
+    Requires parameterization via request.param dict containing:
+    - "id": catalog ID to override (required)
+    - "field_name": name of the field to override (required)
+    - "field_value": value for the field (required)
     """
-    # Get operator-managed default sources ConfigMap
-    default_sources_cm = ConfigMap(
-        name=DEFAULT_MODEL_CATALOG_CM, client=admin_client, namespace=model_registry_namespace, ensure_exists=True
+    # Get fields from pytest param
+    param = getattr(request, "param", None)
+    assert param, "sparse_override_catalog_source requires request.param dict"
+
+    catalog_id = param["id"]
+    field_name = param["field_name"]
+    field_value = param["field_value"]
+
+    # Capture CURRENT catalog state from API before applying sparse override
+    response = execute_get_command(url=f"{model_catalog_rest_url[0]}sources", headers=model_registry_rest_headers)
+    items = response.get("items", [])
+    original_catalog = next((item for item in items if item.get("id") == catalog_id), None)
+    assert original_catalog is not None, f"Original catalog '{catalog_id}' not found in sources"
+
+    # Create sparse override YAML with only id and the field to override
+    catalog_override = {"id": catalog_id, field_name: field_value}
+    sparse_catalog_yaml = yaml.dump(
+        {"catalogs": [catalog_override]},
+        default_flow_style=False,
     )
 
-    # Get the sources.yaml content from default sources
-    default_sources_yaml = default_sources_cm.instance.data.get("sources.yaml", "")
-
-    # Parse the YAML and extract only catalogs, enabling each one
-    parsed_yaml = yaml.safe_load(default_sources_yaml)
-    if not parsed_yaml or "catalogs" not in parsed_yaml:
-        raise RuntimeError("No catalogs found in default sources ConfigMap")
-
-    for catalog in parsed_yaml["catalogs"]:
-        catalog["enabled"] = True
-    enabled_yaml_dict = {"catalogs": parsed_yaml["catalogs"]}
-    enabled_sources_yaml = yaml.dump(enabled_yaml_dict, default_flow_style=False, sort_keys=False)
-
-    LOGGER.info("Adding enabled catalogs to model-catalog-sources ConfigMap")
-
-    # Get user-managed sources ConfigMap
-    user_sources_cm = ConfigMap(
-        name=DEFAULT_CUSTOM_MODEL_CATALOG, client=admin_client, namespace=model_registry_namespace, ensure_exists=True
+    # Write sparse override to custom ConfigMap
+    sources_cm = ConfigMap(
+        name=DEFAULT_CUSTOM_MODEL_CATALOG,
+        client=admin_client,
+        namespace=model_registry_namespace,
     )
+    patches = {"data": {"sources.yaml": sparse_catalog_yaml}}
 
-    patches = {"data": {"sources.yaml": enabled_sources_yaml}}
-
-    with ResourceEditor(patches={user_sources_cm: patches}):
-        # Wait for the model catalog pod to be ready
+    with ResourceEditor(patches={sources_cm: patches}):
         is_model_catalog_ready(client=admin_client, model_registry_namespace=model_registry_namespace)
+        wait_for_model_catalog_api(url=model_catalog_rest_url[0], headers=model_registry_rest_headers)
+        yield {
+            "catalog_id": catalog_id,
+            "field_name": field_name,
+            "field_value": field_value,
+            "original_catalog": original_catalog,
+        }
 
-        # Get the model catalog URL and headers and wait for the API to be fully ready
-        catalog_url, headers = get_catalog_url_and_headers(
-            admin_client=admin_client,
-            model_registry_namespace=model_registry_namespace,
-            token=current_client_token,
-        )
-        wait_for_model_catalog_api(url=catalog_url, headers=headers)
-
-        yield user_sources_cm
+    is_model_catalog_ready(client=admin_client, model_registry_namespace=model_registry_namespace)
+    wait_for_model_catalog_api(url=model_catalog_rest_url[0], headers=model_registry_rest_headers)
 
 
 @pytest.fixture(scope="class")

tests/model_registry/model_catalog/constants.py

@@ -25,12 +25,14 @@
         "type": "yaml",
         "properties": {"yamlCatalogPath": "/shared-data/models-catalog.yaml"},
         "labels": [REDHAT_AI_CATALOG_NAME],
+        "enabled": True,
     },
     "redhat_ai_validated_models": {
         "name": REDHAT_AI_VALIDATED_CATALOG_NAME,
         "type": "yaml",
         "properties": {"yamlCatalogPath": "/shared-data/validated-models-catalog.yaml"},
         "labels": [REDHAT_AI_VALIDATED_CATALOG_NAME],
+        "enabled": True,
     },
 }
 REDHAT_AI_CATALOG_ID: str = "redhat_ai_models"