Skip to content

Commit 90b3ed2

Browse files
fegejgarciao
fege
and
jgarciao
authored
Tests to increase mcp servers coverage (#1267)
* test: mcp regression test coverage increase Signed-off-by: fege <fmosca@redhat.com> * fix: add tier1 marker Signed-off-by: fege <fmosca@redhat.com> --------- Signed-off-by: fege <fmosca@redhat.com> Co-authored-by: Jorge <jgarciao@users.noreply.github.com>
1 parent 6be3cb8 commit 90b3ed2

File tree

4 files changed

+135
-20
lines changed

4 files changed

+135
-20
lines changed

‎tests/model_registry/mcp_servers/config/conftest.py‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@
33
import pytest
44
import yaml
55
from kubernetes.dynamic import DynamicClient
6-
from ocp_resources.config_map import ConfigMap
76
from ocp_resources.resource import ResourceEditor
87
from simple_logger.logger import get_logger
98

10-
from tests.model_registry.constants import DEFAULT_CUSTOM_MODEL_CATALOG
9+
from tests.model_registry.mcp_servers.config.utils import get_mcp_catalog_sources
1110
from tests.model_registry.mcp_servers.constants import (
1211
MCP_CATALOG_INVALID_SOURCE,
1312
MCP_CATALOG_SOURCE,
@@ -37,13 +36,9 @@ def mcp_multi_source_configmap_patch(
3736
Class-scoped fixture that patches the model-catalog-sources ConfigMap
3837
with two MCP catalog sources pointing to two different YAML files.
3938
"""
40-
catalog_config_map = ConfigMap(
41-
name=DEFAULT_CUSTOM_MODEL_CATALOG,
42-
client=admin_client,
43-
namespace=model_registry_namespace,
39+
catalog_config_map, current_data = get_mcp_catalog_sources(
40+
admin_client=admin_client, model_registry_namespace=model_registry_namespace
4441
)
45-
46-
current_data = yaml.safe_load(catalog_config_map.instance.data.get("sources.yaml", "{}") or "{}")
4742
if "mcp_catalogs" not in current_data:
4843
current_data["mcp_catalogs"] = []
4944
current_data["mcp_catalogs"].extend([MCP_CATALOG_SOURCE, MCP_CATALOG_SOURCE2])
@@ -80,13 +75,9 @@ def mcp_invalid_yaml_configmap_patch(
8075
Class-scoped fixture that patches the ConfigMap with a valid MCP source
8176
plus an invalid one (parameterized via request.param as the invalid YAML content).
8277
"""
83-
catalog_config_map = ConfigMap(
84-
name=DEFAULT_CUSTOM_MODEL_CATALOG,
85-
client=admin_client,
86-
namespace=model_registry_namespace,
78+
catalog_config_map, current_data = get_mcp_catalog_sources(
79+
admin_client=admin_client, model_registry_namespace=model_registry_namespace
8780
)
88-
89-
current_data = yaml.safe_load(catalog_config_map.instance.data.get("sources.yaml", "{}") or "{}")
9081
if "mcp_catalogs" not in current_data:
9182
current_data["mcp_catalogs"] = []
9283
current_data["mcp_catalogs"].extend([MCP_CATALOG_SOURCE, MCP_CATALOG_INVALID_SOURCE])
@@ -142,13 +133,9 @@ def mcp_included_excluded_configmap_patch(
142133
if "excludedServers" in filter_params:
143134
source_config["excludedServers"] = filter_params["excludedServers"]
144135

145-
catalog_config_map = ConfigMap(
146-
name=DEFAULT_CUSTOM_MODEL_CATALOG,
147-
client=admin_client,
148-
namespace=model_registry_namespace,
136+
catalog_config_map, current_data = get_mcp_catalog_sources(
137+
admin_client=admin_client, model_registry_namespace=model_registry_namespace
149138
)
150-
151-
current_data = yaml.safe_load(catalog_config_map.instance.data.get("sources.yaml", "{}") or "{}")
152139
if "mcp_catalogs" not in current_data:
153140
current_data["mcp_catalogs"] = []
154141
current_data["mcp_catalogs"].append(source_config)

‎tests/model_registry/mcp_servers/config/test_multi_source.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,22 @@
11
from typing import Any, Self
22

33
import pytest
4+
import yaml
5+
from kubernetes.dynamic import DynamicClient
6+
from ocp_resources.resource import ResourceEditor
47
from simple_logger.logger import get_logger
58

9+
from tests.model_registry.mcp_servers.config.utils import get_mcp_catalog_sources
610
from tests.model_registry.mcp_servers.constants import (
711
EXPECTED_ALL_MCP_SERVER_NAMES,
12+
EXPECTED_MCP_SERVER_NAMES,
813
EXPECTED_MCP_SOURCE_ID_MAP,
14+
MCP_CATALOG_SOURCE2_ID,
15+
)
16+
from tests.model_registry.utils import (
17+
execute_get_command,
18+
wait_for_mcp_catalog_api,
19+
wait_for_model_catalog_pod_ready_after_deletion,
920
)
1021

1122
LOGGER = get_logger(name=__name__)
@@ -34,3 +45,63 @@ def test_servers_tagged_with_correct_source_id(
3445
assert server.get("source_id") == expected_source, (
3546
f"Server '{name}' has source_id '{server.get('source_id')}', expected '{expected_source}'"
3647
)
48+
49+
@pytest.mark.parametrize(
50+
"cleanup_action",
51+
[
52+
pytest.param("disable", id="disable_source"),
53+
pytest.param("remove", id="remove_source"),
54+
],
55+
)
56+
def test_source_cleanup_removes_servers(
57+
self: Self,
58+
admin_client: DynamicClient,
59+
model_registry_namespace: str,
60+
mcp_catalog_rest_urls: list[str],
61+
model_registry_rest_headers: dict[str, str],
62+
cleanup_action: str,
63+
):
64+
"""TC-LOAD-011/012: Verify that disabling or removing a source removes its servers from the catalog."""
65+
response = execute_get_command(
66+
url=f"{mcp_catalog_rest_urls[0]}mcp_servers",
67+
headers=model_registry_rest_headers,
68+
)
69+
server_names = {server["name"] for server in response.get("items", [])}
70+
assert server_names == EXPECTED_ALL_MCP_SERVER_NAMES
71+
72+
catalog_config_map, current_data = get_mcp_catalog_sources(
73+
admin_client=admin_client, model_registry_namespace=model_registry_namespace
74+
)
75+
76+
if cleanup_action == "disable":
77+
for source in current_data.get("mcp_catalogs", []):
78+
if source["id"] == MCP_CATALOG_SOURCE2_ID:
79+
source["enabled"] = False
80+
break
81+
else:
82+
current_data["mcp_catalogs"] = [
83+
source for source in current_data.get("mcp_catalogs", []) if source["id"] != MCP_CATALOG_SOURCE2_ID
84+
]
85+
86+
patches = {"data": {"sources.yaml": yaml.dump(current_data, default_flow_style=False)}}
87+
88+
with ResourceEditor(patches={catalog_config_map: patches}):
89+
wait_for_model_catalog_pod_ready_after_deletion(
90+
client=admin_client, model_registry_namespace=model_registry_namespace
91+
)
92+
wait_for_mcp_catalog_api(url=mcp_catalog_rest_urls[0], headers=model_registry_rest_headers)
93+
94+
response = execute_get_command(
95+
url=f"{mcp_catalog_rest_urls[0]}mcp_servers",
96+
headers=model_registry_rest_headers,
97+
)
98+
remaining_names = {server["name"] for server in response.get("items", [])}
99+
assert remaining_names == EXPECTED_MCP_SERVER_NAMES, (
100+
f"Expected only source1 servers {EXPECTED_MCP_SERVER_NAMES} after {cleanup_action} of source2, "
101+
f"got {remaining_names}"
102+
)
103+
104+
wait_for_model_catalog_pod_ready_after_deletion(
105+
client=admin_client, model_registry_namespace=model_registry_namespace
106+
)
107+
wait_for_mcp_catalog_api(url=mcp_catalog_rest_urls[0], headers=model_registry_rest_headers)

‎tests/model_registry/mcp_servers/config/utils.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import yaml
2+
from kubernetes.dynamic import DynamicClient
3+
from ocp_resources.config_map import ConfigMap
4+
5+
from tests.model_registry.constants import DEFAULT_CUSTOM_MODEL_CATALOG
6+
7+
8+
def get_mcp_catalog_sources(admin_client: DynamicClient, model_registry_namespace: str) -> tuple[ConfigMap, dict]:
9+
"""Return the catalog ConfigMap and its parsed sources.yaml data."""
10+
catalog_config_map = ConfigMap(
11+
name=DEFAULT_CUSTOM_MODEL_CATALOG,
12+
client=admin_client,
13+
namespace=model_registry_namespace,
14+
)
15+
current_data = yaml.safe_load(catalog_config_map.instance.data.get("sources.yaml", "{}") or "{}")
16+
return catalog_config_map, current_data

‎tests/model_registry/test_security.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
from typing import Self
2+
3+
import pytest
4+
import requests
5+
from ocp_resources.route import Route
6+
from simple_logger.logger import get_logger
7+
8+
LOGGER = get_logger(name=__name__)
9+
10+
MCP_CATALOG_API_PATH = "/api/mcp_catalog/v1alpha1/"
11+
MODEL_CATALOG_API_PATH = "/api/model_catalog/v1alpha1/"
12+
13+
14+
@pytest.mark.tier1
15+
@pytest.mark.usefixtures("mcp_servers_configmap_patch")
16+
class TestCatalogSecurity:
17+
"""Tests for catalog endpoint security (TC-SEC-001)."""
18+
19+
@pytest.mark.parametrize(
20+
"api_path",
21+
[
22+
pytest.param(f"{MCP_CATALOG_API_PATH}mcp_servers", id="mcp_catalog_mcp_servers"),
23+
pytest.param(f"{MODEL_CATALOG_API_PATH}models", id="model_catalog_models"),
24+
pytest.param(MODEL_CATALOG_API_PATH, id="model_catalog_root"),
25+
],
26+
)
27+
def test_unauthenticated_access_denied(
28+
self: Self,
29+
model_catalog_routes: list[Route],
30+
api_path: str,
31+
):
32+
"""Verify that requests without an Authorization header are rejected."""
33+
url = f"https://{model_catalog_routes[0].instance.spec.host}:443{api_path}"
34+
response = requests.get(
35+
url=url,
36+
headers={},
37+
verify=False,
38+
timeout=60,
39+
)
40+
LOGGER.info(f"Unauthenticated response to {api_path}: {response.status_code}")
41+
assert response.status_code == 401

0 commit comments

Comments
 (0)