diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 8b616f34b8441..9af6e50764dfd 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1552,6 +1552,16 @@ func (q *querier) DeleteOAuth2ProviderAppTokensByAppAndUserID(ctx context.Contex return q.db.DeleteOAuth2ProviderAppTokensByAppAndUserID(ctx, arg) } +func (q *querier) DeleteOldAuditLogConnectionEvents(ctx context.Context, threshold database.DeleteOldAuditLogConnectionEventsParams) error { + // `ResourceSystem` is deprecated, but it doesn't make sense to add + // `policy.ActionDelete` to `ResourceAuditLog`, since this is the one and + // only time we'll be deleting from the audit log. + if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil { + return err + } + return q.db.DeleteOldAuditLogConnectionEvents(ctx, threshold) +} + func (q *querier) DeleteOldNotificationMessages(ctx context.Context) error { if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceNotificationMessage); err != nil { return err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 2ea27f7d92342..c153974394650 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -337,6 +337,10 @@ func (s *MethodTestSuite) TestAuditLogs() { _ = dbgen.AuditLog(s.T(), db, database.AuditLog{}) check.Args(database.CountAuditLogsParams{}, emptyPreparedAuthorized{}).Asserts(rbac.ResourceAuditLog, policy.ActionRead) })) + s.Run("DeleteOldAuditLogConnectionEvents", s.Subtest(func(db database.Store, check *expects) { + _ = dbgen.AuditLog(s.T(), db, database.AuditLog{}) + check.Args(database.DeleteOldAuditLogConnectionEventsParams{}).Asserts(rbac.ResourceSystem, policy.ActionDelete) + })) } func (s *MethodTestSuite) TestConnectionLogs() { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index a0090a1103279..7a7c3cb2d41c6 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -362,6 +362,13 @@ func (m queryMetricsStore) DeleteOAuth2ProviderAppTokensByAppAndUserID(ctx conte return r0 } +func (m queryMetricsStore) DeleteOldAuditLogConnectionEvents(ctx context.Context, threshold database.DeleteOldAuditLogConnectionEventsParams) error { + start := time.Now() + r0 := m.s.DeleteOldAuditLogConnectionEvents(ctx, threshold) + m.queryLatencies.WithLabelValues("DeleteOldAuditLogConnectionEvents").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) DeleteOldNotificationMessages(ctx context.Context) error { start := time.Now() r0 := m.s.DeleteOldNotificationMessages(ctx) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 723c4f3687e81..fba3deb45e4be 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -635,6 +635,20 @@ func (mr *MockStoreMockRecorder) DeleteOAuth2ProviderAppTokensByAppAndUserID(ctx return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOAuth2ProviderAppTokensByAppAndUserID", reflect.TypeOf((*MockStore)(nil).DeleteOAuth2ProviderAppTokensByAppAndUserID), ctx, arg) } +// DeleteOldAuditLogConnectionEvents mocks base method. +func (m *MockStore) DeleteOldAuditLogConnectionEvents(ctx context.Context, arg database.DeleteOldAuditLogConnectionEventsParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteOldAuditLogConnectionEvents", ctx, arg) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteOldAuditLogConnectionEvents indicates an expected call of DeleteOldAuditLogConnectionEvents. +func (mr *MockStoreMockRecorder) DeleteOldAuditLogConnectionEvents(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOldAuditLogConnectionEvents", reflect.TypeOf((*MockStore)(nil).DeleteOldAuditLogConnectionEvents), ctx, arg) +} + // DeleteOldNotificationMessages mocks base method. func (m *MockStore) DeleteOldNotificationMessages(ctx context.Context) error { m.ctrl.T.Helper() diff --git a/coderd/database/dbpurge/dbpurge.go b/coderd/database/dbpurge/dbpurge.go index b7a308cfd6a06..135d7f40b05dd 100644 --- a/coderd/database/dbpurge/dbpurge.go +++ b/coderd/database/dbpurge/dbpurge.go @@ -18,6 +18,11 @@ import ( const ( delay = 10 * time.Minute maxAgentLogAge = 7 * 24 * time.Hour + // Connection events are now inserted into the `connection_logs` table. + // We'll slowly remove old connection events from the `audit_logs` table, + // but we won't touch the `connection_logs` table. + maxAuditLogConnectionEventAge = 90 * 24 * time.Hour // 90 days + auditLogConnectionEventBatchSize = 1000 ) // 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. return xerrors.Errorf("failed to delete old notification messages: %w", err) } + deleteOldAuditLogConnectionEventsBefore := start.Add(-maxAuditLogConnectionEventAge) + if err := tx.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{ + BeforeTime: deleteOldAuditLogConnectionEventsBefore, + LimitCount: auditLogConnectionEventBatchSize, + }); err != nil { + return xerrors.Errorf("failed to delete old audit log connection events: %w", err) + } + logger.Debug(ctx, "purged old database entries", slog.F("duration", clk.Since(start))) return nil diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go index 4e81868ac73fb..1d57a87e68f48 100644 --- a/coderd/database/dbpurge/dbpurge_test.go +++ b/coderd/database/dbpurge/dbpurge_test.go @@ -490,3 +490,148 @@ func containsProvisionerDaemon(daemons []database.ProvisionerDaemon, name string return d.Name == name }) } + +//nolint:paralleltest // It uses LockIDDBPurge. +func TestDeleteOldAuditLogConnectionEvents(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + clk := quartz.NewMock(t) + now := dbtime.Now() + afterThreshold := now.Add(-91 * 24 * time.Hour) // 91 days ago (older than 90 day threshold) + beforeThreshold := now.Add(-30 * 24 * time.Hour) // 30 days ago (newer than 90 day threshold) + closeBeforeThreshold := now.Add(-89 * 24 * time.Hour) // 89 days ago + clk.Set(now).MustWait(ctx) + + db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) + user := dbgen.User(t, db, database.User{}) + org := dbgen.Organization(t, db, database.Organization{}) + + oldConnectLog := dbgen.AuditLog(t, db, database.AuditLog{ + UserID: user.ID, + OrganizationID: org.ID, + Time: afterThreshold, + Action: database.AuditActionConnect, + ResourceType: database.ResourceTypeWorkspace, + }) + + oldDisconnectLog := dbgen.AuditLog(t, db, database.AuditLog{ + UserID: user.ID, + OrganizationID: org.ID, + Time: afterThreshold, + Action: database.AuditActionDisconnect, + ResourceType: database.ResourceTypeWorkspace, + }) + + oldOpenLog := dbgen.AuditLog(t, db, database.AuditLog{ + UserID: user.ID, + OrganizationID: org.ID, + Time: afterThreshold, + Action: database.AuditActionOpen, + ResourceType: database.ResourceTypeWorkspace, + }) + + oldCloseLog := dbgen.AuditLog(t, db, database.AuditLog{ + UserID: user.ID, + OrganizationID: org.ID, + Time: afterThreshold, + Action: database.AuditActionClose, + ResourceType: database.ResourceTypeWorkspace, + }) + + recentConnectLog := dbgen.AuditLog(t, db, database.AuditLog{ + UserID: user.ID, + OrganizationID: org.ID, + Time: beforeThreshold, + Action: database.AuditActionConnect, + ResourceType: database.ResourceTypeWorkspace, + }) + + oldNonConnectionLog := dbgen.AuditLog(t, db, database.AuditLog{ + UserID: user.ID, + OrganizationID: org.ID, + Time: afterThreshold, + Action: database.AuditActionCreate, + ResourceType: database.ResourceTypeWorkspace, + }) + + nearThresholdConnectLog := dbgen.AuditLog(t, db, database.AuditLog{ + UserID: user.ID, + OrganizationID: org.ID, + Time: closeBeforeThreshold, + Action: database.AuditActionConnect, + ResourceType: database.ResourceTypeWorkspace, + }) + + // Run the purge + done := awaitDoTick(ctx, t, clk) + closer := dbpurge.New(ctx, logger, db, clk) + defer closer.Close() + // Wait for tick + testutil.TryReceive(ctx, t, done) + + // Verify results by querying all audit logs + logs, err := db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{}) + require.NoError(t, err) + + // Extract log IDs for comparison + logIDs := make([]uuid.UUID, len(logs)) + for i, log := range logs { + logIDs[i] = log.AuditLog.ID + } + + require.NotContains(t, logIDs, oldConnectLog.ID, "old connect log should be deleted") + require.NotContains(t, logIDs, oldDisconnectLog.ID, "old disconnect log should be deleted") + require.NotContains(t, logIDs, oldOpenLog.ID, "old open log should be deleted") + require.NotContains(t, logIDs, oldCloseLog.ID, "old close log should be deleted") + require.Contains(t, logIDs, recentConnectLog.ID, "recent connect log should be kept") + require.Contains(t, logIDs, nearThresholdConnectLog.ID, "near threshold connect log should be kept") + require.Contains(t, logIDs, oldNonConnectionLog.ID, "old non-connection log should be kept") +} + +func TestDeleteOldAuditLogConnectionEventsLimit(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + user := dbgen.User(t, db, database.User{}) + org := dbgen.Organization(t, db, database.Organization{}) + + now := dbtime.Now() + threshold := now.Add(-90 * 24 * time.Hour) + + for i := 0; i < 5; i++ { + dbgen.AuditLog(t, db, database.AuditLog{ + UserID: user.ID, + OrganizationID: org.ID, + Time: threshold.Add(-time.Duration(i+1) * time.Hour), + Action: database.AuditActionConnect, + ResourceType: database.ResourceTypeWorkspace, + }) + } + + err := db.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{ + BeforeTime: threshold, + LimitCount: 1, + }) + require.NoError(t, err) + + logs, err := db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{}) + require.NoError(t, err) + + require.Len(t, logs, 4) + + err = db.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{ + BeforeTime: threshold, + LimitCount: 100, + }) + require.NoError(t, err) + + logs, err = db.GetAuditLogsOffset(ctx, database.GetAuditLogsOffsetParams{}) + require.NoError(t, err) + + require.Len(t, logs, 0) +} diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 72f511618838b..24893a9197815 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -96,6 +96,7 @@ type sqlcQuerier interface { DeleteOAuth2ProviderAppCodesByAppAndUserID(ctx context.Context, arg DeleteOAuth2ProviderAppCodesByAppAndUserIDParams) error DeleteOAuth2ProviderAppSecretByID(ctx context.Context, id uuid.UUID) error DeleteOAuth2ProviderAppTokensByAppAndUserID(ctx context.Context, arg DeleteOAuth2ProviderAppTokensByAppAndUserIDParams) error + DeleteOldAuditLogConnectionEvents(ctx context.Context, arg DeleteOldAuditLogConnectionEventsParams) error // Delete all notification messages which have not been updated for over a week. DeleteOldNotificationMessages(ctx context.Context) error // Delete provisioner daemons that have been created at least a week ago diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 676ce75621ded..0ef4553149465 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -566,6 +566,33 @@ func (q *sqlQuerier) CountAuditLogs(ctx context.Context, arg CountAuditLogsParam return count, err } +const deleteOldAuditLogConnectionEvents = `-- name: DeleteOldAuditLogConnectionEvents :exec +DELETE FROM audit_logs +WHERE id IN ( + SELECT id FROM audit_logs + WHERE + ( + action = 'connect' + OR action = 'disconnect' + OR action = 'open' + OR action = 'close' + ) + AND "time" < $1::timestamp with time zone + ORDER BY "time" ASC + LIMIT $2 +) +` + +type DeleteOldAuditLogConnectionEventsParams struct { + BeforeTime time.Time `db:"before_time" json:"before_time"` + LimitCount int32 `db:"limit_count" json:"limit_count"` +} + +func (q *sqlQuerier) DeleteOldAuditLogConnectionEvents(ctx context.Context, arg DeleteOldAuditLogConnectionEventsParams) error { + _, err := q.db.ExecContext(ctx, deleteOldAuditLogConnectionEvents, arg.BeforeTime, arg.LimitCount) + return err +} + const getAuditLogsOffset = `-- name: GetAuditLogsOffset :many SELECT audit_logs.id, audit_logs.time, audit_logs.user_id, audit_logs.organization_id, audit_logs.ip, audit_logs.user_agent, audit_logs.resource_type, audit_logs.resource_id, audit_logs.resource_target, audit_logs.action, audit_logs.diff, audit_logs.status_code, audit_logs.additional_fields, audit_logs.request_id, audit_logs.resource_icon, -- sqlc.embed(users) would be nice but it does not seem to play well with diff --git a/coderd/database/queries/auditlogs.sql b/coderd/database/queries/auditlogs.sql index 6269f21cd27e4..63e8c721c8e4c 100644 --- a/coderd/database/queries/auditlogs.sql +++ b/coderd/database/queries/auditlogs.sql @@ -237,3 +237,19 @@ WHERE -- Authorize Filter clause will be injected below in CountAuthorizedAuditLogs -- @authorize_filter ; + +-- name: DeleteOldAuditLogConnectionEvents :exec +DELETE FROM audit_logs +WHERE id IN ( + SELECT id FROM audit_logs + WHERE + ( + action = 'connect' + OR action = 'disconnect' + OR action = 'open' + OR action = 'close' + ) + AND "time" < @before_time::timestamp with time zone + ORDER BY "time" ASC + LIMIT @limit_count +);