fix(apiserver): validate named and array submissions like any other (bug-1953)
preCheckJobRequest() returned from inside its job-name branch, so every named or array job skipped the description, project and resreq checks, resreq validation and the shell-limit conversion that follow. The name now only sets the job count and array flag, and every submission runs the rest. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
27ef506058
commit
e00526f9f5
2 changed files with 54 additions and 19 deletions
|
|
@ -1307,6 +1307,9 @@ var (
|
|||
// to 400).
|
||||
func preCheckJobRequest(req *models.JobSubmitRequest) (n int, isArray bool, err error) {
|
||||
logger.Debug("Enter preCheckJobRequest")
|
||||
// bug-1953: the name only decides n and isArray; every submission then
|
||||
// goes through the same checks and conversions below
|
||||
n, isArray = 1, false
|
||||
if req.QueueName == "" {
|
||||
req.QueueName = "normal"
|
||||
} else {
|
||||
|
|
@ -1343,27 +1346,26 @@ func preCheckJobRequest(req *models.JobSubmitRequest) (n int, isArray bool, err
|
|||
req.JobName = arrayName
|
||||
logger.Debug("preCheckJobRequest parsed array job name",
|
||||
zap.String("jobName", req.JobName), zap.Int("arraySize", count))
|
||||
return count, true, nil
|
||||
}
|
||||
|
||||
// Names without brackets keep the existing path (1.4).
|
||||
arrayName, idx, slotLimit, err := createArrayName(req.JobName)
|
||||
if err != nil {
|
||||
logger.Error("job name format is invalid", zap.Error(err))
|
||||
return 0, false, errors.New("job array list format is invalid: " + err.Error())
|
||||
}
|
||||
|
||||
if arrayName != "" {
|
||||
if err = isValidArrayName(arrayName); err != nil {
|
||||
logger.Error(err.Error())
|
||||
n, isArray = count, true
|
||||
} else {
|
||||
// Names without brackets keep the existing path (1.4).
|
||||
arrayName, idx, slotLimit, err := createArrayName(req.JobName)
|
||||
if err != nil {
|
||||
logger.Error("job name format is invalid", zap.Error(err))
|
||||
return 0, false, errors.New("job array list format is invalid: " + err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
req.JobName = arrayName
|
||||
logger.Debug("preCheckJobRequest parsed job name is: ", zap.String("jobName", req.JobName),
|
||||
zap.Any("indices", idx), zap.Any("slotLimit", slotLimit))
|
||||
return 1, false, nil
|
||||
if arrayName != "" {
|
||||
if err = isValidArrayName(arrayName); err != nil {
|
||||
logger.Error(err.Error())
|
||||
return 0, false, errors.New("job array list format is invalid: " + err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
req.JobName = arrayName
|
||||
logger.Debug("preCheckJobRequest parsed job name is: ", zap.String("jobName", req.JobName),
|
||||
zap.Any("indices", idx), zap.Any("slotLimit", slotLimit))
|
||||
}
|
||||
} else {
|
||||
if req.Command != "" {
|
||||
if len(req.Command) > 4094 {
|
||||
|
|
@ -1531,7 +1533,7 @@ func preCheckJobRequest(req *models.JobSubmitRequest) (n int, isArray bool, err
|
|||
req.LaunchType = launchType
|
||||
}
|
||||
|
||||
return 1, false, nil
|
||||
return n, isArray, nil
|
||||
}
|
||||
|
||||
// It returns minTasks, maxTasks, and an error if parsing fails or constraints are violated.
|
||||
|
|
|
|||
|
|
@ -270,3 +270,36 @@ func TestPreCheckJobRequest_RejectsMalformedResreq(t *testing.T) {
|
|||
t.Fatalf("malformed resreq must be rejected, got err=%v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1953: a job name (plain or array) must not skip the checks and
|
||||
// conversions every other submission gets.
|
||||
func TestPreCheckJobRequest_NamedJobsGetFullValidation(t *testing.T) {
|
||||
resetJobSubmitState()
|
||||
origCfg := utils.JobYamlConfig
|
||||
utils.JobYamlConfig = &utils.JobConfig{MaxArraySize: 1000}
|
||||
t.Cleanup(func() { utils.JobYamlConfig = origCfg })
|
||||
|
||||
for _, name := range []string{"", "myjob", "arr[1-3]"} {
|
||||
req := &models.JobSubmitRequest{JobName: name, Command: "sleep 1"}
|
||||
req.ShellLimit.CPULimit = 60000
|
||||
n, isArray, err := preCheckJobRequest(req)
|
||||
if err != nil {
|
||||
t.Fatalf("name %q: unexpected error %v", name, err)
|
||||
}
|
||||
if req.CPULimit.Seconds != 60 {
|
||||
t.Errorf("name %q: cpu limit not converted: got %d s, want 60", name, req.CPULimit.Seconds)
|
||||
}
|
||||
if name == "arr[1-3]" && (n != 3 || !isArray || req.JobName != "arr") {
|
||||
t.Errorf("array name: got n=%d isArray=%v name=%q, want 3 true \"arr\"", n, isArray, req.JobName)
|
||||
}
|
||||
if name == "myjob" && (n != 1 || isArray) {
|
||||
t.Errorf("plain name: got n=%d isArray=%v, want 1 false", n, isArray)
|
||||
}
|
||||
|
||||
bad := &models.JobSubmitRequest{JobName: name, Command: "sleep 1",
|
||||
ProjectName: strings.Repeat("p", 512)}
|
||||
if _, _, err := preCheckJobRequest(bad); err == nil {
|
||||
t.Errorf("name %q: an over-long project name was accepted", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue