feat(coderd/database/dbpurge): make API keys retention configurable (#21037)

Replace hardcoded 7-day retention for expired API keys with configurable
retention from deployment settings. Skips deletion entirely when effective
retention is 0.

Depends on #21021
Updates #20743

coderd/database/dbpurge/dbpurge.go

@@ -82,18 +82,24 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, vals *coder
 			if err := tx.ExpirePrebuildsAPIKeys(ctx, dbtime.Time(start)); err != nil {
 				return xerrors.Errorf("failed to expire prebuilds user api keys: %w", err)
 			}
-			expiredAPIKeys, err := tx.DeleteExpiredAPIKeys(ctx, database.DeleteExpiredAPIKeysParams{
-				// Leave expired keys for a week to allow the backend to know the difference
-				// between a 404 and an expired key. This purge code is just to bound the size of
-				// the table to something more reasonable.
-				Before: dbtime.Time(start.Add(time.Hour * 24 * 7 * -1)),
-				// There could be a lot of expired keys here, so set a limit to prevent this
-				// taking too long.
-				// This runs every 10 minutes, so it deletes ~1.5m keys per day at most.
-				LimitCount: 10000,
-			})
-			if err != nil {
-				return xerrors.Errorf("failed to delete expired api keys: %w", err)
+
+			var expiredAPIKeys int64
+			apiKeysRetention := vals.Retention.APIKeys.Value()
+			if apiKeysRetention > 0 {
+				// Delete keys that have been expired for at least the retention period.
+				// A higher retention period allows the backend to return a more helpful
+				// error message when a user tries to use an expired key.
+				deleteExpiredKeysBefore := start.Add(-apiKeysRetention)
+				expiredAPIKeys, err = tx.DeleteExpiredAPIKeys(ctx, database.DeleteExpiredAPIKeysParams{
+					Before: dbtime.Time(deleteExpiredKeysBefore),
+					// There could be a lot of expired keys here, so set a limit to prevent
+					// this taking too long. This runs every 10 minutes, so it deletes
+					// ~1.5m keys per day at most.
+					LimitCount: 10000,
+				})
+				if err != nil {
+					return xerrors.Errorf("failed to delete expired api keys: %w", err)
+				}
 			}
 			deleteOldTelemetryLocksBefore := start.Add(-maxTelemetryHeartbeatAge)
 			if err := tx.DeleteOldTelemetryLocks(ctx, deleteOldTelemetryLocksBefore); err != nil {

coderd/database/dbpurge/dbpurge_test.go

@@ -1245,3 +1245,131 @@ func TestDeleteOldAuditLogs(t *testing.T) {
 		require.NotContains(t, logIDs, oldCreateLog.ID, "old create log should be deleted by audit logs retention")
 	})
 }
+
+func TestDeleteExpiredAPIKeys(t *testing.T) {
+	t.Parallel()
+
+	now := time.Date(2025, 1, 15, 7, 30, 0, 0, time.UTC)
+
+	testCases := []struct {
+		name                    string
+		retentionConfig         codersdk.RetentionConfig
+		oldExpiredTime          time.Time
+		recentExpiredTime       *time.Time // nil means no recent expired key created
+		activeTime              *time.Time // nil means no active key created
+		expectOldExpiredDeleted bool
+		expectedKeysRemaining   int
+	}{
+		{
+			name: "RetentionEnabled",
+			retentionConfig: codersdk.RetentionConfig{
+				APIKeys: serpent.Duration(7 * 24 * time.Hour), // 7 days
+			},
+			oldExpiredTime:          now.Add(-8 * 24 * time.Hour),      // Expired 8 days ago
+			recentExpiredTime:       ptr(now.Add(-6 * 24 * time.Hour)), // Expired 6 days ago
+			activeTime:              ptr(now.Add(24 * time.Hour)),      // Expires tomorrow
+			expectOldExpiredDeleted: true,
+			expectedKeysRemaining:   2, // recent expired + active
+		},
+		{
+			name: "RetentionDisabled",
+			retentionConfig: codersdk.RetentionConfig{
+				APIKeys: serpent.Duration(0),
+			},
+			oldExpiredTime:          now.Add(-365 * 24 * time.Hour), // Expired 1 year ago
+			recentExpiredTime:       nil,
+			activeTime:              nil,
+			expectOldExpiredDeleted: false,
+			expectedKeysRemaining:   1, // old expired is kept
+		},
+
+		{
+			name: "CustomRetention30Days",
+			retentionConfig: codersdk.RetentionConfig{
+				APIKeys: serpent.Duration(30 * 24 * time.Hour), // 30 days
+			},
+			oldExpiredTime:          now.Add(-31 * 24 * time.Hour),      // Expired 31 days ago
+			recentExpiredTime:       ptr(now.Add(-29 * 24 * time.Hour)), // Expired 29 days ago
+			activeTime:              nil,
+			expectOldExpiredDeleted: true,
+			expectedKeysRemaining:   1, // only recent expired remains
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx := testutil.Context(t, testutil.WaitShort)
+			clk := quartz.NewMock(t)
+			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{})
+
+			// Create API key that expired long ago.
+			oldExpiredKey, _ := dbgen.APIKey(t, db, database.APIKey{
+				UserID:    user.ID,
+				ExpiresAt: tc.oldExpiredTime,
+				TokenName: "old-expired-key",
+			})
+
+			// Create API key that expired recently if specified.
+			var recentExpiredKey database.APIKey
+			if tc.recentExpiredTime != nil {
+				recentExpiredKey, _ = dbgen.APIKey(t, db, database.APIKey{
+					UserID:    user.ID,
+					ExpiresAt: *tc.recentExpiredTime,
+					TokenName: "recent-expired-key",
+				})
+			}
+
+			// Create API key that hasn't expired yet if specified.
+			var activeKey database.APIKey
+			if tc.activeTime != nil {
+				activeKey, _ = dbgen.APIKey(t, db, database.APIKey{
+					UserID:    user.ID,
+					ExpiresAt: *tc.activeTime,
+					TokenName: "active-key",
+				})
+			}
+
+			// Run the purge.
+			done := awaitDoTick(ctx, t, clk)
+			closer := dbpurge.New(ctx, logger, db, &codersdk.DeploymentValues{
+				Retention: tc.retentionConfig,
+			}, clk)
+			defer closer.Close()
+			testutil.TryReceive(ctx, t, done)
+
+			// Verify total keys remaining.
+			keys, err := db.GetAPIKeysLastUsedAfter(ctx, time.Time{})
+			require.NoError(t, err)
+			require.Len(t, keys, tc.expectedKeysRemaining, "unexpected number of keys remaining")
+
+			// Verify results.
+			_, err = db.GetAPIKeyByID(ctx, oldExpiredKey.ID)
+			if tc.expectOldExpiredDeleted {
+				require.Error(t, err, "old expired key should be deleted")
+			} else {
+				require.NoError(t, err, "old expired key should NOT be deleted")
+			}
+
+			if tc.recentExpiredTime != nil {
+				_, err = db.GetAPIKeyByID(ctx, recentExpiredKey.ID)
+				require.NoError(t, err, "recently expired key should be kept")
+			}
+
+			if tc.activeTime != nil {
+				_, err = db.GetAPIKeyByID(ctx, activeKey.ID)
+				require.NoError(t, err, "active key should be kept")
+			}
+		})
+	}
+}
+
+// ptr is a helper to create a pointer to a value.
+func ptr[T any](v T) *T {
+	return &v
+}

coderd/database/queries.sql.go

@@ -1107,24 +1107,20 @@ func (q *sqlQuerier) DeleteApplicationConnectAPIKeysByUserID(ctx context.Context
 	return err
 }
 
-const deleteExpiredAPIKeys = `-- name: DeleteExpiredAPIKeys :one
+const deleteExpiredAPIKeys = `-- name: DeleteExpiredAPIKeys :execrows
 WITH expired_keys AS (
 	SELECT id
 	FROM api_keys
 	-- expired keys only
 	WHERE expires_at < $1::timestamptz
 	LIMIT $2
-),
-deleted_rows AS (
-	 DELETE FROM
-		 api_keys
-	 USING
-		 expired_keys
-	 WHERE
-		 api_keys.id = expired_keys.id
-	 RETURNING api_keys.id
- )
-SELECT COUNT(deleted_rows.id) AS deleted_count FROM deleted_rows
+)
+DELETE FROM
+	api_keys
+USING
+	expired_keys
+WHERE
+	api_keys.id = expired_keys.id
 `
 
 type DeleteExpiredAPIKeysParams struct {
@@ -1133,10 +1129,11 @@ type DeleteExpiredAPIKeysParams struct {
 }
 
 func (q *sqlQuerier) DeleteExpiredAPIKeys(ctx context.Context, arg DeleteExpiredAPIKeysParams) (int64, error) {
-	row := q.db.QueryRowContext(ctx, deleteExpiredAPIKeys, arg.Before, arg.LimitCount)
-	var deleted_count int64
-	err := row.Scan(&deleted_count)
-	return deleted_count, err
+	result, err := q.db.ExecContext(ctx, deleteExpiredAPIKeys, arg.Before, arg.LimitCount)
+	if err != nil {
+		return 0, err
+	}
+	return result.RowsAffected()
 }
 
 const expirePrebuildsAPIKeys = `-- name: ExpirePrebuildsAPIKeys :exec

coderd/database/queries/apikeys.sql

@@ -85,25 +85,20 @@ DELETE FROM
 WHERE
 	user_id = $1;
 
--- name: DeleteExpiredAPIKeys :one
+-- name: DeleteExpiredAPIKeys :execrows
 WITH expired_keys AS (
 	SELECT id
 	FROM api_keys
 	-- expired keys only
 	WHERE expires_at < @before::timestamptz
 	LIMIT @limit_count
-),
-deleted_rows AS (
-	 DELETE FROM
-		 api_keys
-	 USING
-		 expired_keys
-	 WHERE
-		 api_keys.id = expired_keys.id
-	 RETURNING api_keys.id
- )
-SELECT COUNT(deleted_rows.id) AS deleted_count FROM deleted_rows;
-;
+)
+DELETE FROM
+	api_keys
+USING
+	expired_keys
+WHERE
+	api_keys.id = expired_keys.id;
 
 -- name: ExpirePrebuildsAPIKeys :exec
 -- Firstly, collect api_keys owned by the prebuilds user that correlate