Files
spark-mcp/internal/mcp/tools/spark_submit_path_test.go
T
tao.chen dab4cd062e 修复 spark_submit 参数重复、上传校验绕过等 8 处缺陷
对应代码审查发现的问题(#1, #2, #4-#8, #10):

#1 CRITICAL:spark_submit 在 argv 中重复拼接 binary。此前
    cmd := []string{binary} 后又把 cmd 作为 Args 传给 executor.Run,
    而 executor 会再拼一次 Binary,导致 OS argv 为 [binary, binary, ...],
    spark-submit 会把自身当作应用 jar。现在 cmd 从 --master 开始,
    executor.Run 使用 Binary + Args,argv 正确。
#2:Validate 曾接受 .meta.json 路径本身。现在显式拒绝 sidecar 路径,
    要求传入数据文件路径。
#4:Sweep 对 sidecar 损坏的数据文件跳过清理。现在损坏 sidecar 会回退
    到数据文件 mtime,超期即删除。
#5:upload_file 描述仍引用已移除的 args 字段,已改为引用 script_path
    及结构化字段。
#6:Deps.UploadStore 改为值类型 uploads.Store,避免 nil 绕过上传校验;
    移除 spark_submit/upload_file 中的 nil 检查。
#7:master/queue/executor_memory 增加空字符串校验。
#8:提取 buildSparkSubmitCommand 构建 argv,消除双写参数的结构性根因。
#10:Validate 失败时记录 slog.Warn("spark_submit.unminted_path_rejected")。

新增测试:
- TestSparkSubmit_StructuredCommand:断言 argv 首行为 --master,末行
  仍为 script_path。
- TestSparkSubmit_EmptyMaster:空 master 返回错误。
- TestStore_Validate_RejectsSidecarPath:拒绝 .meta.json 路径。
- TestStore_Sweep_DeletesDataWithCorruptSidecar:损坏 sidecar 的数据文件
  被清理。

未在本提交处理:
- #9 cluster.DefaultSubmitArgs 弃用留作后续批次。

Co-Authored-By: tao.chen <93983997+taochen-ct@users.noreply.github.com>
2026-07-14 10:09:48 +08:00

235 lines
6.4 KiB
Go

package tools
import (
"context"
"os"
"path/filepath"
"strings"
"testing"
"time"
"github.com/mark3labs/mcp-go/mcp"
"spark-mcp-go/internal/cluster"
)
func TestSparkSubmit_StructuredCommand(t *testing.T) {
deps, repo := testDepsWithDataDir(t)
deps.SparkSubmitTimeout = 5 * time.Second
store := deps.UploadStore
fileID, _, _, _, absPath, err := store.Save([]byte("print('hi')\n"), "hello.py")
if err != nil {
t.Fatalf("save upload: %v", err)
}
echoed := filepath.Join(t.TempDir(), "echoed")
echoScript := filepath.Join(t.TempDir(), "echo-args.sh")
if err := os.WriteFile(echoScript, []byte("#!/bin/sh\nfor arg do\n echo \"$arg\" >> \"$ECHO_FILE\"\ndone\n"), 0o755); err != nil {
t.Fatalf("write echo script: %v", err)
}
t.Setenv("ECHO_FILE", echoed)
createCluster(t, repo, &cluster.Cluster{
ID: "cluster-echo",
Name: "Echo",
IsActive: true,
AuthType: cluster.AuthNone,
SparkSubmitExecuteBin: echoScript,
})
req := newToolRequest(SparkSubmitName, map[string]any{
"cluster_id": "cluster-echo",
"master": "yarn",
"deploy_mode": "cluster",
"script_path": absPath,
"queue": "default",
"executor_memory": "2G",
"executor_cores": 1,
"num_executors": 4,
"spark_conf": map[string]any{"a": "1", "b": "2"},
"extra_args": map[string]any{"name": "wordcount-job"},
})
res, err := deps.SparkSubmitHandler(context.Background(), req)
if err != nil {
t.Fatalf("handler error: %v", err)
}
if res.IsError {
t.Fatalf("unexpected error result: %v", res.Content)
}
got, err := os.ReadFile(echoed)
if err != nil {
t.Fatalf("read echoed args: %v", err)
}
lines := strings.Split(strings.TrimSpace(string(got)), "\n")
want := []string{
"--master", "yarn",
"--queue", "default",
"--executor-memory", "2G",
"--executor-cores", "1",
"--num-executors", "4",
"--conf", "a=1",
"--conf", "b=2",
"--name", "wordcount-job",
absPath,
}
if len(lines) != len(want) {
t.Fatalf("lines=%v\nwant=%v", lines, want)
}
if lines[0] != "--master" {
t.Errorf("line[0]=%q, want \"--master\"", lines[0])
}
for i, l := range lines {
if l != want[i] {
t.Errorf("line[%d]=%q, want %q", i, l, want[i])
}
}
if lines[len(lines)-1] != absPath {
t.Errorf("last line=%q, want script_path %q", lines[len(lines)-1], absPath)
}
if !strings.HasSuffix(absPath, "/"+fileID) {
t.Errorf("absPath=%q does not end with fileID %q", absPath, fileID)
}
}
func TestSparkSubmit_MissingRequiredField(t *testing.T) {
deps, _ := testDepsWithDataDir(t)
store := deps.UploadStore
_, _, _, _, mintedPath, err := store.Save([]byte("# dummy\n"), "dummy.py")
if err != nil {
t.Fatalf("save upload: %v", err)
}
req := newToolRequest(SparkSubmitName, map[string]any{
"cluster_id": "cluster-echo",
"deploy_mode": "cluster",
"script_path": mintedPath,
"queue": "default",
"executor_memory": "2G",
"executor_cores": 1,
"num_executors": 4,
})
res, err := deps.SparkSubmitHandler(context.Background(), req)
if err != nil {
t.Fatalf("handler error: %v", err)
}
if !res.IsError {
t.Fatalf("expected error result, got: %v", res.Content)
}
text, ok := mcp.AsTextContent(res.Content[0])
if !ok {
t.Fatalf("content is not text: %T", res.Content[0])
}
if !strings.Contains(text.Text, "master") {
t.Errorf("error text=%q, want mention of master", text.Text)
}
}
func TestSparkSubmit_BadSparkConfValue(t *testing.T) {
deps, _ := testDepsWithDataDir(t)
store := deps.UploadStore
_, _, _, _, mintedPath, err := store.Save([]byte("# dummy\n"), "dummy.py")
if err != nil {
t.Fatalf("save upload: %v", err)
}
req := newToolRequest(SparkSubmitName, map[string]any{
"cluster_id": "cluster-echo",
"master": "yarn",
"deploy_mode": "cluster",
"script_path": mintedPath,
"queue": "default",
"executor_memory": "2G",
"executor_cores": 1,
"num_executors": 4,
"spark_conf": map[string]any{"a": 1},
})
res, err := deps.SparkSubmitHandler(context.Background(), req)
if err != nil {
t.Fatalf("handler error: %v", err)
}
if !res.IsError {
t.Fatalf("expected error result, got: %v", res.Content)
}
text, ok := mcp.AsTextContent(res.Content[0])
if !ok {
t.Fatalf("content is not text: %T", res.Content[0])
}
if !strings.Contains(text.Text, "spark_conf") {
t.Errorf("error text=%q, want mention of spark_conf", text.Text)
}
}
func TestSparkSubmit_RejectsNonMintedPath(t *testing.T) {
deps, _ := testDepsWithDataDir(t)
unminted := filepath.Join(t.TempDir(), "unminted.py")
if err := os.WriteFile(unminted, []byte("print('not from upload_file')\n"), 0o644); err != nil {
t.Fatalf("write unminted file: %v", err)
}
req := newToolRequest(SparkSubmitName, map[string]any{
"cluster_id": "cluster-echo",
"master": "yarn",
"deploy_mode": "cluster",
"script_path": unminted,
"queue": "default",
"executor_memory": "2G",
"executor_cores": 1,
"num_executors": 4,
})
res, err := deps.SparkSubmitHandler(context.Background(), req)
if err != nil {
t.Fatalf("handler error: %v", err)
}
if !res.IsError {
t.Fatalf("expected error result, got: %v", res.Content)
}
text, ok := mcp.AsTextContent(res.Content[0])
if !ok {
t.Fatalf("content is not text: %T", res.Content[0])
}
if !strings.Contains(text.Text, "not from upload_file") {
t.Errorf("error text=%q, want mention of not from upload_file", text.Text)
}
}
func TestSparkSubmit_EmptyMaster(t *testing.T) {
deps, _ := testDepsWithDataDir(t)
store := deps.UploadStore
_, _, _, _, mintedPath, err := store.Save([]byte("# dummy\n"), "dummy.py")
if err != nil {
t.Fatalf("save upload: %v", err)
}
req := newToolRequest(SparkSubmitName, map[string]any{
"cluster_id": "cluster-echo",
"master": "",
"deploy_mode": "cluster",
"script_path": mintedPath,
"queue": "default",
"executor_memory": "2G",
"executor_cores": 1,
"num_executors": 4,
})
res, err := deps.SparkSubmitHandler(context.Background(), req)
if err != nil {
t.Fatalf("handler error: %v", err)
}
if !res.IsError {
t.Fatalf("expected error result, got: %v", res.Content)
}
text, ok := mcp.AsTextContent(res.Content[0])
if !ok {
t.Fatalf("content is not text: %T", res.Content[0])
}
if !strings.Contains(text.Text, "master is required") && !strings.Contains(text.Text, "master") {
t.Errorf("error text=%q, want mention of master", text.Text)
}
}