Skip to content

Commit 60d24bb

Browse files
ethanndicksonethanndickson
committed
chore!: delete old connection events from audit log
1 parent 4be35f4 commit 60d24bb

File tree

10 files changed

+258
-0
lines changed

10 files changed

+258
-0
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,6 +1510,16 @@ func (q *querier) DeleteOAuth2ProviderAppTokensByAppAndUserID(ctx context.Contex
15101510
return q.db.DeleteOAuth2ProviderAppTokensByAppAndUserID(ctx, arg)
15111511
}
15121512

1513+
func (q *querier) DeleteOldAuditLogConnectionEvents(ctx context.Context, threshold database.DeleteOldAuditLogConnectionEventsParams) error {
1514+
// `ResourceSystem` is deprecated, but it doesn't make sense to add
1515+
// `policy.ActionDelete` to `ResourceAuditLog`, since this is the one and
1516+
// only time we'll be deleting from the audit log.
1517+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
1518+
return err
1519+
}
1520+
return q.db.DeleteOldAuditLogConnectionEvents(ctx, threshold)
1521+
}
1522+
15131523
func (q *querier) DeleteOldNotificationMessages(ctx context.Context) error {
15141524
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceNotificationMessage); err != nil {
15151525
return err

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,10 @@ func (s *MethodTestSuite) TestAuditLogs() {
327327
LimitOpt: 10,
328328
}, emptyPreparedAuthorized{}).Asserts(rbac.ResourceAuditLog, policy.ActionRead)
329329
}))
330+
s.Run("DeleteOldAuditLogConnectionEvents", s.Subtest(func(db database.Store, check *expects) {
331+
_ = dbgen.AuditLog(s.T(), db, database.AuditLog{})
332+
check.Args(database.DeleteOldAuditLogConnectionEventsParams{}).Asserts(rbac.ResourceSystem, policy.ActionDelete)
333+
}))
330334
}
331335

332336
func (s *MethodTestSuite) TestConnectionLogs() {

‎coderd/database/dbmem/dbmem.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2188,6 +2188,27 @@ func (q *FakeQuerier) DeleteOAuth2ProviderAppTokensByAppAndUserID(_ context.Cont
21882188
return nil
21892189
}
21902190

2191+
func (q *FakeQuerier) DeleteOldAuditLogConnectionEvents(ctx context.Context, params database.DeleteOldAuditLogConnectionEventsParams) error {
2192+
q.mutex.Lock()
2193+
defer q.mutex.Unlock()
2194+
2195+
deleted := 0
2196+
newAuditLogs := make([]database.AuditLog, 0, len(q.auditLogs))
2197+
for _, auditLog := range q.auditLogs {
2198+
isConnectionEvent := auditLog.Action == database.AuditActionConnect ||
2199+
auditLog.Action == database.AuditActionDisconnect ||
2200+
auditLog.Action == database.AuditActionOpen ||
2201+
auditLog.Action == database.AuditActionClose
2202+
if isConnectionEvent && auditLog.Time.Before(params.BeforeTime) && deleted < int(params.LimitCount) {
2203+
deleted++
2204+
continue
2205+
}
2206+
newAuditLogs = append(newAuditLogs, auditLog)
2207+
}
2208+
q.auditLogs = newAuditLogs
2209+
return nil
2210+
}
2211+
21912212
func (*FakeQuerier) DeleteOldNotificationMessages(_ context.Context) error {
21922213
return nil
21932214
}

‎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: 14 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: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ import (
1818
const (
1919
delay = 10 * time.Minute
2020
maxAgentLogAge = 7 * 24 * time.Hour
21+
// Connection events are now inserted into the `connection_logs` table.
22+
// We'll slowly remove old connection events from the `audit_log` table,
23+
// but we won't touch the `connection_logs` table.
24+
maxAuditLogConnectionEventAge = 90 * 24 * time.Hour // 90 days
25+
auditLogConnectionEventBatchSize = 1000
2126
)
2227

2328
// New creates a new periodically purging database instance.
@@ -63,6 +68,14 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz.
6368
return xerrors.Errorf("failed to delete old notification messages: %w", err)
6469
}
6570

71+
deleteOldAuditLogConnectionEventsBefore := start.Add(-maxAuditLogConnectionEventAge)
72+
if err := tx.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{
73+
BeforeTime: deleteOldAuditLogConnectionEventsBefore,
74+
LimitCount: auditLogConnectionEventBatchSize,
75+
}); err != nil {
76+
return xerrors.Errorf("failed to delete old audit log connection events: %w", err)
77+
}
78+
6679
logger.Debug(ctx, "purged old database entries", slog.F("duration", clk.Since(start)))
6780

6881
return nil

‎coderd/database/dbpurge/dbpurge_test.go

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,3 +490,148 @@ func containsProvisionerDaemon(daemons []database.ProvisionerDaemon, name string
490490
return d.Name == name
491491
})
492492
}
493+
494+
//nolint:paralleltest // It uses LockIDDBPurge.
495+
func TestDeleteOldAuditLogConnectionEvents(t *testing.T) {
496+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
497+
defer cancel()
498+
499+
clk := quartz.NewMock(t)
500+
now := dbtime.Now()
501+
afterThreshold := now.Add(-91 * 24 * time.Hour) // 91 days ago (older than 90 day threshold)
502+
beforeThreshold := now.Add(-30 * 24 * time.Hour) // 30 days ago (newer than 90 day threshold)
503+
closeBeforeThreshold := now.Add(-89 * 24 * time.Hour) // 89 days ago
504+
clk.Set(now).MustWait(ctx)
505+
506+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
507+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
508+
user := dbgen.User(t, db, database.User{})
509+
org := dbgen.Organization(t, db, database.Organization{})
510+
511+
oldConnectLog := dbgen.AuditLog(t, db, database.AuditLog{
512+
UserID: user.ID,
513+
OrganizationID: org.ID,
514+
Time: afterThreshold,
515+
Action: database.AuditActionConnect,
516+
ResourceType: database.ResourceTypeWorkspace,
517+
})
518+
519+
oldDisconnectLog := dbgen.AuditLog(t, db, database.AuditLog{
520+
UserID: user.ID,
521+
OrganizationID: org.ID,
522+
Time: afterThreshold,
523+
Action: database.AuditActionDisconnect,
524+
ResourceType: database.ResourceTypeWorkspace,
525+
})
526+
527+
oldOpenLog := dbgen.AuditLog(t, db, database.AuditLog{
528+
UserID: user.ID,
529+
OrganizationID: org.ID,
530+
Time: afterThreshold,
531+
Action: database.AuditActionOpen,
532+
ResourceType: database.ResourceTypeWorkspace,
533+
})
534+
535+
oldCloseLog := dbgen.AuditLog(t, db, database.AuditLog{
536+
UserID: user.ID,
537+
OrganizationID: org.ID,
538+
Time: afterThreshold,
539+
Action: database.AuditActionClose,
540+
ResourceType: database.ResourceTypeWorkspace,
541+
})
542+
543+
recentConnectLog := dbgen.AuditLog(t, db, database.AuditLog{
544+
UserID: user.ID,
545+
OrganizationID: org.ID,
546+
Time: beforeThreshold,
547+
Action: database.AuditActionConnect,
548+
ResourceType: database.ResourceTypeWorkspace,
549+
})
550+
551+
oldNonConnectionLog := dbgen.AuditLog(t, db, database.AuditLog{
552+
UserID: user.ID,
553+
OrganizationID: org.ID,
554+
Time: afterThreshold,
555+
Action: database.AuditActionCreate,
556+
ResourceType: database.ResourceTypeWorkspace,
557+
})
558+
559+
nearThresholdConnectLog := dbgen.AuditLog(t, db, database.AuditLog{
560+
UserID: user.ID,
561+
OrganizationID: org.ID,
562+
Time: closeBeforeThreshold,
563+
Action: database.AuditActionConnect,
564+
ResourceType: database.ResourceTypeWorkspace,
565+
})
566+
567+
// Run the purge
568+
done := awaitDoTick(ctx, t, clk)
569+
closer := dbpurge.New(ctx, logger, db, clk)
570+
defer closer.Close()
571+
// Wait for tick
572+
testutil.TryReceive(ctx, t, done)
573+
574+
// Verify results by querying all audit logs
575+
logs, err := db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{})
576+
require.NoError(t, err)
577+
578+
// Extract log IDs for comparison
579+
logIDs := make([]uuid.UUID, len(logs))
580+
for i, log := range logs {
581+
logIDs[i] = log.AuditLog.ID
582+
}
583+
584+
require.NotContains(t, logIDs, oldConnectLog.ID, "old connect log should be deleted")
585+
require.NotContains(t, logIDs, oldDisconnectLog.ID, "old disconnect log should be deleted")
586+
require.NotContains(t, logIDs, oldOpenLog.ID, "old open log should be deleted")
587+
require.NotContains(t, logIDs, oldCloseLog.ID, "old close log should be deleted")
588+
require.Contains(t, logIDs, recentConnectLog.ID, "recent connect log should be kept")
589+
require.Contains(t, logIDs, nearThresholdConnectLog.ID, "near threshold connect log should be kept")
590+
require.Contains(t, logIDs, oldNonConnectionLog.ID, "old non-connection log should be kept")
591+
}
592+
593+
func TestDeleteOldAuditLogConnectionEventsLimit(t *testing.T) {
594+
t.Parallel()
595+
596+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
597+
defer cancel()
598+
599+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
600+
user := dbgen.User(t, db, database.User{})
601+
org := dbgen.Organization(t, db, database.Organization{})
602+
603+
now := dbtime.Now()
604+
threshold := now.Add(-90 * 24 * time.Hour)
605+
606+
for i := 0; i < 5; i++ {
607+
dbgen.AuditLog(t, db, database.AuditLog{
608+
UserID: user.ID,
609+
OrganizationID: org.ID,
610+
Time: threshold.Add(-time.Duration(i+1) * time.Hour),
611+
Action: database.AuditActionConnect,
612+
ResourceType: database.ResourceTypeWorkspace,
613+
})
614+
}
615+
616+
err := db.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{
617+
BeforeTime: threshold,
618+
LimitCount: 1,
619+
})
620+
require.NoError(t, err)
621+
622+
logs, err := db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{})
623+
require.NoError(t, err)
624+
625+
require.Len(t, logs, 4)
626+
627+
err = db.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{
628+
BeforeTime: threshold,
629+
LimitCount: 100,
630+
})
631+
require.NoError(t, err)
632+
633+
logs, err = db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{})
634+
require.NoError(t, err)
635+
636+
require.Len(t, logs, 0)
637+
}

‎coderd/database/querier.go

Lines changed: 1 addition & 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: 27 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: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,3 +156,19 @@ INSERT INTO
156156
)
157157
VALUES
158158
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) RETURNING *;
159+
160+
-- name: DeleteOldAuditLogConnectionEvents :exec
161+
DELETE FROM audit_logs
162+
WHERE id IN (
163+
SELECT id FROM audit_logs
164+
WHERE
165+
(
166+
action = 'connect'
167+
OR action = 'disconnect'
168+
OR action = 'open'
169+
OR action = 'close'
170+
)
171+
AND "time" < @before_time::timestamp with time zone
172+
ORDER BY "time" ASC
173+
LIMIT @limit_count
174+
);

0 commit comments

Comments
 (0)