diff --git a/src/common/k8s.test.ts b/src/common/k8s.test.ts index f2506be0ac..3476ef3b95 100644 --- a/src/common/k8s.test.ts +++ b/src/common/k8s.test.ts @@ -1037,3 +1037,129 @@ describe('getSealedSecretsPEM', () => { expect(MockX509Certificate).toHaveBeenCalledWith('single-cert') }) }) + +describe('createArgoCdRedisSecret', () => { + const password = 'new-password' + const objectApi = { patch: jest.fn() } + + beforeEach(() => { + jest.clearAllMocks() + objectApi.patch.mockResolvedValue({}) + jest.spyOn(k8s.k8s, 'object').mockReturnValue(objectApi as any) + }) + + afterEach(() => jest.restoreAllMocks()) + + const makeDeps = (overrides = {}) => ({ + getK8sSecret: jest.fn(async () => ({ auth: password })), + restartArgoCdRedisConsumers: jest.fn(async (_namespace: string) => {}), + ...overrides, + }) + + it('should restart redis and its consumers when the password changed', async () => { + const deps = makeDeps({ getK8sSecret: jest.fn(async () => ({ auth: 'old-password' })) }) + + await k8s.createArgoCdRedisSecret({ apps: { argocd: { redisPassword: password } } }, deps as any) + + expect(deps.restartArgoCdRedisConsumers).toHaveBeenCalledWith('argocd') + }) + + it('should not restart anything when the password is unchanged', async () => { + const deps = makeDeps() + + await k8s.createArgoCdRedisSecret({ apps: { argocd: { redisPassword: password } } }, deps as any) + + expect(objectApi.patch).toHaveBeenCalled() + expect(deps.restartArgoCdRedisConsumers).not.toHaveBeenCalled() + }) + + it('should not restart anything on a fresh install where the secret does not exist yet', async () => { + const deps = makeDeps({ getK8sSecret: jest.fn(async () => undefined) }) + + await k8s.createArgoCdRedisSecret({ apps: { argocd: { redisPassword: password } } }, deps as any) + + expect(deps.restartArgoCdRedisConsumers).not.toHaveBeenCalled() + }) + + it('should restart when the existing secret cannot be read at all', async () => { + const deps = makeDeps({ + getK8sSecret: jest.fn(async () => { + throw new Error('forbidden') + }), + }) + + await k8s.createArgoCdRedisSecret({ apps: { argocd: { redisPassword: password } } }, deps as any) + + expect(objectApi.patch).toHaveBeenCalled() + expect(deps.restartArgoCdRedisConsumers).toHaveBeenCalledWith('argocd') + }) + + it('should skip reconciliation entirely when no password is supplied', async () => { + const deps = makeDeps() + + await k8s.createArgoCdRedisSecret({ apps: { argocd: {} } }, deps as any) + + expect(objectApi.patch).not.toHaveBeenCalled() + expect(deps.restartArgoCdRedisConsumers).not.toHaveBeenCalled() + }) +}) + +describe('restartArgoCdRedisConsumers', () => { + const makeDeps = (overrides = {}) => ({ + restartDeployment: jest.fn(async (_name: string, _namespace: string) => {}), + restartStatefulSet: jest.fn(async (_name: string, _namespace: string) => {}), + ...overrides, + }) + + it('should restart redis before its clients', async () => { + const deps = makeDeps() + + await k8s.restartArgoCdRedisConsumers('argocd', deps as any) + + expect(deps.restartDeployment).toHaveBeenCalledWith('argocd-redis', 'argocd') + expect(deps.restartDeployment.mock.calls[0][0]).toBe('argocd-redis') + expect(deps.restartDeployment).toHaveBeenCalledWith('argocd-repo-server', 'argocd') + expect(deps.restartStatefulSet).toHaveBeenCalledWith('argocd-application-controller', 'argocd') + }) + + it('should keep going when a target does not exist', async () => { + const notFound = new MockApiException(404, 'not found', {}, {}) + const deps = makeDeps({ + restartDeployment: jest.fn(async (name: string) => { + if (name === 'argocd-server') throw notFound + }), + }) + + await expect(k8s.restartArgoCdRedisConsumers('argocd', deps as any)).resolves.toBeUndefined() + + expect(deps.restartDeployment).toHaveBeenCalledWith('argocd-repo-server', 'argocd') + }) + + it('should not restart the clients when redis itself fails to restart', async () => { + const deps = makeDeps({ + restartDeployment: jest.fn(async (name: string) => { + if (name === 'argocd-redis') throw new Error('boom') + }), + }) + + await expect(k8s.restartArgoCdRedisConsumers('argocd', deps as any)).rejects.toThrow('boom') + + expect(deps.restartDeployment).toHaveBeenCalledTimes(1) + expect(deps.restartDeployment).toHaveBeenCalledWith('argocd-redis', 'argocd') + expect(deps.restartStatefulSet).not.toHaveBeenCalled() + }) + + it('should leave the clients alone when redis is not present at all', async () => { + const notFound = new MockApiException(404, 'not found', {}, {}) + const deps = makeDeps({ + restartDeployment: jest.fn(async (name: string) => { + if (name === 'argocd-redis') throw notFound + }), + }) + + await expect(k8s.restartArgoCdRedisConsumers('argocd', deps as any)).resolves.toBeUndefined() + + expect(deps.restartDeployment).toHaveBeenCalledTimes(1) + expect(deps.restartStatefulSet).not.toHaveBeenCalled() + }) +}) diff --git a/src/common/k8s.ts b/src/common/k8s.ts index 94dbc05224..174e1246ac 100644 --- a/src/common/k8s.ts +++ b/src/common/k8s.ts @@ -819,7 +819,63 @@ export async function setArgoCdAppSync( ) } -export const createArgoCdRedisSecret = async (values: Record): Promise => { +// argocd-redis runs `redis-server --requirepass $(REDIS_PASSWORD)`, so the password is baked into +// the process at pod start and never re-read. Its clients (repo-server, application-controller, +// server) read the same Secret key into an env var, also at their own start. Rewriting the Secret +// under a running redis pod therefore splits the two: redis keeps requiring the old password while +// any client that restarts afterwards presents the new one, and every Application flips to +// Unknown/ComparisonError (WRONGPASS) until redis is restarted by hand. +export const ARGOCD_REDIS_RESTART_TARGETS: Array<{ kind: 'deployment' | 'statefulset'; name: string }> = [ + { kind: 'deployment', name: 'argocd-redis' }, + { kind: 'deployment', name: 'argocd-server' }, + { kind: 'deployment', name: 'argocd-repo-server' }, + { kind: 'statefulset', name: 'argocd-application-controller' }, + { kind: 'deployment', name: 'argocd-application-controller' }, +] + +export const restartArgoCdRedisConsumers = async ( + namespace: string, + deps = { restartDeployment, restartStatefulSet }, +): Promise => { + const d = terminal('common:k8s:restartArgoCdRedisConsumers') + const [redis, ...consumers] = ARGOCD_REDIS_RESTART_TARGETS + + // Redis has to come back on the new password before anything is pointed at it. If it does not + // restart, restarting the clients is worse than doing nothing: they would pick up the new + // password while redis still requires the old one, which is the split this function exists to + // prevent. So redis, and only redis, is allowed to stop the sequence. + try { + await deps.restartDeployment(redis.name, namespace) + d.info(`Restarted ${redis.kind}/${redis.name} after redis password change`) + } catch (error) { + if (error instanceof ApiException && error.code === 404) { + d.debug(`Could not restart ${redis.kind}/${redis.name}: not found — leaving its clients alone`) + return + } + d.error(`Could not restart ${redis.kind}/${redis.name}, not restarting its clients:`, error) + throw error + } + + for (const target of consumers) { + try { + if (target.kind === 'deployment') await deps.restartDeployment(target.name, namespace) + else await deps.restartStatefulSet(target.name, namespace) + d.info(`Restarted ${target.kind}/${target.name} after redis password change`) + } catch (error) { + // Not every target exists in every topology — a missing one is not a failure. + if (error instanceof ApiException && error.code === 404) { + d.debug(`Could not restart ${target.kind}/${target.name}: not found`) + continue + } + d.warn(`Could not restart ${target.kind}/${target.name}:`, error) + } + } +} + +export const createArgoCdRedisSecret = async ( + values: Record, + deps = { getK8sSecret, restartArgoCdRedisConsumers }, +): Promise => { const d = terminal('common:k8s:createArgoCdRedisSecret') const argocdNamespace = 'argocd' const secretName = 'argocd-redis' @@ -831,6 +887,20 @@ export const createArgoCdRedisSecret = async (values: Record): Prom return } + // Read before writing so the restart below is limited to an actual rotation. This runs on every + // install, and restarting the whole Argo stack on an unchanged password would be its own outage. + // getK8sSecret already maps 404 to undefined (first install — nothing to restart), so anything + // that throws here is a real read failure and leaves us unable to tell. Assume it rotated: a + // redundant restart costs seconds, a missed one leaves redis and its clients split on WRONGPASS. + let passwordChanged: boolean + try { + const existingSecret = await deps.getK8sSecret(secretName, argocdNamespace) + passwordChanged = existingSecret !== undefined && existingSecret.auth !== redisPassword + } catch (error) { + d.warn(`Could not read Secret ${secretName} to detect a password change, assuming it rotated:`, error) + passwordChanged = true + } + try { await k8s.object().patch( { @@ -879,6 +949,11 @@ export const createArgoCdRedisSecret = async (values: Record): Prom d.error(`Failed to patch Secret ${secretName} with server-side apply:`, error) throw error } + + if (passwordChanged) { + d.info(`Password of Secret ${secretName} changed, restarting redis and its consumers`) + await deps.restartArgoCdRedisConsumers(argocdNamespace) + } } export async function restartDeployment(name: string, namespace: string): Promise {