Don't execute webhooks/services when above limit

If a single push has a number of changes over the threshold
(`push_event_hooks_limit` setting, 3 by default), the related
project webhooks and services won't be executed.

This way, in an event where bulk changes are pushed, the system
won't be overloaded.

app/helpers/application_settings_helper.rb

@@ -289,7 +289,8 @@ module ApplicationSettingsHelper
       :snowplow_collector_hostname,
       :snowplow_cookie_domain,
       :snowplow_enabled,
-      :snowplow_site_id
+      :snowplow_site_id,
+      :push_event_hooks_limit
     ]
   end
 

app/models/application_setting.rb

@@ -214,6 +214,9 @@ class ApplicationSetting < ApplicationRecord
             length: { maximum: 100, message: N_('is too long (maximum is 100 entries)') },
             allow_nil: false
 
+  validates :push_event_hooks_limit,
+            numericality: { greater_than_or_equal_to: 0 }
+
   SUPPORTED_KEY_TYPES.each do |type|
     validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
   end

app/models/application_setting_implementation.rb

@@ -82,6 +82,7 @@ module ApplicationSettingImplementation
         polling_interval_multiplier: 1,
         project_export_enabled: true,
         protected_ci_variables: false,
+        push_event_hooks_limit: 3,
         raw_blob_request_limit: 300,
         recaptcha_enabled: false,
         login_recaptcha_protection_enabled: false,

app/services/git/base_hooks_service.rb

@@ -62,6 +62,8 @@ module Git
     end
 
     def execute_project_hooks
+      return unless params.fetch(:execute_project_hooks, true)
+
       # Creating push_data invokes one CommitDelta RPC per commit. Only
       # build this data if we actually need it.
       project.execute_hooks(push_data, hook_name) if project.has_active_hooks?(hook_name)

app/services/git/process_ref_changes_service.rb

@@ -15,9 +15,10 @@ module Git
 
     def process_changes_by_action(ref_type, changes)
       changes_by_action = group_changes_by_action(changes)
+      execute_project_hooks = changes.size <= Gitlab::CurrentSettings.push_event_hooks_limit
 
       changes_by_action.each do |_, changes|
-        process_changes(ref_type, changes) if changes.any?
+        process_changes(ref_type, changes, execute_project_hooks: execute_project_hooks) if changes.any?
       end
     end
 
@@ -34,7 +35,7 @@ module Git
       :pushed
     end
 
-    def process_changes(ref_type, changes)
+    def process_changes(ref_type, changes, execute_project_hooks: true)
       push_service_class = push_service_class_for(ref_type)
 
       changes.each do |change|
@@ -43,7 +44,8 @@ module Git
           current_user,
           change: change,
           push_options: params[:push_options],
-          create_pipelines: change[:index] < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, project)
+          create_pipelines: change[:index] < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, project),
+          execute_project_hooks: execute_project_hooks
         ).execute
       end
     end

app/views/admin/application_settings/_performance.html.haml

@@ -20,5 +20,10 @@
       = f.number_field :raw_blob_request_limit, class: 'form-control'
       .form-text.text-muted
         = _('Highest number of requests per minute for each raw path, default to 300. To disable throttling set to 0.')
+    .form-group
+      = f.label :push_event_hooks_limit, class: 'label-bold'
+      = f.number_field :push_event_hooks_limit, class: 'form-control'
+      .form-text.text-muted
+        = _("Number of changes (branches or tags) in a single push to determine whether webhooks and services will be fired or not. Webhooks and services won't be submitted if it surpasses that value.")
 
   = f.submit 'Save changes', class: "btn btn-success"

db/migrate/20191003060227_add_push_event_hooks_limit_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddPushEventHooksLimitToApplicationSettings < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:application_settings, :push_event_hooks_limit, :integer, default: 3)
+  end
+
+  def down
+    remove_column(:application_settings, :push_event_hooks_limit)
+  end
+end

db/schema.rb

@@ -338,6 +338,7 @@ ActiveRecord::Schema.define(version: 2019_10_16_072826) do
     t.boolean "throttle_incident_management_notification_enabled", default: false, null: false
     t.integer "throttle_incident_management_notification_period_in_seconds", default: 3600
     t.integer "throttle_incident_management_notification_per_period", default: 3600
+    t.integer "push_event_hooks_limit", default: 3, null: false
     t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
     t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
     t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"

lib/api/settings.rb

@@ -101,6 +101,7 @@ module API
       optional :polling_interval_multiplier, type: BigDecimal, desc: 'Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.'
       optional :project_export_enabled, type: Boolean, desc: 'Enable project export'
       optional :prometheus_metrics_enabled, type: Boolean, desc: 'Enable Prometheus metrics'
+      optional :push_event_hooks_limit, type: Integer, desc: "Number of changes (branches or tags) in a single push to determine whether webhooks and services will be fired or not. Webhooks and services won't be submitted if it surpasses that value."
       optional :recaptcha_enabled, type: Boolean, desc: 'Helps prevent bots from creating accounts'
       given recaptcha_enabled: ->(val) { val } do
         requires :recaptcha_site_key, type: String, desc: 'Generate site key at http://www.google.com/recaptcha'

locale/gitlab.pot

@@ -11172,6 +11172,9 @@ msgstr ""
 msgid "Number of LOCs per commit"
 msgstr ""
 
+msgid "Number of changes (branches or tags) in a single push to determine whether webhooks and services will be fired or not. Webhooks and services won't be submitted if it surpasses that value."
+msgstr ""
+
 msgid "Number of commits per MR"
 msgstr ""
 

spec/models/application_setting_spec.rb

@@ -56,6 +56,10 @@ describe ApplicationSetting do
     it { is_expected.not_to allow_value(nil).for(:protected_paths) }
     it { is_expected.to allow_value([]).for(:protected_paths) }
 
+    it { is_expected.to allow_value(3).for(:push_event_hooks_limit) }
+    it { is_expected.not_to allow_value('three').for(:push_event_hooks_limit) }
+    it { is_expected.not_to allow_value(nil).for(:push_event_hooks_limit) }
+
     context "when user accepted let's encrypt terms of service" do
       before do
         setting.update(lets_encrypt_terms_of_service_accepted: true)

spec/requests/api/settings_spec.rb

@@ -72,7 +72,8 @@ describe API::Settings, 'Settings' do
             default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE,
             local_markdown_version: 3,
             allow_local_requests_from_web_hooks_and_services: true,
-            allow_local_requests_from_system_hooks: false
+            allow_local_requests_from_system_hooks: false,
+            push_event_hooks_limit: 2
           }
 
         expect(response).to have_gitlab_http_status(200)
@@ -102,6 +103,7 @@ describe API::Settings, 'Settings' do
         expect(json_response['local_markdown_version']).to eq(3)
         expect(json_response['allow_local_requests_from_web_hooks_and_services']).to eq(true)
         expect(json_response['allow_local_requests_from_system_hooks']).to eq(false)
+        expect(json_response['push_event_hooks_limit']).to eq(2)
       end
     end
 

spec/services/git/base_hooks_service_spec.rb

@@ -8,7 +8,6 @@ describe Git::BaseHooksService do
 
   let(:user) { create(:user) }
   let(:project) { create(:project, :repository) }
-
   let(:oldrev) { Gitlab::Git::BLANK_SHA }
   let(:newrev) { "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b" } # gitlab-test: git rev-parse refs/tags/v1.1.0
   let(:ref) { 'refs/tags/v1.1.0' }
@@ -26,7 +25,17 @@ describe Git::BaseHooksService do
 
     let(:project) { create(:project, :repository) }
 
-    subject { TestService.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }) }
+    let(:params) do
+      {
+        change: {
+          oldrev: oldrev,
+          newrev: newrev,
+          ref: ref
+        }
+      }
+    end
+
+    subject { TestService.new(project, user, params) }
 
     context '#execute_hooks' do
       before do
@@ -83,5 +92,21 @@ describe Git::BaseHooksService do
         end
       end
     end
+
+    context 'execute_project_hooks param set to false' do
+      before do
+        params[:execute_project_hooks] = false
+
+        allow(project).to receive(:has_active_hooks?).and_return(true)
+        allow(project).to receive(:has_active_services?).and_return(true)
+      end
+
+      it 'does not execute hooks and services' do
+        expect(project).not_to receive(:execute_hooks)
+        expect(project).not_to receive(:execute_services)
+
+        subject.execute
+      end
+    end
   end
 end

spec/services/git/process_ref_changes_service_spec.rb

@@ -28,12 +28,44 @@ describe Git::ProcessRefChangesService do
     it "calls #{push_service_class}" do
       expect(push_service_class)
         .to receive(:new)
+        .with(project, project.owner, hash_including(execute_project_hooks: true))
         .exactly(changes.count).times
         .and_return(service)
 
       subject.execute
     end
 
+    context 'changes exceed push_event_hooks_limit' do
+      def multiple_changes(change, count)
+        Array.new(count).map.with_index do |n, index|
+          { index: index, oldrev: change[:oldrev], newrev: change[:newrev], ref: "#{change[:ref]}#{n}" }
+        end
+      end
+
+      let(:push_event_hooks_limit) { 3 }
+
+      let(:changes) do
+        multiple_changes(
+          { oldrev: '123456', newrev: '789012', ref: "#{ref_prefix}/test" },
+          push_event_hooks_limit + 1
+        )
+      end
+
+      before do
+        stub_application_setting(push_event_hooks_limit: push_event_hooks_limit)
+      end
+
+      it "calls #{push_service_class} with execute_project_hooks set to false" do
+        expect(push_service_class)
+          .to receive(:new)
+          .with(project, project.owner, hash_including(execute_project_hooks: false))
+          .exactly(changes.count).times
+          .and_return(service)
+
+        subject.execute
+      end
+    end
+
     context 'pipeline creation' do
       context 'with valid .gitlab-ci.yml' do
         before do

spec/workers/post_receive_spec.rb

@@ -93,6 +93,8 @@ describe PostReceive do
     end
 
     context 'with changes' do
+      let(:push_service) { double(execute: true) }
+
       before do
         allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner)
         allow(Gitlab::GlRepository).to receive(:parse).and_return([project, Gitlab::GlRepository::PROJECT])