Skip to content

Commit

Permalink
Fix panic when canonicalizing a jobspec with incorrect job type.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jrasell committed Feb 21, 2020
1 parent 9ced046 commit e1545d7
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 0 deletions.
13 changes: 13 additions & 0 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
64 changes: 64 additions & 0 deletions api/tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

0 comments on commit e1545d7

Please sign in to comment.