Reconcile crashes the whole Authorino process: `worker.Stop()` is not idempotent and `cleanConfigs()` cleans the same evaluator twice when `translateAuthConfig()` fails

[Aman-Cool]

Describe the bug

(*worker).Stop() isn't idempotent, and Reconcile() calls it twice on the same evaluator whenever translateAuthConfig() fails for an already-indexed AuthConfig. The second call closes a closed channel and panics in a goroutine AuthConfig.Clean() spawned, taking the process with it.

An update is meant to be clean the old, then replace it. The error path does the first half only:

// controllers/auth_config_controller.go:190
if err := r.cleanConfigs(ctx, resourceId); err != nil { ... }   // stops the indexed config's workers

translatedAuthConfig, err := r.translateAuthConfig(...)
if err != nil {
    r.StatusReport.Set(resourceId, api.StatusReasonInvalidResource, err.Error(), []string{})
    return ctrl.Result{}, err                                    // cleaned, never replaced
}

1. reconcile Improve test coverage #1 — cleanConfigs() stops the indexed config's refreshers, translateAuthConfig() fails, we return the error before addToIndex(). The index still holds the instance we just tore down.
2. the requeue, ~5ms later.
3. reconcile Review/refactor caching of the configs #2 — cleanConfigs() (:254) pulls that same instance back out and cleans it again → close of closed channel.

Same object, not a copy: Index.Set() stores the struct by value, but IdentityConfigs is a slice, so the copy shares its backing array down to the *worker.

// pkg/workers/worker.go:74
func (w *worker) Stop() error {
    if w.done != nil {
        close(w.done)   // second call panics
    }
    return nil
}

The nil check only covers a worker never started — nothing marks a stopped one stopped. Start() calls Stop() for restarts, likely why it survived there, but oidcProviderVerifier.Clean() (jwt.go:158) and OPAExternalSource.cleanupRefresher() (opa.go:133) call it directly, and cleanConfigs() can't know the instance was already cleaned.

Fatal rather than logged because AuthConfig.Clean() fans out into raw goroutines (config.go:56) and recover() only catches panics in its own goroutine. (Moot anyway — on controller-runtime v0.16.3 RecoverPanic is a *bool we never set, so the controller re-panics at internal/controller/controller.go:116.) Same shape as #651/#652 and #494.

Needs two ordinary things: a refresher (jwt.ttl > 0 or opa.externalPolicy.ttl), and a translate failure after the config was indexed — rotated or deleted credentialsRef Secret, unreachable OPA registry, Rego precompile error, bad selector, CEL build failure. A cold start is safe, since nothing is indexed yet to clean; it needs the config live and serving when the dependency breaks. Which is exactly the incident you hoped to ride out on cached policy and JWKS.

Help us Reproduce it

No cluster — the existing fake-client harness in controllers/ drives the real Reconcile(). AuthConfig with jwt.ttl: 60 against the mock OIDC server TestMain already runs → reconcile (indexes it) → add an oauth2Introspection whose credentialsRef names a missing Secret → reconcile twice.

reconcile #1 ok - authconfig indexed, OIDC refresher running
reconcile #2 failed as expected: secrets "no-such-secret" not found
stale config still indexed with its worker already stopped
panic: close of closed channel

goroutine 82 [running]:
workers.(*worker).Stop                  pkg/workers/worker.go:76
identity.(*oidcProviderVerifier).Clean  pkg/evaluators/identity/jwt.go:158
identity.(*JWTAuthentication).Clean     pkg/evaluators/identity/jwt.go:77
evaluators.(*IdentityConfig).Clean      pkg/evaluators/identity.go:157
evaluators.(*AuthConfig).Clean.func1    pkg/evaluators/config.go:59
created by evaluators.(*AuthConfig).Clean in goroutine 64

created by — child goroutine, no recover boundary. In-cluster is the same three steps: an AuthConfig with jwt.ttl serving traffic, break a translate-time dependency (scale the policy registry to 0, delete the introspection Secret), touch the AuthConfig; every pod dies on the requeue.

Two related problems on the same path

Leaked refreshers. A partway translateAuthConfig() failure drops whatever it already built — nothing cleans a config that never reached the index. If the JWT identity is built before the failing one (map order), its worker is already running and unowned. Measured on the same loop with no worker on the indexed side, so it loops instead of panicking: goroutines before=3 after=51 failed reconciles (delta=+48 over 50). Backoff starts at ~5ms, so dozens within seconds, each polling discovery every ttl forever. Fixing Stop() doesn't touch it.

Stale index keys → the wrong config gets cleaned. Index.Set() appends to c.keys[id] unconditionally (index.go:77) and neither Delete() nor deleteKey() prunes it. Narrow A from [x.com, y.com] to [y.com]: the tree entry goes, c.keys["ns/A"] keeps x.com. Create B claiming x.com and it legitimately gets it. Now every reconcile of A resolves FindKeys("ns/A")[0] → Index.Get("x.com") → B's config → B.Clean(), stopping B's refreshers and shutting its caches while B serves traffic; once more and it's the double Stop() on B's worker. That stale map also keeps Index.Empty() false forever, defeating the re-bootstrap guard in bootstrapIndex().

Expected behavior

A failed translate should keep the last-known-good config indexed, report InvalidResource, and retry. Double-cleaning should be a no-op, no cleanup path should kill the process, and reconciling one AuthConfig should never touch another's evaluators.

The blast radius isn't one pod: every replica reconciles every matching AuthConfig, so they panic together and the whole ext_authz service CrashLoopBackOffs. With failure_mode_allow: true — a common choice precisely to survive authz blips — that window is every request allowed through unauthenticated; false gives a 503 on everything protected.

The aftermath hides it. bootstrapIndex() re-indexes the host with the denyAll placeholder (503 "Busy"); reconcile still fails so it's never replaced, but the placeholder carries no workers, so it doesn't panic again. One crash, a pod that looks recovered, a host stuck on 503, nothing in the panic naming the AuthConfig, and a status reading InvalidResource like a benign user error.

Environment (please complete the following information):

• Cluster information: none required — both reproduce under go test ./controllers/.... Reconcile-lifecycle bug, platform-independent.
• Authorino version: main @ 58fecc6c; present in v0.27.0. Go 1.26.4.
• Link to the Authorino AuthConfig resource: inline above — any AuthConfig with jwt.ttl or opa.externalPolicy.ttl.

Additional context

Not a regression: pkg/workers/worker.go has one commit since it was written (714717a7, the pkg/cron → pkg/workers rename) and Stop() none; index.go likewise only e64de14a. #572 is close history — it flagged races in TestOPAExternalUrlWithTTL and TestOIDCProviderVerifierRefresh, these two exact refreshers, and closed on the races.

Proposed, small and independent:

1. workers/worker.go:74 — nil done after closing (guarded; Start() also calls Stop()). Kills the crash alone.
2. evaluators/config.go:56 — recover() in the per-evaluator goroutine.
3. index/index.go — dedupe in Set(), prune c.keys in deleteKey()/Delete().
4. auth_config_controller.go:196 — clean the partially-built config on the error path.

~30 lines plus regression tests. Happy to split (3) into its own issue — kept here because it's the second trigger for the same panic.

### I have a fix + regression tests ready and am happy to open a PR.

[guicassolato]

Thanks for the thorough description, @Aman-Cool. The bug sounds legit even though I myself haven't reproduced it. The fix looks trivial though. Perhaps a check to control whether the channel has been closed already?