Commit eca72fa1 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch '330817-the-webhook-recent_failures-counter-may-overflow' into 'master'

Resolve "The WebHook `recent_failures` counter may overflow"

See merge request gitlab-org/gitlab!61686
parents a7d0e3f6 56b837de
...@@ -27,6 +27,7 @@ class WebHookService ...@@ -27,6 +27,7 @@ class WebHookService
REQUEST_BODY_SIZE_LIMIT = 25.megabytes REQUEST_BODY_SIZE_LIMIT = 25.megabytes
GITLAB_EVENT_HEADER = 'X-Gitlab-Event' GITLAB_EVENT_HEADER = 'X-Gitlab-Event'
MAX_FAILURES = 100
attr_accessor :hook, :data, :hook_name, :request_options attr_accessor :hook, :data, :hook_name, :request_options
...@@ -141,7 +142,7 @@ class WebHookService ...@@ -141,7 +142,7 @@ class WebHookService
next_backoff = hook.next_backoff next_backoff = hook.next_backoff
hook.update!(disabled_until: next_backoff.from_now, backoff_count: hook.backoff_count + 1) hook.update!(disabled_until: next_backoff.from_now, backoff_count: hook.backoff_count + 1)
else else
hook.update!(recent_failures: hook.recent_failures + 1) hook.update!(recent_failures: hook.recent_failures + 1) if hook.recent_failures < MAX_FAILURES
end end
end end
......
...@@ -240,6 +240,25 @@ RSpec.describe WebHookService do ...@@ -240,6 +240,25 @@ RSpec.describe WebHookService do
it 'does not change the disabled_until attribute' do it 'does not change the disabled_until attribute' do
expect { service_instance.execute }.not_to change(project_hook, :disabled_until) expect { service_instance.execute }.not_to change(project_hook, :disabled_until)
end end
it 'does not allow the failure count to overflow' do
project_hook.update!(recent_failures: 32767)
expect { service_instance.execute }.not_to change(project_hook, :recent_failures)
end
context 'when the web_hooks_disable_failed FF is disabled' do
before do
# Hook will only be executed if the flag is disabled.
stub_feature_flags(web_hooks_disable_failed: false)
end
it 'does not allow the failure count to overflow' do
project_hook.update!(recent_failures: 32767)
expect { service_instance.execute }.not_to change(project_hook, :recent_failures)
end
end
end end
context 'with exception' do context 'with exception' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment