Skip to content

Commit c85d79b

Browse files
mafredrimafredri
authored
feat(coderd/database/dbpurge): add retention for audit logs (#21025)
Add configurable retention policy for audit logs. The DeleteOldAuditLogs query excludes deprecated connection events (connect, disconnect, open, close) which are handled separately by DeleteOldAuditLogConnectionEvents. Disabled (0) by default. Depends on #21021 Updates #20743
1 parent fa7bbe2 commit c85d79b

File tree

9 files changed

+296
-0
lines changed

9 files changed

+296
-0
lines changed

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,6 +1749,13 @@ func (q *querier) DeleteOldAuditLogConnectionEvents(ctx context.Context, thresho
17491749
return q.db.DeleteOldAuditLogConnectionEvents(ctx, threshold)
17501750
}
17511751

1752+
func (q *querier) DeleteOldAuditLogs(ctx context.Context, arg database.DeleteOldAuditLogsParams) (int64, error) {
1753+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
1754+
return 0, err
1755+
}
1756+
return q.db.DeleteOldAuditLogs(ctx, arg)
1757+
}
1758+
17521759
func (q *querier) DeleteOldConnectionLogs(ctx context.Context, arg database.DeleteOldConnectionLogsParams) (int64, error) {
17531760
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
17541761
return 0, err

‎coderd/database/dbauthz/dbauthz_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,10 @@ func (s *MethodTestSuite) TestAuditLogs() {
324324
dbm.EXPECT().DeleteOldAuditLogConnectionEvents(gomock.Any(), database.DeleteOldAuditLogConnectionEventsParams{}).Return(nil).AnyTimes()
325325
check.Args(database.DeleteOldAuditLogConnectionEventsParams{}).Asserts(rbac.ResourceSystem, policy.ActionDelete)
326326
}))
327+
s.Run("DeleteOldAuditLogs", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
328+
dbm.EXPECT().DeleteOldAuditLogs(gomock.Any(), database.DeleteOldAuditLogsParams{}).Return(int64(0), nil).AnyTimes()
329+
check.Args(database.DeleteOldAuditLogsParams{}).Asserts(rbac.ResourceSystem, policy.ActionDelete)
330+
}))
327331
}
328332

329333
func (s *MethodTestSuite) TestConnectionLogs() {

‎coderd/database/dbmetrics/querymetrics.go‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbpurge/dbpurge.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ const (
2727
auditLogConnectionEventBatchSize = 1000
2828
// Batch size for connection log deletion.
2929
connectionLogsBatchSize = 10000
30+
// Batch size for audit log deletion.
31+
auditLogsBatchSize = 10000
3032
// Telemetry heartbeats are used to deduplicate events across replicas. We
3133
// don't need to persist heartbeat rows for longer than 24 hours, as they
3234
// are only used for deduplication across replicas. The time needs to be
@@ -126,10 +128,24 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, vals *coder
126128
}
127129
}
128130

131+
var purgedAuditLogs int64
132+
auditLogsRetention := vals.Retention.AuditLogs.Value()
133+
if auditLogsRetention > 0 {
134+
deleteAuditLogsBefore := start.Add(-auditLogsRetention)
135+
purgedAuditLogs, err = tx.DeleteOldAuditLogs(ctx, database.DeleteOldAuditLogsParams{
136+
BeforeTime: deleteAuditLogsBefore,
137+
LimitCount: auditLogsBatchSize,
138+
})
139+
if err != nil {
140+
return xerrors.Errorf("failed to delete old audit logs: %w", err)
141+
}
142+
}
143+
129144
logger.Debug(ctx, "purged old database entries",
130145
slog.F("expired_api_keys", expiredAPIKeys),
131146
slog.F("aibridge_records", purgedAIBridgeRecords),
132147
slog.F("connection_logs", purgedConnectionLogs),
148+
slog.F("audit_logs", purgedAuditLogs),
133149
slog.F("duration", clk.Since(start)),
134150
)
135151

‎coderd/database/dbpurge/dbpurge_test.go‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,3 +1050,198 @@ func TestDeleteOldAIBridgeRecords(t *testing.T) {
10501050
require.NoError(t, err)
10511051
require.Len(t, newToolUsages, 1, "near threshold tool usages should not be deleted")
10521052
}
1053+
1054+
func TestDeleteOldAuditLogs(t *testing.T) {
1055+
t.Parallel()
1056+
1057+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1058+
retentionPeriod := 30 * 24 * time.Hour
1059+
afterThreshold := now.Add(-retentionPeriod).Add(-24 * time.Hour) // 31 days ago (older than threshold)
1060+
beforeThreshold := now.Add(-15 * 24 * time.Hour) // 15 days ago (newer than threshold)
1061+
1062+
testCases := []struct {
1063+
name string
1064+
retentionConfig codersdk.RetentionConfig
1065+
oldLogTime time.Time
1066+
recentLogTime *time.Time // nil means no recent log created
1067+
expectOldDeleted bool
1068+
expectedLogsRemaining int
1069+
}{
1070+
{
1071+
name: "RetentionEnabled",
1072+
retentionConfig: codersdk.RetentionConfig{
1073+
AuditLogs: serpent.Duration(retentionPeriod),
1074+
},
1075+
oldLogTime: afterThreshold,
1076+
recentLogTime: &beforeThreshold,
1077+
expectOldDeleted: true,
1078+
expectedLogsRemaining: 1, // only recent log remains
1079+
},
1080+
{
1081+
name: "RetentionDisabled",
1082+
retentionConfig: codersdk.RetentionConfig{
1083+
AuditLogs: serpent.Duration(0),
1084+
},
1085+
oldLogTime: now.Add(-365 * 24 * time.Hour), // 1 year ago
1086+
recentLogTime: nil,
1087+
expectOldDeleted: false,
1088+
expectedLogsRemaining: 1, // old log is kept
1089+
},
1090+
}
1091+
1092+
for _, tc := range testCases {
1093+
t.Run(tc.name, func(t *testing.T) {
1094+
t.Parallel()
1095+
1096+
ctx := testutil.Context(t, testutil.WaitShort)
1097+
clk := quartz.NewMock(t)
1098+
clk.Set(now).MustWait(ctx)
1099+
1100+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1101+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1102+
1103+
// Setup test fixtures.
1104+
user := dbgen.User(t, db, database.User{})
1105+
org := dbgen.Organization(t, db, database.Organization{})
1106+
1107+
// Create old audit log.
1108+
oldLog := dbgen.AuditLog(t, db, database.AuditLog{
1109+
UserID: user.ID,
1110+
OrganizationID: org.ID,
1111+
Time: tc.oldLogTime,
1112+
Action: database.AuditActionCreate,
1113+
ResourceType: database.ResourceTypeWorkspace,
1114+
})
1115+
1116+
// Create recent audit log if specified.
1117+
var recentLog database.AuditLog
1118+
if tc.recentLogTime != nil {
1119+
recentLog = dbgen.AuditLog(t, db, database.AuditLog{
1120+
UserID: user.ID,
1121+
OrganizationID: org.ID,
1122+
Time: *tc.recentLogTime,
1123+
Action: database.AuditActionCreate,
1124+
ResourceType: database.ResourceTypeWorkspace,
1125+
})
1126+
}
1127+
1128+
// Run the purge.
1129+
done := awaitDoTick(ctx, t, clk)
1130+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1131+
Retention: tc.retentionConfig,
1132+
}, clk)
1133+
defer closer.Close()
1134+
testutil.TryReceive(ctx, t, done)
1135+
1136+
// Verify results.
1137+
logs, err := db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{
1138+
LimitOpt: 100,
1139+
})
1140+
require.NoError(t, err)
1141+
require.Len(t, logs, tc.expectedLogsRemaining, "unexpected number of logs remaining")
1142+
1143+
logIDs := make([]uuid.UUID, len(logs))
1144+
for i, log := range logs {
1145+
logIDs[i] = log.AuditLog.ID
1146+
}
1147+
1148+
if tc.expectOldDeleted {
1149+
require.NotContains(t, logIDs, oldLog.ID, "old audit log should be deleted")
1150+
} else {
1151+
require.Contains(t, logIDs, oldLog.ID, "old audit log should NOT be deleted")
1152+
}
1153+
1154+
if tc.recentLogTime != nil {
1155+
require.Contains(t, logIDs, recentLog.ID, "recent audit log should be kept")
1156+
}
1157+
})
1158+
}
1159+
1160+
// ConnectionEventsNotDeleted is a special case that tests multiple audit
1161+
// action types, so it's kept as a separate subtest.
1162+
t.Run("ConnectionEventsNotDeleted", func(t *testing.T) {
1163+
t.Parallel()
1164+
1165+
ctx := testutil.Context(t, testutil.WaitShort)
1166+
clk := quartz.NewMock(t)
1167+
clk.Set(now).MustWait(ctx)
1168+
1169+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1170+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1171+
user := dbgen.User(t, db, database.User{})
1172+
org := dbgen.Organization(t, db, database.Organization{})
1173+
1174+
// Create old connection events (should NOT be deleted by audit logs retention).
1175+
oldConnectLog := dbgen.AuditLog(t, db, database.AuditLog{
1176+
UserID: user.ID,
1177+
OrganizationID: org.ID,
1178+
Time: afterThreshold,
1179+
Action: database.AuditActionConnect,
1180+
ResourceType: database.ResourceTypeWorkspace,
1181+
})
1182+
1183+
oldDisconnectLog := dbgen.AuditLog(t, db, database.AuditLog{
1184+
UserID: user.ID,
1185+
OrganizationID: org.ID,
1186+
Time: afterThreshold,
1187+
Action: database.AuditActionDisconnect,
1188+
ResourceType: database.ResourceTypeWorkspace,
1189+
})
1190+
1191+
oldOpenLog := dbgen.AuditLog(t, db, database.AuditLog{
1192+
UserID: user.ID,
1193+
OrganizationID: org.ID,
1194+
Time: afterThreshold,
1195+
Action: database.AuditActionOpen,
1196+
ResourceType: database.ResourceTypeWorkspace,
1197+
})
1198+
1199+
oldCloseLog := dbgen.AuditLog(t, db, database.AuditLog{
1200+
UserID: user.ID,
1201+
OrganizationID: org.ID,
1202+
Time: afterThreshold,
1203+
Action: database.AuditActionClose,
1204+
ResourceType: database.ResourceTypeWorkspace,
1205+
})
1206+
1207+
// Create old non-connection audit log (should be deleted).
1208+
oldCreateLog := dbgen.AuditLog(t, db, database.AuditLog{
1209+
UserID: user.ID,
1210+
OrganizationID: org.ID,
1211+
Time: afterThreshold,
1212+
Action: database.AuditActionCreate,
1213+
ResourceType: database.ResourceTypeWorkspace,
1214+
})
1215+
1216+
// Run the purge with audit logs retention enabled.
1217+
done := awaitDoTick(ctx, t, clk)
1218+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1219+
Retention: codersdk.RetentionConfig{
1220+
AuditLogs: serpent.Duration(retentionPeriod),
1221+
},
1222+
}, clk)
1223+
defer closer.Close()
1224+
testutil.TryReceive(ctx, t, done)
1225+
1226+
// Verify results.
1227+
logs, err := db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{
1228+
LimitOpt: 100,
1229+
})
1230+
require.NoError(t, err)
1231+
require.Len(t, logs, 4, "should have 4 connection event logs remaining")
1232+
1233+
logIDs := make([]uuid.UUID, len(logs))
1234+
for i, log := range logs {
1235+
logIDs[i] = log.AuditLog.ID
1236+
}
1237+
1238+
// Connection events should NOT be deleted by audit logs retention.
1239+
require.Contains(t, logIDs, oldConnectLog.ID, "old connect log should NOT be deleted by audit logs retention")
1240+
require.Contains(t, logIDs, oldDisconnectLog.ID, "old disconnect log should NOT be deleted by audit logs retention")
1241+
require.Contains(t, logIDs, oldOpenLog.ID, "old open log should NOT be deleted by audit logs retention")
1242+
require.Contains(t, logIDs, oldCloseLog.ID, "old close log should NOT be deleted by audit logs retention")
1243+
1244+
// Non-connection event should be deleted.
1245+
require.NotContains(t, logIDs, oldCreateLog.ID, "old create log should be deleted by audit logs retention")
1246+
})
1247+
}

‎coderd/database/querier.go‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go‎

Lines changed: 31 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/auditlogs.sql‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,3 +253,20 @@ WHERE id IN (
253253
ORDER BY "time" ASC
254254
LIMIT @limit_count
255255
);
256+
257+
-- name: DeleteOldAuditLogs :execrows
258+
-- Deletes old audit logs based on retention policy, excluding deprecated
259+
-- connection events (connect, disconnect, open, close) which are handled
260+
-- separately by DeleteOldAuditLogConnectionEvents.
261+
WITH old_logs AS (
262+
SELECT id
263+
FROM audit_logs
264+
WHERE
265+
"time" < @before_time::timestamp with time zone
266+
AND action NOT IN ('connect', 'disconnect', 'open', 'close')
267+
ORDER BY "time" ASC
268+
LIMIT @limit_count
269+
)
270+
DELETE FROM audit_logs
271+
USING old_logs
272+
WHERE audit_logs.id = old_logs.id;

0 commit comments

Comments
 (0)