Skip to content

Allow self-managed to prevent auto disabling hooks

Luke Duncalfe requested to merge 390157-auto-disabling-webhooks-ops-flag into master

What does this MR do and why?

This merge request adds an ops flag to make auto-disabling hooks an opt-in feature. It will allow self-managed to prevent the auto-disabling webhooks feature. By default the feature will be disabled for self-managed #390157 (closed).

Refactoring

The auto-disabling code has undergone a bit of refactoring in this change. The reason is that we previously had two modules:

  • WebHooks::Autodisabling
  • WebHooks::Unstoppable

A webhook model (one that inherits from WebHook) would mix in one of those modules to determine whether auto-disabling applied to the webhook or not.

We're now introducing the idea of a webhook that supports the auto-disabling feature, still not auto-disabling if the ops flag is disabled.

To avoid the codebase getting quite confusing, this change consolidates the two modules into one, with that one module deciding whether to apply the auto-disabling behaviour or not. For auto-disabling to be enabled, support for hook type needs to be defined in a const, and the flag must be toggled on.

It also moves some auto-disabling methods out of WebHook and into the one module, again, mostly to simplify the ability to rationalise the feature and discover the code.

Follow-up merge requests

Auto-disabling webhook behaviour is quite intricate and has been the source of a number of bugs in the past when the behaviour is not 100% correct.

I'm proposing that we merge the change in separate pieces.

  1. This change
  2. Related constants in WebHook !113479 (comment 1308304278).
  3. Related tests of methods being removed from WebHook !113479 (comment 1308319756).

The idea is to allow us at every step to have a good degree of confidence that the behaviours of the feature are unchanged.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #390157 (closed)

Edited by Luke Duncalfe

Merge request reports

Loading