Merge branch 'sh-limit-web-hook-backoffs' into 'master'

Limit updates to Web Hook backoff interval

See merge request gitlab-org/gitlab!69955

app/models/hooks/web_hook.rb

@@ -80,6 +80,8 @@ class WebHook < ApplicationRecord
   end
 
   def backoff!
+    return if backoff_count >= MAX_FAILURES && disabled_until && disabled_until > Time.current
+
     assign_attributes(disabled_until: next_backoff.from_now, backoff_count: backoff_count.succ.clamp(0, MAX_FAILURES))
     save(validate: false)
   end

spec/models/hooks/web_hook_spec.rb

@@ -10,8 +10,12 @@ RSpec.describe WebHook do
   let(:hook) { build(:project_hook, project: project) }
 
   around do |example|
+    if example.metadata[:skip_freeze_time]
+      example.run
+    else
       freeze_time { example.run }
     end
+  end
 
   describe 'associations' do
     it { is_expected.to have_many(:web_hook_logs) }
@@ -326,12 +330,30 @@ RSpec.describe WebHook do
       expect { hook.backoff! }.to change(hook, :backoff_count).by(1)
     end
 
-    it 'does not let the backoff count exceed the maximum failure count' do
-      hook.backoff_count = described_class::MAX_FAILURES
+    context 'when we have backed off MAX_FAILURES times' do
+      before do
+        stub_const("#{described_class}::MAX_FAILURES", 5)
+        5.times { hook.backoff! }
+      end
 
+      it 'does not let the backoff count exceed the maximum failure count' do
         expect { hook.backoff! }.not_to change(hook, :backoff_count)
       end
 
+      it 'does not change disabled_until', :skip_freeze_time do
+        travel_to(hook.disabled_until - 1.minute) do
+          expect { hook.backoff! }.not_to change(hook, :disabled_until)
+        end
+      end
+
+      it 'changes disabled_until when it has elapsed', :skip_freeze_time do
+        travel_to(hook.disabled_until + 1.minute) do
+          expect { hook.backoff! }.to change { hook.disabled_until }
+          expect(hook.backoff_count).to eq(described_class::MAX_FAILURES)
+        end
+      end
+    end
+
     include_examples 'is tolerant of invalid records' do
       def run_expectation
         expect { hook.backoff! }.to change(hook, :backoff_count).by(1)