Scope log retention deletion and floors to (host, service), not host alone
logs rows carry a real per-record `service` (nginx, smtp, ufw, ...) -- already true of the schema (storage/migrations/0001) and wire protocol, not something this feature invents. Both the deletion picker and the retention floor now operate on (host, service) pairs instead of whole hosts, so an operator can delete just one noisy log type from an agent without touching everything else it ships, and can protect one service (e.g. keep smtp a year) longer than the rest of that host's default. api/agents.ConfigOverride gains ServiceLogRetentionDays (map[string]int), owner-only to change like LogRetentionDays -- a service listed there overrides the host's LogRetentionDays default for that service only. Agent config page gets a matching "Per-service log retention overrides" add/remove list next to the existing host-level field. api/logretention: Store's count/delete now take []HostService and build a ClickHouse tuple IN ((?,?),...) over (host, service); AgentRetentionStore. FloorsByHost returns each host's default plus its per-service map, with HostFloor.Effective(service) resolving which one applies. preview/delete moved from GET/DELETE-with-query-params to POST-with-JSON-body (a list of targets needs a real body, not a repeated compound query param), and partitionTargets checks the floor per target so one protected service never blocks deleting a different, unprotected one in the same request. Settings' Log retention section is a two-level picker now: each host row (with a "select all services" checkbox and its default floor badge) expands to its services, each with its own count and effective protected-days badge. Verified live against real ClickHouse/Postgres and in-browser: a host with a 7-day default plus a 365-day smtp override -- deleting nginx+ smtp+ufw together correctly removed nginx and ufw, left smtp's 10 records untouched, and confirmed via a follow-up owner delete that bypassing the floor works. Also verified the full click-through (add a service override on the agent page, see it reflected in Settings' picker, select/preview/cancel) and confirmed no regression from the prior host-only version's tests.
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user