From bc921666deb154c1a9720496b85df26fb2c5280b Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 14 May 2026 03:19:34 +0000 Subject: [PATCH] feat: reject job submissions to 'lost_and_found' reserved queue in preCheckJobRequest --- src/frontend/apiserver/controllers/jobs.go | 7 ++ .../apiserver/controllers/jobs_test.go | 76 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 src/frontend/apiserver/controllers/jobs_test.go diff --git a/src/frontend/apiserver/controllers/jobs.go b/src/frontend/apiserver/controllers/jobs.go index af592d7b..16eecfdc 100644 --- a/src/frontend/apiserver/controllers/jobs.go +++ b/src/frontend/apiserver/controllers/jobs.go @@ -1535,6 +1535,13 @@ func preCheckJobRequest(req *models.JobSubmitRequest) ([]int, error) { if req.QueueName == "" { req.QueueName = "normal" } else { + // Reject submissions to the reserved internal recovery queue. + if req.QueueName == "lost_and_found" { + logger.Error("Job submission to reserved queue rejected", + zap.String("queue_name", req.QueueName)) + return nil, fmt.Errorf( + "cannot submit jobs to 'lost_and_found' — this is an internal recovery queue") + } //validate queue_name whether exists queueInfo, ok := store.Queues.Get(req.QueueName) if !ok { diff --git a/src/frontend/apiserver/controllers/jobs_test.go b/src/frontend/apiserver/controllers/jobs_test.go new file mode 100644 index 00000000..26727c9b --- /dev/null +++ b/src/frontend/apiserver/controllers/jobs_test.go @@ -0,0 +1,76 @@ +package controllers + +import ( + "apiserver/models" + "apiserver/store" + "apiserver/utils" + "strings" + "testing" + + "go.uber.org/zap" +) + +func resetJobSubmitState() { + store.InitStores() + utils.Log = zap.NewNop() +} + +// TestPreCheckJobRequest_LostAndFound verifies that submitting to the +// "lost_and_found" queue is rejected with a clear error message. +func TestPreCheckJobRequest_LostAndFound(t *testing.T) { + resetJobSubmitState() + + req := &models.JobSubmitRequest{ + QueueName: "lost_and_found", + } + + _, err := preCheckJobRequest(req) + if err == nil { + t.Fatal("expected error when submitting to lost_and_found queue, got nil") + } + if !strings.Contains(err.Error(), "lost_and_found") { + t.Errorf("error message should mention 'lost_and_found', got: %v", err) + } + if !strings.Contains(err.Error(), "internal recovery queue") { + t.Errorf("error message should mention 'internal recovery queue', got: %v", err) + } +} + +// TestPreCheckJobRequest_NormalQueue verifies that a normal (non-reserved) +// queue name does not trigger the reserved-name rejection. +func TestPreCheckJobRequest_NormalQueue(t *testing.T) { + resetJobSubmitState() + + // Seed a valid open queue in the store. + store.Queues.Put("normal", models.QueueInfo{ + QueueName: "normal", + QueueStatus: 0, // open + }) + + req := &models.JobSubmitRequest{ + QueueName: "normal", + } + + _, err := preCheckJobRequest(req) + // Should not be rejected for the reserved-name check; may fail for other + // reasons (e.g. missing JobName validation), but not with our new error. + if err != nil && strings.Contains(err.Error(), "internal recovery queue") { + t.Errorf("normal queue should not be rejected with reserved-name error: %v", err) + } +} + +// TestPreCheckJobRequest_EmptyQueue verifies that an empty queue name is +// allowed (defaults to "normal") and is not confused with reserved names. +func TestPreCheckJobRequest_EmptyQueue(t *testing.T) { + resetJobSubmitState() + + req := &models.JobSubmitRequest{ + QueueName: "", + } + + // Empty queue name sets default — should not trigger reserved-name error. + _, err := preCheckJobRequest(req) + if err != nil && strings.Contains(err.Error(), "internal recovery queue") { + t.Errorf("empty queue should not trigger reserved-name error: %v", err) + } +}