diff --git a/.goreleaser.yml b/.goreleaser.yml index 39ac510a..17d7bc1b 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -48,6 +48,7 @@ builds: - GOWORK=off ldflags: - -s + - -X main.version={{.Version}} goos: - linux goarch: diff --git a/e2e/volume_mutation_test.go b/e2e/volume_mutation_test.go index 9fd17e57..5fcedef8 100644 --- a/e2e/volume_mutation_test.go +++ b/e2e/volume_mutation_test.go @@ -60,3 +60,20 @@ func TestRejectedVolumeTargetExplainsRefusalWithoutChangingState(t *testing.T) { t.Fatalf("rejected request changed settled state: %+v err=%v", view, err) } } + +func TestEmptyVolumePatchChangesNothing(t *testing.T) { + image := writeVolumeImage(t, "dataset.img", "dataset-bytes") + stack := startTenantStack(t, "node-token", nil, []config.VolumeSpec{{Name: "dataset", Path: image}}) + sb, err := stack.client.New(t.Context(), "rt:24.04") + if err != nil { + t.Fatal(err) + } + defer sb.Close() + if _, err = sb.SetVolumes(t.Context(), []sandbox.Volume{{Name: "dataset"}}); err != nil { + t.Fatal(err) + } + got, err := sb.PatchVolumes(t.Context(), sandbox.VolumePatch{Add: []sandbox.Volume{}, Remove: []string{}}) + if err != nil || len(got.Volumes) != 1 || got.Volumes[0].Name != "dataset" || got.Converging { + t.Fatalf("empty patch: %+v err=%v", got, err) + } +} diff --git a/e2e/wire_parity_test.go b/e2e/wire_parity_test.go new file mode 100644 index 00000000..84312e84 --- /dev/null +++ b/e2e/wire_parity_test.go @@ -0,0 +1,156 @@ +package e2e + +import ( + "cmp" + "go/ast" + "go/parser" + "go/token" + "maps" + "path/filepath" + "reflect" + "slices" + "strconv" + "strings" + "testing" + + "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/pool" + "github.com/cocoonstack/sandbox/sandboxd/server" + "github.com/cocoonstack/sandbox/sandboxd/types" +) + +const sdkDir = "../sdk/go" + +var wirePairs = []struct { + sdk string + sends bool + server any + allow []string +}{ + {"claimRequest", true, server.ClaimRequest{}, []string{"egress"}}, + {"claimResponse", false, server.ClaimResponse{}, nil}, + {"forkRequest", true, server.ForkRequest{}, []string{"claim_ref_prefix", "on_expire"}}, + {"forkResponse", false, server.ForkResponse{}, nil}, + {"renewRequest", true, server.RenewRequest{}, []string{"on_expire"}}, + {"renewResponse", false, server.RenewResponse{}, nil}, + {"promoteRequest", true, server.PromoteRequest{}, nil}, + {"promoteResponse", false, server.PromoteResponse{}, nil}, + {"previewRequest", true, server.PreviewRequest{}, nil}, + {"previewResponse", false, server.PreviewResponse{}, nil}, + {"checkpointRequest", true, server.CheckpointRequest{}, nil}, + {"checkpointResponse", false, server.CheckpointResponse{}, nil}, + {"checkpointClaimRequest", true, server.CheckpointClaimRequest{}, nil}, + {"checkpointListResponse", false, server.CheckpointListResponse{}, nil}, + {"checkpointRecord", false, types.Checkpoint{}, []string{"key", "tenant", "policy_source", "no_egress", "guest_env", "archive"}}, + {"volumeListResponse", false, server.VolumeListResponse{}, nil}, + {"VolumeInfo", false, types.VolumeInfo{}, nil}, + {"VolumeState", false, types.VolumeState{}, nil}, + {"SandboxVolumesResponse", false, types.SandboxVolumesResponse{}, nil}, + {"VolumePatch", true, types.VolumePatch{}, nil}, + {"Volume", true, types.Volume{}, nil}, + {"EnvVar", true, types.EnvVar{}, nil}, + {"EnvInject", true, types.EnvInject{}, nil}, + {"envBody", true, server.SandboxEnv{}, nil}, + {"envPatchBody", true, server.SandboxEnvPatch{}, nil}, + {"PoolKey", false, types.PoolKey{}, nil}, + {"NodeInfo", false, server.InfoResponse{}, nil}, + {"PoolStatus", false, pool.PoolInfo{}, nil}, + {"TemplateStatus", false, pool.TemplateInfo{}, nil}, + {"SandboxSummary", false, pool.SandboxSummary{}, []string{"token"}}, + {"sandboxListResponse", false, server.SandboxListResponse{}, nil}, + {"poolUpdate", true, server.PoolUpdateRequest{}, nil}, + {"PoolSpec", true, config.PoolSpec{}, []string{"egress", "warmup", "egress_upstream_env", "capture_trim", "storage", "net", "size"}}, + {"tenantsUpdate", true, server.TenantsRequest{}, nil}, + {"tenantUpdate", true, server.TenantRequest{}, nil}, + {"TenantSpec", true, reflect.Zero(reflect.TypeOf(server.TenantsRequest{}.Tenants).Elem()).Interface(), []string{"token"}}, + {"TenantInfo", false, pool.TenantInfo{}, nil}, + {"TenantList", false, pool.TenantPage{}, nil}, +} + +func TestSDKStructsMatchServerWireShape(t *testing.T) { + sdkFields := parseSDKFields(t) + for _, tt := range wirePairs { + t.Run(tt.sdk, func(t *testing.T) { + got, ok := sdkFields[tt.sdk] + if !ok { + t.Fatalf("sdk struct %s not found in %s", tt.sdk, sdkDir) + } + want := serverFields(reflect.TypeOf(tt.server)) + for name, opts := range want { + gotOpts, present := got[name] + differs := !present || (tt.sends && gotOpts != opts) + switch allowed := slices.Contains(tt.allow, name); { + case differs && !allowed: + t.Errorf("%s: field %q differs: sdk present=%t options=%q, server options=%q", tt.sdk, name, present, gotOpts, opts) + case !differs && allowed: + t.Errorf("%s: field %q is allowlisted but matches", tt.sdk, name) + } + } + for name := range got { + if _, present := want[name]; !present { + t.Errorf("%s: field %q is in the SDK but not on the server", tt.sdk, name) + } + } + }) + } +} + +func serverFields(typ reflect.Type) map[string]string { + fields := map[string]string{} + for f := range typ.Fields() { + name, opts := splitJSONTag(f.Tag.Get("json")) + switch { + case name == "-" && opts == "": + case f.Anonymous && name == "" && f.Type.Kind() == reflect.Struct: + maps.Copy(fields, serverFields(f.Type)) + case f.IsExported(): + fields[cmp.Or(name, f.Name)] = opts + } + } + return fields +} + +func parseSDKFields(t *testing.T) map[string]map[string]string { + t.Helper() + paths, err := filepath.Glob(filepath.Join(sdkDir, "*.go")) + if err != nil { + t.Fatal(err) + } + out := map[string]map[string]string{} + for _, path := range paths { + if strings.HasSuffix(path, "_test.go") { + continue + } + src, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.SkipObjectResolution) + if err != nil { + t.Fatal(err) + } + ast.Inspect(src, func(n ast.Node) bool { + spec, ok := n.(*ast.TypeSpec) + if !ok { + return true + } + if st, ok := spec.Type.(*ast.StructType); ok { + out[spec.Name.Name] = map[string]string{} + for _, f := range st.Fields.List { + if f.Tag == nil { + continue + } + tag, _ := strconv.Unquote(f.Tag.Value) + if name, opts := splitJSONTag(reflect.StructTag(tag).Get("json")); name != "" && name != "-" { + out[spec.Name.Name][name] = opts + } + } + } + return true + }) + } + return out +} + +func splitJSONTag(tag string) (name, opts string) { + name, rest, _ := strings.Cut(tag, ",") + options := strings.Split(rest, ",") + slices.Sort(options) + return name, strings.Join(options, ",") +} diff --git a/mcp/server.go b/mcp/server.go index acd2badc..a38912c9 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -24,6 +24,8 @@ const ( defaultToolTTL = time.Hour ) +var version = "devel" + type rpcRequest struct { ID jsontext.Value `json:"id"` Method string `json:"method"` @@ -107,7 +109,7 @@ func (s *server) dispatch(ctx context.Context, req *rpcRequest) rpcResponse { return result(req.ID, map[string]any{ "protocolVersion": protocolVersion, "capabilities": map[string]any{"tools": map[string]any{}}, - "serverInfo": map[string]any{"name": "sandbox-mcp", "version": "0.1.0"}, + "serverInfo": map[string]any{"name": "sandbox-mcp", "version": version}, }) case "tools/list": return result(req.ID, map[string]any{"tools": toolSpecs()}) diff --git a/mcp/server_test.go b/mcp/server_test.go index c86f3fa8..07f7cb56 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -57,6 +57,9 @@ func TestServeSpeaksMCP(t *testing.T) { if initResult["protocolVersion"] != protocolVersion { t.Errorf("initialize protocolVersion = %v", initResult["protocolVersion"]) } + if info := initResult["serverInfo"].(map[string]any); info["version"] != version { + t.Errorf("initialize serverInfo = %v, want version %q", info, version) + } tools := replies[2]["result"].(map[string]any)["tools"].([]any) if len(tools) != len(toolSpecs()) { t.Errorf("tools/list: %d tools, want %d", len(tools), len(toolSpecs())) diff --git a/protocol/wire/frame.go b/protocol/wire/frame.go index eaf0a635..16a5f8ed 100644 --- a/protocol/wire/frame.go +++ b/protocol/wire/frame.go @@ -133,6 +133,8 @@ var ( func(d []byte) Response { return &Stderr{Data: d} }, func(d []byte) Response { return &DataResp{Data: d} }, ) + + dataRespHead = bulkResponses["data"].head ) // Request is a client→server frame; Op is its wire tag. @@ -700,6 +702,11 @@ func DecodeResponseInto(line []byte, scratch *[]byte) (Response, error) { return dec(line) } +// DecodeData slices a canonical data response frame's payload into *scratch without allocating a Response; ok is false for any other shape, which takes DecodeResponseInto. +func DecodeData(line []byte, scratch *[]byte) ([]byte, bool) { + return decodeBulk(line, dataRespHead, scratch) +} + // NewFrameScanner wraps r for newline-delimited frames capped at MaxFrame. func NewFrameScanner(r io.Reader) *bufio.Scanner { sc := bufio.NewScanner(r) diff --git a/protocol/wire/frame_test.go b/protocol/wire/frame_test.go index 95b39b44..6eb316cd 100644 --- a/protocol/wire/frame_test.go +++ b/protocol/wire/frame_test.go @@ -274,6 +274,25 @@ func TestDecodeResponseIntoReusesScratch(t *testing.T) { } } +func TestDecodeDataTakesOnlyCanonicalDataFrames(t *testing.T) { + var scratch []byte + got, ok := DecodeData([]byte(`{"type":"data","data":"aGVsbG8="}`), &scratch) + if !ok || string(got) != "hello" { + t.Fatalf("canonical frame: %q, %v", got, ok) + } + for _, frame := range []string{ + `{"type":"stdout","data":"aGVsbG8="}`, + `{"type":"data","meta":{"data":"WFhY"},"data":"aGk="}`, + `{"type":"data","data":"aGk="}garbage`, + "{\"type\":\"data\",\"data\":\"Q\nQ==\"}", + `{"type":"done"}`, + } { + if data, ok := DecodeData([]byte(frame), &scratch); ok { + t.Errorf("frame %s decoded as %q", frame, data) + } + } +} + func TestDecodeResponseIntoMatchesDecodeResponse(t *testing.T) { entries, err := os.ReadDir(fixtureDir) if err != nil { diff --git a/sandboxd/engine/portconn.go b/sandboxd/engine/portconn.go index a2d3b84b..05312151 100644 --- a/sandboxd/engine/portconn.go +++ b/sandboxd/engine/portconn.go @@ -3,7 +3,6 @@ package engine import ( "bufio" "bytes" - "encoding/base64" "encoding/json/v2" "errors" "fmt" @@ -16,8 +15,6 @@ import ( // portReadBuf fits silkd's data frames in one buffered read. const portReadBuf = wire.BulkChunk*4/3 + 64 -var portDataHead = []byte(`{"type":"data","data":"`) - // guestPortConn adapts silkd's port_forward data frames to a plain net.Conn. type guestPortConn struct { net.Conn @@ -37,7 +34,7 @@ func (g *guestPortConn) Read(p []byte) (int, error) { if err != nil { return 0, err } - if data, ok := g.fastPortData(line); ok { + if data, ok := wire.DecodeData(bytes.TrimSuffix(line, []byte("\n")), &g.rbuf); ok { g.pending = data continue } @@ -102,25 +99,3 @@ func (g *guestPortConn) readFrame() ([]byte, error) { } return long, err } - -// fastPortData slices the canonical data frame's base64 out without a JSON parse. -func (g *guestPortConn) fastPortData(line []byte) ([]byte, bool) { - after, ok := bytes.CutPrefix(line, portDataHead) - if !ok { - return nil, false - } - b64, rest, ok := bytes.Cut(after, []byte(`"`)) - if !ok || !bytes.Equal(rest, []byte("}\n")) { - return nil, false - } - if size := base64.StdEncoding.DecodedLen(len(b64)); cap(g.rbuf) < size { - g.rbuf = make([]byte, size) - } else { - g.rbuf = g.rbuf[:size] - } - n, err := base64.StdEncoding.Decode(g.rbuf, b64) - if err != nil || base64.StdEncoding.EncodedLen(n) != len(b64) { - return nil, false - } - return g.rbuf[:n], true -} diff --git a/sandboxd/types/api.go b/sandboxd/types/api.go index 3699c09b..ac0cd6b4 100644 --- a/sandboxd/types/api.go +++ b/sandboxd/types/api.go @@ -24,8 +24,8 @@ type VolumeInfo struct { // VolumePatch is the wire body of PATCH /v1/sandboxes/{id}/volumes. type VolumePatch struct { - Add []Volume `json:"add,omitempty"` - Remove []string `json:"remove,omitempty"` + Add []Volume `json:"add,omitzero"` + Remove []string `json:"remove,omitzero"` } // VolumePhase is how far one held volume has converged.