From e1545d718f3ddd5713152f5501ff87f95f07c1e1 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Fri, 21 Feb 2020 09:14:36 +0100 Subject: [PATCH] Fix panic when canonicalizing a jobspec with incorrect job type. When canonicalizing the ReschedulePolicy a panic was possible if the passed job type was not valid. This change protects against this possibility, in a verbose way to ensure the code path is clear. --- api/tasks.go | 13 ++++++++++ api/tasks_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/api/tasks.go b/api/tasks.go index f9ad7856bb42..de30897c17df 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -217,6 +217,19 @@ func NewDefaultReschedulePolicy(jobType string) *ReschedulePolicy { MaxDelay: timeToPtr(0), Unlimited: boolToPtr(false), } + + default: + // GH-7203: it is possible an unknown job type is passed to this + // function and we need to ensure a non-nil object is returned so that + // the canonicalization runs without panicking. + dp = &ReschedulePolicy{ + Attempts: intToPtr(0), + Interval: timeToPtr(0), + Delay: timeToPtr(0), + DelayFunction: stringToPtr(""), + MaxDelay: timeToPtr(0), + Unlimited: boolToPtr(false), + } } return dp } diff --git a/api/tasks_test.go b/api/tasks_test.go index f83a91e24ff8..3fd4ccfa0e3b 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -638,3 +638,67 @@ func TestSpread_Canonicalize(t *testing.T) { }) } } + +func Test_NewDefaultReschedulePolicy(t *testing.T) { + testCases := []struct { + desc string + inputJobType string + expected *ReschedulePolicy + }{ + { + desc: "service job type", + inputJobType: "service", + expected: &ReschedulePolicy{ + Attempts: intToPtr(0), + Interval: timeToPtr(0), + Delay: timeToPtr(30 * time.Second), + DelayFunction: stringToPtr("exponential"), + MaxDelay: timeToPtr(1 * time.Hour), + Unlimited: boolToPtr(true), + }, + }, + { + desc: "batch job type", + inputJobType: "batch", + expected: &ReschedulePolicy{ + Attempts: intToPtr(1), + Interval: timeToPtr(24 * time.Hour), + Delay: timeToPtr(5 * time.Second), + DelayFunction: stringToPtr("constant"), + MaxDelay: timeToPtr(0), + Unlimited: boolToPtr(false), + }, + }, + { + desc: "system job type", + inputJobType: "system", + expected: &ReschedulePolicy{ + Attempts: intToPtr(0), + Interval: timeToPtr(0), + Delay: timeToPtr(0), + DelayFunction: stringToPtr(""), + MaxDelay: timeToPtr(0), + Unlimited: boolToPtr(false), + }, + }, + { + desc: "unrecognised job type", + inputJobType: "unrecognised", + expected: &ReschedulePolicy{ + Attempts: intToPtr(0), + Interval: timeToPtr(0), + Delay: timeToPtr(0), + DelayFunction: stringToPtr(""), + MaxDelay: timeToPtr(0), + Unlimited: boolToPtr(false), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + actual := NewDefaultReschedulePolicy(tc.inputJobType) + assert.Equal(t, tc.expected, actual) + }) + } +}