Skip to content

feat: update ModelArtifact CRD to include registry field and improve … #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iasthc merged 1 commit into from
Feb 28, 2026
Merged

feat: update ModelArtifact CRD to include registry field and improve … #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions config/crd/bases/model.otterscale.io_modelartifacts.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ spec:
- jsonPath: .status.conditions[?(@.type=="Ready")].status
name: Ready
type: string
- jsonPath: .spec.target.registry
name: Registry
type: string
- jsonPath: .spec.target.repository
name: Repository
type: string
Expand Down Expand Up @@ -75,9 +78,9 @@ spec:
description: HuggingFace specifies a HuggingFace Hub repository
as the model source.
Comment on lines 78 to 79
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The huggingFace source description still says it specifies a HuggingFace Hub "repository", but the field has been renamed to model. Update this text to avoid confusion for CRD users reading the generated schema.

Suggested change
description: HuggingFace specifies a HuggingFace Hub repository
as the model source.
description: HuggingFace specifies a HuggingFace Hub model
(via the model identifier) as the model source.

Copilot uses AI. Check for mistakes.
properties:
repository:
model:
description: |-
Repository is the HuggingFace model repository identifier (e.g. "microsoft/phi-4").
Model is the HuggingFace model identifier (e.g. "microsoft/phi-4", "facebook/opt-125m").
Must contain only alphanumerics, dots, underscores, hyphens, and slashes.
maxLength: 253
Comment on lines +81 to 85
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming the HuggingFace field from repository to model is a breaking CRD change: existing manifests using spec.source.huggingFace.repository will fail validation (and the unknown field may be pruned on write). If upgrades need to be supported, consider keeping repository as a deprecated alias (with validation to ensure only one is set) or providing a conversion/migration path.

Copilot uses AI. Check for mistakes.
minLength: 1
Expand Down Expand Up @@ -109,7 +112,7 @@ spec:
- name
type: object
required:
- repository
- model
type: object
type: object
x-kubernetes-validations:
Expand Down Expand Up @@ -160,10 +163,17 @@ spec:
Insecure uses an unencrypted connection to the registry instead of TLS.
Only use for development or air-gapped environments.
type: boolean
registry:
description: Registry is the OCI registry host, optionally with
port (e.g. "ghcr.io", "registry.local:5001").
maxLength: 253
minLength: 1
pattern: ^[a-zA-Z0-9][a-zA-Z0-9._/:-]*$
Copy link

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The validation pattern for registry allows slashes (/), which are not valid in a registry hostname. A registry host should be a DNS name or IP, optionally with a port, but should not contain path segments. Please remove the slash from the pattern to enforce a valid registry host format.

                    pattern: ^[a-zA-Z0-9][a-zA-Z0-9._:-]*$

Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CRD schema for .spec.target.registry uses a pattern that allows /, even though registry is described as a host[:port]. This permits values like ghcr.io/myorg and blurs the boundary with .spec.target.repository. Update the regex to reject / and more closely match a registry host (optionally with port).

Suggested change
pattern: ^[a-zA-Z0-9][a-zA-Z0-9._/:-]*$
pattern: ^[a-zA-Z0-9][a-zA-Z0-9._-]*(?::[0-9]{1,5})?$

Copilot uses AI. Check for mistakes.
type: string
repository:
description: |-
Repository is the full OCI registry path (e.g. "ghcr.io/myorg/models/phi-4").
Must contain only alphanumerics, dots, underscores, hyphens, and slashes.
Repository is the OCI repository path within the registry (e.g. "myorg/models/phi-4", "facebook/opt-125m").
Must not include the registry host. Must contain only alphanumerics, dots, underscores, hyphens, and slashes.
maxLength: 253
minLength: 1
pattern: ^[a-zA-Z0-9][a-zA-Z0-9._/-]*$
Expand All @@ -176,6 +186,7 @@ spec:
pattern: ^[a-zA-Z0-9._/-]*$
type: string
required:
- registry
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making .spec.target.registry required is an API-breaking change for any existing ModelArtifacts that only set repository (as a full ref previously). If cluster upgrades must be smooth, consider temporarily allowing registry to be optional with defaulting/derivation from repository, or supporting both forms during a deprecation window.

Suggested change
- registry

Copilot uses AI. Check for mistakes.
- repository
type: object
required:
Expand Down
23 changes: 17 additions & 6 deletions model/v1alpha1/modelartifact_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,13 @@ type ArtifactPhase string
const (
// PhasePending indicates the pipeline has not yet started.
PhasePending ArtifactPhase = "Pending"

// PhaseRunning indicates the import/pack/push Job is in progress.
PhaseRunning ArtifactPhase = "Running"

// PhaseSucceeded indicates the artifact was successfully pushed to the registry.
PhaseSucceeded ArtifactPhase = "Succeeded"

// PhaseFailed indicates the pipeline encountered an error.
PhaseFailed ArtifactPhase = "Failed"
)
Expand Down Expand Up @@ -83,16 +86,16 @@ type ModelSource struct {

// HuggingFaceSource configures model retrieval from HuggingFace Hub.
//
// SECURITY: Repository and Revision are passed to shell scripts. Only users who can
// SECURITY: Model and Revision are passed to shell scripts. Only users who can
// create ModelArtifacts should have access; they already have equivalent privileges.
type HuggingFaceSource struct {
// Repository is the HuggingFace model repository identifier (e.g. "microsoft/phi-4").
// Model is the HuggingFace model identifier (e.g. "microsoft/phi-4", "facebook/opt-125m").
// Must contain only alphanumerics, dots, underscores, hyphens, and slashes.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:Pattern="^[a-zA-Z0-9][a-zA-Z0-9._/-]*$"
// +required
Repository string `json:"repository"`
Model string `json:"model"`

// Revision pins a specific branch, tag, or commit hash.
// If not specified, the default branch is used.
Expand All @@ -110,11 +113,18 @@ type HuggingFaceSource struct {

// OCITarget defines the destination OCI registry for the packaged artifact.
//
// SECURITY: Repository and Tag are passed to shell scripts. Only users who can
// SECURITY: Registry, Repository, and Tag are passed to shell scripts. Only users who can
// create ModelArtifacts should have access; they already have equivalent privileges.
type OCITarget struct {
// Repository is the full OCI registry path (e.g. "ghcr.io/myorg/models/phi-4").
// Must contain only alphanumerics, dots, underscores, hyphens, and slashes.
// Registry is the OCI registry host, optionally with port (e.g. "ghcr.io", "registry.local:5001").
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:Pattern="^[a-zA-Z0-9][a-zA-Z0-9._/:-]*$"
Copy link

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The validation pattern for Registry allows slashes (/), which are not valid in a registry hostname. A registry host should be a DNS name or IP, optionally with a port, but should not contain path segments. Please remove the slash from the pattern to enforce a valid registry host format. This will also fix the generated CRD YAML.

Suggested change
// +kubebuilder:validation:Pattern="^[a-zA-Z0-9][a-zA-Z0-9._/:-]*$"
// +kubebuilder:validation:Pattern="^[a-zA-Z0-9][a-zA-Z0-9._:-]*$"

Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Registry field is documented as an OCI registry host (optionally with port), but the validation pattern currently allows /. That permits path segments (e.g. ghcr.io/myorg), which contradicts the field’s intent and undermines the separation between registry and repository. Tighten the regex to disallow / (and ideally validate host[:port]).

Suggested change
// +kubebuilder:validation:Pattern="^[a-zA-Z0-9][a-zA-Z0-9._/:-]*$"
// +kubebuilder:validation:Pattern="^[a-zA-Z0-9][a-zA-Z0-9._-]*(?::[0-9]{1,5})?$"

Copilot uses AI. Check for mistakes.
// +required
Registry string `json:"registry"`

// Repository is the OCI repository path within the registry (e.g. "myorg/models/phi-4", "facebook/opt-125m").
// Must not include the registry host. Must contain only alphanumerics, dots, underscores, hyphens, and slashes.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:Pattern="^[a-zA-Z0-9][a-zA-Z0-9._/-]*$"
Expand Down Expand Up @@ -228,6 +238,7 @@ type ModelArtifactStatus struct {
// +kubebuilder:resource:scope=Namespaced
// +kubebuilder:printcolumn:name="Phase",type=string,JSONPath=`.status.phase`
// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`
// +kubebuilder:printcolumn:name="Registry",type=string,JSONPath=`.spec.target.registry`
// +kubebuilder:printcolumn:name="Repository",type=string,JSONPath=`.spec.target.repository`
// +kubebuilder:printcolumn:name="Digest",type=string,JSONPath=`.status.digest`,priority=1
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
Expand Down