From b8155e107c61311106ac7a83dd9e0f13be3193f4 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <jliggitt@redhat.com>
Date: Sun, 24 Sep 2017 23:56:27 -0400
Subject: [PATCH] Limit 52-character cronjob name validation to create

---
 pkg/apis/batch/validation/validation.go      |  8 ++++++
 pkg/apis/batch/validation/validation_test.go | 26 ++++++++++++++++++++
 pkg/registry/batch/cronjob/strategy.go       |  2 +-
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go
index 17729942c2..7f9215d700 100644
--- a/pkg/apis/batch/validation/validation.go
+++ b/pkg/apis/batch/validation/validation.go
@@ -176,6 +176,14 @@ func ValidateCronJob(scheduledJob *batch.CronJob) field.ErrorList {
 	return allErrs
 }
 
+func ValidateCronJobUpdate(job, oldJob *batch.CronJob) field.ErrorList {
+	allErrs := apivalidation.ValidateObjectMetaUpdate(&job.ObjectMeta, &oldJob.ObjectMeta, field.NewPath("metadata"))
+	allErrs = append(allErrs, ValidateCronJobSpec(&job.Spec, field.NewPath("spec"))...)
+	// skip the 52-character name validation limit on update validation
+	// to allow old cronjobs with names > 52 chars to be updated/deleted
+	return allErrs
+}
+
 func ValidateCronJobSpec(spec *batch.CronJobSpec, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
diff --git a/pkg/apis/batch/validation/validation_test.go b/pkg/apis/batch/validation/validation_test.go
index 5e9ce08236..f40921e42f 100644
--- a/pkg/apis/batch/validation/validation_test.go
+++ b/pkg/apis/batch/validation/validation_test.go
@@ -347,6 +347,14 @@ func TestValidateCronJob(t *testing.T) {
 		if errs := ValidateCronJob(&v); len(errs) != 0 {
 			t.Errorf("expected success for %s: %v", k, errs)
 		}
+
+		// Update validation should pass same success cases
+		// copy to avoid polluting the testcase object, set a resourceVersion to allow validating update, and test a no-op update
+		v = *v.DeepCopy()
+		v.ResourceVersion = "1"
+		if errs := ValidateCronJobUpdate(&v, &v); len(errs) != 0 {
+			t.Errorf("expected success for %s: %v", k, errs)
+		}
 	}
 
 	negative := int32(-1)
@@ -588,6 +596,24 @@ func TestValidateCronJob(t *testing.T) {
 				t.Errorf("unexpected error: %v, expected: %s", err, k)
 			}
 		}
+
+		// Update validation should fail all failure cases other than the 52 character name limit
+		// copy to avoid polluting the testcase object, set a resourceVersion to allow validating update, and test a no-op update
+		v = *v.DeepCopy()
+		v.ResourceVersion = "1"
+		errs = ValidateCronJobUpdate(&v, &v)
+		if len(errs) == 0 {
+			if k == "metadata.name: must be no more than 52 characters" {
+				continue
+			}
+			t.Errorf("expected failure for %s", k)
+		} else {
+			s := strings.Split(k, ":")
+			err := errs[0]
+			if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
+				t.Errorf("unexpected error: %v, expected: %s", err, k)
+			}
+		}
 	}
 }
 
diff --git a/pkg/registry/batch/cronjob/strategy.go b/pkg/registry/batch/cronjob/strategy.go
index 5b8b35ba0f..ac91c59c7c 100644
--- a/pkg/registry/batch/cronjob/strategy.go
+++ b/pkg/registry/batch/cronjob/strategy.go
@@ -87,7 +87,7 @@ func (cronJobStrategy) AllowCreateOnUpdate() bool {
 
 // ValidateUpdate is the default update validation for an end user.
 func (cronJobStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList {
-	return validation.ValidateCronJob(obj.(*batch.CronJob))
+	return validation.ValidateCronJobUpdate(obj.(*batch.CronJob), old.(*batch.CronJob))
 }
 
 type cronJobStatusStrategy struct {