Fix gitlab:web_hook tasks

changelogs/unreleased/54975-fix-web-hooks-rake-task.yml

+---
+title: Fix gitlab:web_hook tasks
+merge_request: 23635
+author:
+type: fixed

doc/raketasks/web_hooks.md

@@ -38,8 +38,6 @@
 ## List the webhooks from projects in a given **NAMESPACE**:
 
     # omnibus-gitlab
-    sudo gitlab-rake gitlab:web_hook:list NAMESPACE=/
+    sudo gitlab-rake gitlab:web_hook:list NAMESPACE=acme
     # source installations
-    bundle exec rake gitlab:web_hook:list NAMESPACE=/ RAILS_ENV=production
-
-> Note: `/` is the global namespace.
+    bundle exec rake gitlab:web_hook:list NAMESPACE=acme RAILS_ENV=production

lib/tasks/gitlab/web_hook.rake

@@ -25,11 +25,22 @@ namespace :gitlab do
       web_hook_url = ENV['URL']
       namespace_path = ENV['NAMESPACE']
 
-      projects = find_projects(namespace_path)
-      project_ids = projects.pluck(:id)
+      web_hooks = find_web_hooks(namespace_path)
 
       puts "Removing webhooks with the url '#{web_hook_url}' ... "
-      count = WebHook.where(url: web_hook_url, project_id: project_ids, type: 'ProjectHook').delete_all
+
+      # FIXME: Hook URLs are now encrypted, so there is no way to efficiently
+      # find them all in SQL. For now, check them in Ruby. If this is too slow,
+      # we could consider storing a hash of the URL alongside the encrypted
+      # value to speed up searches
+      count = 0
+      web_hooks.find_each do |hook|
+        next unless hook.url == web_hook_url
+
+        hook.destroy!
+        count += 1
+      end
+
       puts "#{count} webhooks were removed."
     end
 
@@ -37,29 +48,37 @@ namespace :gitlab do
     task list: :environment do
       namespace_path = ENV['NAMESPACE']
 
-      projects = find_projects(namespace_path)
-      web_hooks = projects.all.map(&:hooks).flatten
-      web_hooks.each do |hook|
+      web_hooks = find_web_hooks(namespace_path)
+      web_hooks.find_each do |hook|
         puts "#{hook.project.name.truncate(20).ljust(20)} -> #{hook.url}"
       end
 
-      puts "\n#{web_hooks.size} webhooks found."
+      puts "\n#{web_hooks.count} webhooks found."
     end
   end
 
   def find_projects(namespace_path)
     if namespace_path.blank?
       Project
-    elsif namespace_path == '/'
-      Project.in_namespace(nil)
-    else
-      namespace = Namespace.where(path: namespace_path).first
-      if namespace
-        Project.in_namespace(namespace.id)
     else
+      namespace = Namespace.find_by_full_path(namespace_path)
+
+      unless namespace
         puts "Namespace not found: #{namespace_path}".color(:red)
         exit 2
       end
+
+      Project.in_namespace(namespace.id)
+    end
+  end
+
+  def find_web_hooks(namespace_path)
+    if namespace_path.blank?
+      ProjectHook
+    else
+      project_ids = find_projects(namespace_path).select(:id)
+
+      ProjectHook.where(project_id: project_ids)
     end
   end
 end

spec/tasks/gitlab/web_hook_rake_spec.rb

+require 'rake_helper'
+
+describe 'gitlab:web_hook namespace rake tasks' do
+  set(:group) { create(:group) }
+
+  set(:project1) { create(:project, namespace: group) }
+  set(:project2) { create(:project, namespace: group) }
+  set(:other_group_project) { create(:project) }
+
+  let(:url) { 'http://example.com' }
+  let(:hook_urls) { (project1.hooks + project2.hooks).map(&:url) }
+  let(:other_group_hook_urls) { other_group_project.hooks.map(&:url) }
+
+  before do
+    Rake.application.rake_require 'tasks/gitlab/web_hook'
+  end
+
+  describe 'gitlab:web_hook:add' do
+    it 'adds a web hook to all projects' do
+      stub_env('URL' => url)
+      run_rake_task('gitlab:web_hook:add')
+
+      expect(hook_urls).to contain_exactly(url, url)
+      expect(other_group_hook_urls).to contain_exactly(url)
+    end
+
+    it 'adds a web hook to projects in the specified namespace' do
+      stub_env('URL' => url, 'NAMESPACE' => group.full_path)
+      run_rake_task('gitlab:web_hook:add')
+
+      expect(hook_urls).to contain_exactly(url, url)
+      expect(other_group_hook_urls).to be_empty
+    end
+
+    it 'raises an error if an unknown namespace is specified' do
+      stub_env('URL' => url, 'NAMESPACE' => group.full_path)
+
+      group.destroy
+
+      expect { run_rake_task('gitlab:web_hook:add') }.to raise_error(SystemExit)
+    end
+  end
+
+  describe 'gitlab:web_hook:rm' do
+    let!(:hook1) { create(:project_hook, project: project1, url: url) }
+    let!(:hook2) { create(:project_hook, project: project2, url: url) }
+    let!(:other_group_hook) { create(:project_hook, project: other_group_project, url: url) }
+    let!(:other_url_hook) { create(:project_hook, url: other_url, project: project1) }
+
+    let(:other_url) { 'http://other.example.com' }
+
+    it 'removes a web hook from all projects by URL' do
+      stub_env('URL' => url)
+      run_rake_task('gitlab:web_hook:rm')
+
+      expect(hook_urls).to contain_exactly(other_url)
+      expect(other_group_hook_urls).to be_empty
+    end
+
+    it 'removes a web hook from projects in the specified namespace by URL' do
+      stub_env('NAMESPACE' => group.full_path, 'URL' => url)
+      run_rake_task('gitlab:web_hook:rm')
+
+      expect(hook_urls).to contain_exactly(other_url)
+      expect(other_group_hook_urls).to contain_exactly(url)
+    end
+
+    it 'raises an error if an unknown namespace is specified' do
+      stub_env('URL' => url, 'NAMESPACE' => group.full_path)
+
+      group.destroy
+
+      expect { run_rake_task('gitlab:web_hook:rm') }.to raise_error(SystemExit)
+    end
+  end
+
+  describe 'gitlab:web_hook:list' do
+    let!(:hook1) { create(:project_hook, project: project1) }
+    let!(:hook2) { create(:project_hook, project: project2) }
+    let!(:other_group_hook) { create(:project_hook, project: other_group_project) }
+
+    it 'lists all web hooks' do
+      expect { run_rake_task('gitlab:web_hook:list') }.to output(/3 webhooks found/).to_stdout
+    end
+
+    it 'lists web hooks in a particular namespace' do
+      stub_env('NAMESPACE', group.full_path)
+
+      expect { run_rake_task('gitlab:web_hook:list') }.to output(/2 webhooks found/).to_stdout
+    end
+  end
+end