Commit ef11b418 authored by Stan Hu's avatar Stan Hu

Fix GitLab plugins not working without hooks configured

https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/31741, shipped
in GitLab 12.2, reduced Gitaly RPCs by generating commit data only if
hooks or services were actually active. However, GitLab plugins
(https://docs.gitlab.com/ee/administration/plugins.html), are executable
programs that get launched inside `SystemHooksService#execute_hooks`
regardless of whether any hooks are actually active. As a result of this
change, users have to configure a Webhook for plugins to fire.

We restore the previous behavior by checking whether plugins are
available and executing `SystemHooksService#execute_hooks` if any
plugins are available.

Closes https://gitlab.com/gitlab-org/gitlab/issues/194131
parent 637c65be
......@@ -1318,7 +1318,7 @@ class Project < ApplicationRecord
end
def has_active_hooks?(hooks_scope = :push_hooks)
hooks.hooks_for(hooks_scope).any? || SystemHook.hooks_for(hooks_scope).any?
hooks.hooks_for(hooks_scope).any? || SystemHook.hooks_for(hooks_scope).any? || Gitlab::Plugin.any?
end
def has_active_services?(hooks_scope = :push_hooks)
......
---
title: Fix GitLab plugins not working without hooks configured
merge_request: 22409
author:
type: fixed
......@@ -2,10 +2,16 @@
module Gitlab
module Plugin
def self.any?
plugin_glob.any? { |entry| File.file?(entry) }
end
def self.files
Dir.glob(Rails.root.join('plugins/*')).select do |entry|
File.file?(entry)
end
plugin_glob.select { |entry| File.file?(entry) }
end
def self.plugin_glob
Dir.glob(Rails.root.join('plugins/*'))
end
def self.execute_all_async(data)
......
......@@ -3,22 +3,59 @@
require 'spec_helper'
describe Gitlab::Plugin do
let(:plugin) { Rails.root.join('plugins', 'test.rb') }
let(:tmp_file) { Tempfile.new('plugin-dump') }
let(:plugin_source) do
<<~EOS
#!/usr/bin/env ruby
x = STDIN.read
File.write('#{tmp_file.path}', x)
EOS
end
context 'with plugins present' do
before do
File.write(plugin, plugin_source)
end
after do
FileUtils.rm(plugin)
end
describe '.any?' do
it 'returns true' do
expect(described_class.any?).to be true
end
end
describe '.files?' do
it 'returns a list of plugins' do
expect(described_class.files).to match_array([plugin.to_s])
end
end
end
context 'without any plugins' do
describe '.any?' do
it 'returns false' do
expect(described_class.any?).to be false
end
end
describe '.files' do
it 'returns an empty list' do
expect(described_class.files).to be_empty
end
end
end
describe '.execute' do
let(:data) { Gitlab::DataBuilder::Push::SAMPLE_DATA }
let(:plugin) { Rails.root.join('plugins', 'test.rb') }
let(:tmp_file) { Tempfile.new('plugin-dump') }
let(:result) { described_class.execute(plugin.to_s, data) }
let(:success) { result.first }
let(:message) { result.last }
let(:plugin_source) do
<<~EOS
#!/usr/bin/env ruby
x = STDIN.read
File.write('#{tmp_file.path}', x)
EOS
end
before do
File.write(plugin, plugin_source)
end
......
......@@ -4721,6 +4721,13 @@ describe Project do
expect(project.has_active_hooks?(:merge_request_events)).to be_falsey
expect(project.has_active_hooks?).to be_truthy
end
it 'returns true when a plugin exists' do
expect(Gitlab::Plugin).to receive(:any?).twice.and_return(true)
expect(project.has_active_hooks?(:merge_request_events)).to be_truthy
expect(project.has_active_hooks?).to be_truthy
end
end
describe '#has_active_services?' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment