diff --git a/charts/kubeflow-trainer/crds/trainer.kubeflow.org_trainjobs.yaml b/charts/kubeflow-trainer/crds/trainer.kubeflow.org_trainjobs.yaml index 8fc3e30387..59bd1bbe51 100644 --- a/charts/kubeflow-trainer/crds/trainer.kubeflow.org_trainjobs.yaml +++ b/charts/kubeflow-trainer/crds/trainer.kubeflow.org_trainjobs.yaml @@ -3081,6 +3081,11 @@ spec: x-kubernetes-list-type: map type: object type: object + x-kubernetes-validations: + - message: metadata.name must match RFC 1035 DNS label format + rule: self.metadata.name.matches('^[a-z]([-a-z0-9]*[a-z0-9])?$') + - message: metadata.name must be no more than 63 characters + rule: size(self.metadata.name) <= 63 served: true storage: true subresources: diff --git a/manifests/base/crds/trainer.kubeflow.org_trainjobs.yaml b/manifests/base/crds/trainer.kubeflow.org_trainjobs.yaml index 8fc3e30387..59bd1bbe51 100644 --- a/manifests/base/crds/trainer.kubeflow.org_trainjobs.yaml +++ b/manifests/base/crds/trainer.kubeflow.org_trainjobs.yaml @@ -3081,6 +3081,11 @@ spec: x-kubernetes-list-type: map type: object type: object + x-kubernetes-validations: + - message: metadata.name must match RFC 1035 DNS label format + rule: self.metadata.name.matches('^[a-z]([-a-z0-9]*[a-z0-9])?$') + - message: metadata.name must be no more than 63 characters + rule: size(self.metadata.name) <= 63 served: true storage: true subresources: diff --git a/pkg/apis/trainer/v1alpha1/trainjob_types.go b/pkg/apis/trainer/v1alpha1/trainjob_types.go index 6d0f9e122a..e1360c4f35 100644 --- a/pkg/apis/trainer/v1alpha1/trainjob_types.go +++ b/pkg/apis/trainer/v1alpha1/trainjob_types.go @@ -34,6 +34,8 @@ const ( // +kubebuilder:storageversion // +kubebuilder:printcolumn:name="State",type=string,JSONPath=`.status.conditions[-1:].type` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` +// +kubebuilder:validation:XValidation:rule="self.metadata.name.matches('^[a-z]([-a-z0-9]*[a-z0-9])?$')", message="metadata.name must match RFC 1035 DNS label format" +// +kubebuilder:validation:XValidation:rule="size(self.metadata.name) <= 63", message="metadata.name must be no more than 63 characters" // TrainJob represents configuration of a training job. type TrainJob struct { diff --git a/pkg/webhooks/trainjob_webhook.go b/pkg/webhooks/trainjob_webhook.go index 7f4d2dc9e6..ae4e64b92a 100644 --- a/pkg/webhooks/trainjob_webhook.go +++ b/pkg/webhooks/trainjob_webhook.go @@ -21,8 +21,6 @@ import ( "fmt" apiruntime "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -52,20 +50,13 @@ func (w *TrainJobWebhook) ValidateCreate(ctx context.Context, obj apiruntime.Obj log := ctrl.LoggerFrom(ctx).WithName("trainJob-webhook") log.V(5).Info("Validating create", "TrainJob", klog.KObj(trainJob)) - allErrs := field.ErrorList{} - - // Check RFC 1035 name validation errors - for _, err := range validation.IsDNS1035Label(trainJob.Name) { - allErrs = append(allErrs, field.Invalid(field.NewPath("metadata", "name"), trainJob.Name, err)) - } - runtimeRefGK := runtime.RuntimeRefToRuntimeRegistryKey(trainJob.Spec.RuntimeRef) runtime, ok := w.runtimes[runtimeRefGK] if !ok { return nil, fmt.Errorf("unsupported runtime: %s", runtimeRefGK) } warnings, errors := runtime.ValidateObjects(ctx, nil, trainJob) - return warnings, append(allErrs, errors...).ToAggregate() + return warnings, errors.ToAggregate() } func (w *TrainJobWebhook) ValidateUpdate(ctx context.Context, oldObj apiruntime.Object, newObj apiruntime.Object) (admission.Warnings, error) { diff --git a/pkg/webhooks/trainjob_webhook_test.go b/pkg/webhooks/trainjob_webhook_test.go index 9e92e3520d..23f8c6b760 100644 --- a/pkg/webhooks/trainjob_webhook_test.go +++ b/pkg/webhooks/trainjob_webhook_test.go @@ -45,19 +45,6 @@ func TestValidateCreate(t *testing.T) { wantError: nil, wantWarnings: nil, }, - "invalid trainjob name with uppercase letters": { - obj: testingutil.MakeTrainJobWrapper("default", "Invalid-job-name"). - RuntimeRef(trainer.SchemeGroupVersion.WithKind(trainer.ClusterTrainingRuntimeKind), "test-runtime"). - Obj(), - wantError: field.ErrorList{ - &field.Error{ - Type: field.ErrorTypeInvalid, - Field: "metadata.name", - BadValue: "Invalid-job-name", - }, - }, - wantWarnings: nil, - }, "unsupported runtime": { obj: testingutil.MakeTrainJobWrapper("default", "valid-job-name"). RuntimeRef(trainer.SchemeGroupVersion.WithKind(trainer.ClusterTrainingRuntimeKind), "unsupported-runtime"). diff --git a/test/integration/webhooks/trainjob_test.go b/test/integration/webhooks/trainjob_test.go index 3e41e0e425..fca92ea99a 100644 --- a/test/integration/webhooks/trainjob_test.go +++ b/test/integration/webhooks/trainjob_test.go @@ -218,6 +218,54 @@ var _ = ginkgo.Describe("TrainJob Webhook", ginkgo.Ordered, func() { }, testingutil.BeForbiddenError()), ) + ginkgo.DescribeTable("RFC1035-compliant TrainJob name validation", func(trainJob func() *trainer.TrainJob, errorMatcher gomega.OmegaMatcher) { + gomega.Expect(k8sClient.Create(ctx, trainJob())).Should(errorMatcher) + }, + ginkgo.Entry("Should succeed to create TrainJob with valid RFC1035-compliant name", + func() *trainer.TrainJob { + return testingutil.MakeTrainJobWrapper(ns.Name, "valid-job-name"). + RuntimeRef(trainer.SchemeGroupVersion.WithKind(trainer.ClusterTrainingRuntimeKind), runtimeName). + Obj() + }, + gomega.Succeed()), + ginkgo.Entry("Should succeed to create TrainJob with name exactly 63 characters", + func() *trainer.TrainJob { + return testingutil.MakeTrainJobWrapper(ns.Name, + "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk"). // 63 chars + RuntimeRef(trainer.SchemeGroupVersion.WithKind(trainer.ClusterTrainingRuntimeKind), runtimeName). + Obj() + }, + gomega.Succeed()), + ginkgo.Entry("Should fail to create TrainJob with uppercase letters in name", + func() *trainer.TrainJob { + return testingutil.MakeTrainJobWrapper(ns.Name, "Invalid-job-name"). + RuntimeRef(trainer.SchemeGroupVersion.WithKind(trainer.ClusterTrainingRuntimeKind), runtimeName). + Obj() + }, + testingutil.BeInvalidError()), + ginkgo.Entry("Should fail to create TrainJob starting with a digit", + func() *trainer.TrainJob { + return testingutil.MakeTrainJobWrapper(ns.Name, "1jobname"). + RuntimeRef(trainer.SchemeGroupVersion.WithKind(trainer.ClusterTrainingRuntimeKind), runtimeName). + Obj() + }, + testingutil.BeInvalidError()), + ginkgo.Entry("Should fail to create TrainJob ending with a hyphen", + func() *trainer.TrainJob { + return testingutil.MakeTrainJobWrapper(ns.Name, "jobname-"). + RuntimeRef(trainer.SchemeGroupVersion.WithKind(trainer.ClusterTrainingRuntimeKind), runtimeName). + Obj() + }, + testingutil.BeInvalidError()), + ginkgo.Entry("Should fail to create TrainJob with name exceeding 63 characters", + func() *trainer.TrainJob { + return testingutil.MakeTrainJobWrapper(ns.Name, + "this-name-is-way-too-long-for-a-rfc1035-label-and-should-fail-validation"). + RuntimeRef(trainer.SchemeGroupVersion.WithKind(trainer.ClusterTrainingRuntimeKind), runtimeName). + Obj() + }, + testingutil.BeInvalidError()), + ) }) })