From ae13b88a7671bdf3fa534e7656d25174f3b08b8b Mon Sep 17 00:00:00 2001
From: Tom Wanielista <tomwans@gmail.com>
Date: Wed, 27 Mar 2019 09:25:12 -0400
Subject: [PATCH] Avoid panic in cronjob sorting

This change handles the case where the ith cronjob may have its start
time set to nil.

Previously, the Less method could cause a panic in case the ith
cronjob had its start time set to nil, but the jth cronjob did not. It
would panic when calling Before on a nil StartTime.
---
 pkg/controller/cronjob/utils.go      |  9 ++--
 pkg/controller/cronjob/utils_test.go | 65 ++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/pkg/controller/cronjob/utils.go b/pkg/controller/cronjob/utils.go
index 19fb91c4ba..d1044a6146 100644
--- a/pkg/controller/cronjob/utils.go
+++ b/pkg/controller/cronjob/utils.go
@@ -195,13 +195,14 @@ func (o byJobStartTime) Len() int      { return len(o) }
 func (o byJobStartTime) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
 
 func (o byJobStartTime) Less(i, j int) bool {
-	if o[j].Status.StartTime == nil {
-		return o[i].Status.StartTime != nil
+	if o[i].Status.StartTime == nil && o[j].Status.StartTime != nil {
+		return false
+	}
+	if o[i].Status.StartTime != nil && o[j].Status.StartTime == nil {
+		return true
 	}
-
 	if o[i].Status.StartTime.Equal(o[j].Status.StartTime) {
 		return o[i].Name < o[j].Name
 	}
-
 	return o[i].Status.StartTime.Before(o[j].Status.StartTime)
 }
diff --git a/pkg/controller/cronjob/utils_test.go b/pkg/controller/cronjob/utils_test.go
index fb6b569f2e..1e5ee4d036 100644
--- a/pkg/controller/cronjob/utils_test.go
+++ b/pkg/controller/cronjob/utils_test.go
@@ -17,6 +17,8 @@ limitations under the License.
 package cronjob
 
 import (
+	"reflect"
+	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -376,3 +378,66 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) {
 		}
 	}
 }
+
+func TestByJobStartTime(t *testing.T) {
+	now := metav1.NewTime(time.Date(2018, time.January, 1, 2, 3, 4, 5, time.UTC))
+	later := metav1.NewTime(time.Date(2019, time.January, 1, 2, 3, 4, 5, time.UTC))
+	aNil := batchv1.Job{
+		ObjectMeta: metav1.ObjectMeta{Name: "a"},
+		Status:     batchv1.JobStatus{},
+	}
+	bNil := batchv1.Job{
+		ObjectMeta: metav1.ObjectMeta{Name: "b"},
+		Status:     batchv1.JobStatus{},
+	}
+	aSet := batchv1.Job{
+		ObjectMeta: metav1.ObjectMeta{Name: "a"},
+		Status:     batchv1.JobStatus{StartTime: &now},
+	}
+	bSet := batchv1.Job{
+		ObjectMeta: metav1.ObjectMeta{Name: "b"},
+		Status:     batchv1.JobStatus{StartTime: &now},
+	}
+	aSetLater := batchv1.Job{
+		ObjectMeta: metav1.ObjectMeta{Name: "a"},
+		Status:     batchv1.JobStatus{StartTime: &later},
+	}
+
+	testCases := []struct {
+		name            string
+		input, expected []batchv1.Job
+	}{
+		{
+			name:     "both have nil start times",
+			input:    []batchv1.Job{bNil, aNil},
+			expected: []batchv1.Job{aNil, bNil},
+		},
+		{
+			name:     "only the first has a nil start time",
+			input:    []batchv1.Job{aNil, bSet},
+			expected: []batchv1.Job{bSet, aNil},
+		},
+		{
+			name:     "only the second has a nil start time",
+			input:    []batchv1.Job{aSet, bNil},
+			expected: []batchv1.Job{aSet, bNil},
+		},
+		{
+			name:     "both have non-nil, equal start time",
+			input:    []batchv1.Job{bSet, aSet},
+			expected: []batchv1.Job{aSet, bSet},
+		},
+		{
+			name:     "both have non-nil, different start time",
+			input:    []batchv1.Job{aSetLater, bSet},
+			expected: []batchv1.Job{bSet, aSetLater},
+		},
+	}
+
+	for _, testCase := range testCases {
+		sort.Sort(byJobStartTime(testCase.input))
+		if !reflect.DeepEqual(testCase.input, testCase.expected) {
+			t.Errorf("case: '%s', jobs not sorted as expected", testCase.name)
+		}
+	}
+}