fix(apiserver): reject a malformed rusage[] at job submit (bug-1744)
Gitea #47: `xcecli jobs submit -R "rusage[mem=100000000a0]"` was accepted with a normal "Job <...> is submitted" reply. The apiserver only length-checked resreq; the real PEG grammar ran later, asynchronously, in the scheduler's core_build_job(), which logged a server-side WARN and dropped the job -- it then sat PEND forever with no sign of why. - libxce: xce_check_rusage_syntax(resreq, &errmsg) runs parse_rusage() on every rusage[...] clause (extracted the way section.peg does: up to the first ']'), so primary, compound and alternative requests are all covered. No resource manager needed, so select[]/order[] resource names are still only checked by the scheduler (the documented scope). - apiserver/backend: ValidateResreqSyntax() cgo wrapper (resreq_check.go); the !cgo stub returns nil (no job can be submitted without CGO anyway). - controllers/jobs.go preCheckJobRequest(): calls it after the length check via a package var, so a malformed request is refused up front. Tests: lib test_check_rusage_syntax (RED before: the reported input was accepted) and backend TestValidateResreqSyntax (RED before) through the real grammar; controllers TestPreCheckJobRequest_RejectsMalformedResreq checks the call site with an injected validator (written with the call site, so no separate RED). libxce, apiserver backend (cgo) and controllers tests pass. apiserver/middleware TestLoadJobs fails here and on the baseline alike: it needs a live apiserver on :8080. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ac20b2441f
commit
b970ab75b1
8 changed files with 209 additions and 0 deletions
|
|
@ -286,3 +286,20 @@ func TestClibSource_ClusterAndClientConfigsArePackageLevel(t *testing.T) {
|
|||
"runtime life (see status report CONCERNS for the precise finding)")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateResreqSyntax (bug-1744): the real libxce rusage grammar accepts
|
||||
// well-formed requests and rejects the reported malformed one.
|
||||
func TestValidateResreqSyntax(t *testing.T) {
|
||||
for _, ok := range []string{"", "rusage[mem=100]",
|
||||
"select[type==any] rusage[mem=100, swp=2]", "select[type==any]"} {
|
||||
if err := ValidateResreqSyntax(ok); err != nil {
|
||||
t.Errorf("%q: unexpected error %v", ok, err)
|
||||
}
|
||||
}
|
||||
for _, bad := range []string{"rusage[mem=100000000a0]",
|
||||
"select[type==any] rusage[mem=]", "rusage[mem=1"} {
|
||||
if err := ValidateResreqSyntax(bad); err == nil {
|
||||
t.Errorf("%q: expected a syntax error", bad)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
41
src/frontend/apiserver/backend/resreq_check.go
Normal file
41
src/frontend/apiserver/backend/resreq_check.go
Normal file
|
|
@ -0,0 +1,41 @@
|
|||
//go:build cgo
|
||||
|
||||
package backend
|
||||
|
||||
/*
|
||||
#include <stdlib.h>
|
||||
#include "resreq.h"
|
||||
*/
|
||||
import "C"
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"unsafe"
|
||||
)
|
||||
|
||||
// ValidateResreqSyntax (bug-1744) checks every rusage[...] clause of a job's
|
||||
// resource request with libxce's real rusage grammar (xce_check_rusage_syntax,
|
||||
// src/lib/resreq.c) before the job is accepted. The scheduler parses the full
|
||||
// request only later, asynchronously, and silently drops a job it cannot
|
||||
// parse -- after the submitter was already told it was accepted. Needs no
|
||||
// resource manager, so select[]/order[] resource names are not checked here.
|
||||
func ValidateResreqSyntax(resreq string) error {
|
||||
if resreq == "" {
|
||||
return nil
|
||||
}
|
||||
|
||||
cs := C.CString(resreq)
|
||||
defer C.free(unsafe.Pointer(cs))
|
||||
|
||||
var cerr *C.char
|
||||
if rc := C.xce_check_rusage_syntax(cs, &cerr); rc < 0 {
|
||||
msg := "malformed clause"
|
||||
if cerr != nil {
|
||||
msg = C.GoString(cerr)
|
||||
C.free(unsafe.Pointer(cerr))
|
||||
}
|
||||
return fmt.Errorf("invalid rusage in resource request %q: %s",
|
||||
resreq, msg)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
|
@ -46,3 +46,8 @@ func Cleanup() {}
|
|||
func FetchFile(_ uint64, _ string, _ uint64, _ uint32) (*FetchFileResult, error) {
|
||||
return nil, errNoCGO
|
||||
}
|
||||
|
||||
// ValidateResreqSyntax (bug-1744) needs libxce's grammar parser; without CGO
|
||||
// no job can be submitted anyway (SubmitJob returns errNoCGO), so there is
|
||||
// nothing to check.
|
||||
func ValidateResreqSyntax(string) error { return nil }
|
||||
|
|
|
|||
|
|
@ -1530,6 +1530,10 @@ func preCheckJobRestartRequest(req *models.JobRestartRequest) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
// validateResreqSyntax is the resource-request syntax check used by
|
||||
// preCheckJobRequest; a variable so tests can substitute it (bug-1744).
|
||||
var validateResreqSyntax = backend.ValidateResreqSyntax
|
||||
|
||||
func preCheckJobRequest(req *models.JobSubmitRequest) ([]int, error) {
|
||||
logger.Debug("Enter preCheckJobRequest")
|
||||
if req.QueueName == "" {
|
||||
|
|
@ -1603,6 +1607,11 @@ func preCheckJobRequest(req *models.JobSubmitRequest) ([]int, error) {
|
|||
if len(req.Resreq) > 4094 {
|
||||
return nil, errors.New("resreq length exceeds the limit of 4094 characters")
|
||||
}
|
||||
// bug-1744: reject a malformed rusage[] clause now, instead of
|
||||
// accepting the job and letting the scheduler silently drop it.
|
||||
if err := validateResreqSyntax(req.Resreq); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
if req.ShellLimit.CPULimit != 0 {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package controllers
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"apiserver/models"
|
||||
"apiserver/store"
|
||||
"apiserver/utils"
|
||||
|
|
@ -192,3 +193,32 @@ func TestFoundJob_MatchesRealArrayIdxType(t *testing.T) {
|
|||
t.Fatalf("expected to find 1 job, got %d", len(found))
|
||||
}
|
||||
}
|
||||
|
||||
// TestPreCheckJobRequest_RejectsMalformedResreq (bug-1744): a resource request
|
||||
// the syntax check rejects must fail the submission instead of being accepted
|
||||
// and later silently dropped by the scheduler.
|
||||
func TestPreCheckJobRequest_RejectsMalformedResreq(t *testing.T) {
|
||||
resetJobSubmitState()
|
||||
store.Queues.Put("normal", models.QueueInfo{QueueName: "normal"})
|
||||
|
||||
var checked string
|
||||
orig := validateResreqSyntax
|
||||
validateResreqSyntax = func(s string) error {
|
||||
checked = s
|
||||
if strings.Contains(s, "100000000a0") {
|
||||
return errors.New("invalid rusage in resource request")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
defer func() { validateResreqSyntax = orig }()
|
||||
|
||||
req := &models.JobSubmitRequest{QueueName: "normal",
|
||||
Resreq: "rusage[mem=100000000a0]"}
|
||||
_, err := preCheckJobRequest(req)
|
||||
if checked != "rusage[mem=100000000a0]" {
|
||||
t.Fatalf("resreq was not syntax-checked (checked %q)", checked)
|
||||
}
|
||||
if err == nil || !strings.Contains(err.Error(), "invalid rusage") {
|
||||
t.Fatalf("malformed resreq must be rejected, got err=%v", err)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue