diff --git a/api/agents/handler.go b/api/agents/handler.go index ebac861..615d5d1 100644 --- a/api/agents/handler.go +++ b/api/agents/handler.go @@ -169,6 +169,17 @@ func (h *Handler) handleSetConfig(w http.ResponseWriter, r *http.Request) { } } + // service_log_retention_days requires RoleOwner too, for exactly the + // same reason log_retention_days does -- it's the same override-floor + // mechanism, just keyed per service instead of once for the whole + // host, so it needs the same "any change, not just raising" gate. + if identity, ok := authz.IdentityFromContext(r.Context()); ok && !identity.Role.Satisfies(authz.RoleOwner) { + if changesServiceLogRetentionDays(h.currentServiceLogRetentionDays(r.Context(), h.tenantID(r), r.PathValue("host")), override.ServiceLogRetentionDays) { + writeError(w, http.StatusForbidden, "service_log_retention_days requires the owner role") + return + } + } + a, err := h.store.SetOverride(r.Context(), h.tenantID(r), r.PathValue("host"), override, h.updatedBy(r)) if err != nil { h.writeStoreErr(w, err, "setting agent config") @@ -259,6 +270,20 @@ func validateOverride(o ConfigOverride) error { if o.LogRetentionDays != nil && (*o.LogRetentionDays < 1 || *o.LogRetentionDays > 3650) { return errors.New("log_retention_days must be between 1 and 3650") } + // 200 services is far beyond any real host's log source variety -- + // exists only to reject a pathological/malformed request, matching + // extra_file_paths' own cap-for-sanity-not-realistic-use posture. + if len(o.ServiceLogRetentionDays) > 200 { + return errors.New("service_log_retention_days: at most 200 services") + } + for service, days := range o.ServiceLogRetentionDays { + if service == "" { + return errors.New("service_log_retention_days: service name must not be empty") + } + if days < 1 || days > 3650 { + return fmt.Errorf("service_log_retention_days[%q] must be between 1 and 3650", service) + } + } return nil } @@ -367,6 +392,30 @@ func changesLogRetentionDays(current, desired *int) bool { return *current != *desired } +// currentServiceLogRetentionDays mirrors currentLogRetentionDays exactly. +func (h *Handler) currentServiceLogRetentionDays(ctx context.Context, tenantID, host string) map[string]int { + a, err := h.store.Get(ctx, tenantID, host) + if err != nil || a.DesiredOverride == nil { + return nil + } + return a.DesiredOverride.ServiceLogRetentionDays +} + +// changesServiceLogRetentionDays reports whether desired differs from +// current at all -- a full map comparison, same "no safe direction" +// posture as changesLogRetentionDays. +func changesServiceLogRetentionDays(current, desired map[string]int) bool { + if len(current) != len(desired) { + return true + } + for service, days := range desired { + if cur, ok := current[service]; !ok || cur != days { + return true + } + } + return false +} + func (h *Handler) writeStoreErr(w http.ResponseWriter, err error, action string) { if errors.Is(err, ErrNotFound) { writeError(w, http.StatusNotFound, "agent not found") diff --git a/api/agents/handler_test.go b/api/agents/handler_test.go index ea47049..b25c966 100644 --- a/api/agents/handler_test.go +++ b/api/agents/handler_test.go @@ -376,6 +376,72 @@ func TestHandleSetConfigRejectsInvalidLogRetentionDays(t *testing.T) { } } +// TestHandleSetConfigServiceLogRetentionDaysRequiresOwner mirrors +// TestHandleSetConfigLogRetentionDaysRequiresOwner exactly -- the +// per-service map has the same owner-only, no-safe-direction gate as +// the single host-level value. +func TestHandleSetConfigServiceLogRetentionDaysRequiresOwner(t *testing.T) { + s := newFakeStore() + s.put(Agent{TenantID: "default", Host: "web-01"}) + admin := NewHandler(discardLogger(), s, fakeAuthorizer{role: authz.RoleAdmin}, nil) + owner := NewHandler(discardLogger(), s, fakeAuthorizer{role: authz.RoleOwner}, nil) + + rec := doRequest(t, admin, "PUT", "/agents/web-01/config", ConfigOverride{ + ServiceLogRetentionDays: map[string]int{"smtp": 365}, + }) + if rec.Code != http.StatusForbidden { + t.Fatalf("admin setting service_log_retention_days: status = %d, want 403", rec.Code) + } + + rec = doRequest(t, owner, "PUT", "/agents/web-01/config", ConfigOverride{ + ServiceLogRetentionDays: map[string]int{"smtp": 365}, + }) + if rec.Code != http.StatusOK { + t.Fatalf("owner setting service_log_retention_days: status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + var got Agent + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decoding response: %v", err) + } + if got.DesiredOverride == nil || got.DesiredOverride.ServiceLogRetentionDays["smtp"] != 365 { + t.Fatalf("stored override = %+v, want service_log_retention_days[smtp]=365", got.DesiredOverride) + } + + // Changing the value of an existing entry is gated the same as + // adding a new one. + rec = doRequest(t, admin, "PUT", "/agents/web-01/config", ConfigOverride{ + ServiceLogRetentionDays: map[string]int{"smtp": 30}, + }) + if rec.Code != http.StatusForbidden { + t.Fatalf("admin changing service_log_retention_days: status = %d, want 403", rec.Code) + } + + // Clearing it (omitting the field) is gated too. + rec = doRequest(t, admin, "PUT", "/agents/web-01/config", ConfigOverride{}) + if rec.Code != http.StatusForbidden { + t.Fatalf("admin clearing service_log_retention_days: status = %d, want 403", rec.Code) + } +} + +func TestHandleSetConfigRejectsInvalidServiceLogRetentionDays(t *testing.T) { + s := newFakeStore() + s.put(Agent{TenantID: "default", Host: "web-01"}) + owner := NewHandler(discardLogger(), s, fakeAuthorizer{role: authz.RoleOwner}, nil) + + cases := []map[string]int{ + {"smtp": 0}, + {"smtp": -1}, + {"smtp": 3651}, + {"": 30}, + } + for _, days := range cases { + rec := doRequest(t, owner, "PUT", "/agents/web-01/config", ConfigOverride{ServiceLogRetentionDays: days}) + if rec.Code != http.StatusBadRequest { + t.Errorf("service_log_retention_days=%v: status = %d, want 400", days, rec.Code) + } + } +} + func TestHandleSetConfigUnknownHostIsNotFound(t *testing.T) { h := newTestHandler(newFakeStore()) interval := int64(30000) diff --git a/api/agents/store.go b/api/agents/store.go index 4a45211..b9c2359 100644 --- a/api/agents/store.go +++ b/api/agents/store.go @@ -53,6 +53,19 @@ type ConfigOverride struct { // an agent" is exactly what it conceptually is, even though nothing // ever ships it to the agent process itself. LogRetentionDays *int `json:"log_retention_days,omitempty"` + // ServiceLogRetentionDays is LogRetentionDays' per-service refinement: + // this host's `logs` rows are tagged with a `service` value (e.g. + // "nginx", "smtp", "ufw" -- see storage/migrations/0001_create_logs_table.sql + // and /docs/agent-management-design.md's note that distinct services + // on one host come from running separate agent processes, each with + // its own agent.toml `service`), and an operator may want to keep one + // service's logs longer than the rest of the host's default. A + // service present here overrides LogRetentionDays for that service + // only; every other service on the host still falls back to + // LogRetentionDays (or no floor at all if that's unset too). Same + // "never shipped to the agent process, central policy metadata only" + // posture as LogRetentionDays -- see logretention.AgentRetentionStore. + ServiceLogRetentionDays map[string]int `json:"service_log_retention_days,omitempty"` } type Agent struct { diff --git a/api/logretention/agent_floor.go b/api/logretention/agent_floor.go index 374c01d..b3546d3 100644 --- a/api/logretention/agent_floor.go +++ b/api/logretention/agent_floor.go @@ -2,19 +2,34 @@ package logretention import ( "context" + "encoding/json" + "fmt" + "strconv" "github.com/jackc/pgx/v5/pgxpool" ) +// HostFloor is one host's configured retention protection -- +// DefaultDays (from api/agents.ConfigOverride.LogRetentionDays) applies +// to any service on this host with no more specific entry; +// ServiceDays (from ConfigOverride.ServiceLogRetentionDays) overrides +// that default for the services named in it. Either may be absent +// independently (a host can have a service override with no host +// default, or vice versa). +type HostFloor struct { + DefaultDays *int + ServiceDays map[string]int +} + // AgentRetentionStore reads the protective retention floors set on -// agents.ConfigOverride.LogRetentionDays -- a separate, Postgres-backed -// concern from Store's ClickHouse access above, so it lives in its own -// file. Deliberately its own narrow query against the same `agents` -// table api/agents.Store manages, rather than importing api/agents for -// a shared type, matching this codebase's "each package owns direct -// SQL access to what it needs" convention (e.g. alerting and api both -// read dashboards-adjacent tables independently rather than sharing a -// store type across a package boundary). +// agents.ConfigOverride -- a separate, Postgres-backed concern from +// Store's ClickHouse access above, so it lives in its own file. +// Deliberately its own narrow query against the same `agents` table +// api/agents.Store manages, rather than importing api/agents for a +// shared type, matching this codebase's "each package owns direct SQL +// access to what it needs" convention (e.g. alerting and api both read +// dashboards-adjacent tables independently rather than sharing a store +// type across a package boundary). type AgentRetentionStore struct { pool *pgxpool.Pool } @@ -23,31 +38,57 @@ func NewAgentRetentionStore(pool *pgxpool.Pool) *AgentRetentionStore { return &AgentRetentionStore{pool: pool} } -// RetentionDaysByHost reports the configured log_retention_days for -// every agent that has one set, keyed by host -- a host absent from -// this map has no configured floor at all. Per-host rather than a -// single global maximum: now that deletion is host-scoped -// (Handler.partitionHosts), a floor on one host must never block -// deleting another host's logs that happen to be requested in the same -// call. -func (s *AgentRetentionStore) RetentionDaysByHost(ctx context.Context) (map[string]int, error) { +// FloorsByHost reports every host with a configured floor of either +// kind (host-level default, per-service, or both), keyed by host -- a +// host absent from this map has no configured floor at all. +func (s *AgentRetentionStore) FloorsByHost(ctx context.Context) (map[string]HostFloor, error) { rows, err := s.pool.Query(ctx, ` - SELECT host, (desired_override->>'log_retention_days')::int + SELECT host, + desired_override->>'log_retention_days', + (desired_override->'service_log_retention_days')::text FROM agents - WHERE desired_override->>'log_retention_days' IS NOT NULL`) + WHERE desired_override->>'log_retention_days' IS NOT NULL + OR desired_override->'service_log_retention_days' IS NOT NULL`) if err != nil { return nil, err } defer rows.Close() - out := map[string]int{} + out := map[string]HostFloor{} for rows.Next() { var host string - var days int - if err := rows.Scan(&host, &days); err != nil { + var defaultDaysText, serviceDaysJSON *string + if err := rows.Scan(&host, &defaultDaysText, &serviceDaysJSON); err != nil { return nil, err } - out[host] = days + var hf HostFloor + if defaultDaysText != nil { + d, err := strconv.Atoi(*defaultDaysText) + if err != nil { + return nil, fmt.Errorf("parsing log_retention_days for host %q: %w", host, err) + } + hf.DefaultDays = &d + } + if serviceDaysJSON != nil { + hf.ServiceDays = map[string]int{} + if err := json.Unmarshal([]byte(*serviceDaysJSON), &hf.ServiceDays); err != nil { + return nil, fmt.Errorf("parsing service_log_retention_days for host %q: %w", host, err) + } + } + out[host] = hf } return out, rows.Err() } + +// Effective reports the retention floor that applies to service on +// this host: a service-specific override if one exists, otherwise the +// host-level default, otherwise no floor at all. +func (hf HostFloor) Effective(service string) (int, bool) { + if days, ok := hf.ServiceDays[service]; ok { + return days, true + } + if hf.DefaultDays != nil { + return *hf.DefaultDays, true + } + return 0, false +} diff --git a/api/logretention/handler.go b/api/logretention/handler.go index f1bb129..e5cae14 100644 --- a/api/logretention/handler.go +++ b/api/logretention/handler.go @@ -11,20 +11,22 @@ import ( "github.com/sentry/sentry/api/authz" ) +const maxBodyBytes = 1 << 20 // 1 MiB, same cap as localauth/dashboards/agents + // store is the narrow interface Handler depends on -- *Store (store.go) // is the production implementation; tests use a fake, same pattern as // agents.store/dashboards.store. type store interface { - HostsOlderThan(ctx context.Context, cutoff time.Time) ([]HostCount, error) - CountOlderThan(ctx context.Context, cutoff time.Time, hosts []string) (uint64, error) - DeleteOlderThan(ctx context.Context, cutoff time.Time, hosts []string) error + TargetsOlderThan(ctx context.Context, cutoff time.Time) ([]TargetCount, error) + CountOlderThan(ctx context.Context, cutoff time.Time, targets []HostService) (uint64, error) + DeleteOlderThan(ctx context.Context, cutoff time.Time, targets []HostService) error } // retentionFloor is the narrow interface backing the owner-only // override check -- *AgentRetentionStore (agent_floor.go) is the // production implementation. type retentionFloor interface { - RetentionDaysByHost(ctx context.Context) (map[string]int, error) + FloorsByHost(ctx context.Context) (map[string]HostFloor, error) } // maxOlderThanHours bounds the age a caller can specify -- 10 years is @@ -33,11 +35,12 @@ type retentionFloor interface { // digit) with a clear 400 rather than silently accepting it. const maxOlderThanHours = 10 * 365 * 24 -// maxHosts caps how many host filters one request can carry -- 1000 is -// far beyond any real fleet this deployment's homelab/small-scale -// target implies, and exists only to reject a pathological/malformed -// request rather than to meaningfully restrict real usage. -const maxHosts = 1000 +// maxTargets caps how many (host, service) pairs one request can +// carry -- 2000 is far beyond any real fleet this deployment's +// homelab/small-scale target implies, and exists only to reject a +// pathological/malformed request rather than to meaningfully restrict +// real usage. +const maxTargets = 2000 type Handler struct { logger *slog.Logger @@ -56,17 +59,19 @@ func NewHandler(logger *slog.Logger, store store, floor retentionFloor, authoriz // deleting log data is at least as consequential as the RBAC matrix's // other RoleAdmin-floor actions (e.g. issuing an agent restart // command, api/agents/handler.go), so it gets the same floor rather -// than a stricter RoleOwner-only one. +// than a stricter RoleOwner-only one. preview/delete are POST, not +// GET/DELETE-with-query-params, because a request now carries a list +// of (host, service) pairs -- a JSON body is the natural shape for +// that, not a repeated compound query param. func (h *Handler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("GET /logs/retention/hosts", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handleHosts)) - mux.HandleFunc("GET /logs/retention/preview", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handlePreview)) - mux.HandleFunc("DELETE /logs/retention", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handleDelete)) + mux.HandleFunc("POST /logs/retention/preview", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handlePreview)) + mux.HandleFunc("POST /logs/retention/delete", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handleDelete)) } // parseOlderThanHours reads and validates the older_than_hours query -// param shared by all three routes -- a caller must ask for at least 1 -// hour (an accidental empty/zero value must never mean "delete -// everything"). +// param GET /logs/retention/hosts uses -- preview/delete take the same +// value from their JSON body instead (see deletionRequest). func parseOlderThanHours(r *http.Request) (int, bool) { hours, err := strconv.Atoi(r.URL.Query().Get("older_than_hours")) if err != nil || hours < 1 || hours > maxOlderThanHours { @@ -75,72 +80,96 @@ func parseOlderThanHours(r *http.Request) (int, bool) { return hours, true } -// parseHosts reads the repeated host query param shared by preview and -// delete -- deliberately required (at least one), never "omitted means -// every host": the whole point of this parameter existing is letting a -// caller target specific agents' logs instead of wholesale deleting -// everything, so there is no implicit "all hosts" shortcut here. GET -// /logs/retention/hosts is how a caller discovers what to pass. -// Duplicates are silently deduped; an empty host value is rejected -// outright rather than silently dropped, since a caller sending "" almost -// certainly has a client-side bug worth surfacing. -func parseHosts(r *http.Request) ([]string, bool) { - raw := r.URL.Query()["host"] - seen := make(map[string]struct{}, len(raw)) - var hosts []string - for _, host := range raw { - if host == "" { - return nil, false - } - if _, dup := seen[host]; dup { - continue - } - seen[host] = struct{}{} - hosts = append(hosts, host) - } - if len(hosts) == 0 || len(hosts) > maxHosts { - return nil, false - } - return hosts, true +// deletionRequest is the JSON body preview and delete both take -- +// targets is deliberately required and never empty: there is no +// "omitted means every host/service" shortcut anywhere in this +// package, matching the feature's whole point (select what you mean to +// act on, never wholesale-delete by accident). +type deletionRequest struct { + OlderThanHours int `json:"older_than_hours"` + Targets []HostService `json:"targets"` } -// blockedHost is one entry in a preview/delete response's blocked_hosts -// list -- a host the caller asked about that a configured retention -// floor (api/agents.ConfigOverride.LogRetentionDays) protects from -// anyone but an owner. -type blockedHost struct { +// parseTargets validates and normalizes a decoded request's targets: +// deduped, every host and service non-empty, count within maxTargets. +func parseTargets(targets []HostService) ([]HostService, bool) { + seen := make(map[HostService]struct{}, len(targets)) + var out []HostService + for _, t := range targets { + if t.Host == "" || t.Service == "" { + return nil, false + } + if _, dup := seen[t]; dup { + continue + } + seen[t] = struct{}{} + out = append(out, t) + } + if len(out) == 0 || len(out) > maxTargets { + return nil, false + } + return out, true +} + +func decodeDeletionRequest(w http.ResponseWriter, r *http.Request) (deletionRequest, bool) { + var req deletionRequest + r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeError(w, http.StatusBadRequest, "invalid JSON body: "+err.Error()) + return deletionRequest{}, false + } + if req.OlderThanHours < 1 || req.OlderThanHours > maxOlderThanHours { + writeError(w, http.StatusBadRequest, "older_than_hours must be a positive integer") + return deletionRequest{}, false + } + targets, ok := parseTargets(req.Targets) + if !ok { + writeError(w, http.StatusBadRequest, "targets must name at least one non-empty host/service pair") + return deletionRequest{}, false + } + req.Targets = targets + return req, true +} + +// blockedTarget is one entry in a preview/delete response's +// blocked_targets list -- a (host, service) the caller asked about +// that a configured retention floor (api/agents.ConfigOverride. +// LogRetentionDays / ServiceLogRetentionDays) protects from anyone but +// an owner. +type blockedTarget struct { Host string `json:"host"` + Service string `json:"service"` ProtectedDays int `json:"protected_days"` } -// partitionHosts splits hosts into allowed (this caller may act on +// partitionTargets splits targets into allowed (this caller may act on // them) and blocked (a configured floor protects them from anyone but -// an owner, and this caller isn't one) -- role-scoped rather than a -// single all-or-nothing check, so a floor on one host never blocks -// acting on other hosts requested in the same call. An owner always +// an owner, and this caller isn't one) -- per-target rather than a +// single all-or-nothing check, so a floor on one target never blocks +// acting on other targets requested in the same call. An owner always // gets everything back as allowed, no query needed. -func (h *Handler) partitionHosts(ctx context.Context, role authz.Role, hosts []string, cutoff time.Time) ([]string, []blockedHost, error) { +func (h *Handler) partitionTargets(ctx context.Context, role authz.Role, targets []HostService, cutoff time.Time) ([]HostService, []blockedTarget, error) { if role == authz.RoleOwner { - return hosts, nil, nil + return targets, nil, nil } - floors, err := h.floor.RetentionDaysByHost(ctx) + floors, err := h.floor.FloorsByHost(ctx) if err != nil { return nil, nil, err } now := time.Now().UTC() - var allowed []string - var blocked []blockedHost - for _, host := range hosts { - days, hasFloor := floors[host] + var allowed []HostService + var blocked []blockedTarget + for _, t := range targets { + days, hasFloor := floors[t.Host].Effective(t.Service) if !hasFloor { - allowed = append(allowed, host) + allowed = append(allowed, t) continue } protectedBoundary := now.Add(-time.Duration(days) * 24 * time.Hour) if cutoff.After(protectedBoundary) { - blocked = append(blocked, blockedHost{Host: host, ProtectedDays: days}) + blocked = append(blocked, blockedTarget{Host: t.Host, Service: t.Service, ProtectedDays: days}) } else { - allowed = append(allowed, host) + allowed = append(allowed, t) } } return allowed, blocked, nil @@ -159,23 +188,30 @@ func roleForFloorCheck(ctx context.Context) authz.Role { return identity.Role } -type hostEntry struct { - Host string `json:"host"` +type serviceEntry struct { + Service string `json:"service"` Count uint64 `json:"count"` ProtectedDays *int `json:"protected_days,omitempty"` } +type hostEntry struct { + Host string `json:"host"` + ProtectedDays *int `json:"protected_days,omitempty"` + Services []serviceEntry `json:"services"` +} + type hostsResponse struct { Hosts []hostEntry `json:"hosts"` Cutoff time.Time `json:"cutoff"` } -// handleHosts lists every host with at least one log record older than -// the requested age, annotated with any configured retention floor -- -// informational for every caller regardless of role (RegisterRoutes' -// RoleAdmin floor already gates who can reach this at all); it's what -// populates the host picker a caller then selects from for preview/ -// delete, not itself an action that needs partitionHosts. +// handleHosts lists every (host, service) pair with at least one log +// record older than the requested age, grouped by host and annotated +// with any configured retention floor -- informational for every +// caller regardless of role (RegisterRoutes' RoleAdmin floor already +// gates who can reach this at all); it's what populates the picker a +// caller then selects from for preview/delete, not itself an action +// that needs partitionTargets. func (h *Handler) handleHosts(w http.ResponseWriter, r *http.Request) { hours, ok := parseOlderThanHours(r) if !ok { @@ -184,52 +220,57 @@ func (h *Handler) handleHosts(w http.ResponseWriter, r *http.Request) { } cutoff := time.Now().UTC().Add(-time.Duration(hours) * time.Hour) - counts, err := h.store.HostsOlderThan(r.Context(), cutoff) + counts, err := h.store.TargetsOlderThan(r.Context(), cutoff) if err != nil { - h.logger.Error("listing hosts for retention preview", "error", err) + h.logger.Error("listing targets for retention preview", "error", err) writeError(w, http.StatusInternalServerError, "listing hosts failed") return } - floors, err := h.floor.RetentionDaysByHost(r.Context()) + floors, err := h.floor.FloorsByHost(r.Context()) if err != nil { h.logger.Error("reading log retention floors", "error", err) writeError(w, http.StatusInternalServerError, "listing hosts failed") return } - entries := make([]hostEntry, len(counts)) - for i, c := range counts { - e := hostEntry{Host: c.Host, Count: c.Count} - if days, hasFloor := floors[c.Host]; hasFloor { - d := days - e.ProtectedDays = &d + // counts is ordered by host (Store.TargetsOlderThan), so contiguous + // rows for the same host can be grouped in one pass. + var hosts []hostEntry + for _, c := range counts { + hf := floors[c.Host] + if len(hosts) == 0 || hosts[len(hosts)-1].Host != c.Host { + he := hostEntry{Host: c.Host} + if hf.DefaultDays != nil { + d := *hf.DefaultDays + he.ProtectedDays = &d + } + hosts = append(hosts, he) } - entries[i] = e + se := serviceEntry{Service: c.Service, Count: c.Count} + if days, hasFloor := hf.Effective(c.Service); hasFloor { + se.ProtectedDays = &days + } + hosts[len(hosts)-1].Services = append(hosts[len(hosts)-1].Services, se) } - writeJSON(w, http.StatusOK, hostsResponse{Hosts: entries, Cutoff: cutoff}) + + writeJSON(w, http.StatusOK, hostsResponse{Hosts: hosts, Cutoff: cutoff}) } type previewResponse struct { - Count uint64 `json:"count"` - Cutoff time.Time `json:"cutoff"` - Hosts []string `json:"hosts"` - BlockedHosts []blockedHost `json:"blocked_hosts,omitempty"` + Count uint64 `json:"count"` + Cutoff time.Time `json:"cutoff"` + Targets []HostService `json:"targets"` + BlockedTargets []blockedTarget `json:"blocked_targets,omitempty"` } func (h *Handler) handlePreview(w http.ResponseWriter, r *http.Request) { - hours, ok := parseOlderThanHours(r) + req, ok := decodeDeletionRequest(w, r) if !ok { - writeError(w, http.StatusBadRequest, "older_than_hours must be a positive integer") return } - hosts, ok := parseHosts(r) - if !ok { - writeError(w, http.StatusBadRequest, "at least one host must be specified") - return - } - cutoff := time.Now().UTC().Add(-time.Duration(hours) * time.Hour) + cutoff := time.Now().UTC().Add(-time.Duration(req.OlderThanHours) * time.Hour) - allowed, blocked, err := h.partitionHosts(r.Context(), roleForFloorCheck(r.Context()), hosts, cutoff) + allowed, blocked, err := h.partitionTargets(r.Context(), roleForFloorCheck(r.Context()), req.Targets, cutoff) if err != nil { h.logger.Error("checking log retention floor", "error", err) writeError(w, http.StatusInternalServerError, "checking retention policy failed") @@ -245,14 +286,14 @@ func (h *Handler) handlePreview(w http.ResponseWriter, r *http.Request) { return } } - writeJSON(w, http.StatusOK, previewResponse{Count: count, Cutoff: cutoff, Hosts: allowed, BlockedHosts: blocked}) + writeJSON(w, http.StatusOK, previewResponse{Count: count, Cutoff: cutoff, Targets: allowed, BlockedTargets: blocked}) } type deleteResponse struct { - DeletedCount uint64 `json:"deleted_count"` - Cutoff time.Time `json:"cutoff"` - DeletedHosts []string `json:"deleted_hosts"` - BlockedHosts []blockedHost `json:"blocked_hosts,omitempty"` + DeletedCount uint64 `json:"deleted_count"` + Cutoff time.Time `json:"cutoff"` + DeletedTargets []HostService `json:"deleted_targets"` + BlockedTargets []blockedTarget `json:"blocked_targets,omitempty"` } // handleDelete counts immediately before deleting so the response can @@ -264,19 +305,13 @@ type deleteResponse struct { // disclosed margin for an admin-facing summary number, not something // anything downstream depends on for correctness. func (h *Handler) handleDelete(w http.ResponseWriter, r *http.Request) { - hours, ok := parseOlderThanHours(r) + req, ok := decodeDeletionRequest(w, r) if !ok { - writeError(w, http.StatusBadRequest, "older_than_hours must be a positive integer") return } - hosts, ok := parseHosts(r) - if !ok { - writeError(w, http.StatusBadRequest, "at least one host must be specified") - return - } - cutoff := time.Now().UTC().Add(-time.Duration(hours) * time.Hour) + cutoff := time.Now().UTC().Add(-time.Duration(req.OlderThanHours) * time.Hour) - allowed, blocked, err := h.partitionHosts(r.Context(), roleForFloorCheck(r.Context()), hosts, cutoff) + allowed, blocked, err := h.partitionTargets(r.Context(), roleForFloorCheck(r.Context()), req.Targets, cutoff) if err != nil { h.logger.Error("checking log retention floor", "error", err) writeError(w, http.StatusInternalServerError, "checking retention policy failed") @@ -300,10 +335,10 @@ func (h *Handler) handleDelete(w http.ResponseWriter, r *http.Request) { identity, _ := authz.IdentityFromContext(r.Context()) h.logger.Info("logs deleted by retention age", - "deleted_count", count, "cutoff", cutoff, "hosts", allowed, "user_id", identity.UserID, "role", identity.Role) + "deleted_count", count, "cutoff", cutoff, "targets", allowed, "user_id", identity.UserID, "role", identity.Role) } - writeJSON(w, http.StatusOK, deleteResponse{DeletedCount: count, Cutoff: cutoff, DeletedHosts: allowed, BlockedHosts: blocked}) + writeJSON(w, http.StatusOK, deleteResponse{DeletedCount: count, Cutoff: cutoff, DeletedTargets: allowed, BlockedTargets: blocked}) } func writeJSON(w http.ResponseWriter, status int, v any) { diff --git a/api/logretention/handler_test.go b/api/logretention/handler_test.go index 6222077..51235ce 100644 --- a/api/logretention/handler_test.go +++ b/api/logretention/handler_test.go @@ -1,6 +1,7 @@ package logretention import ( + "bytes" "context" "encoding/json" "errors" @@ -9,7 +10,6 @@ import ( "net/http" "net/http/httptest" "reflect" - "strconv" "testing" "time" @@ -20,43 +20,43 @@ func discardLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) } -// hostCall records one CountOlderThan/DeleteOlderThan invocation, so -// tests can assert both the cutoff and the exact host set a call used. -type hostCall struct { - cutoff time.Time - hosts []string +// targetCall records one CountOlderThan/DeleteOlderThan invocation, so +// tests can assert both the cutoff and the exact target set a call used. +type targetCall struct { + cutoff time.Time + targets []HostService } -// fakeStore lets a test inject store errors and a fixed host listing, +// fakeStore lets a test inject store errors and a fixed target listing, // and records every count/delete call it received so tests can assert -// the handler scoped them to the right hosts. +// the handler scoped them to the right targets. type fakeStore struct { - hostList []HostCount - hostsErr error + targetList []TargetCount + targetsErr error count uint64 countErr error deleteErr error - countedWith []hostCall - deletedWith []hostCall + countedWith []targetCall + deletedWith []targetCall } -func (f *fakeStore) HostsOlderThan(_ context.Context, _ time.Time) ([]HostCount, error) { - if f.hostsErr != nil { - return nil, f.hostsErr +func (f *fakeStore) TargetsOlderThan(_ context.Context, _ time.Time) ([]TargetCount, error) { + if f.targetsErr != nil { + return nil, f.targetsErr } - return f.hostList, nil + return f.targetList, nil } -func (f *fakeStore) CountOlderThan(_ context.Context, cutoff time.Time, hosts []string) (uint64, error) { - f.countedWith = append(f.countedWith, hostCall{cutoff, hosts}) +func (f *fakeStore) CountOlderThan(_ context.Context, cutoff time.Time, targets []HostService) (uint64, error) { + f.countedWith = append(f.countedWith, targetCall{cutoff, targets}) if f.countErr != nil { return 0, f.countErr } return f.count, nil } -func (f *fakeStore) DeleteOlderThan(_ context.Context, cutoff time.Time, hosts []string) error { - f.deletedWith = append(f.deletedWith, hostCall{cutoff, hosts}) +func (f *fakeStore) DeleteOlderThan(_ context.Context, cutoff time.Time, targets []HostService) error { + f.deletedWith = append(f.deletedWith, targetCall{cutoff, targets}) return f.deleteErr } @@ -69,14 +69,14 @@ func (f fakeAuthorizer) Authorize(*http.Request) (authz.Identity, error) { } // fakeFloor stands in for AgentRetentionStore -- a nil/empty byHost map -// means no agent has log_retention_days configured, same as every test -// that doesn't care about the floor assumed before it existed. +// means no agent has any retention floor configured, same as every +// test that doesn't care about the floor assumed before it existed. type fakeFloor struct { - byHost map[string]int + byHost map[string]HostFloor err error } -func (f fakeFloor) RetentionDaysByHost(context.Context) (map[string]int, error) { +func (f fakeFloor) FloorsByHost(context.Context) (map[string]HostFloor, error) { return f.byHost, f.err } @@ -94,13 +94,35 @@ func doRequest(t *testing.T, h *Handler, method, path string) *httptest.Response return rec } -func hoursForDays(days int) string { - return strconv.Itoa(days * 24) +func doJSONRequest(t *testing.T, h *Handler, method, path string, body any) *httptest.ResponseRecorder { + t.Helper() + b, err := json.Marshal(body) + if err != nil { + t.Fatalf("marshaling request body: %v", err) + } + req := httptest.NewRequest(method, path, bytes.NewReader(b)) + rec := httptest.NewRecorder() + mux := http.NewServeMux() + h.RegisterRoutes(mux) + mux.ServeHTTP(rec, req) + return rec } -func TestHostsListsHostsWithCountsAndFloors(t *testing.T) { - s := &fakeStore{hostList: []HostCount{{Host: "web-01", Count: 100}, {Host: "web-02", Count: 5}}} - h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"web-02": 90}}, fakeAuthorizer{role: authz.RoleAdmin}) +func hoursForDays(days int) int { + return days * 24 +} + +func intPtr(n int) *int { return &n } + +func TestHostsListsTargetsGroupedByHostWithFloors(t *testing.T) { + s := &fakeStore{targetList: []TargetCount{ + {Host: "web-01", Service: "nginx", Count: 100}, + {Host: "web-01", Service: "smtp", Count: 5}, + {Host: "web-02", Service: "ufw", Count: 20}, + }} + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]HostFloor{ + "web-01": {DefaultDays: intPtr(7), ServiceDays: map[string]int{"smtp": 365}}, + }}, fakeAuthorizer{role: authz.RoleAdmin}) rec := doRequest(t, h, "GET", "/logs/retention/hosts?older_than_hours=24") if rec.Code != http.StatusOK { @@ -113,20 +135,39 @@ func TestHostsListsHostsWithCountsAndFloors(t *testing.T) { if len(resp.Hosts) != 2 { t.Fatalf("len(hosts) = %d, want 2", len(resp.Hosts)) } - if resp.Hosts[0].Host != "web-01" || resp.Hosts[0].Count != 100 || resp.Hosts[0].ProtectedDays != nil { - t.Errorf("hosts[0] = %+v, want web-01/100/no floor", resp.Hosts[0]) + + web01 := resp.Hosts[0] + if web01.Host != "web-01" || web01.ProtectedDays == nil || *web01.ProtectedDays != 7 { + t.Fatalf("hosts[0] = %+v, want web-01 with host default floor 7", web01) } - if resp.Hosts[1].Host != "web-02" || resp.Hosts[1].Count != 5 || resp.Hosts[1].ProtectedDays == nil || *resp.Hosts[1].ProtectedDays != 90 { - t.Errorf("hosts[1] = %+v, want web-02/5/floor=90", resp.Hosts[1]) + if len(web01.Services) != 2 { + t.Fatalf("web-01 services = %+v, want 2 entries", web01.Services) + } + if web01.Services[0].Service != "nginx" || web01.Services[0].Count != 100 || web01.Services[0].ProtectedDays == nil || *web01.Services[0].ProtectedDays != 7 { + t.Errorf("web-01/nginx = %+v, want count=100 protected_days=7 (host default)", web01.Services[0]) + } + if web01.Services[1].Service != "smtp" || web01.Services[1].ProtectedDays == nil || *web01.Services[1].ProtectedDays != 365 { + t.Errorf("web-01/smtp = %+v, want protected_days=365 (service override, not the host default)", web01.Services[1]) + } + + web02 := resp.Hosts[1] + if web02.Host != "web-02" || web02.ProtectedDays != nil { + t.Fatalf("hosts[1] = %+v, want web-02 with no floor", web02) + } + if len(web02.Services) != 1 || web02.Services[0].ProtectedDays != nil { + t.Errorf("web-02 services = %+v, want ufw with no floor", web02.Services) } } -func TestPreviewReturnsCountCutoffAndHosts(t *testing.T) { +func TestPreviewReturnsCountCutoffAndTargets(t *testing.T) { s := &fakeStore{count: 42} h := newTestHandler(s, authz.RoleAdmin) before := time.Now().UTC() - rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01&host=web-02") + rec := doJSONRequest(t, h, "POST", "/logs/retention/preview", deletionRequest{ + OlderThanHours: 24, + Targets: []HostService{{Host: "web-01", Service: "nginx"}, {Host: "web-01", Service: "smtp"}}, + }) after := time.Now().UTC() if rec.Code != http.StatusOK { @@ -139,8 +180,9 @@ func TestPreviewReturnsCountCutoffAndHosts(t *testing.T) { if resp.Count != 42 { t.Errorf("count = %d, want 42", resp.Count) } - if !reflect.DeepEqual(resp.Hosts, []string{"web-01", "web-02"}) { - t.Errorf("hosts = %v, want [web-01 web-02]", resp.Hosts) + want := []HostService{{Host: "web-01", Service: "nginx"}, {Host: "web-01", Service: "smtp"}} + if !reflect.DeepEqual(resp.Targets, want) { + t.Errorf("targets = %v, want %v", resp.Targets, want) } wantEarliest := before.Add(-24 * time.Hour) wantLatest := after.Add(-24 * time.Hour) @@ -150,47 +192,76 @@ func TestPreviewReturnsCountCutoffAndHosts(t *testing.T) { if len(s.deletedWith) != 0 { t.Errorf("preview must never delete anything, but DeleteOlderThan was called %d time(s)", len(s.deletedWith)) } - if len(s.countedWith) != 1 || !reflect.DeepEqual(s.countedWith[0].hosts, []string{"web-01", "web-02"}) { - t.Errorf("CountOlderThan was not scoped to the requested hosts: %+v", s.countedWith) + if len(s.countedWith) != 1 || !reflect.DeepEqual(s.countedWith[0].targets, want) { + t.Errorf("CountOlderThan was not scoped to the requested targets: %+v", s.countedWith) } } -func TestPreviewDedupesHosts(t *testing.T) { +func TestPreviewDedupesTargets(t *testing.T) { s := &fakeStore{count: 1} h := newTestHandler(s, authz.RoleAdmin) - rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01&host=web-01") + rec := doJSONRequest(t, h, "POST", "/logs/retention/preview", deletionRequest{ + OlderThanHours: 24, + Targets: []HostService{ + {Host: "web-01", Service: "nginx"}, + {Host: "web-01", Service: "nginx"}, + }, + }) if rec.Code != http.StatusOK { t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) } - if len(s.countedWith) != 1 || !reflect.DeepEqual(s.countedWith[0].hosts, []string{"web-01"}) { - t.Fatalf("expected a deduped single-host call, got %+v", s.countedWith) + want := []HostService{{Host: "web-01", Service: "nginx"}} + if len(s.countedWith) != 1 || !reflect.DeepEqual(s.countedWith[0].targets, want) { + t.Fatalf("expected a deduped single-target call, got %+v", s.countedWith) } } -func TestPreviewRequiresAtLeastOneHost(t *testing.T) { +func TestPreviewRequiresAtLeastOneTarget(t *testing.T) { h := newTestHandler(&fakeStore{}, authz.RoleAdmin) - rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24") + rec := doJSONRequest(t, h, "POST", "/logs/retention/preview", deletionRequest{OlderThanHours: 24}) if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want 400 with no host specified", rec.Code) + t.Fatalf("status = %d, want 400 with no targets specified", rec.Code) } } -func TestPreviewRejectsEmptyHostValue(t *testing.T) { +func TestPreviewRejectsTargetWithEmptyHostOrService(t *testing.T) { h := newTestHandler(&fakeStore{}, authz.RoleAdmin) - rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=") - if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want 400 with an empty host value", rec.Code) + cases := [][]HostService{ + {{Host: "", Service: "nginx"}}, + {{Host: "web-01", Service: ""}}, + } + for _, targets := range cases { + rec := doJSONRequest(t, h, "POST", "/logs/retention/preview", deletionRequest{OlderThanHours: 24, Targets: targets}) + if rec.Code != http.StatusBadRequest { + t.Errorf("targets %v: status = %d, want 400", targets, rec.Code) + } } } -func TestDeleteReturnsDeletedCountHostsAndCutoff(t *testing.T) { +func TestPreviewRejectsInvalidOlderThanHours(t *testing.T) { + h := newTestHandler(&fakeStore{}, authz.RoleAdmin) + targets := []HostService{{Host: "web-01", Service: "nginx"}} + + cases := []int{0, -5, 999999999} + for _, hours := range cases { + rec := doJSONRequest(t, h, "POST", "/logs/retention/preview", deletionRequest{OlderThanHours: hours, Targets: targets}) + if rec.Code != http.StatusBadRequest { + t.Errorf("older_than_hours=%d: status = %d, want 400", hours, rec.Code) + } + } +} + +func TestDeleteReturnsDeletedCountTargetsAndCutoff(t *testing.T) { s := &fakeStore{count: 7} h := newTestHandler(s, authz.RoleAdmin) - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=720&host=web-01") + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{ + OlderThanHours: 720, + Targets: []HostService{{Host: "web-01", Service: "nginx"}}, + }) if rec.Code != http.StatusOK { t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) } @@ -201,10 +272,11 @@ func TestDeleteReturnsDeletedCountHostsAndCutoff(t *testing.T) { if resp.DeletedCount != 7 { t.Errorf("deleted_count = %d, want 7", resp.DeletedCount) } - if !reflect.DeepEqual(resp.DeletedHosts, []string{"web-01"}) { - t.Errorf("deleted_hosts = %v, want [web-01]", resp.DeletedHosts) + want := []HostService{{Host: "web-01", Service: "nginx"}} + if !reflect.DeepEqual(resp.DeletedTargets, want) { + t.Errorf("deleted_targets = %v, want %v", resp.DeletedTargets, want) } - if len(s.deletedWith) != 1 || !reflect.DeepEqual(s.deletedWith[0].hosts, []string{"web-01"}) { + if len(s.deletedWith) != 1 || !reflect.DeepEqual(s.deletedWith[0].targets, want) { t.Fatalf("expected exactly one scoped DeleteOlderThan call, got %+v", s.deletedWith) } if len(s.countedWith) != 1 || !s.countedWith[0].cutoff.Equal(s.deletedWith[0].cutoff) { @@ -212,61 +284,47 @@ func TestDeleteReturnsDeletedCountHostsAndCutoff(t *testing.T) { } } -func TestRejectsMissingOrInvalidOlderThanHours(t *testing.T) { +func TestDeleteRejectsMissingTargets(t *testing.T) { + s := &fakeStore{} + h := newTestHandler(s, authz.RoleAdmin) + + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{OlderThanHours: 24}) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 with no targets specified", rec.Code) + } + if len(s.deletedWith) != 0 { + t.Error("a request with no targets specified must never reach the store's delete path") + } +} + +func TestDeleteRejectsInvalidJSONBody(t *testing.T) { h := newTestHandler(&fakeStore{}, authz.RoleAdmin) - cases := []string{ - "/logs/retention/preview?host=web-01", - "/logs/retention/preview?older_than_hours=0&host=web-01", - "/logs/retention/preview?older_than_hours=-5&host=web-01", - "/logs/retention/preview?older_than_hours=notanumber&host=web-01", - "/logs/retention/preview?older_than_hours=999999999&host=web-01", - } - for _, path := range cases { - rec := doRequest(t, h, "GET", path) - if rec.Code != http.StatusBadRequest { - t.Errorf("path %q: status = %d, want 400", path, rec.Code) - } - } -} - -func TestDeleteRejectsInvalidOlderThanHours(t *testing.T) { - s := &fakeStore{} - h := newTestHandler(s, authz.RoleAdmin) - - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=0&host=web-01") + req := httptest.NewRequest("POST", "/logs/retention/delete", bytes.NewReader([]byte("not json"))) + rec := httptest.NewRecorder() + mux := http.NewServeMux() + h.RegisterRoutes(mux) + mux.ServeHTTP(rec, req) if rec.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rec.Code) } - if len(s.deletedWith) != 0 { - t.Error("an invalid older_than_hours must never reach the store's delete path") - } -} - -func TestDeleteRejectsMissingHosts(t *testing.T) { - s := &fakeStore{} - h := newTestHandler(s, authz.RoleAdmin) - - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24") - if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want 400 with no host specified", rec.Code) - } - if len(s.deletedWith) != 0 { - t.Error("a request with no host specified must never reach the store's delete path") - } } func TestDeletePropagatesStoreErrors(t *testing.T) { s := &fakeStore{deleteErr: errors.New("clickhouse mutation failed")} h := newTestHandler(s, authz.RoleAdmin) - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24&host=web-01") + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{ + OlderThanHours: 24, + Targets: []HostService{{Host: "web-01", Service: "nginx"}}, + }) if rec.Code != http.StatusInternalServerError { t.Fatalf("status = %d, want 500", rec.Code) } } func TestOwnerAndAdminCanUseRetentionRoutes(t *testing.T) { + targets := []HostService{{Host: "web-01", Service: "nginx"}} for _, role := range []authz.Role{authz.RoleAdmin, authz.RoleOwner} { s := &fakeStore{count: 3} h := newTestHandler(s, role) @@ -275,11 +333,11 @@ func TestOwnerAndAdminCanUseRetentionRoutes(t *testing.T) { if hosts.Code != http.StatusOK { t.Errorf("role %s: hosts status = %d, want 200", role, hosts.Code) } - preview := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01") + preview := doJSONRequest(t, h, "POST", "/logs/retention/preview", deletionRequest{OlderThanHours: 24, Targets: targets}) if preview.Code != http.StatusOK { t.Errorf("role %s: preview status = %d, want 200", role, preview.Code) } - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24&host=web-01") + del := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{OlderThanHours: 24, Targets: targets}) if del.Code != http.StatusOK { t.Errorf("role %s: delete status = %d, want 200", role, del.Code) } @@ -287,6 +345,7 @@ func TestOwnerAndAdminCanUseRetentionRoutes(t *testing.T) { } func TestViewerAndEditorAreForbiddenFromRetentionRoutes(t *testing.T) { + targets := []HostService{{Host: "web-01", Service: "nginx"}} for _, role := range []authz.Role{authz.RoleViewer, authz.RoleEditor} { s := &fakeStore{count: 3} h := newTestHandler(s, role) @@ -295,11 +354,11 @@ func TestViewerAndEditorAreForbiddenFromRetentionRoutes(t *testing.T) { if hosts.Code != http.StatusForbidden { t.Errorf("role %s: hosts status = %d, want 403", role, hosts.Code) } - preview := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01") + preview := doJSONRequest(t, h, "POST", "/logs/retention/preview", deletionRequest{OlderThanHours: 24, Targets: targets}) if preview.Code != http.StatusForbidden { t.Errorf("role %s: preview status = %d, want 403", role, preview.Code) } - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24&host=web-01") + del := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{OlderThanHours: 24, Targets: targets}) if del.Code != http.StatusForbidden { t.Errorf("role %s: delete status = %d, want 403", role, del.Code) } @@ -318,23 +377,31 @@ func TestRetentionRoutesRequireAuth(t *testing.T) { // here too, same as every other RequireRole-wrapped route, rather // than this package accidentally being open or closed by default in // a way inconsistent with the rest of the API. - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24&host=web-01") + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{ + OlderThanHours: 24, + Targets: []HostService{{Host: "web-01", Service: "nginx"}}, + }) if rec.Code != http.StatusOK { t.Fatalf("status with nil authorizer = %d, want 200 (default-open, matches RequireRole elsewhere)", rec.Code) } } -// TestAdminPartiallyBlockedByPerHostRetentionFloor is the core -// regression test for host-scoped floor enforcement: requesting two -// hosts where only one has a protective floor must delete the -// unprotected host and report the other as blocked, not reject the +// TestAdminPartiallyBlockedByPerTargetRetentionFloor is the core +// regression test for target-scoped floor enforcement: requesting two +// targets where only one has a protective floor must delete the +// unprotected target and report the other as blocked, not reject the // whole request. -func TestAdminPartiallyBlockedByPerHostRetentionFloor(t *testing.T) { +func TestAdminPartiallyBlockedByPerTargetRetentionFloor(t *testing.T) { s := &fakeStore{count: 5} - h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"protected-host": 90}}, fakeAuthorizer{role: authz.RoleAdmin}) + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]HostFloor{ + "web-01": {ServiceDays: map[string]int{"smtp": 90}}, + }}, fakeAuthorizer{role: authz.RoleAdmin}) - // 30 days is newer than protected-host's 90-day floor. - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(30)+"&host=protected-host&host=open-host") + // 30 days is newer than smtp's 90-day floor. + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{ + OlderThanHours: hoursForDays(30), + Targets: []HostService{{Host: "web-01", Service: "smtp"}, {Host: "web-01", Service: "nginx"}}, + }) if rec.Code != http.StatusOK { t.Fatalf("status = %d, want 200 (partial success, not an error), body=%s", rec.Code, rec.Body.String()) } @@ -342,26 +409,60 @@ func TestAdminPartiallyBlockedByPerHostRetentionFloor(t *testing.T) { if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { t.Fatalf("decoding response: %v", err) } - if !reflect.DeepEqual(resp.DeletedHosts, []string{"open-host"}) { - t.Errorf("deleted_hosts = %v, want [open-host]", resp.DeletedHosts) + wantDeleted := []HostService{{Host: "web-01", Service: "nginx"}} + if !reflect.DeepEqual(resp.DeletedTargets, wantDeleted) { + t.Errorf("deleted_targets = %v, want %v", resp.DeletedTargets, wantDeleted) } - if len(resp.BlockedHosts) != 1 || resp.BlockedHosts[0].Host != "protected-host" || resp.BlockedHosts[0].ProtectedDays != 90 { - t.Errorf("blocked_hosts = %+v, want [{protected-host 90}]", resp.BlockedHosts) + if len(resp.BlockedTargets) != 1 || resp.BlockedTargets[0] != (blockedTarget{Host: "web-01", Service: "smtp", ProtectedDays: 90}) { + t.Errorf("blocked_targets = %+v, want [{web-01 smtp 90}]", resp.BlockedTargets) } - if len(s.deletedWith) != 1 || !reflect.DeepEqual(s.deletedWith[0].hosts, []string{"open-host"}) { - t.Fatalf("DeleteOlderThan must only ever be scoped to the allowed host, got %+v", s.deletedWith) + if len(s.deletedWith) != 1 || !reflect.DeepEqual(s.deletedWith[0].targets, wantDeleted) { + t.Fatalf("DeleteOlderThan must only ever be scoped to the allowed target, got %+v", s.deletedWith) } } -// TestAllHostsBlockedReturnsZeroCountNotError confirms a request where -// every requested host is protected still succeeds (200), just with -// nothing deleted -- informative, not an error condition, since the -// request itself was perfectly valid. -func TestAllHostsBlockedReturnsZeroCountNotError(t *testing.T) { - s := &fakeStore{count: 100} - h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"protected-host": 90}}, fakeAuthorizer{role: authz.RoleAdmin}) +// TestServiceOverrideBeatsHostDefault confirms Effective's precedence: +// a service-specific floor applies over the host default even when the +// host default alone would have allowed the request. +func TestServiceOverrideBeatsHostDefault(t *testing.T) { + s := &fakeStore{count: 5} + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]HostFloor{ + "web-01": {DefaultDays: intPtr(7), ServiceDays: map[string]int{"smtp": 365}}, + }}, fakeAuthorizer{role: authz.RoleAdmin}) - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(30)+"&host=protected-host") + // 30 days clears the 7-day host default but not smtp's 365-day + // override. + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{ + OlderThanHours: hoursForDays(30), + Targets: []HostService{{Host: "web-01", Service: "smtp"}, {Host: "web-01", Service: "nginx"}}, + }) + var resp deleteResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decoding response: %v", err) + } + wantDeleted := []HostService{{Host: "web-01", Service: "nginx"}} + if !reflect.DeepEqual(resp.DeletedTargets, wantDeleted) { + t.Errorf("deleted_targets = %v, want %v (nginx uses the 7-day default, smtp its own 365-day override)", resp.DeletedTargets, wantDeleted) + } + if len(resp.BlockedTargets) != 1 || resp.BlockedTargets[0].ProtectedDays != 365 { + t.Errorf("blocked_targets = %+v, want smtp blocked at 365 days", resp.BlockedTargets) + } +} + +// TestAllTargetsBlockedReturnsZeroCountNotError confirms a request +// where every requested target is protected still succeeds (200), just +// with nothing deleted -- informative, not an error condition, since +// the request itself was perfectly valid. +func TestAllTargetsBlockedReturnsZeroCountNotError(t *testing.T) { + s := &fakeStore{count: 100} + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]HostFloor{ + "web-01": {ServiceDays: map[string]int{"smtp": 90}}, + }}, fakeAuthorizer{role: authz.RoleAdmin}) + + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{ + OlderThanHours: hoursForDays(30), + Targets: []HostService{{Host: "web-01", Service: "smtp"}}, + }) if rec.Code != http.StatusOK { t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) } @@ -369,38 +470,14 @@ func TestAllHostsBlockedReturnsZeroCountNotError(t *testing.T) { if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { t.Fatalf("decoding response: %v", err) } - if resp.DeletedCount != 0 { - t.Errorf("deleted_count = %d, want 0", resp.DeletedCount) + if resp.DeletedCount != 0 || len(resp.DeletedTargets) != 0 { + t.Errorf("deleted_count/targets = %d/%v, want 0/empty", resp.DeletedCount, resp.DeletedTargets) } - if len(resp.DeletedHosts) != 0 { - t.Errorf("deleted_hosts = %v, want empty", resp.DeletedHosts) - } - if len(resp.BlockedHosts) != 1 || resp.BlockedHosts[0].Host != "protected-host" { - t.Errorf("blocked_hosts = %+v, want [{protected-host 90}]", resp.BlockedHosts) + if len(resp.BlockedTargets) != 1 { + t.Errorf("blocked_targets = %+v, want one entry", resp.BlockedTargets) } if len(s.deletedWith) != 0 || len(s.countedWith) != 0 { - t.Error("the store must never be called when every requested host is blocked") - } -} - -// TestAdminAllowedBeyondRetentionFloor confirms the floor only blocks -// requests that would actually reach into the protected window -- a -// request older than the floor itself is unaffected by it. -func TestAdminAllowedBeyondRetentionFloor(t *testing.T) { - s := &fakeStore{count: 5} - h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"web-01": 90}}, fakeAuthorizer{role: authz.RoleAdmin}) - - // 120 days is older than the 90-day floor -- must be allowed. - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(120)+"&host=web-01") - if del.Code != http.StatusOK { - t.Fatalf("delete at 120d against a 90d floor: status = %d, want 200, body=%s", del.Code, del.Body.String()) - } - var resp deleteResponse - if err := json.Unmarshal(del.Body.Bytes(), &resp); err != nil { - t.Fatalf("decoding response: %v", err) - } - if len(resp.BlockedHosts) != 0 { - t.Errorf("blocked_hosts = %+v, want none", resp.BlockedHosts) + t.Error("the store must never be called when every requested target is blocked") } } @@ -409,31 +486,37 @@ func TestAdminAllowedBeyondRetentionFloor(t *testing.T) { // window that blocks everyone else. func TestOwnerBypassesRetentionFloor(t *testing.T) { s := &fakeStore{count: 100} - h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"web-01": 90}}, fakeAuthorizer{role: authz.RoleOwner}) + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]HostFloor{ + "web-01": {ServiceDays: map[string]int{"smtp": 90}}, + }}, fakeAuthorizer{role: authz.RoleOwner}) - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(1)+"&host=web-01") - if del.Code != http.StatusOK { - t.Fatalf("owner deleting within the floor: status = %d, want 200, body=%s", del.Code, del.Body.String()) - } + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{ + OlderThanHours: hoursForDays(1), + Targets: []HostService{{Host: "web-01", Service: "smtp"}}, + }) var resp deleteResponse - if err := json.Unmarshal(del.Body.Bytes(), &resp); err != nil { + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { t.Fatalf("decoding response: %v", err) } - if !reflect.DeepEqual(resp.DeletedHosts, []string{"web-01"}) { - t.Errorf("deleted_hosts = %v, want [web-01] (owner bypasses the floor entirely)", resp.DeletedHosts) + want := []HostService{{Host: "web-01", Service: "smtp"}} + if !reflect.DeepEqual(resp.DeletedTargets, want) { + t.Errorf("deleted_targets = %v, want %v (owner bypasses the floor entirely)", resp.DeletedTargets, want) } } // TestNoConfiguredFloorNeverBlocksAdmin confirms the default, common -// case (no agent has log_retention_days set) behaves exactly as before -// this feature existed. +// case (no agent has any retention floor set) behaves exactly as +// before this feature existed. func TestNoConfiguredFloorNeverBlocksAdmin(t *testing.T) { s := &fakeStore{count: 9} h := NewHandler(discardLogger(), s, fakeFloor{}, fakeAuthorizer{role: authz.RoleAdmin}) - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=1&host=web-01") - if del.Code != http.StatusOK { - t.Fatalf("status = %d, want 200 with no configured floor", del.Code) + rec := doJSONRequest(t, h, "POST", "/logs/retention/delete", deletionRequest{ + OlderThanHours: 1, + Targets: []HostService{{Host: "web-01", Service: "nginx"}}, + }) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 with no configured floor", rec.Code) } } @@ -441,7 +524,10 @@ func TestRetentionFloorCheckPropagatesStoreErrors(t *testing.T) { s := &fakeStore{} h := NewHandler(discardLogger(), s, fakeFloor{err: errors.New("postgres unreachable")}, fakeAuthorizer{role: authz.RoleAdmin}) - rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01") + rec := doJSONRequest(t, h, "POST", "/logs/retention/preview", deletionRequest{ + OlderThanHours: 24, + Targets: []HostService{{Host: "web-01", Service: "nginx"}}, + }) if rec.Code != http.StatusInternalServerError { t.Fatalf("status = %d, want 500", rec.Code) } diff --git a/api/logretention/store.go b/api/logretention/store.go index a95c318..6d2872f 100644 --- a/api/logretention/store.go +++ b/api/logretention/store.go @@ -1,12 +1,21 @@ // Package logretention lets an owner or admin permanently delete log -// records older than a chosen age, scoped to specific hosts -- deleting -// by age alone (with no way to target which agents' logs) turned out -// to be a real footgun for an operator who only wants to clean up one -// noisy host, not everything; storage/README.md has flagged "no -// TTL/retention clause yet" since Phase 0, this is the on-demand, -// operator-triggered, host-scoped half of that gap (not an automatic -// TTL, which is a different, engine-driven design nobody asked for -// here). +// records older than a chosen age, scoped to specific (host, service) +// targets -- deleting by age alone, with no way to target which +// agents' or which services' logs, turned out to be a real footgun for +// an operator who only wants to clean up one noisy source (e.g. a +// chatty nginx access log) without touching everything else that host +// ships (smtp, ufw, ...); storage/README.md has flagged "no TTL/ +// retention clause yet" since Phase 0, this is the on-demand, +// operator-triggered, host-and-service-scoped half of that gap (not an +// automatic TTL, which is a different, engine-driven design nobody +// asked for here). +// +// service is a genuine per-log-record dimension already, not something +// this package invents: storage/migrations/0001_create_logs_table.sql +// has always had a `service` column, and distinct services on one host +// are a real, already-supported shape (separate sentry-agent processes +// on the same machine, each with its own agent.toml `service` -- see +// /docs/agent-management-design.md), not merely a per-agent label. // // Deliberately scoped to core's single ClickHouse `logs` table, not // enterprise/'s per-tenant ClickHouse routing @@ -40,7 +49,7 @@ import ( // `logs` table -- deliberately not querylang/executor.ChRunner, whose // one method (RunSQL) is scoped to arbitrary SELECT statements for the // query language compiler. This package only ever needs a handful of -// fixed statement shapes (list hosts, count, delete), so keeping them +// fixed statement shapes (list targets, count, delete), so keeping them // separate avoids stretching ChRunner's SELECT-shaped contract to also // cover a DML mutation. type Store struct { @@ -51,59 +60,75 @@ func NewStore(conn driver.Conn) *Store { return &Store{conn: conn} } -type HostCount struct { - Host string `json:"host"` - Count uint64 `json:"count"` +// HostService identifies one (host, service) pair -- the atomic unit a +// caller selects for preview/deletion. Never a wildcard: a request +// naming a host with no service (or vice versa) is invalid at the +// handler layer (see parseTargets), so this package never has to +// reason about "every service on this host." +type HostService struct { + Host string `json:"host"` + Service string `json:"service"` } -// HostsOlderThan lists every host with at least one log record older -// than cutoff, along with how many -- backs the host picker a caller -// selects from before previewing/deleting, so the list only ever shows -// hosts that actually have something to act on for the chosen age. -func (s *Store) HostsOlderThan(ctx context.Context, cutoff time.Time) ([]HostCount, error) { +type TargetCount struct { + Host string + Service string + Count uint64 +} + +// TargetsOlderThan lists every (host, service) pair with at least one +// log record older than cutoff, along with how many -- backs the +// picker a caller selects from before previewing/deleting. Ordered by +// host so the handler can group contiguous rows into a per-host list +// without a second pass. +func (s *Store) TargetsOlderThan(ctx context.Context, cutoff time.Time) ([]TargetCount, error) { rows, err := s.conn.Query(ctx, ` - SELECT host, count() AS n FROM logs WHERE timestamp < ? GROUP BY host ORDER BY n DESC`, cutoff) + SELECT host, service, count() AS n FROM logs + WHERE timestamp < ? + GROUP BY host, service + ORDER BY host, n DESC`, cutoff) if err != nil { return nil, err } defer rows.Close() - var out []HostCount + var out []TargetCount for rows.Next() { - var hc HostCount - if err := rows.Scan(&hc.Host, &hc.Count); err != nil { + var tc TargetCount + if err := rows.Scan(&tc.Host, &tc.Service, &tc.Count); err != nil { return nil, err } - out = append(out, hc) + out = append(out, tc) } return out, rows.Err() } -// hostPlaceholders builds "?, ?, ..." for n hosts and the matching -// []any argument slice (cutoff first, then each host) -- shared by -// CountOlderThan and DeleteOlderThan since both statements have the -// same "timestamp < ? AND host IN (...)" shape. Callers must never -// pass an empty hosts slice (an empty IN () is invalid SQL, and more -// importantly "no hosts specified" must never silently mean "every -// host" -- see Handler.parseHosts, which rejects that before this is -// ever called). -func hostPlaceholders(cutoff time.Time, hosts []string) (string, []any) { - placeholders := make([]string, len(hosts)) - args := make([]any, 0, len(hosts)+1) +// targetPlaceholders builds "(?, ?), (?, ?), ..." for a tuple IN clause +// over (host, service) and the matching []any argument slice (cutoff +// first, then each pair) -- shared by CountOlderThan and +// DeleteOlderThan since both statements have the same +// "timestamp < ? AND (host, service) IN (...)" shape. Callers must +// never pass an empty targets slice (an empty IN () is invalid SQL, and +// more importantly "no targets specified" must never silently mean +// "everything" -- see Handler.parseTargets, which rejects that before +// this is ever called). +func targetPlaceholders(cutoff time.Time, targets []HostService) (string, []any) { + pairs := make([]string, len(targets)) + args := make([]any, 0, len(targets)*2+1) args = append(args, cutoff) - for i, h := range hosts { - placeholders[i] = "?" - args = append(args, h) + for i, t := range targets { + pairs[i] = "(?, ?)" + args = append(args, t.Host, t.Service) } - return strings.Join(placeholders, ", "), args + return strings.Join(pairs, ", "), args } -// CountOlderThan reports how many log records from any of hosts are +// CountOlderThan reports how many log records from any of targets are // older than cutoff -- backs the "this will delete N records" preview // a caller shows before asking for confirmation. -func (s *Store) CountOlderThan(ctx context.Context, cutoff time.Time, hosts []string) (uint64, error) { - ph, args := hostPlaceholders(cutoff, hosts) - row := s.conn.QueryRow(ctx, fmt.Sprintf("SELECT count() FROM logs WHERE timestamp < ? AND host IN (%s)", ph), args...) +func (s *Store) CountOlderThan(ctx context.Context, cutoff time.Time, targets []HostService) (uint64, error) { + ph, args := targetPlaceholders(cutoff, targets) + row := s.conn.QueryRow(ctx, fmt.Sprintf("SELECT count() FROM logs WHERE timestamp < ? AND (host, service) IN (%s)", ph), args...) var n uint64 if err := row.Scan(&n); err != nil { return 0, err @@ -113,13 +138,13 @@ func (s *Store) CountOlderThan(ctx context.Context, cutoff time.Time, hosts []st // DeleteOlderThan issues a synchronous ClickHouse mutation // (SETTINGS mutations_sync = 1) deleting every log record from any of -// hosts older than cutoff. Synchronous rather than fire-and-forget so +// targets older than cutoff. Synchronous rather than fire-and-forget so // a 200 response means the data is actually gone, not just queued -- an // owner/admin confirming a permanent delete should be able to trust the // response. This does block for as long as the mutation takes, which // could be a while against a very large table; a disclosed tradeoff for // this deployment's homelab/small-scale target, not a hidden one. -func (s *Store) DeleteOlderThan(ctx context.Context, cutoff time.Time, hosts []string) error { - ph, args := hostPlaceholders(cutoff, hosts) - return s.conn.Exec(ctx, fmt.Sprintf("ALTER TABLE logs DELETE WHERE timestamp < ? AND host IN (%s) SETTINGS mutations_sync = 1", ph), args...) +func (s *Store) DeleteOlderThan(ctx context.Context, cutoff time.Time, targets []HostService) error { + ph, args := targetPlaceholders(cutoff, targets) + return s.conn.Exec(ctx, fmt.Sprintf("ALTER TABLE logs DELETE WHERE timestamp < ? AND (host, service) IN (%s) SETTINGS mutations_sync = 1", ph), args...) } diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index ee33302..81859a6 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -483,40 +483,47 @@ export function setUserRole(id: string, role: string): Promise { } // --- log retention (owner/admin only, see api/logretention) ----------- -// Deletion is host-scoped, not wholesale: a caller must name which -// hosts' logs to target (listRetentionHosts is how the UI discovers -// what to offer), and api/logretention never treats an omitted host -// list as "every host." +// Deletion is scoped to specific (host, service) targets, not wholesale +// -- a caller must name which agents' *and* which log types' logs to +// target (listRetentionHosts is how the UI discovers what to offer, +// grouped by host with each host's services underneath), and +// api/logretention never treats an omitted target list as "everything." -export type BlockedHost = { host: string; protected_days: number }; -export type RetentionHost = { host: string; count: number; protected_days?: number }; +export type HostService = { host: string; service: string }; +export type BlockedTarget = { host: string; service: string; protected_days: number }; +export type RetentionService = { service: string; count: number; protected_days?: number }; +export type RetentionHost = { host: string; protected_days?: number; services: RetentionService[] }; export type RetentionHostsResult = { hosts: RetentionHost[]; cutoff: string }; -export type LogRetentionPreview = { count: number; cutoff: string; hosts: string[]; blocked_hosts?: BlockedHost[] }; +export type LogRetentionPreview = { + count: number; + cutoff: string; + targets: HostService[]; + blocked_targets?: BlockedTarget[]; +}; export type LogRetentionDeleteResult = { deleted_count: number; cutoff: string; - deleted_hosts: string[]; - blocked_hosts?: BlockedHost[]; + deleted_targets: HostService[]; + blocked_targets?: BlockedTarget[]; }; -function hostsQuery(hosts: string[]): string { - return hosts.map((h) => `host=${encodeURIComponent(h)}`).join('&'); -} - export function listRetentionHosts(olderThanHours: number): Promise { return request(`/logs/retention/hosts?older_than_hours=${olderThanHours}`, { credentials: 'include' }); } -export function previewLogDeletion(olderThanHours: number, hosts: string[]): Promise { - return request(`/logs/retention/preview?older_than_hours=${olderThanHours}&${hostsQuery(hosts)}`, { - credentials: 'include' +export function previewLogDeletion(olderThanHours: number, targets: HostService[]): Promise { + return request('/logs/retention/preview', { + method: 'POST', + credentials: 'include', + body: JSON.stringify({ older_than_hours: olderThanHours, targets }) }); } -export function deleteLogsOlderThan(olderThanHours: number, hosts: string[]): Promise { - return request(`/logs/retention?older_than_hours=${olderThanHours}&${hostsQuery(hosts)}`, { - method: 'DELETE', - credentials: 'include' +export function deleteLogsOlderThan(olderThanHours: number, targets: HostService[]): Promise { + return request('/logs/retention/delete', { + method: 'POST', + credentials: 'include', + body: JSON.stringify({ older_than_hours: olderThanHours, targets }) }); } @@ -617,6 +624,11 @@ export type ConfigOverride = { // reads as a protective floor, not something the agent process itself // ever sees or applies. log_retention_days?: number; + // service_log_retention_days is log_retention_days' per-service + // refinement, also owner-only -- a service present here overrides + // log_retention_days for that service only; every other service on + // this host still falls back to log_retention_days. + service_log_retention_days?: Record; }; export type Agent = { diff --git a/web/src/routes/agents/[host]/+page.svelte b/web/src/routes/agents/[host]/+page.svelte index 9438171..319bd53 100644 --- a/web/src/routes/agents/[host]/+page.svelte +++ b/web/src/routes/agents/[host]/+page.svelte @@ -38,6 +38,11 @@ // one either. Kept as a string ('' = no override) so the input can be // empty rather than defaulting to some arbitrary number of days. let logRetentionDays = $state(''); + // Per-service overrides of logRetentionDays -- a service named here + // (e.g. "smtp") keeps its own retention floor instead of falling back + // to the host default above. Rows with an empty service name are + // dropped at save(), same as extraFilePaths drops blank paths. + let serviceRetention = $state<{ service: string; days: string }[]>([]); function resetForm(a: Agent) { const o = a.desired_override; @@ -48,6 +53,9 @@ journaldUnit = o?.journald_unit ?? ''; extraFilePaths = o?.extra_file_paths ? [...o.extra_file_paths] : []; logRetentionDays = o?.log_retention_days != null ? String(o.log_retention_days) : ''; + serviceRetention = o?.service_log_retention_days + ? Object.entries(o.service_log_retention_days).map(([service, days]) => ({ service, days: String(days) })) + : []; } function addExtraFilePath() { @@ -58,6 +66,14 @@ extraFilePaths = extraFilePaths.filter((_, i) => i !== index); } + function addServiceRetention() { + serviceRetention = [...serviceRetention, { service: '', days: '' }]; + } + + function removeServiceRetention(index: number) { + serviceRetention = serviceRetention.filter((_, i) => i !== index); + } + async function load() { loading = true; error = ''; @@ -72,6 +88,22 @@ } load(); + // Always sent, even when empty ({}) -- unlike logRetentionDays' + // conditional omit, this mirrors extraFilePaths' unconditional style + // (a collection field), so clearing every row genuinely clears the + // stored overrides rather than leaving stale ones behind. + function buildServiceRetentionMap(): Record { + const out: Record = {}; + for (const row of serviceRetention) { + const service = row.service.trim(); + const days = String(row.days).trim(); + if (service !== '' && days !== '') { + out[service] = Number(days); + } + } + return out; + } + async function save() { saving = true; saveError = ''; @@ -91,7 +123,8 @@ // the moment someone edits this field. batch_max_size etc. // above never hit this because Number(x) doesn't care // whether x is already a number. - ...(String(logRetentionDays).trim() !== '' ? { log_retention_days: Number(logRetentionDays) } : {}) + ...(String(logRetentionDays).trim() !== '' ? { log_retention_days: Number(logRetentionDays) } : {}), + service_log_retention_days: buildServiceRetentionMap() }); } catch (e) { saveError = e instanceof Error ? e.message : String(e); @@ -242,6 +275,23 @@

+
+ Per-service log retention overrides +

+ Owner only. Keep a specific log type from this host longer (or shorter) than the general retention + above -- e.g. protect "smtp" for a year while everything else on this host uses the default. A + service not listed here just uses the host default. +

+ {#each serviceRetention as _, i} +
+ + + +
+ {/each} + +
+ {#if saveError}

Error: {saveError}

{/if}
@@ -366,6 +416,9 @@ .path-row :global(input) { flex: 1; } + .service-row :global(input:last-of-type) { + flex: 0 0 6rem; + } .actions { display: flex; gap: var(--space-3); diff --git a/web/src/routes/settings/+page.svelte b/web/src/routes/settings/+page.svelte index 18feee9..10368eb 100644 --- a/web/src/routes/settings/+page.svelte +++ b/web/src/routes/settings/+page.svelte @@ -12,6 +12,7 @@ type LocalSession, type CurrentSession, type RetentionHost, + type HostService, type LogRetentionPreview, type LogRetentionDeleteResult } from '$lib/api'; @@ -70,10 +71,12 @@ return !localAuthEnabled; }); - // --- log retention deletion -- host-scoped, not wholesale: a caller - // picks which hosts to target from what listRetentionHosts reports for - // the chosen age, same "select specific agents, not delete everything" - // requirement api/logretention's own parseHosts enforces server-side. --- + // --- log retention deletion -- scoped to specific (host, service) + // targets, not wholesale: a caller picks which agents' *and* which log + // types' logs to target from what listRetentionHosts reports for the + // chosen age, same "select specific targets, not delete everything" + // requirement api/logretention's own parseTargets enforces + // server-side. --- const retentionOptions: { label: string; hours: number }[] = [ { label: '7 days', hours: 24 * 7 }, { label: '30 days', hours: 24 * 30 }, @@ -86,7 +89,10 @@ let hostsLoading = $state(false); let hosts = $state([]); let hostsError = $state(''); - let selectedHosts = $state>(new Set()); + // Selection keyed by a composite "host service" string for O(1) + // membership checks -- targetKey/targetsFromKeys convert to/from the + // {host, service} objects the API actually wants. + let selectedTargets = $state>(new Set()); let previewing = $state(false); let preview = $state(null); @@ -94,6 +100,17 @@ let deleteResult = $state(null); let retentionError = $state(''); + function targetKey(host: string, service: string): string { + return `${host} ${service}`; + } + + function targetsFromKeys(keys: Iterable): HostService[] { + return [...keys].map((key) => { + const [host, service] = key.split(' '); + return { host, service }; + }); + } + // Reloads whenever retentionHours changes (including on mount, since // $effect runs once immediately too) -- selection/preview/result all // reset because they were scoped to the previous age's host list. @@ -102,7 +119,7 @@ hostsError = ''; preview = null; deleteResult = null; - selectedHosts = new Set(); + selectedTargets = new Set(); try { const result = await listRetentionHosts(hours); hosts = result.hosts; @@ -117,32 +134,56 @@ loadHosts(retentionHours); }); - function toggleHost(host: string) { - const next = new Set(selectedHosts); - if (next.has(host)) next.delete(host); - else next.add(host); - selectedHosts = next; + function toggleTarget(host: string, service: string) { + const key = targetKey(host, service); + const next = new Set(selectedTargets); + if (next.has(key)) next.delete(key); + else next.add(key); + selectedTargets = next; preview = null; deleteResult = null; } - function selectAllHosts() { - selectedHosts = new Set(hosts.map((h) => h.host)); + function isHostFullySelected(host: RetentionHost): boolean { + return host.services.length > 0 && host.services.every((s) => selectedTargets.has(targetKey(host.host, s.service))); + } + + // Toggles every service under one host together -- selects all of + // them if any are currently unselected, otherwise clears all of them. + function toggleHostAll(host: RetentionHost) { + const next = new Set(selectedTargets); + const selectAll = !isHostFullySelected(host); + for (const s of host.services) { + const key = targetKey(host.host, s.service); + if (selectAll) next.add(key); + else next.delete(key); + } + selectedTargets = next; + preview = null; + deleteResult = null; + } + + function selectAllTargets() { + const next = new Set(); + for (const h of hosts) { + for (const s of h.services) next.add(targetKey(h.host, s.service)); + } + selectedTargets = next; preview = null; } - function selectNoHosts() { - selectedHosts = new Set(); + function selectNoTargets() { + selectedTargets = new Set(); preview = null; } async function handlePreview() { - if (selectedHosts.size === 0) return; + if (selectedTargets.size === 0) return; previewing = true; retentionError = ''; deleteResult = null; try { - preview = await previewLogDeletion(retentionHours, [...selectedHosts]); + preview = await previewLogDeletion(retentionHours, targetsFromKeys(selectedTargets)); } catch (e) { retentionError = e instanceof Error ? e.message : String(e); } finally { @@ -159,10 +200,10 @@ deleting = true; retentionError = ''; try { - // preview.hosts, not [...selectedHosts] -- already excludes any - // host the preview found protected, so this never re-asks for a - // host the response just said would be skipped. - const result = await deleteLogsOlderThan(retentionHours, preview.hosts); + // preview.targets, not the current selection -- already excludes + // any target the preview found protected, so this never re-asks + // for a target the response just said would be skipped. + const result = await deleteLogsOlderThan(retentionHours, preview.targets); preview = null; // loadHosts resets deleteResult as part of its "fresh state" load // (stale counts/now-empty hosts shouldn't linger), so it runs @@ -176,6 +217,10 @@ } } + function formatTarget(t: HostService): string { + return `${t.host}/${t.service}`; + } + function formatCutoff(iso: string): string { return new Date(iso).toLocaleString(undefined, { year: 'numeric', @@ -244,8 +289,8 @@

Log retention

- Permanently delete log records from specific hosts, older than a chosen age. Visible to owners and - admins only. + Permanently delete specific log types from specific hosts, older than a chosen age. Visible to + owners and admins only.

@@ -265,32 +310,54 @@ {:else}
- - + +
    {#each hosts as h (h.host)} -
  • -
  • + +
      + {#each h.services as s (s.service)} +
    • + +
    • + {/each} +
  • {/each}
-
{/if} @@ -299,28 +366,29 @@ {#if preview}
- {#if preview.hosts.length > 0} + {#if preview.targets.length > 0}

This will permanently delete {preview.count.toLocaleString()} log record{preview.count === 1 ? '' : 's'} older than {formatCutoff(preview.cutoff)} from - {preview.hosts.length} host{preview.hosts.length === 1 ? '' : 's'} ({preview.hosts.join(', ')}). - This cannot be undone. + {preview.targets.length} target{preview.targets.length === 1 ? '' : 's'} ({preview.targets + .map(formatTarget) + .join(', ')}). This cannot be undone.

{:else} -

Every selected host is protected by a retention policy -- nothing to delete.

+

Every selected target is protected by a retention policy -- nothing to delete.

{/if} - {#if preview.blocked_hosts?.length} + {#if preview.blocked_targets?.length}

- Protected by a retention policy, skipped: {preview.blocked_hosts - .map((b) => `${b.host} (${b.protected_days}d)`) + Protected by a retention policy, skipped: {preview.blocked_targets + .map((b) => `${formatTarget(b)} (${b.protected_days}d)`) .join(', ')}.

{/if}
- {#if preview.hosts.length > 0} + {#if preview.targets.length > 0} @@ -334,10 +402,12 @@ Deleted {deleteResult.deleted_count.toLocaleString()} log record{deleteResult.deleted_count === 1 ? '' : 's'} older than {formatCutoff(deleteResult.cutoff)} - {#if deleteResult.deleted_hosts.length}from {deleteResult.deleted_hosts.join(', ')}{/if}. - {#if deleteResult.blocked_hosts?.length} - Skipped (protected): {deleteResult.blocked_hosts - .map((b) => `${b.host} (${b.protected_days}d)`) + {#if deleteResult.deleted_targets.length}from {deleteResult.deleted_targets + .map(formatTarget) + .join(', ')}{/if}. + {#if deleteResult.blocked_targets?.length} + Skipped (protected): {deleteResult.blocked_targets + .map((b) => `${formatTarget(b)} (${b.protected_days}d)`) .join(', ')}. {/if}

@@ -488,27 +558,45 @@ list-style: none; margin: 0 0 var(--space-3); padding: 0; - max-height: 14rem; + max-height: 20rem; overflow-y: auto; } - .host-list li { + .host-group { border-bottom: 1px solid var(--color-border); } - .host-list li:last-child { + .host-group:last-child { border-bottom: none; } - .host-list label { + .host-row { display: flex; align-items: center; gap: var(--space-2); padding: var(--space-2) var(--space-1); font-size: var(--text-sm); + font-weight: var(--font-weight-medium); + cursor: pointer; + } + .service-list { + list-style: none; + margin: 0; + padding: 0 0 var(--space-2); + } + .service-list label { + display: flex; + align-items: center; + gap: var(--space-2); + padding: var(--space-1) var(--space-1) var(--space-1) var(--space-5); + font-size: var(--text-sm); cursor: pointer; } .host-name { font-family: var(--font-mono); color: var(--color-text); } + .service-name { + font-family: var(--font-mono); + color: var(--color-text-muted); + } .host-count { color: var(--color-text-muted); font-size: var(--text-xs);