diff --git a/adr/2026-08-04-restart-platform-auth-pods-after-oauth2-proxy-healthy.md b/adr/2026-08-04-restart-platform-auth-pods-after-oauth2-proxy-healthy.md new file mode 100644 index 0000000000..865af601ff --- /dev/null +++ b/adr/2026-08-04-restart-platform-auth-pods-after-oauth2-proxy-healthy.md @@ -0,0 +1,41 @@ +# Restart platform-auth pods once oauth2-proxy is healthy + +- Status: accepted + +## Context and Problem Statement + +On install, Keycloak is not yet available when Istio first resolves the JWKS used by `RequestAuthentication`. Istio's sidecar proxy caches whatever it fetched at that point (a dummy/invalid key) and does not reliably re-fetch once Keycloak comes up, so any pod whose sidecar started before Keycloak was ready keeps failing JWT verification ("JWT verification failed") until it happens to self-heal (several minutes) or is restarted. This affects every pod labelled `otomi.io/auth: platform` (see [ADR-2026-06-12](2026-06-12-auth-policy-pod-label.md)), not just Prometheus where it was first observed. + +The sidecar's fetch races Keycloak independently of the app container's own readiness gating (an app's initContainer waiting on Keycloak does not delay its Istio sidecar). Two previously attempted mitigations — tuning `PILOT_JWT_PUB_KEY_REFRESH_INTERVAL` and `PILOT_DEBOUNCE_MAX` — did not solve it and were reverted. + +`oauth2-proxy` already gates its own startup on Keycloak's OIDC issuer being reachable (`wait-for-keycloak` initContainer in `values/oauth2-proxy/oauth2-proxy.gotmpl`), making "oauth2-proxy's ArgoCD Application is Healthy" the most reliable available signal that Keycloak is actually serving real keys. + +## Decision Drivers + +- Must not add a new long-running process or watcher to `apl-operator` — it already runs two concurrent loops and additional operational complexity there is undesirable. +- Cannot hook into the Phase 1 install sequence (`src/cmd/install.ts`): oauth2-proxy's ArgoCD Application is only created by `applyAsApps()`, which is only ever invoked from Phase 2's poll/reconcile loop (`apl-operator.ts`). Phase 1 has nothing to wait for. +- Must fire exactly once per cluster lifetime — restarting the same pods on every apply cycle would cause needless churn. +- Should reuse existing, tested primitives rather than introduce new infrastructure (chart changes, RBAC, shell scripts). + +## Considered Options + +- Add a blocking wait-then-restart step to `src/cmd/install.ts` (Phase 1) — impossible, see decision drivers. +- Start a separate watcher process/goroutine inside `apl-operator` dedicated to this — rejected, adds a third concurrent loop to an already complex operator. +- An ArgoCD `PostSync` hook Job shipped inside the `oauth2-proxy` chart (via its existing `extraObjects` support), restarting labelled pods once the oauth2-proxy Application syncs healthy — viable, keeps the operator untouched, but adds new chart/RBAC/script surface and duplicates the "wait for Keycloak" signal in bash instead of reusing the operator's existing ArgoCD health-check helper. +- Piggyback on the existing Phase 2 poll/reconcile loop in `apl-operator.ts` (chosen) — see Decision Outcome. + +## Decision Outcome + +Chosen option: piggyback on the existing poll/reconcile loop. + +At the end of `runApplyIfNotBusy`'s success path (`apl-operator.ts`), after every successful apply — both `Poll`- and `Reconcile`-triggered — do a single non-blocking check of the oauth2-proxy ArgoCD Application's health (`checkArgoCDAppStatus`, no retry wrapper; the loop's own cadence is the retry). If healthy and a dedicated marker ConfigMap (e.g. `apl-platform-auth-restart-state`, separate from `apl-operator-state` since it tracks an unrelated, one-shot concern) is not yet set: restart every pod labelled `otomi.io/auth: platform` across all namespaces, reusing `restartPodOwner`/`getWorkloadKeyFromPod` from `src/common/runtime-upgrades/restart-istio-sidecars.ts`, then write the marker. + +This adds no new interval, loop, or process — the check rides on cadence that already exists (sub-second while there are git changes to apply during install, at minimum every 5 minutes via the reconcile timer regardless). It also runs unconditionally on upgrades, since there is no cheap way to distinguish "fresh install" from "upgrade" at this point, and a single unnecessary pod-restart round on upgrade is harmless. + +Restarting all `otomi.io/auth: platform` pods unconditionally (rather than only the ones observed failing) is deliberate: the bug is systemic, not app-specific, and scoping it to specific apps would just leave the bug in place for whichever app wasn't listed. + +Fixing sidecar restarts after an Istio version upgrade is a related but separate problem, already solved by `detectAndRestartOutdatedIstioSidecars` (wired as a versioned runtime-upgrade hook in `src/common/runtime-upgrades/runtime-upgrades.ts`); it is out of scope here. + +### Negative Consequences + +- A pod whose sidecar races Keycloak independently of oauth2-proxy's own timing (i.e. starts before Keycloak is ready even though oauth2-proxy is already healthy) is not covered by this one-shot fix. Judged acceptable: Keycloak is a lighter, earlier-starting component than most affected apps, and the ticket already accepts a multi-minute self-heal as tolerable for stragglers. diff --git a/adr/index.md b/adr/index.md index 5c59009b3c..38854105fd 100644 --- a/adr/index.md +++ b/adr/index.md @@ -18,13 +18,13 @@ This log lists the architectural decisions for apl-core. - [ADR-2022-07-02](2022-07-02-node-affinity.md) - Node affinity - [ADR-2022-08-26](2022-08-26-other-dns-provider.md) - Other DNS provider - [ADR-2026-05-20](2026-05-20-gateway-api.md) - Kubernetes Gateway API replaces Ingress CR and Istio IngressGateway -- [ADR-2026-06-02](2026-06-02-release-automation.md) - Release automation: explicit versioning, one branch per release cycle - [ADR-2026-06-02](2026-06-02-release-branch-per-cycle.md) - One release branch per major.minor cycle - [ADR-2026-06-12](2026-06-12-auth-policy-pod-label.md) - Auth policy pod label (`otomi.io/auth-policy`) - [ADR-2026-06-25](2026-06-25-drop-sops-for-sealedsecrets.md) - Drop SOPS in favour of SealedSecrets -- [ADR-2026-06-25](2026-06-25-manifests-directory.md) - Manifests directory in the values repo -- [ADR-2026-06-25](2026-06-25-git-server-as-default-values-repo.md) - Lightweight git-server as the default values repository backend - [ADR-2026-06-25](2026-06-25-git-credential-management.md) - Git credential management via Kubernetes Secret +- [ADR-2026-06-25](2026-06-25-git-server-as-default-values-repo.md) - Lightweight git-server as the default values repository backend +- [ADR-2026-06-25](2026-06-25-manifests-directory.md) - Manifests directory in the values repo +- [ADR-2026-08-04](2026-08-04-restart-platform-auth-pods-after-oauth2-proxy-healthy.md) - Restart platform-auth pods once oauth2-proxy is healthy diff --git a/src/common/constants.ts b/src/common/constants.ts index b48480b27f..7c649dbbd0 100644 --- a/src/common/constants.ts +++ b/src/common/constants.ts @@ -5,6 +5,7 @@ export const APL_OPERATOR_STATUS_CM = 'apl-installation-status' export const OTOMI_NAMESPACE = 'otomi' export const SEALED_SECRETS_NAMESPACE = 'apl-secrets' export const OTOMI_SECRETS = 'otomi-secrets' +export const APL_PLATFORM_AUTH_RESTART_STATE_CM = 'apl-platform-auth-restart-state' export const ARGOCD_APP_PARAMS = { group: 'argoproj.io', version: 'v1alpha1', diff --git a/src/common/runtime-upgrades/restart-platform-auth-pods.test.ts b/src/common/runtime-upgrades/restart-platform-auth-pods.test.ts new file mode 100644 index 0000000000..f825f2aacd --- /dev/null +++ b/src/common/runtime-upgrades/restart-platform-auth-pods.test.ts @@ -0,0 +1,49 @@ +import { CoreV1Api } from '@kubernetes/client-node' +import { PLATFORM_AUTH_LABEL_SELECTOR, restartPlatformAuthPods } from './restart-platform-auth-pods' + +describe('restartPlatformAuthPods', () => { + const mockCoreApi = { + listPodForAllNamespaces: jest.fn(), + } as unknown as jest.Mocked + + const mockDeps = { + getWorkloadKeyFromPod: jest.fn(), + restartPodOwner: jest.fn(), + } + + beforeEach(() => { + jest.clearAllMocks() + }) + + it('lists pods labelled otomi.io/auth=platform', async () => { + mockCoreApi.listPodForAllNamespaces.mockResolvedValue({ items: [] }) + + await restartPlatformAuthPods(mockCoreApi, mockDeps) + + expect(mockCoreApi.listPodForAllNamespaces).toHaveBeenCalledWith({ + labelSelector: PLATFORM_AUTH_LABEL_SELECTOR, + }) + }) + + it('restarts the owner of each matching pod', async () => { + mockDeps.getWorkloadKeyFromPod.mockReturnValueOnce('ns/deploy-a').mockReturnValueOnce('ns/deploy-b') + mockCoreApi.listPodForAllNamespaces.mockResolvedValue({ + items: [{ metadata: { namespace: 'ns', name: 'pod-a' } }, { metadata: { namespace: 'ns', name: 'pod-b' } }], + }) + + await restartPlatformAuthPods(mockCoreApi, mockDeps) + + expect(mockDeps.restartPodOwner).toHaveBeenCalledTimes(2) + }) + + it('restarts each distinct workload only once', async () => { + mockDeps.getWorkloadKeyFromPod.mockReturnValue('ns/deploy-a') + mockCoreApi.listPodForAllNamespaces.mockResolvedValue({ + items: [{ metadata: { namespace: 'ns', name: 'pod-a-1' } }, { metadata: { namespace: 'ns', name: 'pod-a-2' } }], + }) + + await restartPlatformAuthPods(mockCoreApi, mockDeps) + + expect(mockDeps.restartPodOwner).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/common/runtime-upgrades/restart-platform-auth-pods.ts b/src/common/runtime-upgrades/restart-platform-auth-pods.ts new file mode 100644 index 0000000000..896f8a155c --- /dev/null +++ b/src/common/runtime-upgrades/restart-platform-auth-pods.ts @@ -0,0 +1,32 @@ +import { CoreV1Api } from '@kubernetes/client-node' +import { getParsedArgs } from '../yargs' +import { terminal } from '../debug' +import { getWorkloadKeyFromPod, restartPodOwner } from './restart-istio-sidecars' + +export const PLATFORM_AUTH_LABEL_SELECTOR = 'otomi.io/auth=platform' +export const OAUTH2_PROXY_ARGOCD_APP_NAME = 'istio-system-oauth2-proxy' + +export async function restartPlatformAuthPods( + coreV1Api: CoreV1Api, + deps = { getWorkloadKeyFromPod, restartPodOwner }, +): Promise { + const d = terminal('restartPlatformAuthPods') + const parsedArgs = getParsedArgs() + + const podsResponse = await coreV1Api.listPodForAllNamespaces({ labelSelector: PLATFORM_AUTH_LABEL_SELECTOR }) + const pods = podsResponse.items + + d.info(`Found ${pods.length} pods labelled ${PLATFORM_AUTH_LABEL_SELECTOR}`) + + const restartedWorkloads = new Set() + + for (const pod of pods) { + const workloadKey = deps.getWorkloadKeyFromPod(pod) + if (workloadKey && restartedWorkloads.has(workloadKey)) continue + + await deps.restartPodOwner(pod, d, parsedArgs) + if (workloadKey) restartedWorkloads.add(workloadKey) + } + + d.info(`Restarted ${restartedWorkloads.size} workloads with platform-auth pods`) +} diff --git a/src/operator/apl-operator.test.ts b/src/operator/apl-operator.test.ts index f6a2b0dca2..723c276856 100644 --- a/src/operator/apl-operator.test.ts +++ b/src/operator/apl-operator.test.ts @@ -1,9 +1,14 @@ import { getStoredGitRepoConfig, GitRepoConfig } from '../common/git-config' import { waitTillGitRepoAvailable } from '../common/gitea' +import { checkArgoCDAppStatus } from '../common/k8s' +import { + OAUTH2_PROXY_ARGOCD_APP_NAME, + restartPlatformAuthPods, +} from '../common/runtime-upgrades/restart-platform-auth-pods' import { AplOperations } from './apl-operations' import { AplOperator, AplOperatorConfig, ApplyTrigger } from './apl-operator' import { GitRepository } from './git-repository' -import { updateApplyState } from './k8s' +import { hasPlatformAuthPodsRestarted, markPlatformAuthPodsRestarted, updateApplyState } from './k8s' const mockInfoFn = jest.fn() const mockWarnFn = jest.fn() @@ -61,6 +66,18 @@ jest.mock('../cmd/commit', () => ({ jest.mock('./k8s', () => ({ updateApplyState: jest.fn().mockResolvedValue(undefined), appRevisionMatches: jest.fn().mockResolvedValue(true), + hasPlatformAuthPodsRestarted: jest.fn().mockResolvedValue(true), + markPlatformAuthPodsRestarted: jest.fn().mockResolvedValue(undefined), +})) + +jest.mock('../common/k8s', () => ({ + checkArgoCDAppStatus: jest.fn().mockResolvedValue('Healthy'), + k8s: { custom: jest.fn().mockReturnValue({}), core: jest.fn().mockReturnValue({}) }, +})) + +jest.mock('../common/runtime-upgrades/restart-platform-auth-pods', () => ({ + OAUTH2_PROXY_ARGOCD_APP_NAME: 'istio-system-oauth2-proxy', + restartPlatformAuthPods: jest.fn().mockResolvedValue(undefined), })) jest.mock('./git-repository', () => ({ @@ -265,6 +282,59 @@ describe('AplOperator', () => { expect(mockErrorFn).toHaveBeenCalledWith('[poll] Apply process failed', 'Apply failed') }) + + test('restarts platform-auth pods when oauth2-proxy is healthy and marker is unset', async () => { + Object.defineProperty(aplOperator, 'isApplying', { value: false, configurable: true }) + ;(hasPlatformAuthPodsRestarted as jest.Mock).mockResolvedValueOnce(false) + ;(checkArgoCDAppStatus as jest.Mock).mockResolvedValueOnce('Healthy') + + await aplOperator.runApplyIfNotBusy(ApplyTrigger.Poll) + + expect(checkArgoCDAppStatus).toHaveBeenCalledWith( + OAUTH2_PROXY_ARGOCD_APP_NAME, + expect.anything(), + 'health', + 'Healthy', + ) + expect(restartPlatformAuthPods).toHaveBeenCalled() + expect(markPlatformAuthPodsRestarted).toHaveBeenCalled() + }) + + test('does not restart platform-auth pods when the marker is already set', async () => { + Object.defineProperty(aplOperator, 'isApplying', { value: false, configurable: true }) + ;(hasPlatformAuthPodsRestarted as jest.Mock).mockResolvedValueOnce(true) + + await aplOperator.runApplyIfNotBusy(ApplyTrigger.Poll) + + expect(checkArgoCDAppStatus).not.toHaveBeenCalled() + expect(restartPlatformAuthPods).not.toHaveBeenCalled() + expect(markPlatformAuthPodsRestarted).not.toHaveBeenCalled() + }) + + test('does not restart platform-auth pods when oauth2-proxy is not healthy yet', async () => { + Object.defineProperty(aplOperator, 'isApplying', { value: false, configurable: true }) + ;(hasPlatformAuthPodsRestarted as jest.Mock).mockResolvedValueOnce(false) + ;(checkArgoCDAppStatus as jest.Mock).mockRejectedValueOnce(new Error('not healthy yet')) + + await aplOperator.runApplyIfNotBusy(ApplyTrigger.Poll) + + expect(restartPlatformAuthPods).not.toHaveBeenCalled() + expect(markPlatformAuthPodsRestarted).not.toHaveBeenCalled() + expect((aplOperator as any).isApplying).toBe(false) + }) + + test('does not fail the apply when the platform-auth restart check throws', async () => { + Object.defineProperty(aplOperator, 'isApplying', { value: false, configurable: true }) + ;(hasPlatformAuthPodsRestarted as jest.Mock).mockRejectedValueOnce(new Error('configmap read failed')) + + await aplOperator.runApplyIfNotBusy(ApplyTrigger.Poll) + + expect(updateApplyState).toHaveBeenCalledWith( + expect.objectContaining({ + status: 'succeeded', + }), + ) + }) }) describe('pollAndApplyGitChanges', () => { diff --git a/src/operator/apl-operator.ts b/src/operator/apl-operator.ts index 1da973789d..2bc4bf1014 100644 --- a/src/operator/apl-operator.ts +++ b/src/operator/apl-operator.ts @@ -5,11 +5,16 @@ import { env } from '../common/envalid' import { getStoredGitRepoConfig } from '../common/git-config' import { waitTillGitRepoAvailable } from '../common/gitea' import { hfValues } from '../common/hf' +import { checkArgoCDAppStatus, k8s } from '../common/k8s' +import { + OAUTH2_PROXY_ARGOCD_APP_NAME, + restartPlatformAuthPods as restartLabelledPlatformAuthPods, +} from '../common/runtime-upgrades/restart-platform-auth-pods' import { ensureManifestDirectories, ensureTeamGitOpsDirectories } from '../common/utils' import { getDefaultValues, writeValues } from '../common/values' import { AplOperations } from './apl-operations' import { GitRepository } from './git-repository' -import { updateApplyState } from './k8s' +import { hasPlatformAuthPodsRestarted, markPlatformAuthPodsRestarted, updateApplyState } from './k8s' import { getErrorMessage } from './utils' export interface AplOperatorConfig { @@ -105,6 +110,14 @@ export class AplOperator { timestamp: new Date().toISOString(), trigger, }) + try { + // See adr/2026-08-04-restart-platform-auth-pods-after-oauth2-proxy-healthy.md + if (await this.needsPlatformAuthPodsRestart()) { + await this.restartPlatformAuthPods() + } + } catch (error) { + this.d.debug(`Skipping platform-auth pod restart: ${getErrorMessage(error)}`) + } } catch (error) { const errorMessage = getErrorMessage(error) this.d.error(`[${trigger}] Apply process failed`, errorMessage) @@ -120,6 +133,18 @@ export class AplOperator { } } + private async needsPlatformAuthPodsRestart(): Promise { + if (await hasPlatformAuthPodsRestarted()) return false + await checkArgoCDAppStatus(OAUTH2_PROXY_ARGOCD_APP_NAME, k8s.custom(), 'health', 'Healthy') + return true + } + + private async restartPlatformAuthPods(): Promise { + this.d.info('oauth2-proxy is healthy, restarting platform-auth pods') + await restartLabelledPlatformAuthPods(k8s.core()) + await markPlatformAuthPodsRestarted() + } + // Only used in tests: run N iterations and exit public async reconcile(maxIterations = Infinity): Promise { this.d.info('Starting reconciliation loop') diff --git a/src/operator/k8s.test.ts b/src/operator/k8s.test.ts index d004691dd2..557ffe06ed 100644 --- a/src/operator/k8s.test.ts +++ b/src/operator/k8s.test.ts @@ -1,4 +1,4 @@ -import { ApplyState, updateApplyState } from './k8s' +import { ApplyState, hasPlatformAuthPodsRestarted, markPlatformAuthPodsRestarted, updateApplyState } from './k8s' import { CoreV1Api, ApiException } from '@kubernetes/client-node' jest.mock('@kubernetes/client-node', () => { @@ -173,3 +173,61 @@ describe('updateApplyState', () => { expect(mockCoreV1Api.createNamespacedConfigMap).not.toHaveBeenCalled() }) }) + +describe('hasPlatformAuthPodsRestarted', () => { + let mockCoreV1Api + + beforeEach(() => { + jest.clearAllMocks() + mockCoreV1Api = new CoreV1Api({} as any) as jest.Mocked + }) + + test('returns true when the marker configmap exists', async () => { + mockCoreV1Api.readNamespacedConfigMap.mockResolvedValue({ metadata: { name: 'apl-platform-auth-restart-state' } }) + + const result = await hasPlatformAuthPodsRestarted('test-namespace', 'apl-platform-auth-restart-state') + + expect(result).toBe(true) + expect(mockCoreV1Api.readNamespacedConfigMap).toHaveBeenCalledWith({ + name: 'apl-platform-auth-restart-state', + namespace: 'test-namespace', + }) + }) + + test('returns false when the marker configmap does not exist', async () => { + mockCoreV1Api.readNamespacedConfigMap.mockRejectedValue(new ApiException(404, 'Not Found', {}, {})) + + const result = await hasPlatformAuthPodsRestarted('test-namespace', 'apl-platform-auth-restart-state') + + expect(result).toBe(false) + }) + + test('rethrows unexpected errors', async () => { + const unexpectedError = new Error('boom') + mockCoreV1Api.readNamespacedConfigMap.mockRejectedValue(unexpectedError) + + await expect(hasPlatformAuthPodsRestarted('test-namespace', 'apl-platform-auth-restart-state')).rejects.toThrow( + 'boom', + ) + }) +}) + +describe('markPlatformAuthPodsRestarted', () => { + let mockCoreV1Api + + beforeEach(() => { + jest.clearAllMocks() + mockCoreV1Api = new CoreV1Api({} as any) as jest.Mocked + }) + + test('creates the marker configmap', async () => { + mockCoreV1Api.createNamespacedConfigMap.mockResolvedValue({}) + + await markPlatformAuthPodsRestarted('test-namespace', 'apl-platform-auth-restart-state') + + expect(mockCoreV1Api.createNamespacedConfigMap).toHaveBeenCalledWith({ + namespace: 'test-namespace', + body: { metadata: { name: 'apl-platform-auth-restart-state' } }, + }) + }) +}) diff --git a/src/operator/k8s.ts b/src/operator/k8s.ts index 8d963e4331..7a9c2acf0a 100644 --- a/src/operator/k8s.ts +++ b/src/operator/k8s.ts @@ -1,6 +1,6 @@ import { ApiException, CoreV1Api, KubeConfig } from '@kubernetes/client-node' import { writeFileSync } from 'fs' -import { APL_OPERATOR_NS } from '../common/constants' +import { APL_OPERATOR_NS, APL_PLATFORM_AUTH_RESTART_STATE_CM } from '../common/constants' import { terminal } from '../common/debug' import { getErrorMessage } from './utils' @@ -87,3 +87,31 @@ export async function updateApplyState( d.error('Failed to update apply state:', getErrorMessage(error)) } } + +export async function hasPlatformAuthPodsRestarted( + namespace: string = APL_OPERATOR_NS, + configMapName: string = APL_PLATFORM_AUTH_RESTART_STATE_CM, +): Promise { + try { + await k8s.core().readNamespacedConfigMap({ name: configMapName, namespace }) + return true + } catch (error) { + if (error instanceof ApiException && error.code === 404) return false + throw error + } +} + +export async function markPlatformAuthPodsRestarted( + namespace: string = APL_OPERATOR_NS, + configMapName: string = APL_PLATFORM_AUTH_RESTART_STATE_CM, +): Promise { + try { + await k8s.core().createNamespacedConfigMap({ + namespace, + body: { metadata: { name: configMapName } }, + }) + } catch (error) { + if (error instanceof ApiException && error.code === 409) return + throw error + } +}