fix(apiserver): jobs list -l/-o validates against the wrong struct (bug-1674, gitea #41)
GetJobsInfo/GetJobByIDs validated the -o/-l field list against
models.JobSubmitRequest{} -- the job-submission payload -- instead of
models.JobInfo{}, the query/response model (the already-correct
pattern used elsewhere in this file at line 1131). Response-only
fields like pending_reason don't exist on JobSubmitRequest, so any
such field made the whole listing fail outright with "unsupported
query field", not just omit that column.
core_limit/mem_limit exist on JobSubmitRequest but not on JobInfo --
switching wholesale would have newly broken those two previously-valid
fields. Added CoreLimit/MemLimit to JobAdditionalFields, the existing
merge-in bucket already used for pids/pgids/interactive_job_url.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
parent
643741fad5
commit
88c7c05daf
3 changed files with 54 additions and 2 deletions
|
|
@ -550,7 +550,7 @@ func (j JobController) GetJobsInfo(ctx *gin.Context) {
|
|||
found2 := false
|
||||
found3 := false
|
||||
// verify the fields is in supported list
|
||||
supportedFields := utils.GetSupportedFields(models.JobSubmitRequest{})
|
||||
supportedFields := utils.GetSupportedFields(models.JobInfo{})
|
||||
|
||||
utils.AddFieldsToSupportedList(&supportedFields, models.JobAdditionalFields{})
|
||||
logger.Debug("job supportedFields: %v", zap.Any("supportedFields", supportedFields))
|
||||
|
|
@ -898,7 +898,7 @@ func (j JobController) GetJobByIDs(ctx *gin.Context) {
|
|||
found1 := false
|
||||
found2 := false
|
||||
// verify the fields is in supported list
|
||||
supportedFields := utils.GetSupportedFields(models.JobSubmitRequest{})
|
||||
supportedFields := utils.GetSupportedFields(models.JobInfo{})
|
||||
utils.AddFieldsToSupportedList(&supportedFields, models.JobAdditionalFields{})
|
||||
logger.Debug("job supportedFields: %v", zap.Any("supportedFields", supportedFields))
|
||||
|
||||
|
|
|
|||
|
|
@ -36,6 +36,51 @@ func TestPreCheckJobRequest_LostAndFound(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestJobsListFieldValidation_UsesJobInfoNotSubmitRequest is a regression
|
||||
// guard for bug-1674 (gitea #41): GetJobsInfo (jobs.go:553) and GetJobByIDs
|
||||
// (jobs.go:901) validate a caller's requested `-o`/`-l` field list against
|
||||
// utils.GetSupportedFields(models.JobInfo{}) merged with
|
||||
// models.JobAdditionalFields{} -- not models.JobSubmitRequest{}, the
|
||||
// job-submission model. Response-only fields like pending_reason/start_time/
|
||||
// end_time exist on JobInfo but not on JobSubmitRequest; validating against
|
||||
// the wrong struct made `-l` (which requests these fields) fail outright with
|
||||
// "unsupported query field" on every invocation, regardless of which such
|
||||
// field Go's randomized map iteration hit first.
|
||||
//
|
||||
// allXcecliJobFields mirrors xcecli/models/jobs.go's JobFieldsMap value set
|
||||
// verbatim (xcecli is a separate Go module -- see src/frontend/CLAUDE.md --
|
||||
// so it cannot be imported directly here). Every value in that map is a
|
||||
// field xcecli's `-o`/`-l` can actually send the apiserver; this test proves
|
||||
// the combined JobInfo+JobAdditionalFields supported-fields list -- what
|
||||
// both fixed call sites now validate against -- covers every one of them.
|
||||
var allXcecliJobFields = []string{
|
||||
"job_id", "submit_user", "job_status", "queue_name", "command",
|
||||
"job_name", "submit_time", "from_host", "exec_hosts", "core_limit",
|
||||
"mem_limit", "pids", "pgids", "project_name", "start_time", "end_time",
|
||||
"cwd", "cpu_time", "resreq", "job_description", "launch_type",
|
||||
"num_processors", "interactive_job_url", "pending_reason",
|
||||
}
|
||||
|
||||
func TestJobsListFieldValidation_UsesJobInfoNotSubmitRequest(t *testing.T) {
|
||||
supportedFields := utils.GetSupportedFields(models.JobInfo{})
|
||||
utils.AddFieldsToSupportedList(&supportedFields, models.JobAdditionalFields{})
|
||||
|
||||
for _, field := range allXcecliJobFields {
|
||||
if !utils.ContainsField(supportedFields, field) {
|
||||
t.Errorf("GetSupportedFields(models.JobInfo{}) + JobAdditionalFields is missing %q -- xcecli's `-o`/`-l` can request this field (see xcecli/models/jobs.go's JobFieldsMap) and it must validate successfully", field)
|
||||
}
|
||||
}
|
||||
|
||||
// Documents exactly why validating against JobSubmitRequest broke:
|
||||
// response-only fields like pending_reason don't exist there at all.
|
||||
submitRequestFields := utils.GetSupportedFields(models.JobSubmitRequest{})
|
||||
for _, field := range []string{"pending_reason", "start_time", "end_time"} {
|
||||
if utils.ContainsField(submitRequestFields, field) {
|
||||
t.Errorf("unexpected: models.JobSubmitRequest{} contains %q -- this test's premise (that JobSubmitRequest lacks response-only fields) no longer holds", field)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestPreCheckJobRequest_NormalQueue verifies that a normal (non-reserved)
|
||||
// queue name does not trigger the reserved-name rejection.
|
||||
func TestPreCheckJobRequest_NormalQueue(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -612,6 +612,13 @@ type JobAdditionalFields struct {
|
|||
Pids []int `json:"pids"` // List of process IDs associated with the job
|
||||
Pgids []int `json:"pgids"` // List of process group IDs associated with the job
|
||||
InteractiveJobURL string `json:"interactive_job_url"`
|
||||
// CoreLimit/MemLimit: JobInfo has no equivalent field (these are
|
||||
// submission-time resource limits, not job-state fields), but xcecli's
|
||||
// JobFieldsMap accepts them as valid -o/-l display fields (bug-1674) --
|
||||
// merged in here rather than validating against JobSubmitRequest itself,
|
||||
// which lacks response-only fields like pending_reason.
|
||||
CoreLimit CustomLimit `json:"core_limit"`
|
||||
MemLimit CustomLimit `json:"mem_limit"`
|
||||
}
|
||||
|
||||
type JobSubmitRequest struct {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue