From d2bb9de24516a102d6171986955b8b0ae4cc3eb9 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 16 Aug 2026 20:30:45 -0700 Subject: [PATCH] Add sentryctl agents CLI surface sentryctl agents list|get, config get|set|clear, restart -- same list/get shape as dashboards/alerts, plus a config sub-subcommand mirroring dashboards' permissions since an override has its own lifecycle distinct from the agent resource itself. config set is the one command with real logic: since PUT /agents/{host}/config replaces the whole stored override rather than patching individual fields, it fetches the agent's current effective config first and merges only the flags actually passed on top of it, mirroring the web UI's edit form logic in Go. restart requires explicit confirmation (interactive y/N or --yes) and refuses on non-interactive stdin without --yes, the same posture cmd_query.go's --nl/--execute already established for anything that changes what's running. Live-verified the merge logic specifically, since it's the part most likely to hide a real bug: setting one field on a clean agent correctly carried forward its other reported values, and a second config set call correctly carried forward the first call's override rather than resetting it to the reported baseline. config clear and restart --yes both round-tripped against a live agent, with the restart picked up and acted on within one check-in cycle. This closes out the agent-management punch list (restart, fleet-wide alerting, this CLI surface) -- see /docs/agent-management-design.md. --- cli/cmd/sentryctl/cmd_agents.go | 351 +++++++++++++++++++++++++++ cli/cmd/sentryctl/cmd_agents_test.go | 279 +++++++++++++++++++++ cli/cmd/sentryctl/httpclient.go | 21 ++ cli/cmd/sentryctl/main.go | 16 ++ docs/agent-management-design.md | 56 +++++ 5 files changed, 723 insertions(+) create mode 100644 cli/cmd/sentryctl/cmd_agents.go create mode 100644 cli/cmd/sentryctl/cmd_agents_test.go diff --git a/cli/cmd/sentryctl/cmd_agents.go b/cli/cmd/sentryctl/cmd_agents.go new file mode 100644 index 0000000..86219de --- /dev/null +++ b/cli/cmd/sentryctl/cmd_agents.go @@ -0,0 +1,351 @@ +// Command surface for api/agents -- agent inventory, remote config, and +// lifecycle commands (see /docs/agent-management-design.md). Same +// list/get shape as dashboards/alerts, plus a "config" sub-subcommand +// (mirroring dashboards' "permissions") since an agent's remote +// override has its own get/set/clear lifecycle distinct from the +// resource itself. +package main + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "strconv" +) + +func cmdAgents(args []string, stdout, stderr io.Writer) int { + if len(args) == 0 { + fmt.Fprintln(stderr, "sentryctl agents: expected a subcommand (list, get, config, restart)") + return 1 + } + apiURL, rest := extractAPIFlag(args[1:], os.Getenv) + token := resolveToken(os.Getenv) + + switch args[0] { + case "list": + return httpGetJSON(apiURL, "/agents", token, stdout, stderr) + case "get": + if len(rest) == 0 { + fmt.Fprintln(stderr, "sentryctl agents get: missing host") + return 1 + } + return httpGetJSON(apiURL, "/agents/"+rest[0], token, stdout, stderr) + case "config": + if len(rest) == 0 { + fmt.Fprintln(stderr, "sentryctl agents config: expected a subcommand (get, set, clear)") + return 1 + } + return cmdAgentsConfig(rest, apiURL, token, stdout, stderr) + case "restart": + if len(rest) == 0 { + fmt.Fprintln(stderr, "sentryctl agents restart: missing host") + return 1 + } + // os.Stdin passed explicitly at this inner layer (not threaded + // through cmdAgents' own signature) so the confirmation prompt + // is testable the same way cmd_query.go's cmdQueryNL is -- tests + // call cmdAgentsRestart directly with a fake reader. + return cmdAgentsRestart(rest, apiURL, token, os.Stdin, stdout, stderr) + default: + fmt.Fprintf(stderr, "sentryctl agents: unknown subcommand %q (want list, get, config, restart)\n", args[0]) + return 1 + } +} + +func cmdAgentsConfig(args []string, apiURL, token string, stdout, stderr io.Writer) int { + sub, rest := args[0], args[1:] + switch sub { + case "get": + if len(rest) == 0 { + fmt.Fprintln(stderr, "sentryctl agents config get: missing host") + return 1 + } + // Same GET /agents/{host} as plain "get" -- an agent's reported + // config, desired override, and pending/applied status are all + // one resource server-side; a narrower "config-only" response + // shape isn't worth a second endpoint just for this command. + return httpGetJSON(apiURL, "/agents/"+rest[0], token, stdout, stderr) + case "set": + if len(rest) == 0 { + fmt.Fprintln(stderr, "sentryctl agents config set: missing host") + return 1 + } + return cmdAgentsConfigSet(rest[0], rest[1:], apiURL, token, stdout, stderr) + case "clear": + if len(rest) == 0 { + fmt.Fprintln(stderr, "sentryctl agents config clear: missing host") + return 1 + } + return httpMutateNoBody(http.MethodDelete, apiURL, "/agents/"+rest[0]+"/config", token, "", "config override cleared -- agent will run its local agent.toml again", stdout, stderr) + default: + fmt.Fprintf(stderr, "sentryctl agents config: unknown subcommand %q (want get, set, clear)\n", sub) + return 1 + } +} + +// agentInfo is a CLI-local mirror of api/agents.Agent's JSON shape -- +// only the fields config-merging actually needs. Deliberately +// duplicated rather than imported (cli is a separate Go module from +// api), same convention as every other cross-module shared shape in +// this codebase (see ingest/internal/agentregistry.overrideFields). +type agentInfo struct { + SourceKind string `json:"source_kind"` + BatchMaxSize int64 `json:"batch_max_size"` + BatchFlushIntervalMS int64 `json:"batch_flush_interval_ms"` + HeartbeatEnabled bool `json:"heartbeat_enabled"` + HeartbeatIntervalMS int64 `json:"heartbeat_interval_ms"` + DesiredOverride *agentConfigOverride `json:"desired_override,omitempty"` +} + +type agentConfigOverride struct { + BatchMaxSize *int64 `json:"batch_max_size,omitempty"` + BatchFlushIntervalMS *int64 `json:"batch_flush_interval_ms,omitempty"` + HeartbeatEnabled *bool `json:"heartbeat_enabled,omitempty"` + HeartbeatIntervalMS *int64 `json:"heartbeat_interval_ms,omitempty"` + JournaldUnit *string `json:"journald_unit,omitempty"` +} + +// cmdAgentsConfigSet parses --batch-max-size/--batch-flush-interval-ms/ +// --heartbeat-enabled/--heartbeat-interval-ms/--journald-unit, fetches +// the agent's current effective config, and PUTs the complete merged +// override -- api/agents.Store.SetOverride replaces the whole stored +// override, it doesn't patch individual fields (same as the web UI's +// edit form, see /docs/agent-management-design.md), so every field this +// command doesn't touch has to be carried forward from whatever's +// currently in effect (the existing override if one's set, otherwise +// the agent's reported value) rather than silently reset to zero. +func cmdAgentsConfigSet(host string, flagArgs []string, apiURL, token string, stdout, stderr io.Writer) int { + var ( + batchMaxSize, batchFlushMS, heartbeatMS *int64 + heartbeatEnabled *bool + journaldUnit *string + ) + for i := 0; i < len(flagArgs); i++ { + flag := flagArgs[i] + next := func() (string, bool) { + if i+1 >= len(flagArgs) { + return "", false + } + i++ + return flagArgs[i], true + } + switch flag { + case "--batch-max-size": + v, ok := next() + if !ok { + fmt.Fprintln(stderr, "sentryctl agents config set: --batch-max-size requires a value") + return 1 + } + n, err := strconv.ParseInt(v, 10, 64) + if err != nil { + fmt.Fprintf(stderr, "sentryctl agents config set: invalid --batch-max-size %q: %v\n", v, err) + return 1 + } + batchMaxSize = &n + case "--batch-flush-interval-ms": + v, ok := next() + if !ok { + fmt.Fprintln(stderr, "sentryctl agents config set: --batch-flush-interval-ms requires a value") + return 1 + } + n, err := strconv.ParseInt(v, 10, 64) + if err != nil { + fmt.Fprintf(stderr, "sentryctl agents config set: invalid --batch-flush-interval-ms %q: %v\n", v, err) + return 1 + } + batchFlushMS = &n + case "--heartbeat-interval-ms": + v, ok := next() + if !ok { + fmt.Fprintln(stderr, "sentryctl agents config set: --heartbeat-interval-ms requires a value") + return 1 + } + n, err := strconv.ParseInt(v, 10, 64) + if err != nil { + fmt.Fprintf(stderr, "sentryctl agents config set: invalid --heartbeat-interval-ms %q: %v\n", v, err) + return 1 + } + heartbeatMS = &n + case "--heartbeat-enabled": + v, ok := next() + if !ok { + fmt.Fprintln(stderr, "sentryctl agents config set: --heartbeat-enabled requires true or false") + return 1 + } + b, err := strconv.ParseBool(v) + if err != nil { + fmt.Fprintf(stderr, "sentryctl agents config set: invalid --heartbeat-enabled %q: %v\n", v, err) + return 1 + } + heartbeatEnabled = &b + case "--journald-unit": + v, ok := next() + if !ok { + fmt.Fprintln(stderr, "sentryctl agents config set: --journald-unit requires a value (empty string clears the filter)") + return 1 + } + journaldUnit = &v + default: + fmt.Fprintf(stderr, "sentryctl agents config set: unknown flag %q\n", flag) + return 1 + } + } + if batchMaxSize == nil && batchFlushMS == nil && heartbeatMS == nil && heartbeatEnabled == nil && journaldUnit == nil { + fmt.Fprintln(stderr, "sentryctl agents config set: at least one of --batch-max-size, --batch-flush-interval-ms, --heartbeat-enabled, --heartbeat-interval-ms, --journald-unit is required") + return 1 + } + + current, err := fetchAgent(apiURL, host, token) + if err != nil { + fmt.Fprintf(stderr, "sentryctl agents config set: fetching current state: %v\n", err) + return 1 + } + + mergedBatchMax := mergeInt64(batchMaxSize, overrideBatchMaxSize(current), current.BatchMaxSize) + mergedBatchFlush := mergeInt64(batchFlushMS, overrideBatchFlushMS(current), current.BatchFlushIntervalMS) + mergedHeartbeatMS := mergeInt64(heartbeatMS, overrideHeartbeatMS(current), current.HeartbeatIntervalMS) + mergedHeartbeatEnabled := mergeBool(heartbeatEnabled, overrideHeartbeatEnabled(current), current.HeartbeatEnabled) + + merged := agentConfigOverride{ + BatchMaxSize: &mergedBatchMax, + BatchFlushIntervalMS: &mergedBatchFlush, + HeartbeatEnabled: &mergedHeartbeatEnabled, + HeartbeatIntervalMS: &mergedHeartbeatMS, + } + // journald_unit only applies (and is only ever sent) when the + // agent's actual source is journald -- ignored server-side + // otherwise anyway, but sending it for a non-journald agent would + // be misleading in the stored override. Matches + // web/src/routes/agents/[host]/+page.svelte's save() exactly. + if current.SourceKind == "journald" { + unit := "" + if u := overrideJournaldUnit(current); u != nil { + unit = *u + } + if journaldUnit != nil { + unit = *journaldUnit + } + merged.JournaldUnit = &unit + } + + body, err := json.Marshal(merged) + if err != nil { + fmt.Fprintf(stderr, "sentryctl agents config set: encoding request: %v\n", err) + return 1 + } + return httpPutJSON(apiURL, "/agents/"+host+"/config", token, string(body), stdout, stderr) +} + +func mergeInt64(flag, override *int64, reported int64) int64 { + if flag != nil { + return *flag + } + if override != nil { + return *override + } + return reported +} + +func mergeBool(flag, override *bool, reported bool) bool { + if flag != nil { + return *flag + } + if override != nil { + return *override + } + return reported +} + +func overrideBatchMaxSize(a *agentInfo) *int64 { + if a.DesiredOverride == nil { + return nil + } + return a.DesiredOverride.BatchMaxSize +} + +func overrideBatchFlushMS(a *agentInfo) *int64 { + if a.DesiredOverride == nil { + return nil + } + return a.DesiredOverride.BatchFlushIntervalMS +} + +func overrideHeartbeatMS(a *agentInfo) *int64 { + if a.DesiredOverride == nil { + return nil + } + return a.DesiredOverride.HeartbeatIntervalMS +} + +func overrideHeartbeatEnabled(a *agentInfo) *bool { + if a.DesiredOverride == nil { + return nil + } + return a.DesiredOverride.HeartbeatEnabled +} + +func overrideJournaldUnit(a *agentInfo) *string { + if a.DesiredOverride == nil { + return nil + } + return a.DesiredOverride.JournaldUnit +} + +func fetchAgent(apiURL, host, token string) (*agentInfo, error) { + req, err := http.NewRequest(http.MethodGet, apiURL+"/agents/"+host, nil) + if err != nil { + return nil, err + } + setAuth(req, token) + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("request failed: %w", err) + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("reading response: %w", err) + } + if resp.StatusCode != http.StatusOK { + var errResp errorResponseBody + if json.Unmarshal(body, &errResp) == nil && errResp.Error != "" { + return nil, fmt.Errorf("%s", errResp.Error) + } + return nil, fmt.Errorf("status %d", resp.StatusCode) + } + var info agentInfo + if err := json.Unmarshal(body, &info); err != nil { + return nil, fmt.Errorf("decoding response: %w", err) + } + return &info, nil +} + +// cmdAgentsRestart requires explicit confirmation -- interactive y/N, +// or --yes for scripted use -- same "never run something disruptive +// without an explicit signal" posture as cmd_query.go's --execute for +// running an AI-translated query, matching restart's own real (if +// brief) blast radius: it interrupts log collection on that host until +// the service manager brings the agent back up. +func cmdAgentsRestart(args []string, apiURL, token string, stdin io.Reader, stdout, stderr io.Writer) int { + host := args[0] + yes := false + for _, a := range args[1:] { + if a == "--yes" || a == "-y" { + yes = true + } + } + if !yes { + if !isInteractive(stdin) { + fmt.Fprintln(stdout, "Not restarting: pass --yes to confirm non-interactively.") + return 1 + } + prompt := fmt.Sprintf("Restart agent %q? This briefly interrupts log collection on that host.", host) + if !confirmRun(stdin, stdout, prompt) { + fmt.Fprintln(stdout, "Not restarting.") + return 0 + } + } + return httpPutJSON(apiURL, "/agents/"+host+"/command", token, `{"command":"restart"}`, stdout, stderr) +} diff --git a/cli/cmd/sentryctl/cmd_agents_test.go b/cli/cmd/sentryctl/cmd_agents_test.go new file mode 100644 index 0000000..f179d42 --- /dev/null +++ b/cli/cmd/sentryctl/cmd_agents_test.go @@ -0,0 +1,279 @@ +package main + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestCmdAgentsMissingSubcommand(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdAgents(nil, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +func TestCmdAgentsListSuccess(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/agents" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`[{"host":"web-01","service":"web"}]`)) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"list", "--api", srv.URL}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "web-01") { + t.Fatalf("stdout = %q, want it to contain the listed agent", stdout.String()) + } +} + +func TestCmdAgentsGetMissingHost(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"get"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +func TestCmdAgentsGetSuccess(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/agents/web-01" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"host":"web-01"}`)) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"get", "web-01", "--api", srv.URL}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } +} + +func TestCmdAgentsConfigMissingSubcommand(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"config"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +func TestCmdAgentsConfigClearSuccess(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete || r.URL.Path != "/agents/web-01/config" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"config", "clear", "web-01", "--api", srv.URL}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "cleared") { + t.Fatalf("stdout = %q, want a confirmation", stdout.String()) + } +} + +func TestCmdAgentsConfigSetRequiresAtLeastOneFlag(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"config", "set", "web-01"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "at least one of") { + t.Fatalf("stderr = %q, want it to explain a flag is required", stderr.String()) + } +} + +func TestCmdAgentsConfigSetInvalidValue(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"config", "set", "web-01", "--batch-max-size", "not-a-number"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +// TestCmdAgentsConfigSetMergesUntouchedFields is the regression test for +// the whole point of the merge logic: setting only --heartbeat-interval-ms +// must carry forward the agent's OTHER already-set override field +// (batch_max_size) and its reported (non-overridden) values for +// everything else, not silently reset them. +func TestCmdAgentsConfigSetMergesUntouchedFields(t *testing.T) { + var putBody []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/agents/web-01": + w.Write([]byte(`{ + "source_kind": "journald", + "batch_max_size": 500, + "batch_flush_interval_ms": 2000, + "heartbeat_enabled": true, + "heartbeat_interval_ms": 60000, + "desired_override": {"batch_max_size": 1000} + }`)) + case r.Method == http.MethodPut && r.URL.Path == "/agents/web-01/config": + putBody, _ = io.ReadAll(r.Body) + w.Write(putBody) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"config", "set", "web-01", "--heartbeat-interval-ms", "30000", "--api", srv.URL}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } + + var sent agentConfigOverride + if err := json.Unmarshal(putBody, &sent); err != nil { + t.Fatalf("decoding PUT body: %v", err) + } + if sent.BatchMaxSize == nil || *sent.BatchMaxSize != 1000 { + t.Fatalf("BatchMaxSize = %v, want 1000 (carried forward from the existing override)", sent.BatchMaxSize) + } + if sent.BatchFlushIntervalMS == nil || *sent.BatchFlushIntervalMS != 2000 { + t.Fatalf("BatchFlushIntervalMS = %v, want 2000 (carried forward from reported value)", sent.BatchFlushIntervalMS) + } + if sent.HeartbeatEnabled == nil || *sent.HeartbeatEnabled != true { + t.Fatalf("HeartbeatEnabled = %v, want true (carried forward from reported value)", sent.HeartbeatEnabled) + } + if sent.HeartbeatIntervalMS == nil || *sent.HeartbeatIntervalMS != 30000 { + t.Fatalf("HeartbeatIntervalMS = %v, want 30000 (the flag actually passed)", sent.HeartbeatIntervalMS) + } +} + +// TestCmdAgentsConfigSetOmitsJournaldUnitForNonJournaldSource mirrors +// web/src/routes/agents/[host]/+page.svelte's save(): journald_unit +// must never be sent for an agent whose source isn't journald, even if +// a stale override somehow had one. +func TestCmdAgentsConfigSetOmitsJournaldUnitForNonJournaldSource(t *testing.T) { + var putBody []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet: + w.Write([]byte(`{"source_kind":"file","batch_max_size":500,"batch_flush_interval_ms":2000,"heartbeat_enabled":true,"heartbeat_interval_ms":60000}`)) + case r.Method == http.MethodPut: + putBody, _ = io.ReadAll(r.Body) + w.Write(putBody) + } + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"config", "set", "file-host", "--batch-max-size", "100", "--api", srv.URL}, &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } + if strings.Contains(string(putBody), "journald_unit") { + t.Fatalf("PUT body = %s, must not carry journald_unit for a non-journald source", putBody) + } +} + +func TestCmdAgentsConfigSetFetchError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"error":"agent not found"}`)) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"config", "set", "nope", "--batch-max-size", "100", "--api", srv.URL}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "agent not found") { + t.Fatalf("stderr = %q, want the server's actual error surfaced", stderr.String()) + } +} + +func TestCmdAgentsRestartMissingHost(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"restart"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} + +func TestCmdAgentsRestartWithYesSkipsConfirmation(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPut || r.URL.Path != "/agents/web-01/command" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + body, _ := io.ReadAll(r.Body) + if !strings.Contains(string(body), `"command":"restart"`) { + t.Errorf("body = %s, want command=restart", body) + } + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"host":"web-01","pending_command":"restart"}`)) + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + code := cmdAgentsRestart([]string{"web-01", "--yes"}, srv.URL, "", strings.NewReader(""), &stdout, &stderr) + if code != 0 { + t.Fatalf("code = %d, want 0; stderr=%s", code, stderr.String()) + } +} + +// The interactive confirm-prompt branch (confirmRun asked, "y"/"n" +// answered) isn't reachable in a test the way cmdAgentsRestart is +// structured -- isInteractive checks the concrete *os.File type, which +// a strings.Reader can never satisfy, same constraint cmd_query.go's +// own tests work around by testing confirmRun directly (see +// TestConfirmRunAcceptsY/TestConfirmRunRejectsBlankAndOther in +// cmd_query_test.go) rather than through the full non-interactive gate. +// Those two generic tests already cover the y/n logic this command +// relies on; only the two paths actually reachable with a non-tty +// stdin -- --yes and no-confirmation-possible -- are tested below. + +// TestCmdAgentsRestartNonInteractiveWithoutYesRefuses guards against a +// scripted/piped invocation hanging forever waiting for an answer +// nobody can give -- same posture as cmd_query.go's isInteractive check +// for --nl without --execute. +func TestCmdAgentsRestartNonInteractiveWithoutYesRefuses(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("must not call the server without --yes when stdin isn't a terminal") + })) + defer srv.Close() + + var stdout, stderr bytes.Buffer + // strings.Reader is never a *os.File, so isInteractive(it) is + // always false -- exercising the same "piped stdin" path a real + // non-interactive invocation would hit. + code := cmdAgentsRestart([]string{"web-01"}, srv.URL, "", strings.NewReader(""), &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } + if !strings.Contains(stdout.String(), "--yes") { + t.Fatalf("stdout = %q, want it to mention --yes", stdout.String()) + } +} + +func TestCmdAgentsUnknownSubcommand(t *testing.T) { + var stdout, stderr bytes.Buffer + code := cmdAgents([]string{"bogus"}, &stdout, &stderr) + if code != 1 { + t.Fatalf("code = %d, want 1", code) + } +} diff --git a/cli/cmd/sentryctl/httpclient.go b/cli/cmd/sentryctl/httpclient.go index 649432e..be5d391 100644 --- a/cli/cmd/sentryctl/httpclient.go +++ b/cli/cmd/sentryctl/httpclient.go @@ -110,6 +110,27 @@ func httpMutateNoBody(method, baseURL, path, token, body, successMsg string, std return 0 } +// httpPutJSON PUTs body (already-encoded JSON) to path and prints the +// pretty-printed JSON response -- same shape as httpPostFileJSON, but +// for callers that construct the body themselves rather than reading it +// from a file (agents config set, agents restart). +func httpPutJSON(baseURL, path, token, body string, stdout, stderr io.Writer) int { + req, err := http.NewRequest(http.MethodPut, baseURL+path, strings.NewReader(body)) + if err != nil { + fmt.Fprintf(stderr, "building request: %v\n", err) + return 1 + } + req.Header.Set("Content-Type", "application/json") + setAuth(req, token) + resp, err := httpClient.Do(req) + if err != nil { + fmt.Fprintf(stderr, "request failed: %v\n", err) + return 1 + } + defer resp.Body.Close() + return printJSONResponse(resp, stdout, stderr) +} + func printJSONResponse(resp *http.Response, stdout, stderr io.Writer) int { body, err := io.ReadAll(resp.Body) if err != nil { diff --git a/cli/cmd/sentryctl/main.go b/cli/cmd/sentryctl/main.go index 21864bb..315982a 100644 --- a/cli/cmd/sentryctl/main.go +++ b/cli/cmd/sentryctl/main.go @@ -38,6 +38,8 @@ func run(args []string, stdout, stderr io.Writer) int { return cmdDashboards(args[1:], stdout, stderr) case "alerts": return cmdAlerts(args[1:], stdout, stderr) + case "agents": + return cmdAgents(args[1:], stdout, stderr) case "-h", "--help", "help": usage(stdout) return 0 @@ -59,6 +61,12 @@ Usage: sentryctl dashboards permissions grant viewer|editor [--api ] sentryctl dashboards permissions revoke [--api ] sentryctl alerts list|get |apply [--alerting-api ] + sentryctl agents list|get [--api ] + sentryctl agents config get |clear [--api ] + sentryctl agents config set [--batch-max-size N] [--batch-flush-interval-ms N] + [--heartbeat-enabled true|false] [--heartbeat-interval-ms N] + [--journald-unit UNIT] [--api ] + sentryctl agents restart [--yes] [--api ] Commands: ping Checks that the api service is reachable via GET /healthz. @@ -79,6 +87,14 @@ Commands: alerts list/get/apply against alerting's rule CRUD endpoints. "apply " creates a rule from a JSON file with the same shape POST /rules accepts. + agents Agent inventory, remote config, and lifecycle commands + (see /docs/agent-management-design.md). "config set" reads + the agent's current effective config first and PUTs back + the complete merged override -- only the fields you pass + change, everything else carries forward unchanged, same + as the web UI's edit form. "restart" briefly interrupts + log collection on that host and prompts for confirmation + unless --yes is given. --api defaults to $SENTRYCTL_API_URL, or `+defaultAPIURL+` if unset. --alerting-api defaults to $SENTRYCTL_ALERTING_API_URL, or `+defaultAlertingURL+` if unset. diff --git a/docs/agent-management-design.md b/docs/agent-management-design.md index 3742b0d..974cc6b 100644 --- a/docs/agent-management-design.md +++ b/docs/agent-management-design.md @@ -255,3 +255,59 @@ command` is picked up on the agent's very next check-in, logged the process exits cleanly -- `pending_command` confirmed cleared and `ps` confirming the process gone. +## CLI surface (punch-list item 3) + +`sentryctl agents` (`cli/cmd/sentryctl/cmd_agents.go`), same list/get +shape as `dashboards`/`alerts`, plus a `config` sub-subcommand +(mirroring `dashboards permissions`) since an override has its own +get/set/clear lifecycle distinct from the agent resource itself: + +``` +sentryctl agents list|get +sentryctl agents config get |clear +sentryctl agents config set [--batch-max-size N] [--batch-flush-interval-ms N] + [--heartbeat-enabled true|false] [--heartbeat-interval-ms N] + [--journald-unit UNIT] +sentryctl agents restart [--yes] +``` + +`config set` is the one command with real logic beyond a thin HTTP +wrapper: since `PUT /agents/{host}/config` replaces the whole stored +override (not a per-field patch), the command fetches the agent's +current effective config first and merges only the flags actually +passed on top of it -- whatever's already overridden stays overridden, +whatever isn't falls back to the agent's reported value -- exactly +mirroring `web/src/routes/agents/[host]/+page.svelte`'s edit form logic +in Go instead of Svelte. `restart` requires explicit confirmation +(interactive y/N, or `--yes` for scripted use) and refuses outright on +a non-interactive stdin without `--yes`, the same posture +`cmd_query.go`'s `--nl`/`--execute` already established for anything +that can actually change what's running. + +**Verified live**, including the merge logic specifically (the part +most likely to have a real bug): `config set --heartbeat-interval-ms +20000` on an agent with no existing override correctly carried forward +its reported `batch_max_size`/`batch_flush_interval_ms`/ +`heartbeat_enabled`; a second `config set --batch-max-size 750` call +correctly carried forward the *first* call's `heartbeat_interval_ms: +20000` override rather than resetting it to the reported `5000` -- +confirming the read-current-then-merge step actually reads the +override, not just the reported baseline. `config clear` and `restart +--yes` both round-tripped against the live stack; the restart was +picked up by the real agent process on its next check-in, logged, and +the process exited cleanly, closing the loop from a single CLI command +all the way to real process behavior. + +## Punch list: complete + +All three items from `/docs/agent-heartbeat-monitoring.md`'s original +punch list are done: lifecycle commands (restart), fleet-wide alerting +(via the existing raw-SQL escape hatch, no engine changes), and this +CLI surface. Real, disclosed remaining gaps, not oversights: `stop`/ +`uninstall` lifecycle commands (need genuine per-platform OS service- +manager integration), true per-host multi-row alerting from one rule +(needs the alerting engine's own per-group state tracking, a Phase 3 +non-goal for the whole engine, not agent-specific), and a scripted +rule-per-host generator (a real alternative to the fleet-wide aggregate +check, not built since it's tooling that could live under this same CLI +surface if ever wanted).