From e1545d718f3ddd5713152f5501ff87f95f07c1e1 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Fri, 21 Feb 2020 09:14:36 +0100 Subject: [PATCH 1/2] 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 f9ad7856bb4..de30897c17d 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 f83a91e24ff..3fd4ccfa0e3 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) + }) + } +} From 563123b399e319c273f1cf6fc3e8aedbde8b1301 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Fri, 21 Feb 2020 14:21:34 +0100 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40f399ac223..7814a2cd025 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ IMPROVEMENTS: * consul: Added support for configuring `enable_tag_override` on service stanzas. [[GH-2057](https://github.com/hashicorp/nomad/issues/2057)] +BUG FIXES: + + * api: Fixed a panic when canonicalizing a jobspec with an incorrect job type [[GH-7207]](https://github.com/hashicorp/nomad/pull/7207) + ## 0.10.4 (February 19, 2020) FEATURES: