diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2018-02-28 12:16:23 +0200 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2018-02-28 12:16:23 +0200 |
commit | 2577ff7fd656f301c4b1909f0a1c358c8c30a614 (patch) | |
tree | a7c04e89e47df5610fdd189597c58d48f9075e76 | |
parent | 4eed9a12216296709306ce29faf607d8aed2a913 (diff) | |
download | gitlab-ce-2577ff7fd656f301c4b1909f0a1c358c8c30a614.tar.gz |
Refactor plugins feature and make some doc improvements
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
-rw-r--r-- | app/workers/plugin_worker.rb | 8 | ||||
-rw-r--r-- | doc/administration/plugins.md | 5 | ||||
-rw-r--r-- | lib/gitlab/plugin.rb | 10 | ||||
-rw-r--r-- | lib/gitlab/plugin_logger.rb | 8 | ||||
-rw-r--r-- | lib/tasks/plugins.rake | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/plugin_spec.rb | 16 | ||||
-rw-r--r-- | spec/workers/plugin_worker_spec.rb | 14 |
7 files changed, 44 insertions, 25 deletions
diff --git a/app/workers/plugin_worker.rb b/app/workers/plugin_worker.rb index 34a3c8d62ac..bfcc683d99a 100644 --- a/app/workers/plugin_worker.rb +++ b/app/workers/plugin_worker.rb @@ -4,6 +4,12 @@ class PluginWorker sidekiq_options retry: false def perform(file_name, data) - Gitlab::Plugin.execute(file_name, data) + success, message = Gitlab::Plugin.execute(file_name, data) + + unless success + Gitlab::PluginLogger.error("Plugin Error => #{file_name}: #{message}") + end + + true end end diff --git a/doc/administration/plugins.md b/doc/administration/plugins.md index 89ab0b9c8ea..ed1a3480ffc 100644 --- a/doc/administration/plugins.md +++ b/doc/administration/plugins.md @@ -11,7 +11,7 @@ A plugin will run on each event so it's up to you to filter events or projects w ## Setup -Plugins must be placed directly into `plugins` directory, subdirectories will be ignored. There is an `example` directory insider `plugins` where you can find some basic examples. +Plugins must be placed directly into `plugins` directory, subdirectories will be ignored. There is an `example` directory inside `plugins` where you can find some basic examples. Follow the steps below to set up a custom hook: @@ -20,11 +20,12 @@ Follow the steps below to set up a custom hook: `/home/git/gitlab/plugins/`. For Omnibus installs the path is usually `/opt/gitlab/embedded/service/gitlab-rails/plugins`. 1. Inside the `plugins` directory, create a file with a name of your choice, but without spaces or special characters. -1. Make the hook file executable and make sure it's owned by git. +1. Make the hook file executable and make sure it's owned by the git user. 1. Write the code to make the plugin function as expected. Plugin can be in any language. Ensure the 'shebang' at the top properly reflects the language type. For example, if the script is in Ruby the shebang will probably be `#!/usr/bin/env ruby`. +1. The data to the plugin will be provided as JSON on STDIN. It will be exactly same as one for [system hooks] That's it! Assuming the plugin code is properly implemented the hook will fire as appropriate. Plugins file list is updated for each event. There is no need to restart GitLab to apply a new plugin. diff --git a/lib/gitlab/plugin.rb b/lib/gitlab/plugin.rb index 96332362a4c..0d1cb16b378 100644 --- a/lib/gitlab/plugin.rb +++ b/lib/gitlab/plugin.rb @@ -18,15 +18,9 @@ module Gitlab end exit_status = result.status&.exitstatus - - unless exit_status.zero? - Rails.logger.error("Plugin Error => #{file}: #{result.stderr}") - end - - exit_status.zero? + [exit_status.zero?, result.stderr] rescue => e - Rails.logger.error("Plugin Error => #{file}: #{e.message}") - false + [false, e.message] end end end diff --git a/lib/gitlab/plugin_logger.rb b/lib/gitlab/plugin_logger.rb new file mode 100644 index 00000000000..a106a2677ed --- /dev/null +++ b/lib/gitlab/plugin_logger.rb @@ -0,0 +1,8 @@ +module Gitlab + class PluginLogger < Gitlab::Logger + def self.file_name_noext + 'plugin' + end + end +end + diff --git a/lib/tasks/plugins.rake b/lib/tasks/plugins.rake index 11c41f13614..7a9de3afbec 100644 --- a/lib/tasks/plugins.rake +++ b/lib/tasks/plugins.rake @@ -4,12 +4,12 @@ namespace :plugins do puts 'Validating plugins from /plugins directory' Gitlab::Plugin.files.each do |file| - result = Gitlab::Plugin.execute(file, Gitlab::DataBuilder::Push::SAMPLE_DATA) + success, message = Gitlab::Plugin.execute(file, Gitlab::DataBuilder::Push::SAMPLE_DATA) - if result - puts "* #{file} succeed (zero exit code)" + if success + puts "* #{file} succeed (zero exit code)." else - puts "* #{file} failure (non-zero exit code)" + puts "* #{file} failure (non-zero exit code). #{message}" end end end diff --git a/spec/lib/gitlab/plugin_spec.rb b/spec/lib/gitlab/plugin_spec.rb index a01e1383e3b..33dd4f79130 100644 --- a/spec/lib/gitlab/plugin_spec.rb +++ b/spec/lib/gitlab/plugin_spec.rb @@ -5,6 +5,9 @@ describe Gitlab::Plugin 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 @@ -22,8 +25,6 @@ describe Gitlab::Plugin do FileUtils.rm(plugin) end - subject { described_class.execute(plugin.to_s, data) } - context 'successful execution' do before do File.chmod(0o777, plugin) @@ -33,17 +34,19 @@ describe Gitlab::Plugin do tmp_file.close! end - it { is_expected.to be true } + it { expect(success).to be true } + it { expect(message).to be_empty } it 'ensures plugin received data via stdin' do - subject + result expect(File.read(tmp_file.path)).to eq(data.to_json) end end context 'non-executable' do - it { is_expected.to be false } + it { expect(success).to be false } + it { expect(message).to include('Permission denied') } end context 'non-zero exit' do @@ -58,7 +61,8 @@ describe Gitlab::Plugin do File.chmod(0o777, plugin) end - it { is_expected.to be false } + it { expect(success).to be false } + it { expect(message).to be_empty } end end end diff --git a/spec/workers/plugin_worker_spec.rb b/spec/workers/plugin_worker_spec.rb index 60631def8f3..9238a8199bc 100644 --- a/spec/workers/plugin_worker_spec.rb +++ b/spec/workers/plugin_worker_spec.rb @@ -3,16 +3,22 @@ require 'spec_helper' describe PluginWorker do include RepoHelpers - subject { described_class.new } - let(:filename) { 'my_plugin.rb' } + let(:data) { { 'event_name' => 'project_create' } } + + subject { described_class.new } describe '#perform' do it 'executes Gitlab::Plugin with expected values' do - data = { 'event_name' => 'project_create' } + allow(Gitlab::Plugin).to receive(:execute).with(filename, data).and_return([true, '']) + + expect(subject.perform(filename, data)).to be_truthy + end - allow(Gitlab::Plugin).to receive(:execute).with(filename, data).and_return(true) + it 'logs message in case of plugin execution failure' do + allow(Gitlab::Plugin).to receive(:execute).with(filename, data).and_return([false, 'permission denied']) + expect(Gitlab::PluginLogger).to receive(:error) expect(subject.perform(filename, data)).to be_truthy end end |