Instrument the git basic auth IP ban path
What
Adds gitlab_rate_limiter_git_basic_auth_ban_total, a counter on the Git and container
registry authentication IP ban path, with an event label taking failure, ban, blocked,
already_banned and reset.
Also pins the current behaviour of that path in its spec.
Why
First step of gitlab-com/gl-infra/production-engineering#29557, replacing
Rack::Attack::Allow2Ban with a labkit rate limiter. @reprazent asked for metrics on the
existing path first, so the old and new implementations can be compared rather than swapped
blind.
The path has no Prometheus instrumentation at all today. gitlab_rack_attack_events_total
cannot cover it: that comes from a subscriber on Rack::Attack middleware notifications, and
Allow2Ban's class methods publish none when called directly from IpRateLimiter. So today
nobody can say how often bans fire.
Notes for review
- Nothing is emitted when the limiter is skipped. An allowlisted or disabled request is not a ban event at all, and on an instance with the feature off it would be the only thing this counter ever recorded.
resetis high volume by design.Gitlab::Auth.rate_limit!callsreset!after every successful git auth, not only when a ban existed, so this label will dwarf the others. It is kept because it is the only measure of how often the clear-on-success path runs, which is the labkit primitive being designed against. Query the other four labels on their own.- Detecting ban creation needs one extra read.
Allow2Ban#fail!returnsfalseeven on the call that writes the ban, so the only way to see the transition is to checkbanned?straight after registering the failure. That read happens on failed auth only, which is rare. already_bannedis separate fromblockedon purpose. It is the case where a concurrent request banned the IP between this request's pre-auth check and its failure registration, and it is the only path that reaches the "threshold exceeded" auth log today.
Spec changes
The existing spec asserted less than it appeared to. Nothing covered which attempt the ban lands
on, the counter expiring after findtime, the ban expiring after bantime, or reset! clearing
an existing ban. One assertion expected Allow2Ban.reset!, which production never calls, so it
could not fail. All of that is now pinned, because it is the baseline the labkit implementation
will be compared against.