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

[aweingarten]

📌 Summary

Fixes #3424.

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.

Nothing restarts redis when that Secret changes, so the two silently diverge: redis keeps requiring the old password while any client that restarts afterwards presents the new one, and every Application flips to Unknown/ComparisonError:

ComparisonError: Failed to load target state: failed to generate manifest for source 1 of 1:
rpc error: code = Unknown desc = failed to list refs:
WRONGPASS invalid username-password pair or user is disabled.

Because repo-server caches git refs in redis, one bad AUTH turns all manifest generation into a ComparisonError at once — the whole app-of-apps stalls until redis is bounced by hand.

Tracing where the Secret is written made the fix clear, and put it inside apl-core rather than in a reloader:

• values/argocd/argocd.gotmpl sets redisSecretInit.enabled: false — apl deliberately disables the chart's own secret-init and owns this Secret itself
• its replacement, createArgoCdRedisSecret, server-side-applies the Secret on every install with no read-back and no restart

So the one component that writes the password is also the one place that knows it changed. This reads the Secret before writing it and, when the value actually changed, restarts redis and its consumers.

🔍 Reviewer Notes

• The read-back is load-bearing, not an optimisation. createArgoCdRedisSecret runs on every install; restarting the Argo stack unconditionally would be its own outage. Restart happens only when the stored auth differs from the desired password — and never on a fresh install where the Secret doesn't exist yet (redis will start with it anyway).
• The restart list isn't invented. restartArgoCdRedisConsumers is the same set the v60 addRedisSecretForArgoCD migration already restarts by hand, with the same 404-tolerance. That the migration needed it is the tell that it belongs at the point of rotation instead of in a single one-off migration. I left that migration untouched — with this change it will double-restart during the one-time v60 upgrade, which is harmless. Happy to drop its now-redundant block if you'd prefer; I didn't want to alter the behaviour of an already-released migration in the same PR.
• Redis is restarted first, before its clients, so clients come back against a server that already requires the new password.
• A failed restart is logged and the sweep continues rather than failing the install — install.ts already treats this whole call as best-effort.
• Targeting is confirmed, not assumed: redis-ha.enabled is false (chart default, and apl doesn't enable it), so the workload is the argocd-redis Deployment, not a StatefulSet. Both argocd-application-controller kinds are attempted since 404s are tolerated.

6 new unit tests (restart on change, no restart when unchanged, no restart on fresh install, skip without a password, redis-first ordering, 404 tolerance). jest (49 in k8s.test.ts), eslint, tsc --noEmit clean. The only failing suite in a full run is values.test.ts, which fails identically on unmodified main here — it shells out to gucci/htpasswd, which aren't installed locally.

🧹 Checklist

• Code is readable, maintainable, and robust.
• Unit tests added/updated

[Copilot]

Pull request overview

This PR addresses Argo CD outages caused by argocd-redis and its clients diverging when the argocd-redis Secret (auth) rotates: redis bakes the password at process start, while clients read it at pod start, leading to cluster-wide WRONGPASS/ComparisonError until redis is restarted.

Changes:

• Adds conditional restart logic: read existing argocd-redis Secret, patch desired value, and restart redis + consumers only when the password actually changed.
• Introduces a centralized restart target list and a helper to restart redis and its consuming workloads with 404 tolerance.
• Adds unit tests covering restart-on-change behavior, no-op cases, ordering, and missing-target tolerance.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/common/k8s.ts	Adds secret read-before-write rotation detection and restart orchestration for argocd-redis and consumers.
src/common/k8s.test.ts	Adds unit tests validating restart behavior, ordering, and tolerance for missing targets.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/common/k8s.ts:850

• If restarting argocd-redis fails with a non-404 error, the loop currently continues and may restart clients anyway. That can actively worsen the outage (clients pick up the new password while redis is still on the old one). Consider aborting consumer restarts when the redis restart fails.

    } 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

src/common/k8s.ts:875

• getK8sSecret already returns undefined on 404 and throws for other errors; the .catch(() => undefined) here will also swallow non-404 failures (RBAC/transient API issues). In that case we may still patch/rotate the secret but skip the restart, recreating the WRONGPASS divergence this change is meant to prevent.

  // 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.
  const existingSecret = await deps.getK8sSecret(secretName, argocdNamespace).catch(() => undefined)
  const passwordChanged = existingSecret !== undefined && existingSecret.auth !== redisPassword

src/common/k8s.test.ts:1108

• restartArgoCdRedisConsumers has no test for the critical failure mode where restarting argocd-redis itself fails (non-404). Given the ordering intent (redis-first), it would be good to assert that consumers are not restarted in that case.

  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')

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/common/k8s.ts:875

• deps.getK8sSecret(...).catch(() => undefined) will swallow all read errors (RBAC 403, network issues, etc.), which can cause a real password rotation to be missed and therefore skip the restart that prevents the WRONGPASS outage. Since getK8sSecret already returns undefined for 404, it’s safer to let other errors surface (or explicitly handle only 404).

  // 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.
  const existingSecret = await deps.getK8sSecret(secretName, argocdNamespace).catch(() => undefined)
  const passwordChanged = existingSecret !== undefined && existingSecret.auth !== redisPassword