fix(monitoring): stop dropping the OAuth callbacks that fail with a 200 - #421
Merged
Conversation
mon-1 made the three gated hosts log every request and had Alloy drop the 2xx and 304 on the way to Loki, so what reaches Loki is only the refusals. That is right everywhere except one path. When a gate session expires mid-page-load the browser re-requests the page and every asset at once, each starts its own OAuth round trip, and the parallel callbacks overwrite each other's state cookie. Caddy answers 200 to the failures too, so the drop discarded exactly the requests that broke, and the only record left was the outpost's own "oauth state does not match the session". Diagnosing #418 had to be done from that log because the access log could not help. /outpost.goauthentik.io/ is now exempt from the status drop, whatever it returns. It is the auth handshake itself, it is only touched on a login round trip, so it is not the firehose the drop exists to avoid, and it is the one place where a 200 is not evidence that anything worked. The drop is not widened anywhere else. The exemption is a line filter on the selector, because Alloy matches with Go RE2 and RE2 has no negative lookahead: "drop a 2xx unless the URI is an outpost path" cannot be written as one expression. It is the regex form anchored on the "uri" key rather than a plain substring test, because Caddy logs the request headers too, so a bare substring filter would also exempt any ordinary success whose Referer pointed at a handshake page. The client-cancelled-stream drop moves to a block of its own. It carries no URI, so the exemption is meaningless to it and sharing a filtered selector would only make it look conditional on something it cannot depend on. tests/test-alloy-log-drops.sh pins both directions: a failed callback that must now be KEPT, and an asset whose Referer merely quotes the path, which must still go. Its parser had to learn about line filters first, or it would have reported a drop for a line the running pipeline keeps.
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
1 participant
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.
Rebrand board row mon-3. Depends on mon-1 (#417), which is deployed.
The gap
mon-1 made the three gated hosts log every request to stdout, and Alloy drops
the 2xx and 304 on the way to Loki, so everything that arrives is already a
request that did not succeed. Right for ordinary traffic, wrong for one path.
When a gate session expires mid-page-load, the browser re-requests the page and
every asset at once, each one starts its own OAuth round trip, and the parallel
callbacks overwrite each other's state cookie. Caddy returns 200 for those
failures, so the drop discarded them. The only surviving record was the
outpost's own
oauth state does not match the session, which is what #418 hadto be diagnosed from.
The change
/outpost.goauthentik.io/is exempt from the status drop, whatever it returns.The drop is not widened anywhere else, which would just restore the firehose
mon-1 exists to avoid. That path is the auth handshake itself, it is only
touched on a login round trip, and it is the one place where a 200 proves
nothing. All three gates are already on authentik, so it is live on every gated
host today.
Two things shape how it is written:
path" is not expressible as one regex. It is a line filter on the
stage.matchselector instead.
"uri"key, not a plainsubstring test for the path. Caddy's JSON access log records request headers
too, so a bare substring filter would also exempt any ordinary 200 whose
Referer happened to point at a handshake page, silently widening the keep.
The client-cancelled-stream drop moves to a block of its own: it carries no URI,
so the exemption is meaningless to it.
Verification
A
stage.matchselector accepting a line filter was an assumption, so it waschecked in a throwaway
grafana/alloy:v1.18.1(the pinned version) readingfixtures through
loki.echo, before any config was written://outpost.goauthentik.io/callback/, 403 on/beheer, 302 on an outpost pathtests/test-alloy-log-drops.shgains both directions: the failed callback thatmust now be kept, and the Referer case that must still go. Its parser had to
learn about selector line filters first, or it would have reported a drop for a
line the running pipeline keeps, which is the one mistake that file exists to
catch. Both new cases were confirmed load-bearing by mutating the config back
and watching each one fail on its own.
Gates run:
tests/run.sh(40 files, exit 0), shellcheck at error severity,pre-commit --all-files,alloy fmton the real config, and the suite re-rununder mawk since that is what
awkis on the CI runner.Deploying
A normal
deploy.ymlapplies this: #419 putconfig.alloyin the DEP-6 hashlist. No
--force-recreateneeded, unlike mon-1's deploy.Not yet done, and it is the part that matters: causing a 200 on an outpost path
on purpose and watching that specific line reach Loki, cross-checked against
docker logs caddy --sincefor the same window.