From 276c7e860dea4ff3366b2795700345399e599bb9 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 16 Dec 2022 12:15:52 -0800 Subject: [PATCH 1/7] Change DNS Label validation to check for dots This will produce a better error message for the more common case of using a DNS subdomain where a label is needed. --- pkg/apis/core/validation/validation_test.go | 20 +++++++++++++++---- .../pkg/util/validation/validation.go | 8 +++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 56cfdd09a88..c11184e9dca 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -3230,11 +3230,23 @@ func TestValidateVolumes(t *testing.T) { }}, }, { - name: "name not a DNS label", + name: "name has dots", vol: core.Volume{ Name: "a.b.c", VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}}, }, + errs: []verr{{ + etype: field.ErrorTypeInvalid, + field: "name", + detail: "must not contain dots", + }}, + }, + { + name: "name not a DNS label", + vol: core.Volume{ + Name: "Not a DNS label!", + VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}}, + }, errs: []verr{{ etype: field.ErrorTypeInvalid, field: "name", @@ -5042,13 +5054,13 @@ func TestValidateVolumes(t *testing.T) { for i, err := range errs { expErr := tc.errs[i] if err.Type != expErr.etype { - t.Errorf("unexpected error type: got %v, want %v", expErr.etype, err.Type) + t.Errorf("unexpected error type:\n\twant: %q\n\t got: %q", expErr.etype, err.Type) } if !strings.HasSuffix(err.Field, "."+expErr.field) { - t.Errorf("unexpected error field: got %v, want %v", expErr.field, err.Field) + t.Errorf("unexpected error field:\n\twant: %q\n\t got: %q", expErr.field, err.Field) } if !strings.Contains(err.Detail, expErr.detail) { - t.Errorf("unexpected error detail: got %v, want %v", expErr.detail, err.Detail) + t.Errorf("unexpected error detail:\n\twant: %q\n\t got: %q", expErr.detail, err.Detail) } } }) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go index e767092dd87..0b8a6cb354a 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go @@ -191,7 +191,13 @@ func IsDNS1123Label(value string) []string { errs = append(errs, MaxLenError(DNS1123LabelMaxLength)) } if !dns1123LabelRegexp.MatchString(value) { - errs = append(errs, RegexError(dns1123LabelErrMsg, dns1123LabelFmt, "my-name", "123-abc")) + if dns1123SubdomainRegexp.MatchString(value) { + // It was a valid subdomain and not a valid label. Since we + // already checked length, it must be dots. + errs = append(errs, "must not contain dots") + } else { + errs = append(errs, RegexError(dns1123LabelErrMsg, dns1123LabelFmt, "my-name", "123-abc")) + } } return errs } From c555d290c1674e32124ff912533ac85333f6ae44 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 11 Dec 2022 13:31:58 -0800 Subject: [PATCH 2/7] pod: API warn when name is not DNS label --- pkg/registry/core/pod/strategy.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/registry/core/pod/strategy.go b/pkg/registry/core/pod/strategy.go index 4021b4eb4fb..4f48457530f 100644 --- a/pkg/registry/core/pod/strategy.go +++ b/pkg/registry/core/pod/strategy.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilnet "k8s.io/apimachinery/pkg/util/net" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/storage" @@ -44,7 +45,7 @@ import ( podutil "k8s.io/kubernetes/pkg/api/pod" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper/qos" - "k8s.io/kubernetes/pkg/apis/core/validation" + corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/client" proxyutil "k8s.io/kubernetes/pkg/proxy/util" @@ -105,12 +106,18 @@ func (podStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object func (podStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { pod := obj.(*api.Pod) opts := podutil.GetValidationOptionsFromPodSpecAndMeta(&pod.Spec, nil, &pod.ObjectMeta, nil) - return validation.ValidatePodCreate(pod, opts) + return corevalidation.ValidatePodCreate(pod, opts) } // WarningsOnCreate returns warnings for the creation of the given object. func (podStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return podutil.GetWarningsForPod(ctx, obj.(*api.Pod), nil) + newPod := obj.(*api.Pod) + var warnings []string + if msgs := utilvalidation.IsDNS1123Label(newPod.Name); len(msgs) != 0 { + warnings = append(warnings, fmt.Sprintf("metadata.name: this is used in Pod names and hostnames, which can result in surprising behavior; a DNS label is recommended: %v", msgs)) + } + warnings = append(warnings, podutil.GetWarningsForPod(ctx, newPod, nil)...) + return warnings } // Canonicalize normalizes the object after validation. @@ -128,7 +135,7 @@ func (podStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) pod := obj.(*api.Pod) oldPod := old.(*api.Pod) opts := podutil.GetValidationOptionsFromPodSpecAndMeta(&pod.Spec, &oldPod.Spec, &pod.ObjectMeta, &oldPod.ObjectMeta) - return validation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod), opts) + return corevalidation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod), opts) } // WarningsOnUpdate returns warnings for the given update. @@ -213,7 +220,7 @@ func (podStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Ob oldPod := old.(*api.Pod) opts := podutil.GetValidationOptionsFromPodSpecAndMeta(&pod.Spec, &oldPod.Spec, &pod.ObjectMeta, &oldPod.ObjectMeta) - return validation.ValidatePodStatusUpdate(obj.(*api.Pod), old.(*api.Pod), opts) + return corevalidation.ValidatePodStatusUpdate(obj.(*api.Pod), old.(*api.Pod), opts) } // WarningsOnUpdate returns warnings for the given update. @@ -251,7 +258,7 @@ func (podEphemeralContainersStrategy) ValidateUpdate(ctx context.Context, obj, o newPod := obj.(*api.Pod) oldPod := old.(*api.Pod) opts := podutil.GetValidationOptionsFromPodSpecAndMeta(&newPod.Spec, &oldPod.Spec, &newPod.ObjectMeta, &oldPod.ObjectMeta) - return validation.ValidatePodEphemeralContainersUpdate(newPod, oldPod, opts) + return corevalidation.ValidatePodEphemeralContainersUpdate(newPod, oldPod, opts) } // WarningsOnUpdate returns warnings for the given update. From e27cf7509450708efc684a549a37a9078894ae60 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 11 Dec 2022 13:32:21 -0800 Subject: [PATCH 3/7] rc: API warn when name is not DNS label --- .../core/replicationcontroller/strategy.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/registry/core/replicationcontroller/strategy.go b/pkg/registry/core/replicationcontroller/strategy.go index be056a32cfe..cdb32dd5f28 100644 --- a/pkg/registry/core/replicationcontroller/strategy.go +++ b/pkg/registry/core/replicationcontroller/strategy.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" @@ -40,7 +41,7 @@ import ( "k8s.io/kubernetes/pkg/api/pod" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper" - "k8s.io/kubernetes/pkg/apis/core/validation" + corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -122,13 +123,18 @@ func (rcStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) func (rcStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { controller := obj.(*api.ReplicationController) opts := pod.GetValidationOptionsFromPodTemplate(controller.Spec.Template, nil) - return validation.ValidateReplicationController(controller, opts) + return corevalidation.ValidateReplicationController(controller, opts) } // WarningsOnCreate returns warnings for the creation of the given object. func (rcStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { newRC := obj.(*api.ReplicationController) - return pod.GetWarningsForPodTemplate(ctx, field.NewPath("template"), newRC.Spec.Template, nil) + var warnings []string + if msgs := utilvalidation.IsDNS1123Label(newRC.Name); len(msgs) != 0 { + warnings = append(warnings, fmt.Sprintf("metadata.name: this is used in Pod names and hostnames, which can result in surprising behavior; a DNS label is recommended: %v", msgs)) + } + warnings = append(warnings, pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), newRC.Spec.Template, nil)...) + return warnings } // Canonicalize normalizes the object after validation. @@ -147,8 +153,8 @@ func (rcStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) f newRc := obj.(*api.ReplicationController) opts := pod.GetValidationOptionsFromPodTemplate(newRc.Spec.Template, oldRc.Spec.Template) - validationErrorList := validation.ValidateReplicationController(newRc, opts) - updateErrorList := validation.ValidateReplicationControllerUpdate(newRc, oldRc, opts) + validationErrorList := corevalidation.ValidateReplicationController(newRc, opts) + updateErrorList := corevalidation.ValidateReplicationControllerUpdate(newRc, oldRc, opts) errs := append(validationErrorList, updateErrorList...) for key, value := range helper.NonConvertibleFields(oldRc.Annotations) { @@ -240,7 +246,7 @@ func (rcStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.O } func (rcStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateReplicationControllerStatusUpdate(obj.(*api.ReplicationController), old.(*api.ReplicationController)) + return corevalidation.ValidateReplicationControllerStatusUpdate(obj.(*api.ReplicationController), old.(*api.ReplicationController)) } // WarningsOnUpdate returns warnings for the given update. From 820e2fff0d7a114e8429c74ba0741f1c0cbb7bc9 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 11 Dec 2022 13:32:37 -0800 Subject: [PATCH 4/7] rs: API warn when name is not DNS label --- pkg/registry/apps/replicaset/strategy.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/registry/apps/replicaset/strategy.go b/pkg/registry/apps/replicaset/strategy.go index add8c23927d..6d090440dee 100644 --- a/pkg/registry/apps/replicaset/strategy.go +++ b/pkg/registry/apps/replicaset/strategy.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" @@ -39,7 +40,7 @@ import ( "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/pod" "k8s.io/kubernetes/pkg/apis/apps" - "k8s.io/kubernetes/pkg/apis/apps/validation" + appsvalidation "k8s.io/kubernetes/pkg/apis/apps/validation" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -113,13 +114,18 @@ func (rsStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) func (rsStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { rs := obj.(*apps.ReplicaSet) opts := pod.GetValidationOptionsFromPodTemplate(&rs.Spec.Template, nil) - return validation.ValidateReplicaSet(rs, opts) + return appsvalidation.ValidateReplicaSet(rs, opts) } // WarningsOnCreate returns warnings for the creation of the given object. func (rsStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { newRS := obj.(*apps.ReplicaSet) - return pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newRS.Spec.Template, nil) + var warnings []string + if msgs := utilvalidation.IsDNS1123Label(newRS.Name); len(msgs) != 0 { + warnings = append(warnings, fmt.Sprintf("metadata.name: this is used in Pod names and hostnames, which can result in surprising behavior; a DNS label is recommended: %v", msgs)) + } + warnings = append(warnings, pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newRS.Spec.Template, nil)...) + return warnings } // Canonicalize normalizes the object after validation. @@ -138,8 +144,8 @@ func (rsStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) f oldReplicaSet := old.(*apps.ReplicaSet) opts := pod.GetValidationOptionsFromPodTemplate(&newReplicaSet.Spec.Template, &oldReplicaSet.Spec.Template) - allErrs := validation.ValidateReplicaSet(obj.(*apps.ReplicaSet), opts) - allErrs = append(allErrs, validation.ValidateReplicaSetUpdate(newReplicaSet, oldReplicaSet, opts)...) + allErrs := appsvalidation.ValidateReplicaSet(obj.(*apps.ReplicaSet), opts) + allErrs = append(allErrs, appsvalidation.ValidateReplicaSetUpdate(newReplicaSet, oldReplicaSet, opts)...) // Update is not allowed to set Spec.Selector for all groups/versions except extensions/v1beta1. // If RequestInfo is nil, it is better to revert to old behavior (i.e. allow update to set Spec.Selector) @@ -228,7 +234,7 @@ func (rsStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.O } func (rsStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateReplicaSetStatusUpdate(obj.(*apps.ReplicaSet), old.(*apps.ReplicaSet)) + return appsvalidation.ValidateReplicaSetStatusUpdate(obj.(*apps.ReplicaSet), old.(*apps.ReplicaSet)) } // WarningsOnUpdate returns warnings for the given update. From b65cec86eb75892da9b934a65c58ed2f68f590c1 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 11 Dec 2022 13:32:55 -0800 Subject: [PATCH 5/7] deployment: API warn when name is not DNS label --- pkg/registry/apps/deployment/strategy.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/pkg/registry/apps/deployment/strategy.go b/pkg/registry/apps/deployment/strategy.go index 71938f3280f..b3e447c302e 100644 --- a/pkg/registry/apps/deployment/strategy.go +++ b/pkg/registry/apps/deployment/strategy.go @@ -18,6 +18,7 @@ package deployment import ( "context" + "fmt" appsv1beta1 "k8s.io/api/apps/v1beta1" extensionsv1beta1 "k8s.io/api/extensions/v1beta1" @@ -25,6 +26,7 @@ import ( apivalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" @@ -32,7 +34,7 @@ import ( "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/pod" "k8s.io/kubernetes/pkg/apis/apps" - "k8s.io/kubernetes/pkg/apis/apps/validation" + appsvalidation "k8s.io/kubernetes/pkg/apis/apps/validation" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -84,13 +86,18 @@ func (deploymentStrategy) PrepareForCreate(ctx context.Context, obj runtime.Obje func (deploymentStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { deployment := obj.(*apps.Deployment) opts := pod.GetValidationOptionsFromPodTemplate(&deployment.Spec.Template, nil) - return validation.ValidateDeployment(deployment, opts) + return appsvalidation.ValidateDeployment(deployment, opts) } // WarningsOnCreate returns warnings for the creation of the given object. func (deploymentStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { newDeployment := obj.(*apps.Deployment) - return pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newDeployment.Spec.Template, nil) + var warnings []string + if msgs := utilvalidation.IsDNS1123Label(newDeployment.Name); len(msgs) != 0 { + warnings = append(warnings, fmt.Sprintf("metadata.name: this is used in Pod names and hostnames, which can result in surprising behavior; a DNS label is recommended: %v", msgs)) + } + warnings = append(warnings, pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newDeployment.Spec.Template, nil)...) + return warnings } // Canonicalize normalizes the object after validation. @@ -125,7 +132,7 @@ func (deploymentStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.O oldDeployment := old.(*apps.Deployment) opts := pod.GetValidationOptionsFromPodTemplate(&newDeployment.Spec.Template, &oldDeployment.Spec.Template) - allErrs := validation.ValidateDeploymentUpdate(newDeployment, oldDeployment, opts) + allErrs := appsvalidation.ValidateDeploymentUpdate(newDeployment, oldDeployment, opts) // Update is not allowed to set Spec.Selector for all groups/versions except extensions/v1beta1. // If RequestInfo is nil, it is better to revert to old behavior (i.e. allow update to set Spec.Selector) @@ -189,7 +196,7 @@ func (deploymentStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old r // ValidateUpdate is the default update validation for an end user updating status func (deploymentStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateDeploymentStatusUpdate(obj.(*apps.Deployment), old.(*apps.Deployment)) + return appsvalidation.ValidateDeploymentStatusUpdate(obj.(*apps.Deployment), old.(*apps.Deployment)) } // WarningsOnUpdate returns warnings for the given update. From fec8e721b2060031a3bc26fec8fe31094a0a5c59 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 11 Dec 2022 13:33:19 -0800 Subject: [PATCH 6/7] job: API warn when name is not DNS label --- pkg/registry/batch/job/strategy.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/pkg/registry/batch/job/strategy.go b/pkg/registry/batch/job/strategy.go index ceae5bb6879..6f9789ad488 100644 --- a/pkg/registry/batch/job/strategy.go +++ b/pkg/registry/batch/job/strategy.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" @@ -39,7 +40,7 @@ import ( "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/pod" "k8s.io/kubernetes/pkg/apis/batch" - "k8s.io/kubernetes/pkg/apis/batch/validation" + batchvalidation "k8s.io/kubernetes/pkg/apis/batch/validation" "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/features" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" @@ -154,10 +155,10 @@ func (jobStrategy) Validate(ctx context.Context, obj runtime.Object) field.Error generateSelector(job) } opts := validationOptionsForJob(job, nil) - return validation.ValidateJob(job, opts) + return batchvalidation.ValidateJob(job, opts) } -func validationOptionsForJob(newJob, oldJob *batch.Job) validation.JobValidationOptions { +func validationOptionsForJob(newJob, oldJob *batch.Job) batchvalidation.JobValidationOptions { var newPodTemplate, oldPodTemplate *core.PodTemplateSpec if newJob != nil { newPodTemplate = &newJob.Spec.Template @@ -165,7 +166,7 @@ func validationOptionsForJob(newJob, oldJob *batch.Job) validation.JobValidation if oldJob != nil { oldPodTemplate = &oldJob.Spec.Template } - opts := validation.JobValidationOptions{ + opts := batchvalidation.JobValidationOptions{ PodValidationOptions: pod.GetValidationOptionsFromPodTemplate(newPodTemplate, oldPodTemplate), AllowTrackingAnnotation: true, } @@ -192,7 +193,12 @@ func validationOptionsForJob(newJob, oldJob *batch.Job) validation.JobValidation // WarningsOnCreate returns warnings for the creation of the given object. func (jobStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { newJob := obj.(*batch.Job) - return pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newJob.Spec.Template, nil) + var warnings []string + if msgs := utilvalidation.IsDNS1123Label(newJob.Name); len(msgs) != 0 { + warnings = append(warnings, fmt.Sprintf("metadata.name: this is used in Pod names and hostnames, which can result in surprising behavior; a DNS label is recommended: %v", msgs)) + } + warnings = append(warnings, pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "template"), &newJob.Spec.Template, nil)...) + return warnings } // generateSelector adds a selector to a job and labels to its template @@ -264,8 +270,8 @@ func (jobStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) oldJob := old.(*batch.Job) opts := validationOptionsForJob(job, oldJob) - validationErrorList := validation.ValidateJob(job, opts) - updateErrorList := validation.ValidateJobUpdate(job, oldJob, opts) + validationErrorList := batchvalidation.ValidateJob(job, opts) + updateErrorList := batchvalidation.ValidateJobUpdate(job, oldJob, opts) return append(validationErrorList, updateErrorList...) } @@ -303,7 +309,7 @@ func (jobStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime. } func (jobStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateJobUpdateStatus(obj.(*batch.Job), old.(*batch.Job)) + return batchvalidation.ValidateJobUpdateStatus(obj.(*batch.Job), old.(*batch.Job)) } // WarningsOnUpdate returns warnings for the given update. From 8f62b94991aab153a2acd13bb5afb8878b5da04d Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 11 Dec 2022 13:33:28 -0800 Subject: [PATCH 7/7] cronjob: API warn when name is not DNS label --- pkg/registry/batch/cronjob/strategy.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/registry/batch/cronjob/strategy.go b/pkg/registry/batch/cronjob/strategy.go index c1d42d1ae05..6a52d381f47 100644 --- a/pkg/registry/batch/cronjob/strategy.go +++ b/pkg/registry/batch/cronjob/strategy.go @@ -25,6 +25,7 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" @@ -33,7 +34,7 @@ import ( "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/pod" "k8s.io/kubernetes/pkg/apis/batch" - "k8s.io/kubernetes/pkg/apis/batch/validation" + batchvalidation "k8s.io/kubernetes/pkg/apis/batch/validation" "k8s.io/kubernetes/pkg/features" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -120,13 +121,17 @@ func (cronJobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob func (cronJobStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { cronJob := obj.(*batch.CronJob) opts := pod.GetValidationOptionsFromPodTemplate(&cronJob.Spec.JobTemplate.Spec.Template, nil) - return validation.ValidateCronJobCreate(cronJob, opts) + return batchvalidation.ValidateCronJobCreate(cronJob, opts) } // WarningsOnCreate returns warnings for the creation of the given object. func (cronJobStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { newCronJob := obj.(*batch.CronJob) - warnings := pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "jobTemplate", "spec", "template"), &newCronJob.Spec.JobTemplate.Spec.Template, nil) + var warnings []string + if msgs := utilvalidation.IsDNS1123Label(newCronJob.Name); len(msgs) != 0 { + warnings = append(warnings, fmt.Sprintf("metadata.name: this is used in Pod names and hostnames, which can result in surprising behavior; a DNS label is recommended: %v", msgs)) + } + warnings = append(warnings, pod.GetWarningsForPodTemplate(ctx, field.NewPath("spec", "jobTemplate", "spec", "template"), &newCronJob.Spec.JobTemplate.Spec.Template, nil)...) if strings.Contains(newCronJob.Spec.Schedule, "TZ") { warnings = append(warnings, fmt.Sprintf("CRON_TZ or TZ used in %s is not officially supported, see https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/ for more details", field.NewPath("spec", "spec", "schedule"))) } @@ -152,7 +157,7 @@ func (cronJobStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Obje oldCronJob := old.(*batch.CronJob) opts := pod.GetValidationOptionsFromPodTemplate(&newCronJob.Spec.JobTemplate.Spec.Template, &oldCronJob.Spec.JobTemplate.Spec.Template) - return validation.ValidateCronJobUpdate(newCronJob, oldCronJob, opts) + return batchvalidation.ValidateCronJobUpdate(newCronJob, oldCronJob, opts) } // WarningsOnUpdate returns warnings for the given update.