Remove grpc as required param in local mr resource

[dbasunag]

Add local resource to remove grpc as required param and add kube_rbac_proxy option.
Update test to not pass grpc ={} in MR creation call

Description

How Has This Been Tested?

Merge criteria:

• The commits are squashed in a cohesive manner and have meaningful messages.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

• Refactor

  • Consolidated ModelRegistry into a single utilities location and added an updated resource with streamlined configuration handling and validation.
  • Standardized ModelRegistry construction and usage across the codebase.

• Tests

  • Updated tests to reference the consolidated implementation.
  • Improved test reliability and removed an obsolete log warning.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[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

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaces test imports to utilities.resources.model_registry_modelregistry_opendatahub_io, adds a new ModelRegistry resource class under utilities/resources, removes grpc (and an istio=None) constructor argument from some usages, and removes one log warning in a test fixture. No other API signature changes.

Changes

Cohort / File(s)	Summary
Import path migration (tests) tests/model_registry/async_job/conftest.py, tests/model_registry/negative_tests/test_model_registry_creation_negative.py, tests/model_registry/rbac/conftest.py, tests/model_registry/rbac/test_mr_rbac.py, tests/model_registry/rest_api/test_model_registry_rest_api.py, tests/model_registry/rest_api/test_model_registry_secure_db.py, tests/model_registry/rest_api/test_multiple_mr.py, tests/model_registry/upgrade/model_registry/test_model_registry_upgrade.py	Updated imports to use utilities.resources.model_registry_modelregistry_opendatahub_io.ModelRegistry instead of ocp_resources.model_registry_modelregistry_opendatahub_io. No other behavioral changes.
Fixture adjustment (log removal) tests/model_registry/conftest.py	Removed a LOGGER.warning("Requested Oauth Proxy configuration:") call from the model_registry_instance fixture.
Constructor argument removals tests/model_registry/rest_api/conftest.py, tests/model_registry/utils.py	Removed the grpc keyword argument from ModelRegistry(...) constructor calls; also removed an istio=None argument in tests/model_registry/rest_api/conftest.py. rest argument remains.
New resource implementation utilities/resources/model_registry_modelregistry_opendatahub_io.py	Added ModelRegistry class (subclass of NamespacedResource) with api_group attribute, parameterized __init__ accepting config sections (downgrade_db_schema_version, enable_database_upgrade, grpc, kube_rbac_proxy, mysql, oauth_proxy, postgres, rest, **kwargs) and a to_dict() that validates presence of rest and serializes provided sections into spec.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review utilities/resources/model_registry_modelregistry_opendatahub_io.py: subclassing, ApiGroup constant, constructor typing, and to_dict() validation/serialization.
• Verify tests that previously passed grpc (or istio) still behave correctly after argument removal.
• Confirm the changed import paths resolve correctly in test environment and there are no duplicate/conflicting definitions.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing grpc as a required parameter in the local ModelRegistry resource, which aligns with the changeset's primary objective.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ae03bd and 0cde7b3.

📒 Files selected for processing (1)

• tests/model_registry/utils.py (1 hunks)

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

• tests/model_registry/utils.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/model_registry/rest_api/conftest.py (1)

143-167: Consider dropping grpc={} (and possibly istio=None) here

Now that the local ModelRegistry wrapper treats grpc as optional, you can likely remove grpc={} from this call (and istio=None if not used anywhere downstream) for consistency with other creation sites and the “no required gRPC” goal.

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

62-75: ModelRegistry wrapper looks solid; verify behavior of grpc=None serialization

The wrapper correctly makes grpc optional and wires kube_rbac_proxy into kubeRBACProxy, while still enforcing rest as required. One nuance: to_dict always sets _spec["grpc"] = self.grpc, so callers that omit grpc will send grpc: null in the spec. If the CRD expects this field to be truly absent when not used, consider guarding it like the other optional sections:

-            _spec["grpc"] = self.grpc
+            if self.grpc is not None:
+                _spec["grpc"] = self.grpc

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c29e38 and 2c99501.

📒 Files selected for processing (12)

• tests/model_registry/async_job/conftest.py (1 hunks)
• tests/model_registry/conftest.py (2 hunks)
• tests/model_registry/negative_tests/test_model_registry_creation_negative.py (1 hunks)
• tests/model_registry/rbac/conftest.py (1 hunks)
• tests/model_registry/rbac/test_mr_rbac.py (1 hunks)
• tests/model_registry/rest_api/conftest.py (1 hunks)
• tests/model_registry/rest_api/test_model_registry_rest_api.py (1 hunks)
• tests/model_registry/rest_api/test_model_registry_secure_db.py (1 hunks)
• tests/model_registry/rest_api/test_multiple_mr.py (1 hunks)
• tests/model_registry/upgrade/model_registry/test_model_registry_upgrade.py (1 hunks)
• tests/model_registry/utils.py (1 hunks)
• utilities/resources/model_registry_modelregistry_opendatahub_io.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (10)

tests/model_registry/rbac/test_mr_rbac.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/negative_tests/test_model_registry_creation_negative.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/rest_api/test_model_registry_secure_db.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/rest_api/conftest.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/rbac/conftest.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/rest_api/test_multiple_mr.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/upgrade/model_registry/test_model_registry_upgrade.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/rest_api/test_model_registry_rest_api.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/utils.py (1)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

tests/model_registry/conftest.py (3)

utilities/resources/model_registry_modelregistry_opendatahub_io.py (1)

• ModelRegistry (9-94)

utilities/general.py (1)

• wait_for_pods_running (438-471)

tests/conftest.py (1)

• admin_client (78-79)

🔇 Additional comments (14)

tests/model_registry/rbac/conftest.py (1)

10-10: Import path relocation looks good.

The ModelRegistry import has been correctly updated to the new module location.

tests/model_registry/rest_api/test_model_registry_rest_api.py (1)

5-5: Import path relocation looks good.

The ModelRegistry import has been correctly updated to the new module location.

tests/model_registry/rest_api/test_multiple_mr.py (1)

6-6: Import path relocation looks good.

The ModelRegistry import has been correctly updated to the new module location.

tests/model_registry/rest_api/test_model_registry_secure_db.py (1)

8-8: Import path relocation looks good.

The ModelRegistry import has been correctly updated to the new module location.

tests/model_registry/rbac/test_mr_rbac.py (1)

36-36: Import path relocation looks good.

The ModelRegistry import has been correctly updated to the new module location.

tests/model_registry/negative_tests/test_model_registry_creation_negative.py (2)

6-6: Import path relocation looks good.

The ModelRegistry import has been correctly updated to the new module location.

---

53-67: Verify if grpc={} is still necessary.

Line 62 still passes grpc={}. According to the PR objectives, grpc is being removed as a required parameter. Consider whether this empty grpc dict is still needed for this negative test, or if it can be omitted.

tests/model_registry/upgrade/model_registry/test_model_registry_upgrade.py (1)

10-10: Import path relocation looks good.

The ModelRegistry import has been correctly updated to the new module location.

tests/model_registry/async_job/conftest.py (1)

28-28: Import path relocation looks good.

The ModelRegistry import has been correctly updated to the new module location.

tests/model_registry/conftest.py (2)

28-28: ModelRegistry import path update looks correct

Switching to the utilities-backed ModelRegistry keeps tests decoupled from ocp_resources and matches the new local wrapper usage elsewhere.

---

113-120: Workaround for default DB is safely scoped

The extra wait_for_pods_running gated on db_name == "default" cleanly scopes the RHOAIENG-40875 workaround to the default postgres path without impacting other backends.

tests/model_registry/utils.py (2)

15-15: ModelRegistry import migration is consistent

Using utilities.resources.model_registry_modelregistry_opendatahub_io.ModelRegistry here keeps this helper aligned with the new local resource wrapper.

---

538-571: ModelRegistry construction matches new optional-grpc API

get_model_registry_objects now instantiates ModelRegistry with rest={} and no grpc argument, which is consistent with making gRPC optional while still satisfying the required REST spec.

tests/model_registry/rest_api/conftest.py (1)

36-36: Import now targets local ModelRegistry wrapper

This aligns the REST API tests with the utilities-backed ModelRegistry, consistent with the rest of the suite.

[fege]

/lgtm

[github-actions]

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