Revisit webhook rate limit feature flags for application settings conversion

Context

This issue tracks the conversion of two webhook rate limit feature flags to application settings:

  • web_hook_event_resend_api_endpoint_rate_limit
  • web_hook_test_api_endpoint_rate_limit

Both are ops flags, default_enabled: true, and each gates a hardcoded limit of 5 requests/minute (lib/gitlab/application_rate_limiter.rb). The flag is an on/off switch; the number itself is not configurable today.

As noted in !218726 (merged), these ops flags should be converted to application settings instead of remaining as long-lived feature flags. From @SamWord's review comments:

"I'm not really sure why this needs to be an ops flag when we could make the webhook resend API rate limit configurable in application settings"

"This flag functions the same as web_hook_event_resend_api_endpoint_rate_limit but for a different rate limit. I also don't see why this shouldn't be a rate limit that's configurable in application settings"

Design

One integer setting per limit in the rate_limits JSONB column on application_settings. 0 disables the limit, the interval stays hardcoded at 1 minute, and there's no separate on/off boolean. This is the same shape as most existing ApplicationRateLimiter settings (groups_api_limit, users_api_limit_followers, project_jobs_api_rate_limit, and about 25 others), and the limiter already treats a threshold of 0 as disabled.

New setting Replaces flag Default
web_hook_event_resend_limit web_hook_event_resend_api_endpoint_rate_limit 5
web_hook_test_limit web_hook_test_api_endpoint_rate_limit 5

Prior art: the archive_rate_limit conversion (#480919, commits gitlab-org/gitlab@caff07402f29 and gitlab-org/gitlab@02698faf3a8f) and the ops flag migration guideline.

One behaviour nuance: the controller paths (Projects::HooksController, WebHooks::HookLogActions, Groups::HooksController in EE) already call check_rate_limit! with the same keys and no flag guard. So disabling the flag today only lifts the limit on the API endpoints; after this change, setting the limit to 0 disables it everywhere, UI included. Instances that disabled the flag were presumably after exactly that, so this is an improvement rather than a regression, but it should be mentioned in the MR.

Implementation plan

One MR with two commits, mirroring the archive_rate_limit conversion (which shipped both steps in a single MR on one branch). A two-MR split was considered but rejected: between the MRs, an instance that had explicitly disabled a flag would see a settings field in the admin UI that has no effect on the API endpoints. Shipping both commits together avoids that inconsistent state, and the backfill migration and code switch must ship in the same milestone anyway.

Before merging, verify via chatops that GitLab.com has both flags in default state (/chatops run feature get <flag>); if a flag was explicitly disabled there, the backfill writes 0 on GitLab.com and that should be a conscious decision. (Checked 2026-07-13: web_hook_test_api_endpoint_rate_limit does not exist on gprd, so the backfill writes nothing there.)

Commit 1: add the settings (no behaviour change)

The defaults match the current hardcoded values and the flag guards stay in place, so this commit is safe on its own.

  1. Add both settings to rate_limits_definition in app/models/application_setting.rb ([:integer, { default: 5 }]) and to the rate-limit attribute allow-list in the same file.
  2. Add both entries to app/validators/json_schemas/application_setting_rate_limits.json (type: integer, minimum: 0). The schema has additionalProperties: false, so this is required.
  3. Add both to visible_attributes in app/helpers/application_settings_helper.rb. This also exposes them through PUT /application/settings.
  4. Wire the thresholds in lib/gitlab/application_rate_limiter.rb: replace threshold: 5 with threshold: -> { application_settings.web_hook_event_resend_limit } (and the test equivalent).
  5. Update the matching entries in lib/gitlab/application_rate_limiter/labkit_adapter/supported_rate_limits.rb to use the same lambdas. The parity spec requires both registries to agree.
  6. Admin UI: new "Webhook rate limits" section on Admin > Settings > Network (app/views/admin/application_settings/network.html.haml), with a partial modelled on _users_api_limits.html.haml: two number fields, min: 0, and the usual "Set to 0 to disable rate limits." text.
  7. Docs: new doc/administration/settings/rate_limit_on_webhook_operations.md page (matching the existing per-limit page convention), plus two rows and the example payload in doc/api/settings.md.
  8. Specs: model spec (rate_limits sections in spec/models/application_setting_spec.rb), helper spec, parity spec.

Commit 2: replace the flags

  1. Remove the Feature.enabled? guards from lib/api/hooks/resend_hook.rb and lib/api/hooks/trigger_test.rb, keeping the bare check_rate_limit! calls. Update the desc/detail strings, which currently name the flags.

  2. Delete config/feature_flags/ops/web_hook_event_resend_api_endpoint_rate_limit.yml and config/feature_flags/ops/web_hook_test_api_endpoint_rate_limit.yml.

  3. Backfill migration (regular migration, restrict_gitlab_migration gitlab_schema: :gitlab_main): for each flag, if the flag was explicitly disabled with a boolean=false gate, write 0 to the corresponding rate_limits key on the application_settings row; otherwise write nothing and let the default of 5 apply. That covers flags never set, flags explicitly enabled, and flags with percentage or actor gates — a partial gate on a default-enabled flag means the limit was mostly active, so keeping it active is the safer fallback. The stock up_migrate_to_jsonb_setting helper only writes booleans, so the migration checks the feature_gates table and writes the integer itself with the same jsonb_set SQL. down removes the keys.

  4. Migration spec covering: flag never set (no write), flag explicitly enabled (no write), flag with a percentage/actor gate (no write), flag explicitly disabled (writes 0), only one flag disabled (writes 0 for that one), rollback.

  5. Update the rate limit contexts in spec/support/shared_examples/requests/api/hooks_shared_examples.rb: replace the "when ops flag is disabled" contexts with "when the limit is 0" contexts, and add a custom-threshold example.

  6. Docs: update doc/api/project_webhooks.md and doc/api/group_webhooks.md (they currently tell admins to disable the flags), and the Webhook Testing section in doc/security/rate_limits.md.

    While doing this, also fix a scope inaccuracy: doc/api/project_webhooks.md says the resend limit applies "for each project webhook and authenticated user", but the actual scope is [hook.parent, current_user], so the counter is shared by all webhooks in the same project. The group webhooks page ("for each group and authenticated user") is correct. Use the "per user, for a given project or group" wording established in commit 1.

  7. Also regenerate the OpenAPI docs (doc/api/openapi/openapi_v{2,3}.yaml) since the endpoint detail strings and settings params changed: bundle exec rake gitlab:openapi:v2:generate gitlab:openapi:v3:generate.

  8. Changelog: added on commit 1, changed on commit 2.

Related

Edited by 🤖 GitLab Bot 🤖