V1: Introduce the CustomResourceValidationExpressions feature (CEL validation)

[tenzen-y]

/kind feature

Since k8s v1.25, the CustomResourceValidationExpressions feature to validate CRDs using Common Expression Language (CEL) without webhook servers is enabled by default.

Since this feature helps to find CRD validation errors for users, we need to introduce that feature once we stop supporting K8s v1.24.

For example, if the container name of replicaSpec is invalid, for now, we only output that error to controller logs; once we introduce CEL validation, we can return that error to end-users.

https://github.com/kubeflow/training-operator/blob/69813fbc37f33857e59044d14caa7e2765851d98/pkg/apis/kubeflow.org/v1/tensorflow_validation.go#L69-L74

[tenzen-y]

/assign

[tenzen-y]

Through my investigation, I found that we can not replace all validations with CEL validation due to exceeding cost budget.

For example, we can replace the below validation

https://github.com/kubeflow/training-operator/blob/4dd0d0909210edc7e1a31a5987cb32dc834d682b/pkg/apis/kubeflow.org/v1/pytorch_validation.go#L69-L72

with the below CEL validation:

// ReplicaSpec is a description of the replica
type ReplicaSpec struct {
...
	// Template is the object that describes the pod that
	// will be created for this replica. RestartPolicy in PodTemplateSpec
	// will be overide by RestartPolicy in ReplicaSpec
	// +kubebuilder:validation:XValidation:rule="has(self.spec.containers) && self.spec.containers.all(c, has(c.image) && size(c.image) > 0)",message=""
	Template v1.PodTemplateSpec `json:"template,omitempty"`
...

Forbidden: contributed to estimated rule cost total exceeding cost limit for entire OpenAPIv3 schema, spec.validation.openAPIV3Schema: Forbidden: x-kubernetes-validations estimated rule cost total for entire OpenAPIv3 schema exceeds budget by factor of more than 100x (try simplifying the rule, or adding maxItems, maxProperties, and maxLength where arrays, maps, and strings are declared)]

So, I think that we need to introduce the webhook validation instead of replacing the current validation with CEL validation.

@kubeflow/wg-training-leads WDYT?

[johnugeorge]

I remember, we had a discussion regarding webhooks. The arguments were deployment easiness(without web hooks) vs clean validation(with webhooks)

[tenzen-y]

The arguments were deployment easiness

@johnugeorge Are they concerned about certs for the webhook?

[johnugeorge]

Yes. That was the major raod block from what I remember

[tenzen-y]

@johnugeorge I think we can remove webhook installation barriers once we introduce cert generation logic similar to katib. WDYT?

Actually, I have experience implementing internal cert generation logic in 2 components (kubeflow/katib, kubernetes-sigs/kueue).

[tenzen-y]

Maybe, we generalize the katib cert generator, and then just import the cert-generator to the training-operator.

https://github.com/kubeflow/katib/blob/eac65bf8e31fe4d3a2c4dbb20be30d600c086157/cmd/katib-controller/v1beta1/main.go#L137-L145

https://github.com/kubeflow/katib/tree/eac65bf8e31fe4d3a2c4dbb20be30d600c086157/pkg/certgenerator/v1beta1

cc: @andreyvelich