From 659d5d48303c4550a2c4ef7cc938d17b5b45d4c9 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 3 Jun 2016 15:30:38 -0300 Subject: [PATCH 1/3] Disable Webhooks before proceeding with the GitHub import --- lib/gitlab/github_import/hook_formatter.rb | 23 +++++++ lib/gitlab/github_import/importer.rb | 26 ++++++-- .../github_import/hook_formatter_spec.rb | 65 +++++++++++++++++++ 3 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 lib/gitlab/github_import/hook_formatter.rb create mode 100644 spec/lib/gitlab/github_import/hook_formatter_spec.rb 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 9d077e79c39..860ced5474c 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -69,6 +69,9 @@ module Gitlab end def import_pull_requests + hooks = client.hooks(repo).map { |raw| HookFormatter.new(raw) }.select(&:valid?) + disable_webhooks(hooks) + pull_requests = client.pull_requests(repo, state: :all, sort: :created, direction: :asc) .map { |raw| PullRequestFormatter.new(project, raw) } .select(&:valid?) @@ -77,7 +80,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 = MergeRequest.new(pull_request.attributes) @@ -93,10 +96,25 @@ module Gitlab rescue ActiveRecord::RecordInvalid => e raise Projects::ImportService::Error, e.message ensure - delete_refs(branches_removed) + clean_up_restored_branches(branches_removed) + clean_up_disabled_webhooks(hooks) end - def create_refs(branches) + 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 restore_branches(branches) branches.each do |name, sha| client.create_ref(repo, "refs/heads/#{name}", sha) end @@ -104,7 +122,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, _| client.delete_ref(repo, "heads/#{name}") project.repository.rm_branch(project.creator, 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 From 17152602b05021b90261fc8f82024e323724d458 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 3 Jun 2016 15:31:34 -0300 Subject: [PATCH 2/3] Add a message warning user that Webhooks will be disabled --- app/views/import/github/status.html.haml | 4 ++++ 1 file changed, 4 insertions(+) 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 From 1fb1e64fdd3c608f265a59607c748ba4a999d928 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 6 Jun 2016 11:28:20 -0300 Subject: [PATCH 3/3] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index fe9b9bec868..e44ec023959 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -45,6 +45,7 @@ v 8.8.4 (unreleased) - Fix todos page throwing errors when you have a project pending deletion - Reduce number of SQL queries when rendering user references - Upgrade to jQuery 2 + - Disable Webhooks before proceeding with the GitHub import v 8.8.3 - Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312