From 03c4e1d9ba9b53e8fa195436edff50139401b82b Mon Sep 17 00:00:00 2001 From: Utku Ozdemir Date: Mon, 27 Apr 2026 23:23:34 +0200 Subject: [PATCH] fix: stop logging Kubernetes read checks Dry-run requests and permission checks no longer add noisy Kubernetes access entries to the audit log. Kubernetes writes continue to be recorded. Fixes: siderolabs/omni#2745 Signed-off-by: Utku Ozdemir --- internal/backend/runtime/omni/audit/audit.go | 20 ++- .../backend/runtime/omni/audit/audit_test.go | 138 ++++++++++++++++++ 2 files changed, 157 insertions(+), 1 deletion(-) diff --git a/internal/backend/runtime/omni/audit/audit.go b/internal/backend/runtime/omni/audit/audit.go index 3a2e389f..fec38bf7 100644 --- a/internal/backend/runtime/omni/audit/audit.go +++ b/internal/backend/runtime/omni/audit/audit.go @@ -171,7 +171,7 @@ func (l *Log) AuditTalosAccess(ctx context.Context, fullMethodName string, clust // Wrap wraps the http.Handler with audit logging. func (l *Log) Wrap(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - if req.Method == http.MethodGet || req.Method == http.MethodHead || req.Method == http.MethodOptions { + if !shouldAuditK8SAccess(req) { next.ServeHTTP(w, req) return @@ -201,6 +201,24 @@ func (l *Log) Wrap(next http.Handler) http.Handler { }) } +func shouldAuditK8SAccess(req *http.Request) bool { + switch req.Method { + case http.MethodGet, http.MethodHead, http.MethodOptions: + return false + } + + if slices.Contains(req.URL.Query()["dryRun"], "All") { // is a dry-run request + return false + } + + // This endpoint is used by kubectl auth can-i and equivalent permission checks. + if req.Method == http.MethodPost && req.URL.Path == "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews" { + return false + } + + return true +} + // RunCleanup runs [Logger.Remove] once a minute, deleting all log entries older than the configured // retention period. func (l *Log) RunCleanup(ctx context.Context) error { diff --git a/internal/backend/runtime/omni/audit/audit_test.go b/internal/backend/runtime/omni/audit/audit_test.go index ea6f51bf..4634b796 100644 --- a/internal/backend/runtime/omni/audit/audit_test.go +++ b/internal/backend/runtime/omni/audit/audit_test.go @@ -11,6 +11,8 @@ import ( "encoding/json" "errors" "io" + "net/http" + "net/http/httptest" "path/filepath" "strings" "testing" @@ -201,6 +203,142 @@ func TestPhaseChangeIsAudited(t *testing.T) { require.Equal(t, "teardown", events[1]["event_type"]) } +func TestK8SAccessAuditSkipsReadLikeRequests(t *testing.T) { + testCases := []struct { + name string + method string + target string + expectAudit bool + }{ + { + name: "get", + method: http.MethodGet, + target: "/api/v1/namespaces/default/pods", + }, + { + name: "head", + method: http.MethodHead, + target: "/api/v1/namespaces/default/pods", + }, + { + name: "options", + method: http.MethodOptions, + target: "/api/v1/namespaces/default/pods", + }, + { + name: "server side dry run patch", + method: http.MethodPatch, + target: "/apis/apps/v1/namespaces/default/deployments/guestbook-ui?dryRun=All&fieldManager=argocd-controller", + }, + { + name: "server side dry run delete", + method: http.MethodDelete, + target: "/api/v1/namespaces/default/configmaps/test?dryRun=All", + }, + { + name: "invalid bare dry run remains audited", + method: http.MethodPatch, + target: "/apis/apps/v1/namespaces/default/deployments/guestbook-ui?dryRun", + expectAudit: true, + }, + { + name: "invalid false dry run remains audited", + method: http.MethodPatch, + target: "/apis/apps/v1/namespaces/default/deployments/guestbook-ui?dryRun=false", + expectAudit: true, + }, + { + name: "self subject access review", + method: http.MethodPost, + target: "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews", + }, + { + name: "subject access review remains audited", + method: http.MethodPost, + target: "/apis/authorization.k8s.io/v1/subjectaccessreviews", + expectAudit: true, + }, + { + name: "create remains audited", + method: http.MethodPost, + target: "/api/v1/namespaces", + expectAudit: true, + }, + { + name: "patch remains audited", + method: http.MethodPatch, + target: "/apis/apps/v1/namespaces/default/deployments/guestbook-ui", + expectAudit: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + config := config.LogsAudit{ + Enabled: new(true), + } + + l := must.Value(audit.NewLog(t.Context(), config, testDB(t), zaptest.NewLogger(t)))(t) + handler := l.Wrap(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + + ctx := ctxstore.WithValue(t.Context(), &auditlog.Data{ + K8SAccess: &auditlog.K8SAccess{ + FullMethodName: tc.method + " " + strings.Split(tc.target, "?")[0], + ClusterName: "cluster1", + }, + Session: auditlog.Session{ + Email: "user@example.com", + }, + }) + req := httptest.NewRequestWithContext(ctx, tc.method, tc.target, nil) + + handler.ServeHTTP(httptest.NewRecorder(), req) + + events := readAuditEvents(t, l) + if !tc.expectAudit { + require.Empty(t, events) + + return + } + + require.Len(t, events, 1) + require.Equal(t, "k8s_access", events[0]["event_type"]) + }) + } +} + +func readAuditEvents(t *testing.T, l *audit.Log) []map[string]any { + t.Helper() + + rdr, err := l.Reader(t.Context(), auditlog.ReadFilters{End: time.Now().Add(5 * time.Second)}) + require.NoError(t, err) + + t.Cleanup(func() { + require.NoError(t, rdr.Close()) + }) + + var events []map[string]any + + for { + data, err := rdr.Read() + if errors.Is(err, io.EOF) { + break + } + + require.NoError(t, err) + + var event map[string]any + + require.NoError(t, json.Unmarshal(data, &event)) + + events = append(events, event) + } + + return events +} + func cmpIgnoreTime(t *testing.T, expected string, actual string) { expectedEvents := loadEvents(t, expected) actualEvents := loadEvents(t, actual)