fix(decofile): make missing reload-token diagnosable on /.decofile/reload 401#32
Open
nicacioliveira wants to merge 3 commits into
Open
fix(decofile): make missing reload-token diagnosable on /.decofile/reload 401#32nicacioliveira wants to merge 3 commits into
nicacioliveira wants to merge 3 commits into
Conversation
The /.decofile/reload endpoint in the deco runtime became fail-closed (deco-cx/deco 51af3349): a pod with no DECO_RELEASE_RELOAD_TOKEN now returns 401 for every operator notification instead of silently allowing it. The token is only injected by this operator's mutating webhook on Service create/update, so a revision created before injection was active (or on the webhook's non-blocking "Decofile not found" skip path) boots tokenless and can never be hot-reloaded — the failure only surfaces on the next block-only publish as "max retries reached: pod returned status 401" with no indication of the real cause. This does not change control flow or add RBAC; it makes the condition observable: - notifier: classify terminal notification failures where the pod has no reload token via a new ErrMissingReloadToken sentinel (propagated through the batch aggregate error). - decofile_controller: when notification fails with ErrMissingReloadToken, set PodsNotified=False with a distinct reason "MissingReloadToken" and an actionable message (redeploy to re-inject the token) instead of the generic "NotificationFailed". - service_webhook: keep the "Decofile not found" path non-blocking (it guards the cross-cluster fanout propagation race), but log it as an explicit WARNING that the Service is being created without a reload token and will 401 on reload until redeployed — instead of a silent info-level skip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…reason Wire the ErrMissingReloadToken sentinel from the notifier into the Decofile reconciler: when notification fails because the target pods have no reload token, set PodsNotified=False with reason "MissingReloadToken" and an actionable message (redeploy to re-inject the token) instead of the generic "NotificationFailed", and log the actionable line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Narrow the ErrMissingReloadToken classification: require BOTH that the operator had no token to send (no DECO_RELEASE_RELOAD_TOKEN in the pod env) AND that the pod actually answered 401. A 401 with a token present means the token was rejected (a different problem), and non-401 failures (5xx, connection reset / broken pipe) are not necessarily token-related. Track the last observed status code and gate on http.StatusUnauthorized. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hugo-ccabral
approved these changes
Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contexto
Incidente em produção (eks-serverless): o controller de decofile falhou em notificar 16 pods de um site com uma mistura de
pod returned status 401ewrite: broken pipe. Investigando os dois lados (operator + runtimedeco-cx/deco), a causa raiz não é o commit publicado, payload grande, nem timeout.O que realmente acontece
POST /.decofile/reloadno runtime virou fail-closed emdeco-cx/deco@51af3349(correção da vuln de takeover): um pod semDECO_RELEASE_RELOAD_TOKENagora responde 401 a toda notificação, em vez de permitir silenciosamente (fail-open antigo).create/updatede Knative Service comdeco.sites/decofile-inject: "true".Decofile not found → return nil) — sobe sem token. Como revisions são imutáveis, ela nunca mais consegue ser recarregada.broken pipeé o mesmo 401: o check de auth roda antes deawait req.json(), então o server fecha o socket enquanto o operator ainda escreve o body do decofile.Hoje isso chega no operador como
max retries reached: pod returned status 401, sem nenhuma pista da causa real. A correção definitiva é um redeploy (nova revision → webhook reinjeta o token), mas ninguém tem como saber disso pelos sinais atuais.O que este PR faz
Não muda control flow, não adiciona RBAC, não escreve em recursos do admin. Só torna a condição diagnosticável:
notifier.go: novo sentinelErrMissingReloadToken. A classificação é deliberadamente estreita — só marca falta de token quando há evidência dura: o operator não enviou header de auth (não achouDECO_RELEASE_RELOAD_TOKENno env do pod) E o pod respondeu 401 (lastStatusCode == http.StatusUnauthorized). O sentinel é propagado pelo erro agregado do batch (viaerrors.Is).missingTokense qualquer pod bateu no sentinel, o caso real (mistura de 401 limpo + broken pipe) é rotulado certo desde que ao menos um pod devolva o 401 limpo.decofile_controller.go: quando a notificação falha comErrMissingReloadToken, setaPodsNotified=Falsecom reason distintoMissingReloadTokene mensagem acionável ("redeploy para reinjetar o token"), em vez do genéricoNotificationFailed.service_webhook.go: mantém o caminhoDecofile not foundnão-bloqueante (ele protege a corrida de propagação do Decofile no fanout cross-cluster — hard-fail quebraria deploys válidos), mas loga como WARNING explícito de que a Service está subindo sem token e vai dar 401 no reload até ser redeployada — em vez de um info-level silencioso.O que este PR NÃO faz (de propósito)
Auto-heal via redeploy disparado pelo operator (
operator "resolver" o 401 forçando nova revision) foi deliberadamente deixado de fora, porque exigiria: (a) escalação de RBAC do operator paraserving.knative.dev/services: update;patch, (b) escrever na Service que é fonte de verdade do admin (descasa o tracking dedeploymentId), e (c) risco de tempestade de redeploy (notify falha por N motivos além de token). Se o time quiser, fica como follow-up atrás de feature-flag default-off com guarda de "uma vez por revision".Fix complementar recomendado (outro repo)
O gate de hot-swap no admin (
deco-sites/adminhosting/kubernetes/platform.ts) decideskipPromotepela versão do deco, não pela presença do token na revision-alvo. O ideal é: só permitir hot-swap se a revision atual tiverDECO_RELEASE_RELOAD_TOKEN; senão, cair pra deploy completo (que reinjeta o token). Isso auto-drena as landmines existentes no próximo publish de cada site. Não faz parte deste PR (é no admin).Teste
go build ./...+go test ./internal/...no CI do repo (não consegui rodar Go localmente).notifier/controller cobrindo: pod semDECO_RELEASE_RELOAD_TOKEN+ 401 ⇒ erro casa comErrMissingReloadToken⇒ conditionPodsNotified=False, reason=MissingReloadToken; e o inverso: 401 com token, ou falha não-401 ⇒ não casa (ficaNotificationFailed).🤖 Generated with Claude Code