feat: add validation for ReplicatedJobPatch name#3515
Conversation
Signed-off-by: Pulkit Agrawal <97938993+pulkit-999@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
🎉 Welcome to the Kubeflow Trainer! 🎉 Thanks for opening your first PR! We're happy to have you as part of our community 🚀 Here's what happens next:
Join the community:
Feel free to ask questions in the comments if you need any help or clarification! |
There was a problem hiding this comment.
Pull request overview
Adds a CEL XValidation rule on ReplicatedJobPatch.name to enforce an RFC 1035-style DNS label pattern, regenerates the CRD manifests, and adds integration tests covering valid and invalid names.
Changes:
- Add
XValidationrule with regex^[a-z0-9]([-a-z0-9]*[a-z0-9])?$and message onReplicatedJobPatch.Name. - Regenerate
x-kubernetes-validationsentries in both base and Helm chart CRDs. - Add positive and negative integration test cases for replicated job name validation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/apis/trainer/v1alpha1/trainjob_types.go | Adds the CEL XValidation marker to ReplicatedJobPatch.Name. |
| manifests/base/crds/trainer.kubeflow.org_trainjobs.yaml | Generated CRD update reflecting the new validation. |
| charts/kubeflow-trainer/crds/trainer.kubeflow.org_trainjobs.yaml | Generated Helm chart CRD update reflecting the new validation. |
| test/integration/webhooks/trainjob_test.go | Adds integration test entries for invalid/valid replicated job names. |
Signed-off-by: Pulkit Agrawal <97938993+pulkit-999@users.noreply.github.com>
This prevents invalid replicated job identifiers from being accepted into the API and aligns validation behavior with existing Kubernetes CRD validation conventions.