Merge branch 'gh-disable-webhooks'

5 files changed, 116 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index adab410f58c..e44ee2b518c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 8.8.5
   - Import GitHub repositories respecting the API rate limit
   - Fix todos page throwing errors when you have a project pending deletion
+  - Disable Webhooks before proceeding with the GitHub import
 
 v 8.8.4
   - Fix LDAP-based login for users with 2FA enabled. !4493
diff --git a/app/views/import/github/status.html.haml b/app/views/import/github/status.html.haml
index 5b7f11440c1..6c4a9d68d1f 100644
--- a/app/views/import/github/status.html.haml
+++ b/app/views/import/github/status.html.haml
@@ -4,6 +4,10 @@
   %i.fa.fa-github
   Import projects from GitHub
 
+%p
+  %i.fa.fa-warning
+  To import GitHub pull requests, any pull request source branches that had been deleted are temporarily restored on GitHub. To prevent any connected CI services from being overloaded with dozens of irrelevant branches being created and deleted again, GitHub webhooks are temporarily disabled during the import process.
+
 %p.light
   Select projects you want to import.
 %hr
diff --git a/lib/gitlab/github_import/hook_formatter.rb b/lib/gitlab/github_import/hook_formatter.rb
new file mode 100644
index 00000000000..db1fabaa18a
--- /dev/null
+++ b/lib/gitlab/github_import/hook_formatter.rb
@@ -0,0 +1,23 @@
+module Gitlab
+  module GithubImport
+    class HookFormatter
+      EVENTS = %w[* create delete pull_request push].freeze
+
+      attr_reader :raw
+
+      delegate :id, :name, :active, to: :raw
+
+      def initialize(raw)
+        @raw = raw
+      end
+
+      def config
+        raw.config.attrs
+      end
+
+      def valid?
+        (EVENTS & raw.events).any? && active
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb
index 6d263fd8a2f..442b4c389fe 100644
--- a/lib/gitlab/github_import/importer.rb
+++ b/lib/gitlab/github_import/importer.rb
@@ -109,6 +109,9 @@ module Gitlab
       end
 
       def import_pull_requests
+        hooks = client.hooks(repo).map { |raw| HookFormatter.new(raw) }.select(&:valid?)
+        disable_webhooks(hooks)
+
         pull_requests = paginate { client.pull_requests(repo, state: :all, sort: :created, direction: :asc, per_page: 100) }
         pull_requests = pull_requests.map { |raw| PullRequestFormatter.new(project, raw) }.select(&:valid?)
 
@@ -116,7 +119,7 @@ module Gitlab
         target_branches_removed = pull_requests.reject(&:target_branch_exists?).map { |pr| [pr.target_branch_name, pr.target_branch_sha] }
         branches_removed = source_branches_removed | target_branches_removed
 
-        create_refs(branches_removed)
+        restore_branches(branches_removed)
 
         pull_requests.each do |pull_request|
           merge_request = pull_request.create!
@@ -125,14 +128,29 @@ module Gitlab
           import_comments_on_diff(merge_request)
         end
 
-        delete_refs(branches_removed)
-
         true
       rescue ActiveRecord::RecordInvalid => e
         raise Projects::ImportService::Error, e.message
+      ensure
+        clean_up_restored_branches(branches_removed)
+        clean_up_disabled_webhooks(hooks)
+      end
+
+      def disable_webhooks(hooks)
+        update_webhooks(hooks, active: false)
+      end
+
+      def clean_up_disabled_webhooks(hooks)
+        update_webhooks(hooks, active: true)
+      end
+
+      def update_webhooks(hooks, options)
+        hooks.each do |hook|
+          client.edit_hook(repo, hook.id, hook.name, hook.config, options)
+        end
       end
 
-      def create_refs(branches)
+      def restore_branches(branches)
         branches.each do |name, sha|
           sleep rate_limit_sleep_time if rate_limit_exceed?
           client.create_ref(repo, "refs/heads/#{name}", sha)
@@ -141,7 +159,7 @@ module Gitlab
         project.repository.fetch_ref(repo_url, '+refs/heads/*', 'refs/heads/*')
       end
 
-      def delete_refs(branches)
+      def clean_up_restored_branches(branches)
         branches.each do |name, _|
           sleep rate_limit_sleep_time if rate_limit_exceed?
           client.delete_ref(repo, "heads/#{name}")
diff --git a/spec/lib/gitlab/github_import/hook_formatter_spec.rb b/spec/lib/gitlab/github_import/hook_formatter_spec.rb
new file mode 100644
index 00000000000..110ba428258
--- /dev/null
+++ b/spec/lib/gitlab/github_import/hook_formatter_spec.rb
@@ -0,0 +1,65 @@
+require 'spec_helper'
+
+describe Gitlab::GithubImport::HookFormatter, lib: true do
+  describe '#id' do
+    it 'returns raw id' do
+      raw = double(id: 100000)
+      formatter = described_class.new(raw)
+      expect(formatter.id).to eq 100000
+    end
+  end
+
+  describe '#name' do
+    it 'returns raw id' do
+      raw = double(name: 'web')
+      formatter = described_class.new(raw)
+      expect(formatter.name).to eq 'web'
+    end
+  end
+
+  describe '#config' do
+    it 'returns raw config.attrs' do
+      raw = double(config: double(attrs: { url: 'http://something.com/webhook' }))
+      formatter = described_class.new(raw)
+      expect(formatter.config).to eq({ url: 'http://something.com/webhook' })
+    end
+  end
+
+  describe '#valid?' do
+    it 'returns true when events contains the wildcard event' do
+      raw = double(events: ['*', 'commit_comment'], active: true)
+      formatter = described_class.new(raw)
+      expect(formatter.valid?).to eq true
+    end
+
+    it 'returns true when events contains the create event' do
+      raw = double(events: ['create', 'commit_comment'], active: true)
+      formatter = described_class.new(raw)
+      expect(formatter.valid?).to eq true
+    end
+
+    it 'returns true when events contains delete event' do
+      raw = double(events: ['delete', 'commit_comment'], active: true)
+      formatter = described_class.new(raw)
+      expect(formatter.valid?).to eq true
+    end
+
+    it 'returns true when events contains pull_request event' do
+      raw = double(events: ['pull_request', 'commit_comment'], active: true)
+      formatter = described_class.new(raw)
+      expect(formatter.valid?).to eq true
+    end
+
+    it 'returns false when events does not contains branch related events' do
+      raw = double(events: ['member', 'commit_comment'], active: true)
+      formatter = described_class.new(raw)
+      expect(formatter.valid?).to eq false
+    end
+
+    it 'returns false when hook is not active' do
+      raw = double(events: ['pull_request', 'commit_comment'], active: false)
+      formatter = described_class.new(raw)
+      expect(formatter.valid?).to eq false
+    end
+  end
+end