diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index d8590a02..c8f6cd3a 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -805,6 +805,8 @@ func deliveryCommand(ctx context.Context, args []string) error { moduleDirs := set.String("module-dirs", "", "the directories holding a module.json at the head, comma-separated") removed := set.String("removed", "", "the files it deletes, comma-separated") membersJSON := set.String("members", "", "a group's members, as JSON") + membersFile := set.String("members-file", "", "a file of a group's members, as JSON: the same as --members, for a list "+ + "too long for an argument (novox/hq issue 1272)") group := set.String("group", "", "a delivery group's id") ready := set.Bool("ready", false, "the head's pull request is marked ready: its check is queued before every other check "+ "(novox/hq ADR 0406 §3)") @@ -819,6 +821,13 @@ func deliveryCommand(ctx context.Context, args []string) error { return err } var members []orderMember + if *membersFile != "" { + raw, err := os.ReadFile(*membersFile) + if err != nil { + return fmt.Errorf("--members-file: %w", err) + } + *membersJSON = string(raw) + } if *membersJSON != "" { if err := json.Unmarshal([]byte(*membersJSON), &members); err != nil { return fmt.Errorf("the members are not readable: %w", err) diff --git a/cmd/mesh-controller/members_file_test.go b/cmd/mesh-controller/members_file_test.go new file mode 100644 index 00000000..23dc27b0 --- /dev/null +++ b/cmd/mesh-controller/members_file_test.go @@ -0,0 +1,93 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "slices" + "strings" + "testing" +) + +// aLongGroup is members as JSON over 200 KB: 1666 paths of the shape of mesh-delivery #5's change (novox/hq issue 1272). +func aLongGroup(t *testing.T) string { + t.Helper() + var paths []string + for i := 0; i < 1666; i++ { + paths = append(paths, fmt.Sprintf("modules/mesh-delivery-forge-bridge/vendor/github.com/nats-io/nats-server/v2/server/certstore/certstore_windows_file_%04d.go", i)) + } + body, err := json.Marshal([]map[string]any{{"id": "novox/mesh-delivery#5", "paths": paths}, {"id": "novox/mesh-catalog#374"}}) + if err != nil { + t.Fatal(err) + } + if len(body) < 200<<10 { + t.Fatalf("the members are only %d bytes", len(body)) + } + return string(body) +} + +func readBackFromTheCommand(t *testing.T, argv []string, members string) { + t.Helper() + for _, a := range argv { + if len(a) > 64<<10 { + t.Errorf("an argument of %d bytes is handed to the command", len(a)) + } + } + i := slices.Index(argv, "--members-file") + if i < 0 { + t.Fatalf("the command does not name a members file: %.200v", argv) + } + raw, err := os.ReadFile(argv[i+1]) + if err != nil || string(raw) != members { + t.Errorf("the file does not hold the whole members list (%d bytes, %v)", len(raw), err) + } + removeMembersFile(argv) + if _, err := os.Stat(argv[i+1]); !os.IsNotExist(err) { + t.Errorf("the file is still there after the command: %v", err) + } +} + +func TestDeliveryCheckHandsALongMembersListInAFile(t *testing.T) { // 1272/AC1 + members := aLongGroup(t) + argv, err := argvFor("delivery-check", map[string]any{"group": "feat/x", "members": members}) + if err != nil { + t.Fatal(err) + } + if argv[0] != "delivery" || argv[1] != "check" || !slices.Contains(argv, "--group") { + t.Errorf("the command is %.100v", strings.Join(argv, " ")) + } + readBackFromTheCommand(t, argv, members) +} + +func TestDeliveryOrderHandsALongMembersListInAFile(t *testing.T) { // 1272/AC2 + members := aLongGroup(t) + argv, err := argvFor("delivery-order", map[string]any{"members": members}) + if err != nil { + t.Fatal(err) + } + if argv[0] != "delivery" || argv[1] != "order" { + t.Errorf("the command is %.100v", strings.Join(argv, " ")) + } + readBackFromTheCommand(t, argv, members) +} + +func TestAShortMembersListIsStillTheArgumentItWas(t *testing.T) { // 1272/AC3 + members := `[{"id":"novox/a#1"},{"id":"novox/b#2"}]` + argv, err := argvFor("delivery-check", map[string]any{"group": "feat/x", "members": members, "ready": "true"}) + want := []string{"delivery", "check", "--group", "feat/x", "--members", members, "--ready"} + if err != nil || !slices.Equal(argv, want) { + t.Errorf("a short list gives %v, %v, want %v", argv, err, want) + } +} + +func TestTheCommandReadsTheMembersFromTheFileAsFromTheArgument(t *testing.T) { // 1272/AC1 + members := `[{"id":"novox/a#1"}]` + argv, _ := membersFlags(strings.Repeat(" ", membersInArgument) + members) + defer removeMembersFile(argv) + if len(argv) != 2 || argv[0] != "--members-file" { + t.Fatalf("a long list gives %v", argv) + } + if st, err := os.Stat(argv[1]); err != nil || st.Mode().Perm() != 0o600 { + t.Errorf("the file is %v, %v", st, err) + } +} diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 99c1204e..da238e3f 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -10,6 +10,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "slices" "sort" "strings" @@ -408,14 +409,22 @@ func (a *verbArguments) commandLine() ([]string, error) { if err := need("members"); err != nil { return nil, err } - return []string{"delivery", "order", "--members", str("members")}, nil + members, err := membersFlags(str("members")) + if err != nil { + return nil, err + } + return append([]string{"delivery", "order"}, members...), nil case "delivery-check": if err := need("members"); err != nil { return nil, err } - argv := []string{"delivery", "check", "--members", str("members")} + members, err := membersFlags(str("members")) + if err != nil { + return nil, err + } + argv := append([]string{"delivery", "check"}, members...) if g := str("group"); g != "" { - argv = []string{"delivery", "check", "--group", g, "--members", str("members")} + argv = append([]string{"delivery", "check", "--group", g}, members...) } // Marked ready (novox/hq ADR 0406 §3): the check is queued before every other check. if r := strings.ToLower(str("ready")); r != "" && r != "false" && r != "no" && r != "0" { @@ -1193,6 +1202,7 @@ func runVerb(ctx context.Context, argv []string) (verbAnswer, error) { if handingOver.Load() { return verbAnswer{}, fmt.Errorf("%w: this controller is stopping and runs no new command", link.ErrHandingOver) } + defer removeMembersFile(argv) cmd := selfCommand(ctx, argv) // And who asked, so an act it does by hand is recorded as theirs (novox/hq to-be 45 §7). caller := link.CallerIn(ctx) @@ -1858,3 +1868,41 @@ func terminalOnly(argv []string) error { "decide who may become root on it (novox/hq ADR 0266). A verb may run node list and node show. "+ "Nothing was done", sub) } + +// membersInArgument is how long a group's members may be as one argument of the command a verb re-executes: over +// the kernel's 128 KiB for one argument the command cannot start (novox/hq issue 1272). +const membersInArgument = 32 << 10 + +// membersDirPrefix names the private directory a long list of members is handed over in. +const membersDirPrefix = "mesh-members-" + +// membersFlags is a group's members as the flags of the command a verb re-executes: `--members ` while the +// list is short, as a person would type it, and `--members-file ` to a file in a private directory of its own +// when it is not. runVerb removes the directory once the command has ended. +func membersFlags(members string) ([]string, error) { + if len(members) <= membersInArgument { + return []string{"--members", members}, nil + } + dir, err := os.MkdirTemp("", membersDirPrefix) + if err != nil { + return nil, fmt.Errorf("the members cannot be handed to the command: %w", err) + } + file := filepath.Join(dir, "members.json") + if err := os.WriteFile(file, []byte(members), 0o600); err != nil { + os.RemoveAll(dir) + return nil, fmt.Errorf("the members cannot be handed to the command: %w", err) + } + return []string{"--members-file", file}, nil +} + +// removeMembersFile removes the private directory membersFlags made for a command line, and only such a directory. +func removeMembersFile(argv []string) { + for i, a := range argv { + if a == "--members-file" && i+1 < len(argv) { + dir := filepath.Dir(argv[i+1]) + if strings.HasPrefix(filepath.Base(dir), membersDirPrefix) { + os.RemoveAll(dir) + } + } + } +}