Skip to content

Commit 0165a08

Browse files
mafredrimafredri
committed
feat(coderd/database/dbpurge): make API keys retention configurable
Replace hardcoded 7-day retention for expired API keys with configurable retention from deployment settings. Falls back to global retention when not set, and skips deletion entirely when effective retention is 0. Depends on #21021 Updates #20743
1 parent 9ca58c3 commit 0165a08

File tree

2 files changed

+217
-12
lines changed

2 files changed

+217
-12
lines changed

‎coderd/database/dbpurge/dbpurge.go‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -83,18 +83,27 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, vals *coder
8383
if err := tx.ExpirePrebuildsAPIKeys(ctx, dbtime.Time(start)); err != nil {
8484
return xerrors.Errorf("failed to expire prebuilds user api keys: %w", err)
8585
}
86-
expiredAPIKeys, err := tx.DeleteExpiredAPIKeys(ctx, database.DeleteExpiredAPIKeysParams{
87-
// Leave expired keys for a week to allow the backend to know the difference
88-
// between a 404 and an expired key. This purge code is just to bound the size of
89-
// the table to something more reasonable.
90-
Before: dbtime.Time(start.Add(time.Hour * 24 * 7 * -1)),
91-
// There could be a lot of expired keys here, so set a limit to prevent this
92-
// taking too long.
93-
// This runs every 10 minutes, so it deletes ~1.5m keys per day at most.
94-
LimitCount: 10000,
95-
})
96-
if err != nil {
97-
return xerrors.Errorf("failed to delete expired api keys: %w", err)
86+
87+
var expiredAPIKeys int64
88+
apiKeysRetention := vals.Retention.APIKeys.Value()
89+
if apiKeysRetention == 0 {
90+
apiKeysRetention = vals.Retention.Global.Value()
91+
}
92+
if apiKeysRetention > 0 {
93+
// Delete keys that have been expired for at least the retention period.
94+
// This allows the backend to return a more helpful error when a user
95+
// tries to use an expired key.
96+
deleteExpiredKeysBefore := start.Add(-apiKeysRetention)
97+
expiredAPIKeys, err = tx.DeleteExpiredAPIKeys(ctx, database.DeleteExpiredAPIKeysParams{
98+
Before: dbtime.Time(deleteExpiredKeysBefore),
99+
// There could be a lot of expired keys here, so set a limit to prevent
100+
// this taking too long. This runs every 10 minutes, so it deletes
101+
// ~1.5m keys per day at most.
102+
LimitCount: 10000,
103+
})
104+
if err != nil {
105+
return xerrors.Errorf("failed to delete expired api keys: %w", err)
106+
}
98107
}
99108
deleteOldTelemetryLocksBefore := start.Add(-maxTelemetryHeartbeatAge)
100109
if err := tx.DeleteOldTelemetryLocks(ctx, deleteOldTelemetryLocksBefore); err != nil {

‎coderd/database/dbpurge/dbpurge_test.go‎

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,3 +1267,199 @@ func TestDeleteOldAuditLogs(t *testing.T) {
12671267
require.NotContains(t, logIDs, oldCreateLog.ID, "old create log should be deleted by audit logs retention")
12681268
})
12691269
}
1270+
1271+
func TestDeleteExpiredAPIKeys(t *testing.T) {
1272+
t.Parallel()
1273+
1274+
t.Run("RetentionEnabled", func(t *testing.T) {
1275+
t.Parallel()
1276+
1277+
ctx := testutil.Context(t, testutil.WaitShort)
1278+
1279+
clk := quartz.NewMock(t)
1280+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1281+
retentionPeriod := 7 * 24 * time.Hour // 7 days
1282+
expiredLongAgo := now.Add(-retentionPeriod).Add(-24 * time.Hour) // Expired 8 days ago (should be deleted)
1283+
expiredRecently := now.Add(-retentionPeriod).Add(24 * time.Hour) // Expired 6 days ago (should be kept)
1284+
notExpired := now.Add(24 * time.Hour) // Expires tomorrow (should be kept)
1285+
clk.Set(now).MustWait(ctx)
1286+
1287+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1288+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1289+
user := dbgen.User(t, db, database.User{})
1290+
1291+
// Create API key that expired long ago (should be deleted)
1292+
oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1293+
UserID: user.ID,
1294+
ExpiresAt: expiredLongAgo,
1295+
TokenName: "old-expired-key",
1296+
})
1297+
1298+
// Create API key that expired recently (should be kept)
1299+
recentExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1300+
UserID: user.ID,
1301+
ExpiresAt: expiredRecently,
1302+
TokenName: "recent-expired-key",
1303+
})
1304+
1305+
// Create API key that hasn't expired yet (should be kept)
1306+
activeKey, _ := dbgen.APIKey(t, db, database.APIKey{
1307+
UserID: user.ID,
1308+
ExpiresAt: notExpired,
1309+
TokenName: "active-key",
1310+
})
1311+
1312+
// Run the purge with configured retention period
1313+
done := awaitDoTick(ctx, t, clk)
1314+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1315+
Retention: codersdk.RetentionConfig{
1316+
APIKeys: serpent.Duration(retentionPeriod),
1317+
},
1318+
}, clk)
1319+
defer closer.Close()
1320+
testutil.TryReceive(ctx, t, done)
1321+
1322+
// Verify results
1323+
_, err := db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
1324+
require.Error(t, err, "old expired key should be deleted")
1325+
1326+
_, err = db.GetAPIKeyByID(ctx, recentExpiredKey.ID)
1327+
require.NoError(t, err, "recently expired key should be kept")
1328+
1329+
_, err = db.GetAPIKeyByID(ctx, activeKey.ID)
1330+
require.NoError(t, err, "active key should be kept")
1331+
})
1332+
1333+
t.Run("RetentionDisabled", func(t *testing.T) {
1334+
t.Parallel()
1335+
1336+
ctx := testutil.Context(t, testutil.WaitShort)
1337+
1338+
clk := quartz.NewMock(t)
1339+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1340+
expiredLongAgo := now.Add(-365 * 24 * time.Hour) // Expired 1 year ago
1341+
clk.Set(now).MustWait(ctx)
1342+
1343+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1344+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1345+
user := dbgen.User(t, db, database.User{})
1346+
1347+
// Create API key that expired long ago (should NOT be deleted when retention is 0)
1348+
oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1349+
UserID: user.ID,
1350+
ExpiresAt: expiredLongAgo,
1351+
TokenName: "old-expired-key",
1352+
})
1353+
1354+
// Run the purge with retention disabled (0)
1355+
done := awaitDoTick(ctx, t, clk)
1356+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1357+
Retention: codersdk.RetentionConfig{
1358+
APIKeys: serpent.Duration(0), // disabled
1359+
},
1360+
}, clk)
1361+
defer closer.Close()
1362+
testutil.TryReceive(ctx, t, done)
1363+
1364+
// Verify old expired key is still present
1365+
_, err := db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
1366+
require.NoError(t, err, "old expired key should NOT be deleted when retention is disabled")
1367+
})
1368+
1369+
t.Run("GlobalRetentionFallback", func(t *testing.T) {
1370+
t.Parallel()
1371+
1372+
ctx := testutil.Context(t, testutil.WaitShort)
1373+
1374+
clk := quartz.NewMock(t)
1375+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1376+
retentionPeriod := 14 * 24 * time.Hour // 14 days global
1377+
expiredLongAgo := now.Add(-retentionPeriod).Add(-24 * time.Hour) // Expired 15 days ago (should be deleted)
1378+
expiredRecently := now.Add(-retentionPeriod).Add(24 * time.Hour) // Expired 13 days ago (should be kept)
1379+
clk.Set(now).MustWait(ctx)
1380+
1381+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1382+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1383+
user := dbgen.User(t, db, database.User{})
1384+
1385+
// Create API key that expired long ago (should be deleted)
1386+
oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1387+
UserID: user.ID,
1388+
ExpiresAt: expiredLongAgo,
1389+
TokenName: "old-expired-key",
1390+
})
1391+
1392+
// Create API key that expired recently (should be kept)
1393+
recentExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1394+
UserID: user.ID,
1395+
ExpiresAt: expiredRecently,
1396+
TokenName: "recent-expired-key",
1397+
})
1398+
1399+
// Run the purge with global retention (API keys retention is 0, so it falls back)
1400+
done := awaitDoTick(ctx, t, clk)
1401+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1402+
Retention: codersdk.RetentionConfig{
1403+
Global: serpent.Duration(retentionPeriod), // Use global
1404+
APIKeys: serpent.Duration(0), // Not set, should fall back to global
1405+
},
1406+
}, clk)
1407+
defer closer.Close()
1408+
testutil.TryReceive(ctx, t, done)
1409+
1410+
// Verify results
1411+
_, err := db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
1412+
require.Error(t, err, "old expired key should be deleted via global retention")
1413+
1414+
_, err = db.GetAPIKeyByID(ctx, recentExpiredKey.ID)
1415+
require.NoError(t, err, "recently expired key should be kept")
1416+
})
1417+
1418+
t.Run("CustomRetention30Days", func(t *testing.T) {
1419+
t.Parallel()
1420+
1421+
ctx := testutil.Context(t, testutil.WaitShort)
1422+
1423+
clk := quartz.NewMock(t)
1424+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1425+
retentionPeriod := 30 * 24 * time.Hour // 30 days
1426+
expiredLongAgo := now.Add(-retentionPeriod).Add(-24 * time.Hour) // Expired 31 days ago (should be deleted)
1427+
expiredRecently := now.Add(-retentionPeriod).Add(24 * time.Hour) // Expired 29 days ago (should be kept)
1428+
clk.Set(now).MustWait(ctx)
1429+
1430+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1431+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1432+
user := dbgen.User(t, db, database.User{})
1433+
1434+
// Create API key that expired long ago (should be deleted)
1435+
oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1436+
UserID: user.ID,
1437+
ExpiresAt: expiredLongAgo,
1438+
TokenName: "old-expired-key",
1439+
})
1440+
1441+
// Create API key that expired recently (should be kept)
1442+
recentExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1443+
UserID: user.ID,
1444+
ExpiresAt: expiredRecently,
1445+
TokenName: "recent-expired-key",
1446+
})
1447+
1448+
// Run the purge with 30-day retention
1449+
done := awaitDoTick(ctx, t, clk)
1450+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1451+
Retention: codersdk.RetentionConfig{
1452+
APIKeys: serpent.Duration(retentionPeriod),
1453+
},
1454+
}, clk)
1455+
defer closer.Close()
1456+
testutil.TryReceive(ctx, t, done)
1457+
1458+
// Verify results
1459+
_, err := db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
1460+
require.Error(t, err, "old expired key should be deleted with 30-day retention")
1461+
1462+
_, err = db.GetAPIKeyByID(ctx, recentExpiredKey.ID)
1463+
require.NoError(t, err, "recently expired key should be kept with 30-day retention")
1464+
})
1465+
}

0 commit comments

Comments
 (0)