feat: extract shared infrastructure into internal/platform layer by Al-Pragliola · Pull Request #2434 · kubeflow/hub

Al-Pragliola · 2026-03-18T15:12:10Z

Description

Introduces internal/platform/, a new package tree that separates reusable infrastructure from model-registry-specific internals, then migrates the model registry to use it.

Problem

The catalog (and any future registry) must import packages from the model registry's internal/ tree — internal/db/, internal/datastore/, internal/apiutils/, etc. These packages mix generic database infrastructure with model-registry-specific logic. This makes the model registry the platform itself rather than a consumer of it, and makes it impossible to add a new registry without depending on model-specific code.

What this does

Creates internal/platform/ with all shared infrastructure: database abstractions (entity model, filtering, query building, repository, schema, scopes, migrations), datastore connectors, error types, TLS config, proxy/health infrastructure, server middleware, and API utilities.
Migrates the model registry to import from internal/platform/ — converts shared packages (db/models/, db/filter/, db/service/, db/types/) to thin re-export shims, updates all internal consumers (core/, converter/, mapper/, proxy/, server/, etc.), and deletes the now-redundant originals.
Migrates the catalog — all catalog files switch from internal/db/, internal/datastore/, and pkg/api imports to their internal/platform/ equivalents. The catalog no longer depends on any model-registry-specific internal package.
Updates tooling — gorm-gen, Makefile, and scripts/remove_gorm_defaults.sh updated to reference the new platform migration and schema paths.

Design constraint

The platform layer has zero imports from model-specific packages. No file in internal/platform/ references internal/core/, internal/db/models/, internal/converter/, internal/server/openapi/, or internal/defaults/.

How Has This Been Tested?

go build ./... passes from root and catalog
go test ./... -short passes
go vet ./... clean (pre-existing participle parser tag warnings only)
No file in internal/platform/ imports from model-specific packages
Existing functionality unchanged — pure structural refactor

Merge criteria:

All the commits have been signed-off (To pass the DCO check)

The commits have meaningful messages
Automated tests are provided as part of the PR for major new functionalities; 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.
Code changes follow the kubeflow contribution guidelines.
For first time contributors: Please reach out to the Reviewers to ensure all tests are being run, ensuring the label ok-to-test has been added to the PR.

google-oss-prow · 2026-03-18T15:12:18Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from al-pragliola. For more information see the Kubernetes Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

pboyd

Looks good, in general. I didn't go line-by-line through the added files because I assume they've just been copied. Let me know if there's anything I should pay particular attention to.

I think there's some lost code (at least #2424), and some of the removed comments were probably helpful (mostly, I agree with removing them--but a few have information that's not obvious from the code).

pboyd · 2026-03-18T16:52:17Z

-	switch propertyName {
-	case "externalId":
-		// Match any source prefix: sourceId:externalId
-		return "%:" + escapeLike(strVal), true
-	case "name":
-		// If value already contains ":", treat as full namespaced name — exact match only.
-		if strings.Contains(strVal, ":") {
-			return nil, false
-		}
-		// Match namespaced form so model name without source prefix still finds the row.
-		return "%:" + escapeLike(strVal), true
-	default:
-		return nil, false
-	}


This reverts the change from #2424. Assuming that was unintentional?

pboyd · 2026-03-18T17:07:38Z

@@ -3,23 +3,13 @@ package datastore
 import "reflect"

 // Spec is the specification for the datastore.
-// Each entry


These doc comments aren't perfect, but I think they do have some value. Can we keep them? Looks like they've been removed from a other few files too.

pboyd · 2026-03-18T17:20:10Z

+		if isNewEntity {
+			if existingUpdateTime == 0 {
+				r.setLastUpdateTime(&schemaEntity, now)
+			}
+			if existingCreateTime == 0 {
+				r.setCreateTime(&schemaEntity, now)
+			}
+		} else {
+			if existingUpdateTime == 0 {
+				r.setLastUpdateTime(&schemaEntity, now)
+			}
+			if existingCreateTime == 0 {
+				r.setCreateTime(&schemaEntity, now)
+			}
+		}


I know this was just copied here, but I took some AI help with this review and so I'll pass along the finding. The if/else branches here are identical.

Suggested change

if isNewEntity {

if existingUpdateTime == 0 {

r.setLastUpdateTime(&schemaEntity, now)

}

if existingCreateTime == 0 {

r.setCreateTime(&schemaEntity, now)

}

} else {

if existingUpdateTime == 0 {

r.setLastUpdateTime(&schemaEntity, now)

}

if existingCreateTime == 0 {

r.setCreateTime(&schemaEntity, now)

}

}

if existingUpdateTime == 0 {

r.setLastUpdateTime(&schemaEntity, now)

}

if existingCreateTime == 0 {

r.setCreateTime(&schemaEntity, now)

}

pboyd · 2026-03-18T17:22:59Z

@@ -166,7 +150,6 @@ func DecodeCursor(token string) (*Cursor, error) {
 		return nil, err
 	}

-	// Split only on the first ":" so the value can contain colons (e.g. stored names "sourceId:modelName").


A lot of the removed comments are junk, but this one seems like a helpful explanation of something subtle.

pboyd · 2026-03-18T17:24:45Z

-			// If we can't parse query parameters, let the request continue
-			// The parsing error will be handled elsewhere


This is another helpful comment.

pboyd · 2026-03-18T17:26:12Z

-// parseCipherSuites parses a colon-separated list of cipher suite names
-// and returns the corresponding cipher suite IDs


Not the best doc comment. But it at least gives some indication of the expected input format.

Extract reusable infrastructure into internal/platform/ to decouple shared database, datastore, proxy, and server components from model-registry-specific internals. This enables new registries (e.g., catalog) to import platform packages without depending on model-specific code. New platform packages: - errors, apiutils, tls - db/entity, db/filter, db/repository, db/schema, db/scopes - db/types, db/constants, db/dbutil, db/utils - db/{mysql,postgres} with connection, migration, and SQL files - db/drivers for explicit backend registration - datastore (connector and repository interfaces) - proxy (dynamic router and health checks) - server/middleware (validation and router) Updates model registry and catalog imports to use platform packages, converts shared db packages to re-export shims, deletes redundant originals, and updates tooling paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>

The platform extraction accidentally reverted the name filter expansion from PR kubeflow#2424. Restores the GetEqualityExpansion logic that allows name = "modelName" to match namespaced stored values (sourceId:modelName), along with the associated tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>

- Restore doc comments on Spec struct fields (datastore/repos.go) - Collapse identical if/else branches in PreserveHistoricalTimes logic (generic_repository.go) - Restore helpful comments in paginate.go, validation.go, and tls/config.go Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Paul Boyd <pboyd@users.noreply.github.com> Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>

PR kubeflow#2433 added internal/apiutils import to the MCP catalog test file. After rebase, update to internal/platform/apiutils. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>

Al-Pragliola · 2026-03-19T10:30:47Z

Looks good, in general. I didn't go line-by-line through the added files because I assume they've just been copied. Let me know if there's anything I should pay particular attention to.

I think there's some lost code (at least #2424), and some of the removed comments were probably helpful (mostly, I agree with removing them--but a few have information that's not obvious from the code).

internal/db/filter/ was a single package that mixed generic query-building with model-registry-specific
entity mappings. I split it so the platform side defines interfaces (RestEntityType, EntityMappingFunctions, EqualityExpander in a new entity_types.go) and the query builder accepts those via injection instead of hardcoding defaultEntityMappings. The model-registry-specific mappings stay in internal/db/filter/. This is the change that actually enables other registries to plug in their own entity types

proxy/health.go was extracted from readiness.go. The generic health check plumbing (HealthChecker, DatabaseHealthChecker, GeneralReadinessHandler) moved to platform, and readiness.go kept only the model-registry-specific readiness logic

errors/errors.go + pkg/api/error.go — sentinel errors moved to platform, and pkg/api re-exports them as var ErrX = platformerrors.ErrX. Worth a sanity check that errors.Is still works through the re-export (it does — same pointer identity — but it's the kind of thing that's easy to get wrong)

db/drivers/drivers.go groups the MySQL and Postgres blank imports into one package so consumers import _ "internal/platform/db/drivers" instead of blank-importing each backend separately. It's a small readability win — someone new to the codebase can look at one place to see what backends are available

db/entity/ files are extractions from db/models/ — Properties, Pagination, Entity[T], BaseEntity[T], etc. The originals are now type alias shims. Straightforward but there's enough surface area that it's worth confirming nothing got lost in translation

pboyd

Thanks for the changes, @Al-Pragliola.

/lgtm

Al-Pragliola · 2026-04-02T09:49:22Z

/hold will need to port all the bug fixes from main after the next release

Al-Pragliola · 2026-04-27T13:12:49Z

closed in favor of #2644

google-oss-prow Bot added the do-not-merge/work-in-progress label Mar 18, 2026

google-oss-prow Bot added the size/XXL label Mar 18, 2026

github-actions Bot added Area/Go REST server and removed size/XXL labels Mar 18, 2026

google-oss-prow Bot requested review from andreyvelich, fege and rareddy March 18, 2026 15:13

Al-Pragliola mentioned this pull request Mar 18, 2026

feat: add internal/platform layer for shared infrastructure #2431

Closed

9 tasks

google-oss-prow Bot added the size/XXL label Mar 18, 2026

github-actions Bot added Area/Model Catalog and removed size/XXL labels Mar 18, 2026

google-oss-prow Bot added the size/XXL label Mar 18, 2026

Al-Pragliola force-pushed the platform/2-mr-refactor branch 2 times, most recently from 4927e76 to bb5a665 Compare March 18, 2026 15:54

github-actions Bot added the Area/UI label Mar 18, 2026

Al-Pragliola force-pushed the platform/2-mr-refactor branch from bb5a665 to a4df2d1 Compare March 18, 2026 15:58

Al-Pragliola marked this pull request as ready for review March 18, 2026 16:14

google-oss-prow Bot removed the do-not-merge/work-in-progress label Mar 18, 2026

google-oss-prow Bot requested a review from chambridge March 18, 2026 16:14

Al-Pragliola requested review from pboyd and removed request for chambridge March 18, 2026 16:14

Al-Pragliola removed the Area/UI label Mar 18, 2026

pboyd reviewed Mar 18, 2026

View reviewed changes

Al-Pragliola and others added 4 commits March 19, 2026 10:52

Al-Pragliola force-pushed the platform/2-mr-refactor branch from 1f76bcb to fd8691e Compare March 19, 2026 09:55

Al-Pragliola requested a review from pboyd March 19, 2026 10:42

pboyd approved these changes Mar 19, 2026

View reviewed changes

google-oss-prow Bot assigned pboyd Mar 19, 2026

google-oss-prow Bot added the lgtm label Mar 19, 2026

google-oss-prow Bot added the do-not-merge/hold label Apr 2, 2026

Al-Pragliola closed this Apr 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: extract shared infrastructure into internal/platform layer#2434

feat: extract shared infrastructure into internal/platform layer#2434
Al-Pragliola wants to merge 4 commits intokubeflow:mainfrom
Al-Pragliola:platform/2-mr-refactor

Al-Pragliola commented Mar 18, 2026 •

edited

Loading

Uh oh!

google-oss-prow Bot commented Mar 18, 2026

Uh oh!

pboyd left a comment

Uh oh!

pboyd Mar 18, 2026

Uh oh!

pboyd Mar 18, 2026

Uh oh!

pboyd Mar 18, 2026

Uh oh!

pboyd Mar 18, 2026

Uh oh!

pboyd Mar 18, 2026

Uh oh!

pboyd Mar 18, 2026

Uh oh!

Al-Pragliola commented Mar 19, 2026 •

edited

Loading

Uh oh!

pboyd left a comment

Uh oh!

Al-Pragliola commented Apr 2, 2026

Uh oh!

Al-Pragliola commented Apr 27, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// If we can't parse query parameters, let the request continue
		// The parsing error will be handled elsewhere

		// parseCipherSuites parses a colon-separated list of cipher suite names
		// and returns the corresponding cipher suite IDs

Conversation

Al-Pragliola commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

What this does

Design constraint

How Has This Been Tested?

Merge criteria:

Uh oh!

google-oss-prow Bot commented Mar 18, 2026

Uh oh!

pboyd left a comment

Choose a reason for hiding this comment

Uh oh!

pboyd Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

pboyd Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

pboyd Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

pboyd Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

pboyd Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

pboyd Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Al-Pragliola commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pboyd left a comment

Choose a reason for hiding this comment

Uh oh!

Al-Pragliola commented Apr 2, 2026

Uh oh!

Al-Pragliola commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Al-Pragliola commented Mar 18, 2026 •

edited

Loading

Al-Pragliola commented Mar 19, 2026 •

edited

Loading

Al-Pragliola commented Apr 27, 2026 •

edited

Loading