fix(argocd): restart redis and its clients when the password rotates

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()
+  })
+})

src/common/k8s.ts

@@ -819,7 +819,63 @@ export async function setArgoCdAppSync(
   )
 }
 
-export const createArgoCdRedisSecret = async (values: Record<string, any>): Promise<void> => {
+// 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<void> => {
+  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<string, any>,
+  deps = { getK8sSecret, restartArgoCdRedisConsumers },
+): Promise<void> => {
   const d = terminal('common:k8s:createArgoCdRedisSecret')
   const argocdNamespace = 'argocd'
   const secretName = 'argocd-redis'
@@ -831,6 +887,20 @@ export const createArgoCdRedisSecret = async (values: Record<string, any>): 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<string, any>): 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<void> {