Skip to content

feat(api): v1alpha2 CRD types with typed provider slices (Phase 1)#264

Closed
rhuss wants to merge 9 commits intollamastack:mainfrom
rhuss:002-reimpl
Closed

feat(api): v1alpha2 CRD types with typed provider slices (Phase 1)#264
rhuss wants to merge 9 commits intollamastack:mainfrom
rhuss:002-reimpl

Conversation

@rhuss
Copy link
Copy Markdown
Collaborator

@rhuss rhuss commented Mar 10, 2026

Summary

Phase 1 implementation of the v1alpha2 CRD schema for operator-generated config. This PR adds the complete type definitions with CEL validation rules, ready for team review before proceeding to Phase 2 (config generation engine).

What's included

The api/v1alpha2/ package with:

  • ProvidersSpec with typed []ProviderConfig slices (no polymorphic JSON)
  • ProviderConfig with explicit secretRefs field (no heuristic secret detection)
  • ResourcesSpec with typed []ModelConfig (only name required)
  • StateStorageSpec with KV (sqlite/redis) and SQL (sqlite/postgres) backends
  • NetworkingSpec with typed ExposeConfig (enabled + hostname fields)
  • WorkloadSpec consolidating all deployment settings from v1alpha1
  • Status types, condition constants, and event type definitions
  • +kubebuilder:storageversion marker (v1alpha2 is the hub)

15 CEL validation rules

All compiled and verified with controller-gen:

  • 4 mutual exclusivity (providers/resources/storage/disabled vs overrideConfig)
  • 5 provider ID required (per API type, when list has > 1 element)
  • 2 distribution (name/image mutual exclusivity + at-least-one)
  • TLS secretName required when enabled: true
  • Redis endpoint required when type: redis
  • Postgres connectionString required when type: postgres

How to test

Apply the CRD and try creating invalid CRs to verify CEL validation:

kubectl apply -f config/crd/bases/llamastack.io_llamastackdistributions.yaml

# Should succeed:
cat <<YAML | kubectl apply -f -
apiVersion: llamastack.io/v1alpha2
kind: LlamaStackDistribution
metadata:
  name: test
spec:
  distribution:
    name: starter
  providers:
    inference:
      - provider: vllm
        endpoint: "http://vllm:8000"
YAML

# Should fail (both name and image):
cat <<YAML | kubectl apply -f -
apiVersion: llamastack.io/v1alpha2
kind: LlamaStackDistribution
metadata:
  name: test-invalid
spec:
  distribution:
    name: starter
    image: "registry/image:tag"
YAML

Review focus

  • Does api/v1alpha2/llamastackdistribution_types.go feel right for Kubernetes users?
  • Is inference: [{provider: vllm}] (always-list syntax) acceptable?
  • Are the CEL error messages clear and actionable?
  • Is the secretRefs map[string]SecretKeyRef design intuitive?

Task progress

This project uses beads for task tracking:

bd backup restore       # restore state from git
bd list                 # all tasks with status
bd list --status closed # Phase 1: 13/13 complete
bd list --status open   # Phase 2-12: 78 tasks remaining

Current: Phase 1 complete (13/13). Waiting for review before Phase 2 (config generation engine).

Spec reference

Full spec: specs/002-operator-generated-config/spec.md (merged in #263)
Review guide: specs/002-operator-generated-config/REVIEW.md

Assisted-by: 🤖 Claude Code

rhuss added 9 commits March 9, 2026 19:08
This specification defines the v1alpha2 API for the LlamaStackDistribution
CRD, enabling operator-generated server configuration. Key features:

- New spec fields: providers, resources, storage, networking, workload
- Config generation from high-level abstracted spec
- Base config extraction from OCI image labels
- Polymorphic field support (single object or list)
- Backward compatibility via conversion webhook
- Integration with spec 001 external providers

Includes implementation plan (5 phases) and detailed task breakdown
(33 tasks with dependencies).

Assisted-by: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
Adds a condensed 2-page summary of the v1alpha2 spec for easier team
review. Includes before/after example, key design decisions, and
specific questions for reviewers.

Assisted-by: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
Assisted-by: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
Apply fixes from consistency analysis across spec, plan, and tasks:

- Fix expose:{} handling in plan (ShouldExposeRoute returned false)
- Align autoscaling field name to targetCPUUtilizationPercentage
- Fix hub/spoke conversion pattern (Hub() marker, not ConvertTo)
- Restructure plan section 2.2 for phased base config approach
- Add v1alpha2 printer columns (Phase, Providers, Available, Age)
- Add contracts, data-model, quickstart, and research artifacts
- Update review_summary with changelog for reviewers

Assisted-By: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
…findings

Major changes to the v1alpha2 operator-generated config specification:

- Replace polymorphic JSON types with typed []ProviderConfig slices,
  enabling kubebuilder validation and CEL rules (FR-004)
- Add explicit secretRefs field on ProviderConfig, eliminating heuristic
  secret detection that caused false positives (FR-005)
- Fix CEL validation feasibility: FR-071/FR-072 now work because
  providers are typed slices, not opaque JSON
- Add missing CEL rules for TLS, Redis, and Postgres conditional
  field requirements (FR-079, FR-079a-c)
- Add webhook validation for distribution name (FR-079d)
- Change disabled+provider conflict from warning to error (OQ-002)
- Clarify merge semantics with concrete before/after examples in
  config-generation contract
- Add ConfigMap cleanup requirement (FR-025a, retain last 2)
- Add ConfigResolver interface for Phase 2 extensibility (FR-027a1)
- Add Kubernetes Events requirement (NFR-007)
- Update all YAML examples across spec, quickstart, and contracts
- Add 30-minute review recipe in review_summary.md

Assisted-by: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
The []ProviderConfig type annotations were interpreted as YAML flow
sequences by the pre-commit YAML validator. Quoting them preserves
readability while keeping the file valid YAML.

Assisted-by: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
The review summary mentioned providers polymorphism removal but didn't
explicitly list the models and expose changes as separate line items.
Added both to the "Key Changes from PR llamastack#253" table and expanded the
"Decision 1" section to cover all three polymorphic field replacements.

Assisted-by: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
The old name implied "a summary of a review that happened" when the
file is actually "the document that guides reviewers through their
review." REVIEW.md follows the convention of README.md, CHANGELOG.md,
and CONTRIBUTING.md, making it immediately recognizable as the entry
point for reviewers.

Assisted-by: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
Add the complete v1alpha2 API types for LlamaStackDistribution:

- ProvidersSpec with typed []ProviderConfig slices (no polymorphic JSON)
- ProviderConfig with explicit secretRefs field (no heuristic detection)
- ResourcesSpec with typed []ModelConfig (name required, provider optional)
- StateStorageSpec with KV (sqlite/redis) and SQL (sqlite/postgres) backends
- NetworkingSpec with typed ExposeConfig (enabled + hostname fields)
- WorkloadSpec consolidating all deployment settings
- Status types: ResolvedDistributionStatus, ConfigGenerationStatus
- 4 condition types with reason constants and event types

CEL validation rules (15 total):
- 4 mutual exclusivity rules (providers/resources/storage/disabled vs overrideConfig)
- 5 provider ID required rules (per API type, when list > 1)
- 2 distribution name/image rules
- TLS secretName required when enabled
- Redis endpoint required, Postgres connectionString required

Addresses FR-001 through FR-014, FR-070-072, FR-079, FR-079a-c from spec.

Assisted-by: 🤖 Claude Code
Signed-off-by: Roland Huß <rhuss@redhat.com>
Copy link
Copy Markdown
Collaborator

@eoinfennessy eoinfennessy left a comment

Choose a reason for hiding this comment

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

Generally looks good. I added lots and lots of suggestions to strengthen our validations.

Also one question about why we use a pointer to a boolean for ExposeConfig.Enabled

Comment on lines +183 to +206
// Inference providers (e.g., vllm, ollama).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Inference []ProviderConfig `json:"inference,omitempty"`

// Safety providers (e.g., llama-guard).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Safety []ProviderConfig `json:"safety,omitempty"`

// VectorIo providers (e.g., pgvector, chromadb).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
VectorIo []ProviderConfig `json:"vectorIo,omitempty"`

// ToolRuntime providers (e.g., brave-search, rag-runtime).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
ToolRuntime []ProviderConfig `json:"toolRuntime,omitempty"`

// Telemetry providers (e.g., opentelemetry).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Telemetry []ProviderConfig `json:"telemetry,omitempty"`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We can also add validation here to ensure that when a list is provided it must have one or more items. This ensures empty lists cannot be provided explicitly:

Suggested change
// Inference providers (e.g., vllm, ollama).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Inference []ProviderConfig `json:"inference,omitempty"`
// Safety providers (e.g., llama-guard).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Safety []ProviderConfig `json:"safety,omitempty"`
// VectorIo providers (e.g., pgvector, chromadb).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
VectorIo []ProviderConfig `json:"vectorIo,omitempty"`
// ToolRuntime providers (e.g., brave-search, rag-runtime).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
ToolRuntime []ProviderConfig `json:"toolRuntime,omitempty"`
// Telemetry providers (e.g., opentelemetry).
// +optional
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Telemetry []ProviderConfig `json:"telemetry,omitempty"`
// Inference providers (e.g., vllm, ollama).
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Inference []ProviderConfig `json:"inference,omitempty"`
// Safety providers (e.g., llama-guard).
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Safety []ProviderConfig `json:"safety,omitempty"`
// VectorIo providers (e.g., pgvector, chromadb).
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
VectorIo []ProviderConfig `json:"vectorIo,omitempty"`
// ToolRuntime providers (e.g., brave-search, rag-runtime).
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
ToolRuntime []ProviderConfig `json:"toolRuntime,omitempty"`
// Telemetry providers (e.g., opentelemetry).
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:XValidation:rule="self.size() <= 1 || self.all(p, has(p.id))",message="each provider must have an explicit id when multiple providers are specified"
Telemetry []ProviderConfig `json:"telemetry,omitempty"`

Comment on lines +211 to +215
// ID is a unique provider identifier. Required when multiple providers are
// specified for the same API type. Auto-generated from provider field for
// single-element lists.
// +optional
ID string `json:"id,omitempty"`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For optional strings, we can add CEL validation to ensure they are not empty if provided:

// +kubebuilder:validation:XValidation:rule="!has(self.id) || self.id != ''",message="id must not be empty if specified"

WDYT?

Comment on lines +232 to +235
// SecretRefs are named secret references for provider-specific connection fields.
// Each key becomes the env var field suffix: LLSD_<PROVIDER_ID>_<KEY>.
// +optional
SecretRefs map[string]SecretKeyRef `json:"secretRefs,omitempty"`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similar validation here to gaurd against empty maps when provided:

Suggested change
// SecretRefs are named secret references for provider-specific connection fields.
// Each key becomes the env var field suffix: LLSD_<PROVIDER_ID>_<KEY>.
// +optional
SecretRefs map[string]SecretKeyRef `json:"secretRefs,omitempty"`
// SecretRefs are named secret references for provider-specific connection fields.
// Each key becomes the env var field suffix: LLSD_<PROVIDER_ID>_<KEY>.
// +optional
// +kubebuilder:validation:MinProperties=1
SecretRefs map[string]SecretKeyRef `json:"secretRefs,omitempty"`

Comment on lines +261 to +271
// Models to register with inference providers.
// +optional
Models []ModelConfig `json:"models,omitempty"`

// Tools are tool group names to register with the toolRuntime provider.
// +optional
Tools []string `json:"tools,omitempty"`

// Shields are safety shield names to register with the safety provider.
// +optional
Shields []string `json:"shields,omitempty"`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similarly to above, MinItems gaurds against explicitly empty lists. Using MinLength also gaurds against empty strings in the Tools and Shields slices.

Suggested change
// Models to register with inference providers.
// +optional
Models []ModelConfig `json:"models,omitempty"`
// Tools are tool group names to register with the toolRuntime provider.
// +optional
Tools []string `json:"tools,omitempty"`
// Shields are safety shield names to register with the safety provider.
// +optional
Shields []string `json:"shields,omitempty"`
// Models to register with inference providers.
// +optional
// +kubebuilder:validation:MinItems=1
Models []ModelConfig `json:"models,omitempty"`
// Tools are tool group names to register with the toolRuntime provider.
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:items:MinLength=1
Tools []string `json:"tools,omitempty"`
// Shields are safety shield names to register with the safety provider.
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:items:MinLength=1
Shields []string `json:"shields,omitempty"`

Comment on lines +274 to +297
// ModelConfig defines a model to register.
type ModelConfig struct {
// Name is the model identifier (e.g., llama3.2-8b).
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinLength=1
Name string `json:"name"`

// Provider is the provider ID to register this model with.
// Defaults to the first inference provider when omitted.
// +optional
Provider string `json:"provider,omitempty"`

// ContextLength is the model context window size.
// +optional
ContextLength *int `json:"contextLength,omitempty"`

// ModelType is the model type classification.
// +optional
ModelType string `json:"modelType,omitempty"`

// Quantization is the quantization method used.
// +optional
Quantization string `json:"quantization,omitempty"`
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Like above, ensure optional strings are not empty if provided.

Suggested change
// ModelConfig defines a model to register.
type ModelConfig struct {
// Name is the model identifier (e.g., llama3.2-8b).
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinLength=1
Name string `json:"name"`
// Provider is the provider ID to register this model with.
// Defaults to the first inference provider when omitted.
// +optional
Provider string `json:"provider,omitempty"`
// ContextLength is the model context window size.
// +optional
ContextLength *int `json:"contextLength,omitempty"`
// ModelType is the model type classification.
// +optional
ModelType string `json:"modelType,omitempty"`
// Quantization is the quantization method used.
// +optional
Quantization string `json:"quantization,omitempty"`
}
// ModelConfig defines a model to register.
// +kubebuilder:validation:XValidation:rule="!has(self.provider) || self.provider != ''",message="provider must not be empty if specified"
// +kubebuilder:validation:XValidation:rule="!has(self.modelType) || self.modelType != ''",message="modelType must not be empty if specified"
// +kubebuilder:validation:XValidation:rule="!has(self.quantization) || self.quantization != ''",message="quantization must not be empty if specified"
type ModelConfig struct {
// Name is the model identifier (e.g., llama3.2-8b).
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinLength=1
Name string `json:"name"`
// Provider is the provider ID to register this model with.
// Defaults to the first inference provider when omitted.
// +optional
Provider string `json:"provider,omitempty"`
// ContextLength is the model context window size.
// +optional
ContextLength *int `json:"contextLength,omitempty"`
// ModelType is the model type classification.
// +optional
ModelType string `json:"modelType,omitempty"`
// Quantization is the quantization method used.
// +optional
Quantization string `json:"quantization,omitempty"`
}

Comment on lines +482 to +493
// PVCStorageSpec configures persistent volume storage.
type PVCStorageSpec struct {
// Size is the PVC size (e.g., "10Gi").
// +optional
Size *resource.Quantity `json:"size,omitempty"`

// MountPath is where storage is mounted in the container.
// +optional
// +kubebuilder:default:="/.llama"
MountPath string `json:"mountPath,omitempty"`
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ensure MountPath is not empty when provided

Suggested change
// PVCStorageSpec configures persistent volume storage.
type PVCStorageSpec struct {
// Size is the PVC size (e.g., "10Gi").
// +optional
Size *resource.Quantity `json:"size,omitempty"`
// MountPath is where storage is mounted in the container.
// +optional
// +kubebuilder:default:="/.llama"
MountPath string `json:"mountPath,omitempty"`
}
// PVCStorageSpec configures persistent volume storage.
// +kubebuilder:validation:XValidation:rule="!has(self.mountPath) || self.mountPath != ''",message="mountPath must not be empty if specified"
type PVCStorageSpec struct {
// Size is the PVC size (e.g., "10Gi").
// +optional
Size *resource.Quantity `json:"size,omitempty"`
// MountPath is where storage is mounted in the container.
// +optional
// +kubebuilder:default:="/.llama"
MountPath string `json:"mountPath,omitempty"`
}

Comment on lines +494 to +503
// PodDisruptionBudgetSpec defines voluntary disruption controls.
type PodDisruptionBudgetSpec struct {
// MinAvailable is the minimum number of pods that must remain available.
// +optional
MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty"`

// MaxUnavailable is the maximum number of pods that can be disrupted.
// +optional
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ensure one and only one field is provided. (mutually exclusive, similar to actual k8s PDBs)

Suggested change
// PodDisruptionBudgetSpec defines voluntary disruption controls.
type PodDisruptionBudgetSpec struct {
// MinAvailable is the minimum number of pods that must remain available.
// +optional
MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty"`
// MaxUnavailable is the maximum number of pods that can be disrupted.
// +optional
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`
}
// PodDisruptionBudgetSpec defines voluntary disruption controls.
// +kubebuilder:validation:XValidation:rule="has(self.minAvailable) || has(self.maxUnavailable)",message="at least one of minAvailable or maxUnavailable must be specified"
// +kubebuilder:validation:XValidation:rule="!(has(self.minAvailable) && has(self.maxUnavailable))",message="minAvailable and maxUnavailable are mutually exclusive"
type PodDisruptionBudgetSpec struct {
// MinAvailable is the minimum number of pods that must remain available.
// +optional
MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty"`
// MaxUnavailable is the maximum number of pods that can be disrupted.
// +optional
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`
}

Comment on lines +505 to +530
// WorkloadOverrides provides low-level Pod customization.
type WorkloadOverrides struct {
// ServiceAccountName overrides the ServiceAccount.
// +optional
ServiceAccountName string `json:"serviceAccountName,omitempty"`

// Env adds environment variables to the container.
// +optional
Env []corev1.EnvVar `json:"env,omitempty"`

// Command overrides the container entrypoint.
// +optional
Command []string `json:"command,omitempty"`

// Args overrides the container arguments.
// +optional
Args []string `json:"args,omitempty"`

// Volumes adds volumes to the Pod.
// +optional
Volumes []corev1.Volume `json:"volumes,omitempty"`

// VolumeMounts adds volume mounts to the container.
// +optional
VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ensure all fields are non-empty if provided. Ensure no empty strings in lists.

Suggested change
// WorkloadOverrides provides low-level Pod customization.
type WorkloadOverrides struct {
// ServiceAccountName overrides the ServiceAccount.
// +optional
ServiceAccountName string `json:"serviceAccountName,omitempty"`
// Env adds environment variables to the container.
// +optional
Env []corev1.EnvVar `json:"env,omitempty"`
// Command overrides the container entrypoint.
// +optional
Command []string `json:"command,omitempty"`
// Args overrides the container arguments.
// +optional
Args []string `json:"args,omitempty"`
// Volumes adds volumes to the Pod.
// +optional
Volumes []corev1.Volume `json:"volumes,omitempty"`
// VolumeMounts adds volume mounts to the container.
// +optional
VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
}
// WorkloadOverrides provides low-level Pod customization.
// +kubebuilder:validation:XValidation:rule="!has(self.serviceAccountName) || self.serviceAccountName != ''",message="serviceAccountName must not be empty if specified"
type WorkloadOverrides struct {
// +optional
ServiceAccountName string `json:"serviceAccountName,omitempty"`
// +optional
// +kubebuilder:validation:MinItems=1
Env []corev1.EnvVar `json:"env,omitempty"`
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:items:MinLength=1
Command []string `json:"command,omitempty"`
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:items:MinLength=1
Args []string `json:"args,omitempty"`
// +optional
// +kubebuilder:validation:MinItems=1
Volumes []corev1.Volume `json:"volumes,omitempty"`
// +optional
// +kubebuilder:validation:MinItems=1
VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
}

Comment on lines +534 to +539
// ExternalProvidersSpec configures external provider injection.
type ExternalProvidersSpec struct {
// Inference external providers.
// +optional
Inference []ExternalProviderConfig `json:"inference,omitempty"`
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ensure Inference slice is not empty if provided.

Suggested change
// ExternalProvidersSpec configures external provider injection.
type ExternalProvidersSpec struct {
// Inference external providers.
// +optional
Inference []ExternalProviderConfig `json:"inference,omitempty"`
}
// ExternalProvidersSpec configures external provider injection.
type ExternalProvidersSpec struct {
// Inference external providers.
// +optional
// +kubebuilder:validation:items:MinLength=1
Inference []ExternalProviderConfig `json:"inference,omitempty"`
}

Comment on lines +541 to +550
// ExternalProviderConfig defines an external provider sidecar.
type ExternalProviderConfig struct {
// ProviderID is the unique identifier for this provider.
// +kubebuilder:validation:Required
ProviderID string `json:"providerId"`

// Image is the container image for the provider sidecar.
// +kubebuilder:validation:Required
Image string `json:"image"`
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ensure all fields are non-empty strings.

Suggested change
// ExternalProviderConfig defines an external provider sidecar.
type ExternalProviderConfig struct {
// ProviderID is the unique identifier for this provider.
// +kubebuilder:validation:Required
ProviderID string `json:"providerId"`
// Image is the container image for the provider sidecar.
// +kubebuilder:validation:Required
Image string `json:"image"`
}
// ExternalProviderConfig defines an external provider sidecar.
type ExternalProviderConfig struct {
// ProviderID is the unique identifier for this provider.
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinLength=1
ProviderID string `json:"providerId"`
// Image is the container image for the provider sidecar.
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinLength=1
Image string `json:"image"`
}

rhuss added a commit to rhuss/llama-stack-k8s-operator that referenced this pull request Mar 11, 2026
…alidation

Incorporates all review feedback from @eoinfennessy on PR llamastack#264:

- Add MinItems=1 on all optional slices to reject explicitly empty lists
  (provider slices, models, tools, shields, env, command, args, volumes,
  volumeMounts, topologySpreadConstraints, configMapKeys, namespaces, labels)
- Add MinProperties=1 on secretRefs map
- Add MinLength=1 on required string fields (provider, name, configMapName,
  providerId, image, secretKeyRef name/key)
- Add CEL non-empty guards on optional strings (id, provider, modelType,
  quantization, hostname, serviceAccountName, mountPath, configMapNamespace)
- Add reverse validation on storage types: endpoint/password only valid for
  redis, connectionString only valid for postgres
- Add TLS reverse validation: secretName and caBundle only valid when enabled
- Add port range validation (1-65535)
- Add replicas Minimum=0
- Add autoscaling: minReplicas/maxReplicas Minimum=1, percentage range 1-100,
  CEL rule maxReplicas >= minReplicas
- Add PDB mutual exclusivity: minAvailable and maxUnavailable cannot both be
  set, at least one required
- Document *bool semantics on ExposeConfig.Enabled (nil vs false vs true)

Assisted-By: 🤖 Claude Code

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rhuss
Copy link
Copy Markdown
Collaborator Author

rhuss commented Mar 11, 2026

Closing in favor of #266, which includes the v1alpha2 types with all review feedback from this PR incorporated (see #266 (comment) for the full list of changes).

@rhuss rhuss closed this Mar 11, 2026
rhuss added a commit to rhuss/llama-stack-k8s-operator that referenced this pull request Mar 11, 2026
…alidation

Incorporates all review feedback from @eoinfennessy on PR llamastack#264:

- Add MinItems=1 on all optional slices to reject explicitly empty lists
  (provider slices, models, tools, shields, env, command, args, volumes,
  volumeMounts, topologySpreadConstraints, configMapKeys, namespaces, labels)
- Add MinProperties=1 on secretRefs map
- Add MinLength=1 on required string fields (provider, name, configMapName,
  providerId, image, secretKeyRef name/key)
- Add CEL non-empty guards on optional strings (id, provider, modelType,
  quantization, hostname, serviceAccountName, mountPath, configMapNamespace)
- Add reverse validation on storage types: endpoint/password only valid for
  redis, connectionString only valid for postgres
- Add TLS reverse validation: secretName and caBundle only valid when enabled
- Add port range validation (1-65535)
- Add replicas Minimum=0
- Add autoscaling: minReplicas/maxReplicas Minimum=1, percentage range 1-100,
  CEL rule maxReplicas >= minReplicas
- Add PDB mutual exclusivity: minAvailable and maxUnavailable cannot both be
  set, at least one required
- Document *bool semantics on ExposeConfig.Enabled (nil vs false vs true)

Assisted-By: 🤖 Claude Code

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Roland Huß <rhuss@redhat.com>
rhuss added a commit to rhuss/llama-stack-k8s-operator that referenced this pull request Mar 11, 2026
…alidation

Incorporates all review feedback from @eoinfennessy on PR llamastack#264:

- Add MinItems=1 on all optional slices to reject explicitly empty lists
  (provider slices, models, tools, shields, env, command, args, volumes,
  volumeMounts, topologySpreadConstraints, configMapKeys, namespaces, labels)
- Add MinProperties=1 on secretRefs map
- Add MinLength=1 on required string fields (provider, name, configMapName,
  providerId, image, secretKeyRef name/key)
- Add CEL non-empty guards on optional strings (id, provider, modelType,
  quantization, hostname, serviceAccountName, mountPath, configMapNamespace)
- Add reverse validation on storage types: endpoint/password only valid for
  redis, connectionString only valid for postgres
- Add TLS reverse validation: secretName and caBundle only valid when enabled
- Add port range validation (1-65535)
- Add replicas Minimum=0
- Add autoscaling: minReplicas/maxReplicas Minimum=1, percentage range 1-100,
  CEL rule maxReplicas >= minReplicas
- Add PDB mutual exclusivity: minAvailable and maxUnavailable cannot both be
  set, at least one required
- Document *bool semantics on ExposeConfig.Enabled (nil vs false vs true)

Assisted-By: 🤖 Claude Code

Signed-off-by: Roland Huß <rhuss@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants