From 346d2f213efc935c8a6335743bd14afb29712c36 Mon Sep 17 00:00:00 2001 From: x Date: Thu, 1 May 2025 17:33:24 -0400 Subject: [PATCH] improvements + review fixes --- backend/e2e-test/mocks/keystore.ts | 32 +++++++++++++------ backend/src/keystore/keystore.ts | 19 ++++++++--- backend/src/keystore/memory.ts | 27 +++++++++++----- backend/src/lib/delay/index.ts | 4 +++ backend/src/server/routes/v1/admin-router.ts | 10 ++++-- .../super-admin/super-admin-service.ts | 7 +++- .../src/services/telemetry/telemetry-types.ts | 1 + .../OverviewPage/components/CachingPanel.tsx | 17 ++++++---- 8 files changed, 84 insertions(+), 33 deletions(-) create mode 100644 backend/src/lib/delay/index.ts diff --git a/backend/e2e-test/mocks/keystore.ts b/backend/e2e-test/mocks/keystore.ts index ddf57bfd5..577e3c871 100644 --- a/backend/e2e-test/mocks/keystore.ts +++ b/backend/e2e-test/mocks/keystore.ts @@ -1,7 +1,9 @@ -import { TKeyStoreFactory } from "@app/keystore/keystore"; -import { Lock } from "@app/lib/red-lock"; import RE2 from "re2"; +import { TKeyStoreFactory } from "@app/keystore/keystore"; +import { delay as delayMs } from "@app/lib/delay"; +import { Lock } from "@app/lib/red-lock"; + export const mockKeyStore = (): TKeyStoreFactory => { const store: Record = {}; @@ -19,16 +21,26 @@ export const mockKeyStore = (): TKeyStoreFactory => { delete store[key]; return 1; }, - deleteItems: async (pattern) => { - const regex = new RE2(pattern.replace(/\*/g, ".*")); - let deletedCount = 0; - for (const key of Object.keys(store)) { - if (regex.test(key)) { - delete store[key]; - deletedCount += 1; + deleteItems: async ({ pattern, batchSize = 500, delay = 1500, jitter = 200 }) => { + const regex = new RE2(`^${pattern.replace(/[-[\]/{}()+?.\\^$|]/g, "\\$&").replace(/\*/g, ".*")}$`); + let totalDeleted = 0; + const keys = Object.keys(store); + + for (let i = 0; i < keys.length; i += batchSize) { + const batch = keys.slice(i, i + batchSize); + + for (const key of batch) { + if (regex.test(key)) { + delete store[key]; + totalDeleted += 1; + } } + + // eslint-disable-next-line no-await-in-loop + await delayMs(Math.max(0, delay + Math.floor((Math.random() * 2 - 1) * jitter))); } - return deletedCount; + + return totalDeleted; }, getItem: async (key) => { const value = store[key]; diff --git a/backend/src/keystore/keystore.ts b/backend/src/keystore/keystore.ts index 9371cebb8..0e1c3e35e 100644 --- a/backend/src/keystore/keystore.ts +++ b/backend/src/keystore/keystore.ts @@ -1,6 +1,7 @@ import { Redis } from "ioredis"; import { pgAdvisoryLockHashText } from "@app/lib/crypto/hashtext"; +import { delay as delayMs } from "@app/lib/delay"; import { Redlock, Settings } from "@app/lib/red-lock"; export const PgSqlLock = { @@ -48,6 +49,13 @@ export const KeyStoreTtls = { AccessTokenStatusUpdateInSeconds: 120 }; +type TDeleteItems = { + pattern: string; + batchSize?: number; + delay?: number; + jitter?: number; +}; + type TWaitTillReady = { key: string; waitingCb?: () => void; @@ -57,8 +65,6 @@ type TWaitTillReady = { jitter?: number; }; -const DELETION_BATCH_SIZE = 500; - export const keyStoreFactory = (redisUrl: string) => { const redis = new Redis(redisUrl); const redisLock = new Redlock([redis], { retryCount: 2, retryDelay: 200 }); @@ -77,7 +83,7 @@ export const keyStoreFactory = (redisUrl: string) => { const deleteItem = async (key: string) => redis.del(key); - const deleteItems = async (pattern: string) => { + const deleteItems = async ({ pattern, batchSize = 500, delay = 1500, jitter = 200 }: TDeleteItems) => { let cursor = "0"; let totalDeleted = 0; @@ -87,8 +93,8 @@ export const keyStoreFactory = (redisUrl: string) => { const [nextCursor, keys] = await redis.scan(cursor, "MATCH", pattern, "COUNT", 1000); // Count should be 1000 - 5000 for prod loads cursor = nextCursor; - for (let i = 0; i < keys.length; i += DELETION_BATCH_SIZE) { - const batch = keys.slice(i, i + DELETION_BATCH_SIZE); + for (let i = 0; i < keys.length; i += batchSize) { + const batch = keys.slice(i, i + batchSize); const pipeline = redis.pipeline(); for (const key of batch) { pipeline.unlink(key); @@ -96,6 +102,9 @@ export const keyStoreFactory = (redisUrl: string) => { // eslint-disable-next-line no-await-in-loop await pipeline.exec(); totalDeleted += batch.length; + + // eslint-disable-next-line no-await-in-loop + await delayMs(Math.max(0, delay + Math.floor((Math.random() * 2 - 1) * jitter))); } } while (cursor !== "0"); diff --git a/backend/src/keystore/memory.ts b/backend/src/keystore/memory.ts index 55699207f..eab3a32dd 100644 --- a/backend/src/keystore/memory.ts +++ b/backend/src/keystore/memory.ts @@ -1,5 +1,6 @@ import RE2 from "re2"; +import { delay as delayMs } from "@app/lib/delay"; import { Lock } from "@app/lib/red-lock"; import { TKeyStoreFactory } from "./keystore"; @@ -21,16 +22,26 @@ export const inMemoryKeyStore = (): TKeyStoreFactory => { delete store[key]; return 1; }, - deleteItems: async (pattern) => { - const regex = new RE2(pattern.replace(/\*/g, ".*")); - let deletedCount = 0; - for (const key of Object.keys(store)) { - if (regex.test(key)) { - delete store[key]; - deletedCount += 1; + deleteItems: async ({ pattern, batchSize = 500, delay = 1500, jitter = 200 }) => { + const regex = new RE2(`^${pattern.replace(/[-[\]/{}()+?.\\^$|]/g, "\\$&").replace(/\*/g, ".*")}$`); + let totalDeleted = 0; + const keys = Object.keys(store); + + for (let i = 0; i < keys.length; i += batchSize) { + const batch = keys.slice(i, i + batchSize); + + for (const key of batch) { + if (regex.test(key)) { + delete store[key]; + totalDeleted += 1; + } } + + // eslint-disable-next-line no-await-in-loop + await delayMs(Math.max(0, delay + Math.floor((Math.random() * 2 - 1) * jitter))); } - return deletedCount; + + return totalDeleted; }, getItem: async (key) => { const value = store[key]; diff --git a/backend/src/lib/delay/index.ts b/backend/src/lib/delay/index.ts new file mode 100644 index 000000000..32cb8ebfc --- /dev/null +++ b/backend/src/lib/delay/index.ts @@ -0,0 +1,4 @@ +export const delay = (ms: number) => + new Promise((resolve) => { + setTimeout(resolve, ms); + }); diff --git a/backend/src/server/routes/v1/admin-router.ts b/backend/src/server/routes/v1/admin-router.ts index c1dbe3805..b436e2e18 100644 --- a/backend/src/server/routes/v1/admin-router.ts +++ b/backend/src/server/routes/v1/admin-router.ts @@ -553,19 +553,25 @@ export const registerAdminRouter = async (server: FastifyZodProvider) => { }) } }, + onRequest: (req, res, done) => { + verifyAuth([AuthMode.JWT, AuthMode.IDENTITY_ACCESS_TOKEN])(req, res, () => { + verifySuperAdmin(req, res, done); + }); + }, handler: async (req) => { - await server.services.superAdmin.invalidateCache(req.body.type); + const keysCleared = await server.services.superAdmin.invalidateCache(req.body.type); await server.services.telemetry.sendPostHogEvents({ event: PostHogEventTypes.InvalidateCache, distinctId: getTelemetryDistinctId(req), properties: { + keysCleared, ...req.auditLogInfo } }); return { - message: "Successfully purged cache" + message: `Successfully invalidated ${keysCleared} cached items` }; } }); diff --git a/backend/src/services/super-admin/super-admin-service.ts b/backend/src/services/super-admin/super-admin-service.ts index f993d2398..e1136cf0e 100644 --- a/backend/src/services/super-admin/super-admin-service.ts +++ b/backend/src/services/super-admin/super-admin-service.ts @@ -572,7 +572,12 @@ export const superAdminServiceFactory = ({ }; const invalidateCache = async (type: CacheType) => { - if (type === CacheType.ALL || type === CacheType.SECRETS) await keyStore.deleteItems("secret-manager:*"); + let totalKeysCleared = 0; + + if (type === CacheType.ALL || type === CacheType.SECRETS) + totalKeysCleared += await keyStore.deleteItems({ pattern: "secret-manager:*" }); + + return totalKeysCleared; }; return { diff --git a/backend/src/services/telemetry/telemetry-types.ts b/backend/src/services/telemetry/telemetry-types.ts index 9e046cdbd..74dd7448c 100644 --- a/backend/src/services/telemetry/telemetry-types.ts +++ b/backend/src/services/telemetry/telemetry-types.ts @@ -207,6 +207,7 @@ export type TIssueCertificateEvent = { export type TInvalidateCacheEvent = { event: PostHogEventTypes.InvalidateCache; properties: { + keysCleared: number; userAgent?: string; }; }; diff --git a/frontend/src/pages/admin/OverviewPage/components/CachingPanel.tsx b/frontend/src/pages/admin/OverviewPage/components/CachingPanel.tsx index a30042999..338faaa4e 100644 --- a/frontend/src/pages/admin/OverviewPage/components/CachingPanel.tsx +++ b/frontend/src/pages/admin/OverviewPage/components/CachingPanel.tsx @@ -11,7 +11,7 @@ export const CachingPanel = () => { const { mutateAsync: invalidateCache } = useInvalidateCache(); const { membership } = useOrgPermission(); - const [type, setType] = useState(CacheType.ALL); + const [type, setType] = useState(null); const [isLoading, setIsLoading] = useState(false); const { popUp, handlePopUpOpen, handlePopUpClose, handlePopUpToggle } = usePopUp([ @@ -19,25 +19,28 @@ export const CachingPanel = () => { ] as const); const handleInvalidateCacheSubmit = async () => { - try { - setIsLoading(true); + if (!type) return; + setIsLoading(true); + try { await invalidateCache({ type }); createNotification({ - text: `Successfully purged ${type} cache`, + text: `Successfully invalidated ${type} cache`, type: "success" }); - setIsLoading(false); + setType(null); handlePopUpClose("invalidateCache"); } catch (err) { console.error(err); createNotification({ - text: `Failed to purge ${type} cache`, + text: `Failed to invalidate ${type} cache`, type: "error" }); } + + setIsLoading(false); }; return ( @@ -90,7 +93,7 @@ export const CachingPanel = () => { handlePopUpToggle("invalidateCache", isOpen)} deleteKey="confirm" onDeleteApproved={handleInvalidateCacheSubmit}