Skip to content

Commit db9274f

Browse files
EmyrkEmyrk
authored
feat: purge expired api keys in dbpurge (#20863) (#21040)
Backport of #20863
1 parent a75205a commit db9274f

File tree

9 files changed

+114
-2
lines changed

9 files changed

+114
-2
lines changed

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,6 +1509,15 @@ func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCusto
15091509
return q.db.DeleteCustomRole(ctx, arg)
15101510
}
15111511

1512+
func (q *querier) DeleteExpiredAPIKeys(ctx context.Context, arg database.DeleteExpiredAPIKeysParams) (int64, error) {
1513+
// Requires DELETE across all API keys.
1514+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceApiKey); err != nil {
1515+
return 0, err
1516+
}
1517+
1518+
return q.db.DeleteExpiredAPIKeys(ctx, arg)
1519+
}
1520+
15121521
func (q *querier) DeleteExternalAuthLink(ctx context.Context, arg database.DeleteExternalAuthLinkParams) error {
15131522
return fetchAndExec(q.log, q.auth, policy.ActionUpdatePersonal, func(ctx context.Context, arg database.DeleteExternalAuthLinkParams) (database.ExternalAuthLink, error) {
15141523
//nolint:gosimple

‎coderd/database/dbauthz/dbauthz_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,14 @@ func (s *MethodTestSuite) TestAPIKey() {
213213
dbm.EXPECT().DeleteAPIKeyByID(gomock.Any(), key.ID).Return(nil).AnyTimes()
214214
check.Args(key.ID).Asserts(key, policy.ActionDelete).Returns()
215215
}))
216+
s.Run("DeleteExpiredAPIKeys", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
217+
args := database.DeleteExpiredAPIKeysParams{
218+
Before: time.Date(2025, 11, 21, 0, 0, 0, 0, time.UTC),
219+
LimitCount: 1000,
220+
}
221+
dbm.EXPECT().DeleteExpiredAPIKeys(gomock.Any(), args).Return(int64(0), nil).AnyTimes()
222+
check.Args(args).Asserts(rbac.ResourceApiKey, policy.ActionDelete).Returns(int64(0))
223+
}))
216224
s.Run("GetAPIKeyByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
217225
key := testutil.Fake(s.T(), faker, database.APIKey{})
218226
dbm.EXPECT().GetAPIKeyByID(gomock.Any(), key.ID).Return(key, nil).AnyTimes()

‎coderd/database/dbgen/dbgen.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,13 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func
173173
}
174174
}
175175

176+
// It does not make sense for the created_at to be after the expires_at.
177+
// So if expires is set, change the default created_at to be 24 hours before.
178+
var createdAt time.Time
179+
if !seed.ExpiresAt.IsZero() && seed.CreatedAt.IsZero() {
180+
createdAt = seed.ExpiresAt.Add(-24 * time.Hour)
181+
}
182+
176183
params := database.InsertAPIKeyParams{
177184
ID: takeFirst(seed.ID, id),
178185
// 0 defaults to 86400 at the db layer
@@ -182,7 +189,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey, munge ...func
182189
UserID: takeFirst(seed.UserID, uuid.New()),
183190
LastUsed: takeFirst(seed.LastUsed, dbtime.Now()),
184191
ExpiresAt: takeFirst(seed.ExpiresAt, dbtime.Now().Add(time.Hour)),
185-
CreatedAt: takeFirst(seed.CreatedAt, dbtime.Now()),
192+
CreatedAt: takeFirst(seed.CreatedAt, createdAt, dbtime.Now()),
186193
UpdatedAt: takeFirst(seed.UpdatedAt, dbtime.Now()),
187194
LoginType: takeFirst(seed.LoginType, database.LoginTypePassword),
188195
Scope: takeFirst(seed.Scope, database.APIKeyScopeAll),

‎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: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz.
7171
if err := tx.ExpirePrebuildsAPIKeys(ctx, dbtime.Time(start)); err != nil {
7272
return xerrors.Errorf("failed to expire prebuilds user api keys: %w", err)
7373
}
74+
expiredAPIKeys, err := tx.DeleteExpiredAPIKeys(ctx, database.DeleteExpiredAPIKeysParams{
75+
// Leave expired keys for a week to allow the backend to know the difference
76+
// between a 404 and an expired key. This purge code is just to bound the size of
77+
// the table to something more reasonable.
78+
Before: dbtime.Time(start.Add(time.Hour * 24 * 7 * -1)),
79+
// There could be a lot of expired keys here, so set a limit to prevent this
80+
// taking too long.
81+
// This runs every 10 minutes, so it deletes ~1.5m keys per day at most.
82+
LimitCount: 10000,
83+
})
84+
if err != nil {
85+
return xerrors.Errorf("failed to delete expired api keys: %w", err)
86+
}
7487

7588
deleteOldAuditLogConnectionEventsBefore := start.Add(-maxAuditLogConnectionEventAge)
7689
if err := tx.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{
@@ -80,7 +93,7 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz.
8093
return xerrors.Errorf("failed to delete old audit log connection events: %w", err)
8194
}
8295

83-
logger.Debug(ctx, "purged old database entries", slog.F("duration", clk.Since(start)))
96+
logger.Debug(ctx, "purged old database entries", slog.F("expired_api_keys", expiredAPIKeys), slog.F("duration", clk.Since(start)))
8497

8598
return nil
8699
}, database.DefaultTXOptions().WithID("db_purge")); err != nil {

‎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: 32 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/apikeys.sql‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,26 @@ DELETE FROM
8484
WHERE
8585
user_id = $1;
8686

87+
-- name: DeleteExpiredAPIKeys :one
88+
WITH expired_keys AS (
89+
SELECT id
90+
FROM api_keys
91+
-- expired keys only
92+
WHERE expires_at < @before::timestamptz
93+
LIMIT @limit_count
94+
),
95+
deleted_rows AS (
96+
DELETE FROM
97+
api_keys
98+
USING
99+
expired_keys
100+
WHERE
101+
api_keys.id = expired_keys.id
102+
RETURNING api_keys.id
103+
)
104+
SELECT COUNT(deleted_rows.id) AS deleted_count FROM deleted_rows;
105+
;
106+
87107
-- name: ExpirePrebuildsAPIKeys :exec
88108
-- Firstly, collect api_keys owned by the prebuilds user that correlate
89109
-- to workspaces no longer owned by the prebuilds user.

0 commit comments

Comments
 (0)