Skip to content

Apply rate-limiting to webhook executions [RUN AS-IF-FOSS] [RUN ALL RSPEC]

Markus Koller requested to merge 329207-rate-limit-webhooks into master

What does this MR do?

This applies a rate limit to WebHookWorker jobs, scoped to the current hook ID, with thresholds defined in plan limits, and a hard-coded interval of 1 minute.

The rate-limiting is disabled behind a FF and we don't define any plan limits yet, this will be done in an upcoming MR.

I decided to scope the rate-limit to each individual hook, rather than the project or group as we first discussed in https://gitlab.com/gitlab-org/gitlab/-/issues/329207, because:

  • It's easier to implement 😸
  • It's easier to reason about from a user perspective, and limits the impact to the webhook that is actually causing problems.
  • It would allow us to highlight rate-limited webhooks in the UI later, similar to what @alexkalderimis was preparing in !60837 (merged) for auto-disabled webhooks.

Issue: #329207

Migration output

Up:

== 20210503131747 AddWebHookCallsToPlanLimits: migrating ======================
-- add_column(:plan_limits, :web_hook_calls, :integer, {:null=>false, :default=>0})
   -> 0.0051s
== 20210503131747 AddWebHookCallsToPlanLimits: migrated (0.0052s) =============

Down:

== 20210503131747 AddWebHookCallsToPlanLimits: reverting ======================
-- remove_column(:plan_limits, :web_hook_calls, :integer, {:null=>false, :default=>0})
   -> 0.0026s
== 20210503131747 AddWebHookCallsToPlanLimits: reverted (0.0040s) =============

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Edited by Markus Koller

Merge request reports