periodic sync upstream KF to midstream ODH

[Al-Pragliola]

superseeds #276

• keep chore: remove unused dep (go-enum) kubeflow/model-registry#1323
• keep fix: centered header buttons kubeflow/model-registry#1303
• keep chore: fix "Reviewers" URL in pull request template kubeflow/model-registry#1329
• keep feat: migrate remainder of User Story from Robot to pytest (and DocArtifact) kubeflow/model-registry#1331
• keep feat: remove linting/gen from dockerfile kubeflow/model-registry#1334
• keep fix: remove incorrect prepare workflow call in build-and-push-image.yml kubeflow/model-registry#1336
• keep feat: migrate "Regression" user stories from Robot to pytest  kubeflow/model-registry#1332
• keep Make the openapi client a separate Go module kubeflow/model-registry#1322
• keep refactor: remove remaining Robot Framework after migration kubeflow/model-registry#1338
• keep feat: address high code security alerts kubeflow/model-registry#1339
• keep build(deps): bump gorm.io/driver/postgres from 1.5.7 to 1.6.0 kubeflow/model-registry#1349
• keep build(deps): bump github.com/go-logr/logr from 1.4.2 to 1.4.3 kubeflow/model-registry#1348
• keep build(deps): bump github.com/testcontainers/testcontainers-go from 0.37.0 to 0.38.0 kubeflow/model-registry#1345
• keep build(deps): bump k8s.io/apimachinery from 0.33.2 to 0.33.3 kubeflow/model-registry#1347
• keep build(deps): bump github.com/testcontainers/testcontainers-go/modules/mysql from 0.37.0 to 0.38.0 kubeflow/model-registry#1352
• keep chore: move to more general purpose tag for go 1.24 kubeflow/model-registry#1355
• keep build(deps): bump k8s.io/api from 0.33.2 to 0.33.3 kubeflow/model-registry#1354
• keep build(deps): bump google.golang.org/grpc from 1.72.1 to 1.73.0 kubeflow/model-registry#1353
• keep fix: build issues post go.work file introduction kubeflow/model-registry#1358
• merge commit

Summary by CodeRabbit

• New Features

  • Improved error handling and validation for pagination and input parameters, enhancing security and robustness.
  • Expanded support for storing and updating model descriptions and documentation via the Python client and REST API.

• Bug Fixes

  • Structured error messages are now returned for malformed API requests, improving clarity for end-users.
  • Enhanced security for pagination queries to prevent injection vulnerabilities.

• Refactor

  • Updated build and deployment workflows for improved reliability and maintainability.
  • Updated Dockerfiles and dependency management for better consistency and compatibility.

• Style

  • Improved UI layout for model registry tables to enhance usability.

• Documentation

  • Updated README and diagrams to reflect current testing strategies and tooling.

• Tests

  • Replaced Robot Framework end-to-end tests with new Python-based tests, focusing on user story validation and error handling.
  • Added comprehensive unit tests for pagination and TLS configuration.

• Chores

  • Removed legacy Robot Framework test files and related resources.
  • Upgraded multiple dependencies for improved security and performance.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

Walkthrough

This update restructures the testing and build workflows, enhances security in pagination and TLS configuration, and modernizes dependency management. Robot Framework-based end-to-end tests and related resources are removed, replaced by new Python-based tests. Build and Dockerfile processes are streamlined, and several code areas—including pagination, TLS, and client artifact handling—receive targeted improvements and new tests.

Changes

File(s) / Path(s)	Change Summary
.github/pull_request_template.md, .gitignore, README.md	Minor documentation and metadata updates (links, ignore rules, image references).
.github/workflows/build-and-push-image.yml, .github/workflows/build.yml, .github/workflows/prepare.yml	Refactored CI workflows: introduced reusable prepare job, updated job dependencies, removed Python setup, and streamlined build steps.
.github/workflows/check-db-schema-structs.yaml	Changed workflow trigger from PR/path-based to push-based on main branch and tags.
.github/workflows/run-robot-tests.yaml, test/robot/Regression.robot, test/robot/UserStory.robot, test/robot/Setup.resource, test/robot/MRkeywords.resource, test/robot/ModelRegistry.py, test/robot/data.yaml, test/robot/requirements.txt	Removed Robot Framework-based end-to-end test suite and supporting files.
Dockerfile, Dockerfile.odh, cmd/controller/Dockerfile.controller, cmd/csi/Dockerfile.csi	Updated Go toolset base image tags, added/corrected workspace and module copying, and removed dev/dependency install steps.
Makefile	Removed go-enum dependency, adjusted test/build dependencies, set GOWORK=off for GORM codegen.
go.mod, pkg/openapi/go.mod	Added local module, updated and replaced dependencies, introduced new module for openapi.
internal/converter/openapi_mlmd_converter_util.go	Changed string-to-int32 conversion to use strconv.ParseInt for explicitness.
internal/db/scopes/paginate.go	Hardened pagination: added input validation, whitelisting, parameterized queries, and secure cursor handling.
internal/db/scopes/paginate_test.go	Added comprehensive unit tests for pagination input validation and cursor decoding.
internal/tls/config.go, internal/tls/config_test.go	Enforced TLS 1.2+, removed insecure cipher suites, and rejected insecure ciphers at runtime; updated tests accordingly.
clients/python/Makefile	Made image build optional during deployment via new BUILD_IMAGE variable.
clients/python/src/model_registry/types/artifacts.py	Implemented create and update methods for DocArtifact class.
clients/python/src/model_registry/types/contexts.py	Added registered_model_id to ModelVersion, updated methods for new field.
clients/python/tests/regression_test.py	Added async tests for structured error responses on malformed entity submissions.
clients/python/tests/test_client.py	Added async tests for model/description/documentation management and artifact handling.
clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx, clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableToolbar.tsx	Wrapped toolbar elements in <Flex> containers for improved (temporary) layout alignment.

Sequence Diagram(s)

sequenceDiagram
    participant Developer
    participant GitHub Actions
    participant Prepare Workflow
    participant Build Workflow

    Developer->>GitHub Actions: Push or PR triggers build
    GitHub Actions->>Prepare Workflow: Start prepare job (checkout, Go setup, prep build)
    Prepare Workflow-->>GitHub Actions: Prepare job completes
    GitHub Actions->>Build Workflow: Start build job (depends on prepare)
    Build Workflow-->>GitHub Actions: Build completes

Loading

sequenceDiagram
    participant Client
    participant Model Registry API

    Client->>Model Registry API: POST malformed RegisteredModel/ModelVersion
    Model Registry API-->>Client: Structured error response (400/422 with JSON error)

Loading

Estimated code review effort

4 (~90 minutes)

Suggested labels

lgtm, tide/merge-method-merge, Area/Go REST server, Area/MR Python client, Area/UI, Area/GitHub

Suggested reviewers

• nehachopra27

Poem

In the warren of code, a bunny did hop,
Refactoring workflows—no sign of a stop!
Robot tests retired, Python now leads,
Secure pagination, TLS—no more bad deeds.
With Docker and Makefile, dependencies pruned,
This release is robust—by rabbits, fine-tuned! 🐇✨

✨ Finishing Touches

• 📝 Generate Docstrings

---

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Al-Pragliola

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [Al-Pragliola]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (11)

README.md (2)

176-176: Add descriptive alt-text and check image path

The image reference lacks alt-text, triggering markdownlint MD045 and decreasing accessibility.
Replace the bare link with a descriptive label and verify that the .drawio.png file actually exists in docs/ after the upstream sync.

-![](/docs/Model%20Registry%20Testing%20areas.drawio.png)
+![Testing-strategy diagram](/docs/Model%20Registry%20Testing%20areas.drawio.png)

If the file was renamed or deleted, update the path accordingly.

---

183-183: Fix typos and follow project-wide emphasis / hyphenation rules

Grammar & linting issues:

1. “higher-lever” → “higher-level”
2. Missing hyphen in “high-level perspective” (compound adjective)
3. Project linter expects underscore emphasis, not asterisks (MD049).

-End-to-end testing is developed with KinD and Pytest; this higher-lever layer of testing is used to demonstrate *User Stories* from high level perspective.
+End-to-end testing is developed with KinD and Pytest; this higher-level layer of testing is used to demonstrate _User Stories_ from a high-level perspective.

internal/db/scopes/paginate.go (3)

19-25: Remove duplicate map entry for "id".

The allowedOrderByColumns map contains a duplicate entry for "id" (lines 21 and 24). In Go, map keys must be unique, so the second entry will overwrite the first. Since both map to the same value "id", this is redundant.

 var allowedOrderByColumns = map[string]string{
 	"ID":               "id",
 	"CREATE_TIME":      "create_time_since_epoch",
 	"LAST_UPDATE_TIME": "last_update_time_since_epoch",
-	"id":               "id", // default fallback
 }

---

34-41: Consider pre-compiling the regex for better performance.

The regex pattern is constant and called on every validation. Pre-compiling it would improve performance, especially under high load.

+var tablePrefixRegex = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`)
+
 // isValidTablePrefix validates table prefix to prevent SQL injection
 func isValidTablePrefix(tablePrefix string) bool {
 	if tablePrefix == "" {
 		return true
 	}
 	// Only allow alphanumeric characters and underscores, common table naming convention
-	matched, _ := regexp.MatchString(`^[a-zA-Z_][a-zA-Z0-9_]*$`, tablePrefix)
-	return matched
+	return tablePrefixRegex.MatchString(tablePrefix)
 }

---

67-67: Fix truncated comment.

The comment appears to be incomplete.

-			// Validate and sanitize so
+			// Validate and sanitize sortOrder

internal/db/scopes/paginate_test.go (1)

1-265: Consider adding integration tests for the full pagination flow.

While the unit tests excellently cover the validation logic, consider adding tests for the Paginate and PaginateWithTablePrefix functions to ensure the full pagination flow works correctly with the new security measures.

Would you like me to generate integration tests for the complete pagination flow?

Dockerfile.odh (1)

2-2: Consider the implications of using a general base image tag.

The change from 1.24.4 to 1.24 reduces build reproducibility since the specific patch version is no longer pinned. While this provides flexibility for receiving patch updates, it could lead to inconsistent builds across environments.

Consider whether build reproducibility or flexibility is more important for your use case. If reproducibility is preferred, revert to a specific patch version.

cmd/csi/Dockerfile.csi (1)

2-2: Consider the implications of using a general base image tag.

Similar to Dockerfile.odh, the change from 1.24.4 to 1.24 reduces build reproducibility. Ensure this change aligns with your build consistency requirements across all Dockerfiles.

Consider maintaining version consistency across all Dockerfiles in the repository.

Dockerfile (1)

2-2: Consider using specific version tags for reproducible builds

Changing from 1.24.4 to 1.24 means the build could use different patch versions over time, potentially introducing unexpected changes. For production builds, consider pinning to a specific patch version.

clients/python/src/model_registry/types/contexts.py (1)

52-58: Breaking API change: create() now requires registered_model_id

The create() method signature change to require registered_model_id is a breaking change for existing code. Ensure this is documented in release notes and migration guides.

Consider providing a deprecation path or backward compatibility if this is a public API.

clients/python/tests/test_client.py (1)

1099-1120: LGTM: Comprehensive test for selective model property updates.

This test effectively verifies that updating a model's description preserves other properties like owner. The test structure follows good practices with clear setup, action, and verification phases, ensuring partial updates work correctly.

Minor syntax issue on line 1117.

There's a missing space in the assignment: rm =client.update(rm) should be rm = client.update(rm).

-    rm =client.update(rm)
+    rm = client.update(rm)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efb1f31 and 27cd670.

⛔ Files ignored due to path filters (7)

• clients/ui/bff/go.work is excluded by !**/*.work
• clients/ui/bff/go.work.sum is excluded by !**/*.sum
• docs/Model Registry Testing areas.drawio.png is excluded by !**/*.png
• docs/Model Registry Testing areas.png is excluded by !**/*.png
• go.sum is excluded by !**/*.sum
• go.work is excluded by !**/*.work
• go.work.sum is excluded by !**/*.sum

📒 Files selected for processing (35)

• .github/pull_request_template.md (1 hunks)
• .github/workflows/build-and-push-image.yml (1 hunks)
• .github/workflows/build.yml (1 hunks)
• .github/workflows/check-db-schema-structs.yaml (1 hunks)
• .github/workflows/prepare.yml (1 hunks)
• .github/workflows/run-robot-tests.yaml (0 hunks)
• .gitignore (0 hunks)
• Dockerfile (1 hunks)
• Dockerfile.odh (1 hunks)
• Makefile (3 hunks)
• README.md (1 hunks)
• clients/python/Makefile (2 hunks)
• clients/python/src/model_registry/types/artifacts.py (2 hunks)
• clients/python/src/model_registry/types/contexts.py (2 hunks)
• clients/python/tests/regression_test.py (3 hunks)
• clients/python/tests/test_client.py (3 hunks)
• clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx (2 hunks)
• clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableToolbar.tsx (2 hunks)
• cmd/controller/Dockerfile.controller (1 hunks)
• cmd/csi/Dockerfile.csi (1 hunks)
• docs/Model Registry Testing areas.drawio (0 hunks)
• go.mod (9 hunks)
• internal/converter/openapi_mlmd_converter_util.go (1 hunks)
• internal/db/scopes/paginate.go (4 hunks)
• internal/db/scopes/paginate_test.go (1 hunks)
• internal/tls/config.go (2 hunks)
• internal/tls/config_test.go (1 hunks)
• pkg/openapi/go.mod (1 hunks)
• test/robot/MRkeywords.resource (0 hunks)
• test/robot/ModelRegistry.py (0 hunks)
• test/robot/Regression.robot (0 hunks)
• test/robot/Setup.resource (0 hunks)
• test/robot/UserStory.robot (0 hunks)
• test/robot/data.yaml (0 hunks)
• test/robot/requirements.txt (0 hunks)

💤 Files with no reviewable changes (10)

• .gitignore
• test/robot/requirements.txt
• test/robot/data.yaml
• .github/workflows/run-robot-tests.yaml
• docs/Model Registry Testing areas.drawio
• test/robot/Regression.robot
• test/robot/Setup.resource
• test/robot/ModelRegistry.py
• test/robot/UserStory.robot
• test/robot/MRkeywords.resource

🧰 Additional context used

🧬 Code Graph Analysis (7)

clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx (2)

clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts (1)

• navigate (66-69)

clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts (2)

• registerVersionForModelUrl (53-56)
• modelVersionArchiveUrl (32-33)

clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableToolbar.tsx (2)

clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts (1)

• navigate (66-69)

clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts (3)

• registerModelUrl (47-48)
• registerVersionUrl (50-51)
• registeredModelArchiveUrl (10-11)

clients/python/src/model_registry/types/artifacts.py (7)

clients/python/src/mr_openapi/models/doc_artifact_create.py (1)

• DocArtifactCreate (25-128)

pkg/openapi/model_doc_artifact_create.go (1)

• DocArtifactCreate (21-34)

clients/python/src/mr_openapi/models/doc_artifact_update.py (1)

• DocArtifactUpdate (25-122)

pkg/openapi/model_doc_artifact_update.go (1)

• DocArtifactUpdate (21-32)

clients/python/src/model_registry/types/base.py (3)

• _map_custom_properties (53-87)
• _props_as_dict (110-118)
• update (45-46)

clients/python/src/model_registry/types/contexts.py (2)

• update (61-66)
• update (124-129)

clients/python/src/model_registry/_client.py (1)

• update (359-371)

clients/python/tests/regression_test.py (1)

clients/python/tests/conftest.py (1)

• client (154-155)

internal/db/scopes/paginate.go (1)

internal/db/models/pagination.go (4)

• SortOrderAsc (4-4)
• SortOrderDesc (5-5)
• DefaultOrderBy (12-12)
• DefaultSortOrder (10-10)

internal/db/scopes/paginate_test.go (1)

internal/db/models/pagination.go (1)

• DefaultOrderBy (12-12)

clients/python/tests/test_client.py (4)

clients/python/src/model_registry/types/artifacts.py (3)

• DocArtifact (80-135)
• update (101-108)
• update (183-190)

clients/python/tests/conftest.py (1)

• client (154-155)

clients/python/src/model_registry/_client.py (2)

• ModelRegistry (62-621)
• update (359-371)

clients/python/src/model_registry/core.py (6)

• get_model_version_by_params (256-258)
• get_model_version_by_params (261-261)
• get_model_version_by_params (270-298)
• get_model_artifact_by_params (365-369)
• get_model_artifact_by_params (372-372)
• get_model_artifact_by_params (381-407)

🪛 LanguageTool

README.md

[uncategorized] ~183-~183: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...used to demonstrate User Stories from high level perspective. ## FAQ ### How do I dele...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

🪛 markdownlint-cli2 (0.17.2)

README.md

176-176: Images should have alternate text (alt text)

(MD045, no-alt-text)

---

183-183: Emphasis style
Expected: underscore; Actual: asterisk

(MD049, emphasis-style)

---

183-183: Emphasis style
Expected: underscore; Actual: asterisk

(MD049, emphasis-style)

🔇 Additional comments (45)

.github/pull_request_template.md (2)

21-21: Absolute URL is safer but targets the upstream OWNERS file – please confirm that’s intentional
Changing the link from a relative path to an absolute URL avoids breakage when forks move files around, which is 👍.
However, the URL now points at kubeflow/model-registry, not opendatahub-io/model-registry; first-time contributors may ping the wrong reviewer set. If this repo maintains its own OWNERS file (or plans to), the link should reference it instead.

---

21-21: Nice move to an absolute link
Switching to a fully-qualified URL eliminates the ambiguous relative reference and works from forks and different directories.

clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableToolbar.tsx (2)

6-6: LGTM: Flex import added for layout fix.

The import addition is correct and necessary for the temporary layout workaround.

---

43-44: LGTM: Temporary Flex wrapper addresses toolbar centering issue.

The Flex container appropriately wraps the toolbar elements to address the known ToolbarContent centering limitation. The TODO comment correctly indicates this is a temporary measure pending upstream PatternFly improvements.

Also applies to: 119-119

clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx (2)

8-8: LGTM: Flex import added for layout fix.

The import addition is consistent with the coordinated toolbar layout improvements across model registry screens.

---

136-137: LGTM: Consistent temporary Flex wrapper implementation.

The Flex container implementation matches the approach in RegisteredModelsTableToolbar.tsx, providing a coordinated solution to the ToolbarContent centering issue. The TODO comment appropriately indicates this is a temporary measure.

Also applies to: 227-227

internal/db/scopes/paginate.go (2)

58-73: Excellent security improvements in pagination handling.

The validation and sanitization of orderBy and sortOrder parameters using whitelists effectively prevents SQL injection attacks. The fallback to safe defaults ensures the query remains functional even with invalid inputs.

---

108-151: Excellent refactoring to use parameterized queries.

The conversion from raw SQL string concatenation to parameterized queries using GORM's Where method with placeholders is a crucial security improvement. This effectively prevents SQL injection attacks while maintaining the pagination logic.

internal/db/scopes/paginate_test.go (4)

12-75: Comprehensive test coverage for table prefix validation.

Excellent test coverage including valid cases, edge cases, and multiple SQL injection attempts. The table-driven test approach with descriptive test cases makes the tests easy to understand and maintain.

---

77-144: Well-structured column validation tests.

The tests properly validate the column whitelist mechanism and ensure that invalid or malicious inputs fall back to the default value. Good coverage of various SQL injection patterns.

---

146-198: Good test coverage for sort order validation.

The tests properly validate the sort order whitelist, including case sensitivity and SQL injection attempts. The strict validation (only uppercase ASC/DESC) aligns with the security-focused approach.

---

200-265: Thorough cursor decoding tests with security focus.

Excellent test coverage for cursor decoding, including malicious payloads. The tests correctly verify that malicious cursors can be decoded but will be safely handled by the parameterized queries in the main implementation.

internal/converter/openapi_mlmd_converter_util.go (1)

44-49: LGTM! Improved platform-independent integer parsing.

The change from strconv.Atoi to strconv.ParseInt(idString, 10, 32) is an improvement that ensures consistent 32-bit integer parsing across platforms and properly validates the range constraints.

clients/python/Makefile (2)

6-6: Good addition for build flexibility.

The new BUILD_IMAGE variable allows users to control whether the model-registry server image should be built during deployment, which is useful for CI/CD scenarios where the image might already exist.

---

22-26: Correct implementation of conditional image building.

The conditional logic properly checks the BUILD_IMAGE variable and only executes the image build step when set to true. The syntax is correct and maintains backward compatibility.

cmd/controller/Dockerfile.controller (1)

2-2: Consider the implications of using a floating tag.

Changing from go-toolset:1.24.4 to go-toolset:1.24 reduces build determinism as it will automatically pull patch updates. While this can provide security fixes, it may also introduce unexpected changes. Consider if this trade-off is acceptable for your deployment strategy.

.github/workflows/check-db-schema-structs.yaml (1)

3-8: Workflow now runs more frequently.

The trigger change from path-filtered pull requests to push events on main branch and version tags will cause this workflow to run more frequently. This ensures database schema consistency is checked on every main branch update and release, but will consume more CI resources.

Dockerfile.odh (1)

6-7: LGTM! Proper workspace and module support added.

The addition of go.work and pkg/openapi/go.mod correctly supports the new Go workspace and module structure. This ensures the build context includes all necessary dependency manifests before running go mod download.

.github/workflows/build-and-push-image.yml (2)

22-23: LGTM! Good use of reusable workflows.

Adding the prepare job using a reusable workflow centralizes common preparation steps and reduces duplication across workflows.

---

26-26: LGTM! Proper job dependency established.

The needs: prepare dependency ensures the preparation steps complete successfully before the image build begins, maintaining proper workflow execution order.

internal/tls/config_test.go (1)

355-360: LGTM! Security improvement by removing weaker cipher suite.

The removal of TLS_RSA_WITH_AES_128_CBC_SHA from the test aligns with enhanced TLS security policies. RSA-based cipher suites are generally considered less secure than ECDHE-based ones, so this change improves the overall security posture.

cmd/csi/Dockerfile.csi (1)

8-9: LGTM! Consistent workspace and module support.

The addition of go.work and pkg/openapi/go.mod is consistent with other Dockerfiles and properly supports the new Go workspace structure.

.github/workflows/build.yml (3)

17-18: LGTM! Consistent use of reusable workflow.

Adding the prepare job maintains consistency with the image build workflow and centralizes preparation steps.

---

20-20: LGTM! Proper workflow dependency.

The needs: prepare ensures preparation steps complete before building, maintaining proper execution order.

---

30-30: LGTM! Logical separation of build phases.

Changing from make clean build to make build/compile makes sense since the clean and prepare steps are now handled by the prepare job. This creates a cleaner separation between preparation and compilation phases.

.github/workflows/prepare.yml (1)

12-12: Ignore Go version concern – 1.24 is the current stable release
The workflow’s go-version: "1.24" is correct (Go 1.24.5 was released July 8, 2025). No change needed here.

Likely an incorrect or invalid review comment.

internal/tls/config.go (2)

67-67: Good security enhancement: TLS 1.2 minimum

Setting the minimum TLS version to 1.2 is a security best practice that prevents downgrade attacks to older, vulnerable TLS versions.

---

122-126: Excellent addition of insecure cipher validation

The runtime check against tls.InsecureCipherSuites() provides an additional layer of security by preventing the use of known insecure cipher suites. This is a proactive security measure.

Dockerfile (1)

8-9: Verified go.work File Tracking

.gitignore does not include go.work, and the existing go.work (Go 1.24.4) correctly references . and ./pkg/openapi in its workspace.

clients/python/src/model_registry/types/contexts.py (1)

49-49: Proper implementation of registered_model_id field

The addition of registered_model_id field and its mapping in from_basemodel is correctly implemented, maintaining consistency with the rest of the codebase.

Also applies to: 80-80

clients/python/src/model_registry/types/artifacts.py (1)

91-98: Well-implemented create() and update() methods for DocArtifact

The implementation of create() and update() methods follows the established patterns from ModelArtifact class, ensuring consistency across the codebase. The explicit setting of artifactType="doc-artifact" and proper handling of custom properties is correct.

Also applies to: 101-108

Makefile (3)

155-155: LGTM: Good addition of GOWORK=off environment variable.

This change prevents Go workspace mode from interfering with GORM code generation, which is a common best practice when running code generation tools that may have different module requirements.

Also applies to: 162-162

---

252-252: LGTM: Dependency cleanup is appropriate.

The removal of bin/go-enum from the deps target aligns with the broader build simplification effort described in the PR summary.

---

303-303: LGTM: Test target simplification improves build modularity.

Removing the gen prerequisite from test targets allows code generation to be handled separately (likely in the new prepare CI job), improving build flexibility and reducing unnecessary dependencies during testing.

Also applies to: 307-307, 311-311

clients/python/tests/regression_test.py (3)

7-8: LGTM: Module-level imports improve code organization.

Moving REGISTRY_HOST and REGISTRY_PORT imports to module level makes dependencies explicit and avoids repeated imports across test functions.

---

139-149: LGTM: Comprehensive test for malformed RegisteredModel validation.

This test effectively verifies that the API returns structured error messages for unknown fields. The direct HTTP approach is appropriate for testing raw API behavior, and the assertions properly validate both status code and error message structure.

---

152-173: LGTM: Well-structured test for required field validation.

This test properly sets up prerequisites (valid registered model) before testing malformed ModelVersion creation. The 422 status code assertion is appropriate for validation errors, and the structured error response validation ensures consistent API behavior.

go.mod (4)

8-8: LGTM: Routine dependency updates.

These dependency version bumps appear to be routine maintenance updates that likely include bug fixes and security patches. The incremental nature of the version changes reduces the risk of introducing breaking changes.

Also applies to: 21-22, 25-25, 28-28, 30-31

---

13-13: LGTM: Properly structured local module addition.

The addition of the pkg/openapi local module with version v0.0.0 and corresponding replace directive (line 213) follows Go best practices for workspace modules and aligns with the build context changes mentioned in the AI summary.

---

38-38: LGTM: Indirect dependency updates support direct changes.

These indirect dependency updates are expected consequences of the direct dependency updates and include necessary supporting library updates. The addition of new containerd/errdefs dependencies aligns with container runtime improvements.

Also applies to: 47-47, 56-58, 61-61, 105-106, 117-117, 124-124, 146-146, 174-174, 186-186, 209-209

---

213-213: LGTM: Required replace directive for local module.

This replace directive is necessary to map the pkg/openapi module to its local path, enabling the Go workspace to properly resolve the local module dependency.

clients/python/tests/test_client.py (4)

10-10: LGTM: Import addition supports new functionality.

The DocArtifact import is necessary for the new documentation test and is properly scoped to import only the required class.

---

26-27: LGTM: Enhanced test assertions improve coverage.

The changes strengthen the existing test by:

• Using a more realistic URI format
• Adding comprehensive assertions to verify model component relationships
• Ensuring proper linkage between registered models, model versions, and artifacts

These improvements enhance test reliability and coverage.

Also applies to: 32-32, 38-39, 44-46, 49-49

---

1122-1153: LGTM: Thorough test for description management across model components.

This test comprehensively verifies description storage and updates across all model registry components (registered model, model version, and model artifact). The test structure properly validates the component hierarchy and ensures descriptions can be independently managed on each level.

---

1155-1178: LGTM: Well-focused test for documentation artifact management.

This test effectively verifies the ability to store longer documentation as dedicated artifacts. The use of DocArtifact and upsert_model_version_artifact demonstrates proper artifact management patterns, and the assertions ensure the documentation is correctly stored and retrievable.

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Verify Go version consistency across the project.

The module specifies Go 1.23, but other files in the project reference Go 1.24. Ensure this version difference is intentional and won't cause compatibility issues.

---

🏁 Script executed:

#!/bin/bash
# Description: Check Go versions used across the project
# Expected: Identify any version inconsistencies

echo "=== Checking Go versions in go.mod files ==="
fd -t f "go.mod" --exec grep -H "go [0-9]"

echo -e "\n=== Checking Go versions in GitHub workflows ==="
fd -t f -e yml -e yaml . .github/workflows --exec grep -H "go-version"

echo -e "\n=== Checking Go versions in Dockerfiles ==="
fd -t f "Dockerfile*" --exec grep -H "go-toolset"

Length of output: 1421

---

Align Go version in pkg/openapi/go.mod with the rest of the project

pkg/openapi/go.mod is set to Go 1.23, while the root module and other sub-modules use Go 1.24.x. This mismatch can lead to unexpected compatibility issues.

Locations to update:

• pkg/openapi/go.mod: change go 1.23 to match the project’s Go 1.24.x (e.g. go 1.24.4)

Proposed diff:

 pkg/openapi/go.mod
 line 3

- go 1.23
+ go 1.24.4

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	go 1.23
	module github.com/kubeflow/model-registry/pkg/openapi
	- go 1.23
	+ go 1.24.4

🤖 Prompt for AI Agents

In pkg/openapi/go.mod at line 3, update the Go version from 1.23 to 1.24.x (for
example, 1.24.4) to align with the root module and other sub-modules. This
ensures consistency and avoids compatibility issues across the project.

[tarilabs]

#285 (comment)

/lgtm