test: label endpoint with asset type

[fege]

Pull Request

Summary

Related Issues

• Fixes:
• JIRA: RHOAIENG-52379

How it has been tested

• Locally
• Jenkins

Additional Requirements

• If this PR introduces a new test image, did you create a PR to mirror it in disconnected environment?
• If this PR introduces new marker(s)/adds a new component, was relevant ticket created to update relevant Jenkins job?

Summary by CodeRabbit

• Tests
  • Tests updated to inject multiple test labels and validate label handling more accurately.
  • Label retrieval and verification now distinguish asset types (MCP servers vs. models) and validate each set separately.
  • Improved label comparison logic to produce clearer, consolidated assertion results when labels are missing or unexpected.

[github-actions]

The following are automatically added/executed:

• PR size label.
• Run pre-commit
• Run tox
• Add PR author as the PR assignee
• Build image based on the PR

Available user actions:

• To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
• To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
• To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
  lgtm label removed on each new commit push.
• To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
  verified label removed on each new commit push.
• To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
  and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
• To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
  pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.

Supported labels

{'/build-push-pr-image', '/cherry-pick', '/verified', '/wip', '/lgtm', '/hold'}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 91c37e7a-c466-4430-9c2e-85ad7ee00788

📥 Commits

Reviewing files that changed from the base of the PR and between 9967e41 and c36084a.

📒 Files selected for processing (2)

• tests/model_registry/model_catalog/metadata/test_labels_endpoint.py
• tests/model_registry/model_catalog/metadata/utils.py

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/model_registry/model_catalog/metadata/utils.py
• tests/model_registry/model_catalog/metadata/test_labels_endpoint.py

---

📝 Walkthrough

Walkthrough

Tests updated to handle MCP server labels separately from model labels: fixture now injects two labels into the ConfigMap; a utility gains an optional asset_type filter for API requests and improved label comparison; the test verifies model labels and MCP labels via separate API calls.

Changes

Cohort / File(s)	Summary
Test fixtures tests/model_registry/model_catalog/conftest.py	Fixture changed to add two labels (test-dynamic, mcp-test-label) to ConfigMap labels array instead of appending one.
Test utilities tests/model_registry/model_catalog/metadata/utils.py	get_labels_from_api now accepts optional `asset_type: Literal["models","mcp_servers"]
Test logic tests/model_registry/model_catalog/metadata/test_labels_endpoint.py	Test reworked to fetch ConfigMap labels once, split into MCP vs. model labels, and verify default /labels returns only model labels while an assetType=mcp_servers request returns only MCP labels.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Security note (actionable): tests now inject labels into a ConfigMap without explicit validation of label fields — verify production code validating label names/values to avoid configuration injection or malformed metadata (CWE-20: Improper Input Validation).

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The Summary section is empty/incomplete; the PR description fails to explain why these test changes are needed or what problem they address.	Fill in the Summary section with a clear explanation of the test changes, their purpose, and why they are needed.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'test: label endpoint with asset type' accurately summarizes the main change—adding test coverage for the label endpoint with asset type filtering.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Set the reviews.auto_title_instructions setting to generate a title for your PR based on the changes in the PR with custom instructions.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/model_registry/model_catalog/conftest.py`:
- Around line 372-387: The fixture unconditionally extends
current_data["labels"] with new_labels which can introduce duplicates; update
the patching logic in this fixture to deduplicate before appending by comparing
each label's unique key (use tuple (label.get("name"), label.get("assetType"))),
build a set of existing keys from current_data["labels"], filter new_labels to
only those whose key is not present, and then extend current_data["labels"] with
the filtered list so duplicates are avoided.

In `@tests/model_registry/model_catalog/metadata/test_labels_endpoint.py`:
- Around line 66-76: The test currently only asserts that expected labels are
present but not that unexpected labels are absent; update the assertions around
get_labels_from_api and verify_labels_match to enforce asset-type isolation by
(1) asserting that none of mcp_expected_labels appear in api_labels (default
without asset_type) and (2) asserting that none of model_expected_labels appear
in mcp_api_labels (when asset_type="mcp_servers"), using the existing
model_expected_labels and mcp_expected_labels lists to drive these negative
checks; keep using get_labels_from_api and verify_labels_match for positive
checks but add explicit absence assertions (e.g., ensuring no label with
assetType == "mcp_servers" exists in api_labels and no label with assetType !=
"mcp_servers" exists in mcp_api_labels).

In `@tests/model_registry/model_catalog/metadata/utils.py`:
- Around line 462-463: Reject unsupported asset_type values before calling
execute_get_command: validate the asset_type argument against an allowlist
(e.g., the expected asset types used in tests), and if asset_type is not None
and not in that allowlist, raise an explicit error (ValueError or
AssertionError) to fail fast; then construct params as before (params =
{"assetType": asset_type} if asset_type is not None else None) and call
execute_get_command(url=url, headers=headers, params=params). This validation
should be added near the use of asset_type/params in the same helper
(referencing the asset_type variable and execute_get_command) so typos like
"mcp_server" are caught before the request is issued.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 63b329a5-7a5e-44e8-b903-28150c33c27c

📥 Commits

Reviewing files that changed from the base of the PR and between 6237646 and 9967e41.

📒 Files selected for processing (3)

• tests/model_registry/model_catalog/conftest.py
• tests/model_registry/model_catalog/metadata/test_labels_endpoint.py
• tests/model_registry/model_catalog/metadata/utils.py

[github-actions]

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.