Skip to content

Commit bd8e537

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 3f24dd9 commit bd8e537

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
@@ -1406,3 +1406,199 @@ func TestDeleteOldAuditLogs(t *testing.T) {
14061406
require.NotContains(t, logIDs, oldCreateLog.ID, "old create log should be deleted by audit logs retention")
14071407
})
14081408
}
1409+
1410+
func TestDeleteExpiredAPIKeys(t *testing.T) {
1411+
t.Parallel()
1412+
1413+
t.Run("RetentionEnabled", func(t *testing.T) {
1414+
t.Parallel()
1415+
1416+
ctx := testutil.Context(t, testutil.WaitShort)
1417+
1418+
clk := quartz.NewMock(t)
1419+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1420+
retentionPeriod := 7 * 24 * time.Hour // 7 days
1421+
expiredLongAgo := now.Add(-retentionPeriod).Add(-24 * time.Hour) // Expired 8 days ago (should be deleted)
1422+
expiredRecently := now.Add(-retentionPeriod).Add(24 * time.Hour) // Expired 6 days ago (should be kept)
1423+
notExpired := now.Add(24 * time.Hour) // Expires tomorrow (should be kept)
1424+
clk.Set(now).MustWait(ctx)
1425+
1426+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1427+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1428+
user := dbgen.User(t, db, database.User{})
1429+
1430+
// Create API key that expired long ago (should be deleted)
1431+
oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1432+
UserID: user.ID,
1433+
ExpiresAt: expiredLongAgo,
1434+
TokenName: "old-expired-key",
1435+
})
1436+
1437+
// Create API key that expired recently (should be kept)
1438+
recentExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1439+
UserID: user.ID,
1440+
ExpiresAt: expiredRecently,
1441+
TokenName: "recent-expired-key",
1442+
})
1443+
1444+
// Create API key that hasn't expired yet (should be kept)
1445+
activeKey, _ := dbgen.APIKey(t, db, database.APIKey{
1446+
UserID: user.ID,
1447+
ExpiresAt: notExpired,
1448+
TokenName: "active-key",
1449+
})
1450+
1451+
// Run the purge with configured retention period
1452+
done := awaitDoTick(ctx, t, clk)
1453+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1454+
Retention: codersdk.RetentionConfig{
1455+
APIKeys: serpent.Duration(retentionPeriod),
1456+
},
1457+
}, clk)
1458+
defer closer.Close()
1459+
testutil.TryReceive(ctx, t, done)
1460+
1461+
// Verify results
1462+
_, err := db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
1463+
require.Error(t, err, "old expired key should be deleted")
1464+
1465+
_, err = db.GetAPIKeyByID(ctx, recentExpiredKey.ID)
1466+
require.NoError(t, err, "recently expired key should be kept")
1467+
1468+
_, err = db.GetAPIKeyByID(ctx, activeKey.ID)
1469+
require.NoError(t, err, "active key should be kept")
1470+
})
1471+
1472+
t.Run("RetentionDisabled", func(t *testing.T) {
1473+
t.Parallel()
1474+
1475+
ctx := testutil.Context(t, testutil.WaitShort)
1476+
1477+
clk := quartz.NewMock(t)
1478+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1479+
expiredLongAgo := now.Add(-365 * 24 * time.Hour) // Expired 1 year ago
1480+
clk.Set(now).MustWait(ctx)
1481+
1482+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1483+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1484+
user := dbgen.User(t, db, database.User{})
1485+
1486+
// Create API key that expired long ago (should NOT be deleted when retention is 0)
1487+
oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1488+
UserID: user.ID,
1489+
ExpiresAt: expiredLongAgo,
1490+
TokenName: "old-expired-key",
1491+
})
1492+
1493+
// Run the purge with retention disabled (0)
1494+
done := awaitDoTick(ctx, t, clk)
1495+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1496+
Retention: codersdk.RetentionConfig{
1497+
APIKeys: serpent.Duration(0), // disabled
1498+
},
1499+
}, clk)
1500+
defer closer.Close()
1501+
testutil.TryReceive(ctx, t, done)
1502+
1503+
// Verify old expired key is still present
1504+
_, err := db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
1505+
require.NoError(t, err, "old expired key should NOT be deleted when retention is disabled")
1506+
})
1507+
1508+
t.Run("GlobalRetentionFallback", func(t *testing.T) {
1509+
t.Parallel()
1510+
1511+
ctx := testutil.Context(t, testutil.WaitShort)
1512+
1513+
clk := quartz.NewMock(t)
1514+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1515+
retentionPeriod := 14 * 24 * time.Hour // 14 days global
1516+
expiredLongAgo := now.Add(-retentionPeriod).Add(-24 * time.Hour) // Expired 15 days ago (should be deleted)
1517+
expiredRecently := now.Add(-retentionPeriod).Add(24 * time.Hour) // Expired 13 days ago (should be kept)
1518+
clk.Set(now).MustWait(ctx)
1519+
1520+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1521+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1522+
user := dbgen.User(t, db, database.User{})
1523+
1524+
// Create API key that expired long ago (should be deleted)
1525+
oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1526+
UserID: user.ID,
1527+
ExpiresAt: expiredLongAgo,
1528+
TokenName: "old-expired-key",
1529+
})
1530+
1531+
// Create API key that expired recently (should be kept)
1532+
recentExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1533+
UserID: user.ID,
1534+
ExpiresAt: expiredRecently,
1535+
TokenName: "recent-expired-key",
1536+
})
1537+
1538+
// Run the purge with global retention (API keys retention is 0, so it falls back)
1539+
done := awaitDoTick(ctx, t, clk)
1540+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1541+
Retention: codersdk.RetentionConfig{
1542+
Global: serpent.Duration(retentionPeriod), // Use global
1543+
APIKeys: serpent.Duration(0), // Not set, should fall back to global
1544+
},
1545+
}, clk)
1546+
defer closer.Close()
1547+
testutil.TryReceive(ctx, t, done)
1548+
1549+
// Verify results
1550+
_, err := db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
1551+
require.Error(t, err, "old expired key should be deleted via global retention")
1552+
1553+
_, err = db.GetAPIKeyByID(ctx, recentExpiredKey.ID)
1554+
require.NoError(t, err, "recently expired key should be kept")
1555+
})
1556+
1557+
t.Run("CustomRetention30Days", func(t *testing.T) {
1558+
t.Parallel()
1559+
1560+
ctx := testutil.Context(t, testutil.WaitShort)
1561+
1562+
clk := quartz.NewMock(t)
1563+
now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
1564+
retentionPeriod := 30 * 24 * time.Hour // 30 days
1565+
expiredLongAgo := now.Add(-retentionPeriod).Add(-24 * time.Hour) // Expired 31 days ago (should be deleted)
1566+
expiredRecently := now.Add(-retentionPeriod).Add(24 * time.Hour) // Expired 29 days ago (should be kept)
1567+
clk.Set(now).MustWait(ctx)
1568+
1569+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
1570+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
1571+
user := dbgen.User(t, db, database.User{})
1572+
1573+
// Create API key that expired long ago (should be deleted)
1574+
oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1575+
UserID: user.ID,
1576+
ExpiresAt: expiredLongAgo,
1577+
TokenName: "old-expired-key",
1578+
})
1579+
1580+
// Create API key that expired recently (should be kept)
1581+
recentExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
1582+
UserID: user.ID,
1583+
ExpiresAt: expiredRecently,
1584+
TokenName: "recent-expired-key",
1585+
})
1586+
1587+
// Run the purge with 30-day retention
1588+
done := awaitDoTick(ctx, t, clk)
1589+
closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
1590+
Retention: codersdk.RetentionConfig{
1591+
APIKeys: serpent.Duration(retentionPeriod),
1592+
},
1593+
}, clk)
1594+
defer closer.Close()
1595+
testutil.TryReceive(ctx, t, done)
1596+
1597+
// Verify results
1598+
_, err := db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
1599+
require.Error(t, err, "old expired key should be deleted with 30-day retention")
1600+
1601+
_, err = db.GetAPIKeyByID(ctx, recentExpiredKey.ID)
1602+
require.NoError(t, err, "recently expired key should be kept with 30-day retention")
1603+
})
1604+
}

0 commit comments

Comments
 (0)