From edad8c73854d613d3a8e00a7a9a6852d2dbd91a0 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sat, 10 Oct 2026 11:48:45 +0000 Subject: [PATCH] fix(apiserver): reject submit options the job never gets; add exec_home (bug-1936) Cause: CreateJob accepted in_file, out_file, err_file, out_dir, asked_hosts, begin_time, term_time and transfer_files and returned success, but neither the controller's submit data/start info nor the worker carries any of them (the worker never stages transferred files), so they were silently dropped. Exec home had no submit field at all, and a relative cwd would have been resolved against the worker unit's directory. Fix: new preCheckSubmitOptions() runs right after binding, before file uploads and ID allocation, and answers 400 naming every unsupported option that is set; cwd and the new exec_home field must be absolute and fit the worker's job_assignment_t buffers (1023 bytes). exec_home goes to JobInfo.ExecHome and from there (submitStartInfo) to the start info's exechome. num_files already reaches exdata as nofile_limit; a handler test now pins it. The array %J/%I submit-time substitution test (9.4) checks cwd instead of out_file/err_file, which are now refused. Swagger regenerated with `swag init -g cmd/main.go -o docs`. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/frontend/apiserver/controllers/jobs.go | 68 ++++++++++- .../controllers/jobs_handler_test.go | 114 +++++++++++++++++- src/frontend/apiserver/docs/docs.go | 8 +- src/frontend/apiserver/docs/swagger.json | 8 +- src/frontend/apiserver/docs/swagger.yaml | 9 +- src/frontend/apiserver/models/jobs.go | 5 +- 6 files changed, 200 insertions(+), 12 deletions(-) diff --git a/src/frontend/apiserver/controllers/jobs.go b/src/frontend/apiserver/controllers/jobs.go index 36150786..ce65599e 100644 --- a/src/frontend/apiserver/controllers/jobs.go +++ b/src/frontend/apiserver/controllers/jobs.go @@ -1085,7 +1085,8 @@ func (j JobController) RestartJobByID(ctx *gin.Context) { // CreateJob godoc // @Summary Create a new job -// @Description Submit a new job with specified requirements and parameters +// @Description Submit a new job with specified requirements and parameters. +// @Description in_file, out_file, err_file, out_dir, asked_hosts, begin_time, term_time and transfer_files are not supported yet and are rejected with 400; cwd and exec_home must be absolute paths. // @Tags jobs // @Accept json // @Produce json @@ -1114,6 +1115,14 @@ func (j JobController) CreateJob(ctx *gin.Context) { return } + // bug-1936: refuse an option the job would never get, before anything + // (file uploads, ID allocation) happens for it. + if err := preCheckSubmitOptions(&req); err != nil { + logger.Error("Job submission rejected", zap.Error(err)) + ReturnError(ctx, common.INVALID_PARAMS_CODE, err.Error(), nil) + return + } + // precheck the request parameter before generating the job ID arraySize, isArray, err := preCheckJobRequest(&req) if err != nil { @@ -1220,6 +1229,7 @@ func (j JobController) CreateJob(ctx *gin.Context) { UserPriority: req.JobPriority, DependCond: req.DependCond, CWD: req.CWD, + ExecHome: req.ExecHome, // bug-1936: exechome of the start info FromHost: req.FromHost, // bug-1676: client-asserted hostname, passed through unchanged PreExecCmd: req.PreExecCmd, PostExecCmd: req.PostExecCmd, @@ -1281,6 +1291,62 @@ func (j JobController) CreateJob(ctx *gin.Context) { ReturnSuccess(ctx, common.CREATED, common.SUCCESS_MSG, response) } +// maxWorkerPathLen is the longest cwd or exec home the worker takes: xcecd +// hands both to xcecd_worker in job_assignment_t's cwd/home[PATH_LENGTH] +// (1024 bytes, src/worker/include/uv_ipc.h), NUL included. +const maxWorkerPathLen = 1024 - 1 + +// preCheckSubmitOptions rejects what a submission asks for but the job +// would not get (bug-1936). The controller's submit data and start info +// have no field for stdin/stdout/stderr paths, an output directory, asked +// hosts or a begin/termination time, and the worker never stages +// transferred files in or out, so each of those is refused instead of +// being accepted and dropped. cwd and exec_home do reach the worker, which +// runs the job there with HOME set; both must be absolute and fit its +// buffers. +func preCheckSubmitOptions(req *models.JobSubmitRequest) error { + unsupported := []struct { + field string + set bool + }{ + {"in_file", req.InFile != ""}, + {"out_file", req.OutFile != ""}, + {"err_file", req.ErrFile != ""}, + {"out_dir", req.OutDir != ""}, + {"asked_hosts", strings.TrimSpace(req.AskedHosts) != ""}, + {"begin_time", !req.BeginTime.Time.IsZero()}, + {"term_time", !req.TermTime.Time.IsZero()}, + {"transfer_files", len(req.TransferFiles) > 0}, + } + var names []string + for _, u := range unsupported { + if u.set { + names = append(names, u.field) + } + } + if len(names) > 0 { + return fmt.Errorf("job submission option(s) %s not supported: the job would not get them", + strings.Join(names, ", ")) + } + + for _, p := range []struct{ field, value string }{ + {"cwd", req.CWD}, + {"exec_home", req.ExecHome}, + } { + if p.value == "" { + continue + } + if !strings.HasPrefix(p.value, "/") { + return fmt.Errorf("%s %q must be an absolute path", p.field, p.value) + } + if len(p.value) > maxWorkerPathLen { + return fmt.Errorf("%s is too long: %d bytes, more than the limit of %d", + p.field, len(p.value), maxWorkerPathLen) + } + } + return nil +} + func preCheckJobRestartRequest(req *models.JobRestartRequest) error { logger.Debug("Enter preCheckJobRestartRequest") if err := validateAskedHosts(req.AskedHosts); err != nil { diff --git a/src/frontend/apiserver/controllers/jobs_handler_test.go b/src/frontend/apiserver/controllers/jobs_handler_test.go index 44339cb8..23144ac9 100644 --- a/src/frontend/apiserver/controllers/jobs_handler_test.go +++ b/src/frontend/apiserver/controllers/jobs_handler_test.go @@ -694,15 +694,17 @@ func TestJobHandler_SubmitOrdinaryUnchanged(t *testing.T) { } // TestJobHandler_SubmitArrayFileNameSubstitution: at submission a new -// array's file names get the first element's values, %I = 1 and -// %J = [1] (9.4). +// array's path names get the first element's values, %I = 1 and +// %J = [1] (9.4). Checked on cwd: out_file/err_file are refused +// since bug-1936 (the worker never writes them), cwd takes the same +// substitution. func TestJobHandler_SubmitArrayFileNameSubstitution(t *testing.T) { f := newHandlerFixture(t) f.seedQueue("normal") submitData(t, f.submit(map[string]interface{}{ "job_name": "arr[1-3]", "command": "sleep 1", "queue_name": "normal", - "out_file": "out.%J.%I", "err_file": "err.%J.%I"})) + "cwd": "/w/out.%J.%I"})) subs := f.submissions() if len(subs) != 1 { @@ -713,9 +715,8 @@ func TestJobHandler_SubmitArrayFileNameSubstitution(t *testing.T) { if got := backend.FormatJobID(int64(backend.MakeElementID(handlerTestArrayBase, 1))); got != elem { t.Fatalf("first element formats as %q, want %q", got, elem) } - wantOut, wantErr := "out."+elem+".1", "err."+elem+".1" - if subs[0].Job.OutFile != wantOut || subs[0].Job.ErrFile != wantErr { - t.Errorf("files = {%q, %q}, want {%q, %q}", subs[0].Job.OutFile, subs[0].Job.ErrFile, wantOut, wantErr) + if want := "/w/out." + elem + ".1"; subs[0].Job.CWD != want { + t.Errorf("cwd = %q, want %q", subs[0].Job.CWD, want) } } @@ -1603,3 +1604,104 @@ func TestJobHandler_SubmitQueueRejectedByController(t *testing.T) { } } } + +// bug-1936: a submit option the job never gets must be refused with a 400 +// naming it, not accepted and dropped. Neither the controller's submit data +// nor the worker carries stdin/stdout/stderr paths, an output directory, +// asked hosts or a begin/termination time, and the worker never stages +// transferred files in or out. +func TestJobHandler_SubmitRejectsUnsupportedOptions(t *testing.T) { + for _, tc := range []struct { + field string + value interface{} + }{ + {"in_file", "/tmp/in.txt"}, + {"out_file", "/tmp/out.%J"}, + {"err_file", "/tmp/err.%J"}, + {"out_dir", "/tmp/outdir"}, + {"asked_hosts", "host1"}, + {"begin_time", "2030-01-02 03:04"}, + {"term_time", "2030-01-02 03:04"}, + {"transfer_files", []string{"/tmp/a.txt > a.txt"}}, + } { + f := newHandlerFixture(t) + f.seedQueue("normal") + w := f.submit(map[string]interface{}{ + "command": "sleep 1", "queue_name": "normal", tc.field: tc.value, + }) + assert400(t, tc.field, w, tc.field) + assert400(t, tc.field, w, "not supported") + f.assertNothingCreated() + } +} + +// bug-1936: the job runs in cwd and with HOME set to exec_home on the +// worker, so both must be absolute paths that fit the worker's +// job_assignment_t buffers (PATH_LENGTH, 1024 bytes with the NUL). +func TestJobHandler_SubmitRejectsBadCwdAndExecHome(t *testing.T) { + long := "/" + strings.Repeat("d", 1023) + for _, tc := range []struct { + field, value, want string + }{ + {"cwd", "relative/dir", "absolute"}, + {"cwd", long, "too long"}, + {"exec_home", "home/u", "absolute"}, + {"exec_home", long, "too long"}, + } { + f := newHandlerFixture(t) + f.seedQueue("normal") + w := f.submit(map[string]interface{}{ + "command": "sleep 1", "queue_name": "normal", tc.field: tc.value, + }) + assert400(t, tc.field+"="+tc.value[:6], w, tc.want) + f.assertNothingCreated() + } +} + +// bug-1936: exec_home is a submit field and reaches the submitted job (and +// from there submitStartInfo's exechome), next to cwd. +func TestJobHandler_SubmitCarriesCwdAndExecHome(t *testing.T) { + f := newHandlerFixture(t) + f.seedQueue("normal") + + submitData(t, f.submit(map[string]interface{}{ + "command": "sleep 1", "queue_name": "normal", + "cwd": "/data/run/%J", "exec_home": "/home/alice", + })) + subs := f.submissions() + if len(subs) != 1 { + t.Fatalf("submitJob calls = %d, want 1", len(subs)) + } + wantCwd := "/data/run/" + backend.FormatJobID(int64(handlerTestOrdinaryBase)) + if subs[0].Job.CWD != wantCwd || subs[0].Job.ExecHome != "/home/alice" { + t.Errorf("cwd, exec_home = %q, %q; want %q, %q", + subs[0].Job.CWD, subs[0].Job.ExecHome, wantCwd, "/home/alice") + } +} + +// bug-1936: num_files (and shell_limit.num_files) is the job's open-file +// limit; it lands in the exdata limits as nofile_limit, the key the +// worker's parse_limits() reads. +func TestJobHandler_SubmitNofileLimitInExdata(t *testing.T) { + for _, body := range []map[string]interface{}{ + {"command": "sleep 1", "queue_name": "normal", "num_files": 2048}, + {"command": "sleep 1", "queue_name": "normal", "shell_limit": map[string]interface{}{"num_files": 2048}}, + } { + f := newHandlerFixture(t) + f.seedQueue("normal") + submitData(t, f.submit(body)) + subs := f.submissions() + if len(subs) != 1 { + t.Fatalf("submitJob calls = %d, want 1", len(subs)) + } + var ex struct { + Limits map[string]float64 `json:"limits"` + } + if err := json.Unmarshal([]byte(subs[0].Job.ExtendData.String), &ex); err != nil { + t.Fatalf("exdata %q: %v", subs[0].Job.ExtendData.String, err) + } + if ex.Limits["nofile_limit"] != 2048 { + t.Errorf("exdata %q: nofile_limit = %v, want 2048", subs[0].Job.ExtendData.String, ex.Limits["nofile_limit"]) + } + } +} diff --git a/src/frontend/apiserver/docs/docs.go b/src/frontend/apiserver/docs/docs.go index 5b3b9524..abe59936 100644 --- a/src/frontend/apiserver/docs/docs.go +++ b/src/frontend/apiserver/docs/docs.go @@ -141,7 +141,7 @@ const docTemplate = `{ }, "/jobs/create": { "post": { - "description": "Submit a new job with specified requirements and parameters", + "description": "Submit a new job with specified requirements and parameters.\nin_file, out_file, err_file, out_dir, asked_hosts, begin_time, term_time and transfer_files are not supported yet and are rejected with 400; cwd and exec_home must be absolute paths.", "consumes": [ "application/json" ], @@ -476,6 +476,7 @@ const docTemplate = `{ ] }, "cwd": { + "description": "absolute; the job runs here on the worker (bug-1936)", "type": "string" }, "data_limit": { @@ -502,6 +503,11 @@ const docTemplate = `{ "exclusive": { "type": "boolean" }, + "exec_home": { + "description": "absolute; HOME of the job on the worker (bug-1936)", + "type": "string", + "example": "/home/alice" + }, "extend_data": { "type": "string" }, diff --git a/src/frontend/apiserver/docs/swagger.json b/src/frontend/apiserver/docs/swagger.json index 0a345fe0..428d5c1c 100644 --- a/src/frontend/apiserver/docs/swagger.json +++ b/src/frontend/apiserver/docs/swagger.json @@ -135,7 +135,7 @@ }, "/jobs/create": { "post": { - "description": "Submit a new job with specified requirements and parameters", + "description": "Submit a new job with specified requirements and parameters.\nin_file, out_file, err_file, out_dir, asked_hosts, begin_time, term_time and transfer_files are not supported yet and are rejected with 400; cwd and exec_home must be absolute paths.", "consumes": [ "application/json" ], @@ -470,6 +470,7 @@ ] }, "cwd": { + "description": "absolute; the job runs here on the worker (bug-1936)", "type": "string" }, "data_limit": { @@ -496,6 +497,11 @@ "exclusive": { "type": "boolean" }, + "exec_home": { + "description": "absolute; HOME of the job on the worker (bug-1936)", + "type": "string", + "example": "/home/alice" + }, "extend_data": { "type": "string" }, diff --git a/src/frontend/apiserver/docs/swagger.yaml b/src/frontend/apiserver/docs/swagger.yaml index e5c8ead4..abb34010 100644 --- a/src/frontend/apiserver/docs/swagger.yaml +++ b/src/frontend/apiserver/docs/swagger.yaml @@ -109,6 +109,7 @@ definitions: - $ref: '#/definitions/models.CustomInt' description: k change type cwd: + description: absolute; the job runs here on the worker (bug-1936) type: string data_limit: $ref: '#/definitions/models.CustomLimit' @@ -126,6 +127,10 @@ definitions: type: string exclusive: type: boolean + exec_home: + description: absolute; HOME of the job on the worker (bug-1936) + example: /home/alice + type: string extend_data: type: string file_limit: @@ -430,7 +435,9 @@ paths: post: consumes: - application/json - description: Submit a new job with specified requirements and parameters + description: |- + Submit a new job with specified requirements and parameters. + in_file, out_file, err_file, out_dir, asked_hosts, begin_time, term_time and transfer_files are not supported yet and are rejected with 400; cwd and exec_home must be absolute paths. parameters: - description: Job submission parameters in: body diff --git a/src/frontend/apiserver/models/jobs.go b/src/frontend/apiserver/models/jobs.go index 426990d9..719b8802 100644 --- a/src/frontend/apiserver/models/jobs.go +++ b/src/frontend/apiserver/models/jobs.go @@ -673,8 +673,9 @@ type JobSubmitRequest struct { ChkpntPeriod int `json:"chkpnt_period,omitempty"` ChkpntDir string `json:"chkpnt_dir,omitempty"` InitChkpntPeriod int `json:"init_chkpnt_period,omitempty"` - CWD string `json:"cwd,omitempty"` - FromHost string `json:"from_host,omitempty"` // client-asserted submitting hostname (bug-1676) -- passed through unchanged, not derived server-side + CWD string `json:"cwd,omitempty"` // absolute; the job runs here on the worker (bug-1936) + ExecHome string `json:"exec_home,omitempty" example:"/home/alice"` // absolute; HOME of the job on the worker (bug-1936) + FromHost string `json:"from_host,omitempty"` // client-asserted submitting hostname (bug-1676) -- passed through unchanged, not derived server-side OutDir string `json:"out_dir,omitempty"` InFile string `json:"in_file,omitempty"` OutFile string `json:"out_file,omitempty"`