From e6af20384b6c1a717fa1729a71d4634a308c14d2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 8 Feb 2018 17:00:09 +0200 Subject: [PATCH 01/24] Add plugins dir Signed-off-by: Dmitriy Zaporozhets --- plugins/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 plugins/.gitkeep diff --git a/plugins/.gitkeep b/plugins/.gitkeep new file mode 100644 index 00000000000..e69de29bb2d From 1ffa07e6f35fb7832ba8c6fd764da6c201e521bd Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 8 Feb 2018 17:00:25 +0200 Subject: [PATCH 02/24] Ignore content in plugins dir Signed-off-by: Dmitriy Zaporozhets --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 2004c2a09b4..fa39ae01ff0 100644 --- a/.gitignore +++ b/.gitignore @@ -66,3 +66,4 @@ eslint-report.html /locale/**/LC_MESSAGES /locale/**/*.time_stamp /.rspec +/plugins/* From 9be0c2734ae3e3cc84196cf167a0c327c7cf8570 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 8 Feb 2018 17:51:02 +0200 Subject: [PATCH 03/24] Add external plugins support to extend system hooks Signed-off-by: Dmitriy Zaporozhets --- app/services/system_hooks_service.rb | 9 +++++++++ config/application.rb | 1 + config/initializers/9_plugins.rb | 29 ++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 config/initializers/9_plugins.rb diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index a6b7a6e1416..71de74e5a82 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -11,6 +11,15 @@ class SystemHooksService SystemHook.hooks_for(hooks_scope).find_each do |hook| hook.async_execute(data, 'system_hooks') end + + # Execute external plugins + PLUGINS.each do |plugin| + begin + plugin.new.execute(data) + rescue => e + Rails.logger.warn("GitLab -> Plugins -> #{plugin.class.name} raised an axception during execution. #{e}") + end + end end private diff --git a/config/application.rb b/config/application.rb index 918bd4d57cf..f2fc6270748 100644 --- a/config/application.rb +++ b/config/application.rb @@ -28,6 +28,7 @@ module Gitlab config.eager_load_paths.push(*%W[#{config.root}/lib #{config.root}/app/models/hooks #{config.root}/app/models/members + #{config.root}/plugins #{config.root}/app/models/project_services #{config.root}/app/workers/concerns #{config.root}/app/services/concerns diff --git a/config/initializers/9_plugins.rb b/config/initializers/9_plugins.rb new file mode 100644 index 00000000000..9f252ccd296 --- /dev/null +++ b/config/initializers/9_plugins.rb @@ -0,0 +1,29 @@ +class PluginsSystem + attr_accessor :plugins, :files + + def initialize + @files = Dir.glob(Rails.root.join('plugins', '*_plugin.rb')) + end + + def valid_plugins + files.map do |file| + file_name = File.basename(file, '.rb') + + # Just give sample data to method and expect it to not crash. + begin + klass = Object.const_get(file_name.classify) + klass.new.execute(Gitlab::DataBuilder::Push::SAMPLE_DATA) + rescue => e + Rails.logger.warn("GitLab -> Plugins -> #{file_name} raised an exception during boot check. #{e}") + next + else + Rails.logger.info "GitLab -> Plugins -> #{file_name} passed boot check" + klass + end + end + end +end + +# Load external plugins from /plugins directory +# and set into PLUGINS variable +PLUGINS = PluginsSystem.new.valid_plugins From b985fe95b6c30bc83725e9b5e18a79a8ceb900d3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 8 Feb 2018 18:14:10 +0200 Subject: [PATCH 04/24] Add generator for plugins Signed-off-by: Dmitriy Zaporozhets --- generator_templates/plugins/template.rb | 16 ++++++++++++++++ lib/tasks/plugins.rake | 25 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 generator_templates/plugins/template.rb create mode 100644 lib/tasks/plugins.rake diff --git a/generator_templates/plugins/template.rb b/generator_templates/plugins/template.rb new file mode 100644 index 00000000000..16c87f2c2b2 --- /dev/null +++ b/generator_templates/plugins/template.rb @@ -0,0 +1,16 @@ +# Requirements +# * File name must end with _s.rb. For example, jenkins_plugin.rb. +# * All code should be inside class. No code should be executed on file load. +# * Class name must be same as file name. +# If file name is jenkins_plugin.rb then class name must be JenkinsPlugin. +# +# Reccomendations +# * Code should not depend on or use GitLab classes and other code. +# * Consider contributing your plugin to GitLab source code so we can test it +# and make sure it will work in further version. +# +class $NAMEPlugin + def execute(data) + # TODO: Implement me + end +end diff --git a/lib/tasks/plugins.rake b/lib/tasks/plugins.rake new file mode 100644 index 00000000000..fac6070ea9b --- /dev/null +++ b/lib/tasks/plugins.rake @@ -0,0 +1,25 @@ +namespace :plugins do + desc 'Generate skeleton for new plugin' + task generate: :environment do + ARGV.each { |a| task a.to_sym { } } + name = ARGV[1] + + unless name.present? + puts 'Error. You need to specify a name for the plugin' + exit 1 + end + + class_name = name.classify + param = name.underscore + file_path = Rails.root.join('plugins', param + '_plugin.rb') + template = File.read(Rails.root.join('generator_templates', 'plugins', 'template.rb')) + template.gsub!('$NAME', class_name) + + if File.write(file_path, template) + puts "Done. Your plugin saved under #{file_path}." + puts 'Feel free to edit it.' + else + puts "Failed to save #{file_path}." + end + end +end From 5bb435d0e75ad84e2fc379208cc36a25a4574453 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Feb 2018 19:20:53 +0200 Subject: [PATCH 05/24] Remove plugin initializer and add plugins:validate rake task Signed-off-by: Dmitriy Zaporozhets --- app/services/system_hooks_service.rb | 2 +- config/initializers/9_plugins.rb | 29 ---------------------------- lib/gitlab/plugin.rb | 25 ++++++++++++++++++++++++ lib/tasks/plugins.rake | 15 ++++++++++++++ 4 files changed, 41 insertions(+), 30 deletions(-) delete mode 100644 config/initializers/9_plugins.rb create mode 100644 lib/gitlab/plugin.rb diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 71de74e5a82..ba46c0074e4 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -13,7 +13,7 @@ class SystemHooksService end # Execute external plugins - PLUGINS.each do |plugin| + Gitlab::Plugin.all.each do |plugin| begin plugin.new.execute(data) rescue => e diff --git a/config/initializers/9_plugins.rb b/config/initializers/9_plugins.rb deleted file mode 100644 index 9f252ccd296..00000000000 --- a/config/initializers/9_plugins.rb +++ /dev/null @@ -1,29 +0,0 @@ -class PluginsSystem - attr_accessor :plugins, :files - - def initialize - @files = Dir.glob(Rails.root.join('plugins', '*_plugin.rb')) - end - - def valid_plugins - files.map do |file| - file_name = File.basename(file, '.rb') - - # Just give sample data to method and expect it to not crash. - begin - klass = Object.const_get(file_name.classify) - klass.new.execute(Gitlab::DataBuilder::Push::SAMPLE_DATA) - rescue => e - Rails.logger.warn("GitLab -> Plugins -> #{file_name} raised an exception during boot check. #{e}") - next - else - Rails.logger.info "GitLab -> Plugins -> #{file_name} passed boot check" - klass - end - end - end -end - -# Load external plugins from /plugins directory -# and set into PLUGINS variable -PLUGINS = PluginsSystem.new.valid_plugins diff --git a/lib/gitlab/plugin.rb b/lib/gitlab/plugin.rb new file mode 100644 index 00000000000..cbc57a5cce3 --- /dev/null +++ b/lib/gitlab/plugin.rb @@ -0,0 +1,25 @@ +module Gitlab + module Plugin + def self.all + files.map do |file| + file_name = File.basename(file, '.rb') + + # Just give sample data to method and expect it to not crash. + begin + klass = Object.const_get(file_name.classify) + klass.new.execute(Gitlab::DataBuilder::Push::SAMPLE_DATA) + rescue => e + Rails.logger.warn("GitLab -> Plugins -> #{file_name} raised an exception during boot check. #{e}") + next + else + Rails.logger.info "GitLab -> Plugins -> #{file_name} passed validation check" + klass + end + end + end + + def self.files + Dir.glob(Rails.root.join('plugins', '*_plugin.rb')) + end + end +end diff --git a/lib/tasks/plugins.rake b/lib/tasks/plugins.rake index fac6070ea9b..8728e232c9c 100644 --- a/lib/tasks/plugins.rake +++ b/lib/tasks/plugins.rake @@ -22,4 +22,19 @@ namespace :plugins do puts "Failed to save #{file_path}." end end + + desc 'Validate existing plugins' + task validate: :environment do + puts 'Validating plugins from /plugins directory' + + Gitlab::Plugin.all.each do |plugin| + begin + plugin.new.execute(Gitlab::DataBuilder::Push::SAMPLE_DATA) + rescue => e + puts "- #{plugin} raised an exception during boot check. #{e}" + else + puts "- #{plugin} passed validation check" + end + end + end end From eff5746b5e7cf4075edd6d1c76fdcd24c1603bb4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 19 Feb 2018 19:55:54 +0200 Subject: [PATCH 06/24] Redesign plugins system Signed-off-by: Dmitriy Zaporozhets --- app/services/system_hooks_service.rb | 9 +------- app/workers/plugin_worker.rb | 9 ++++++++ lib/gitlab/plugin.rb | 34 +++++++++++++--------------- lib/tasks/plugins.rake | 12 +++++----- 4 files changed, 32 insertions(+), 32 deletions(-) create mode 100644 app/workers/plugin_worker.rb diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index ba46c0074e4..af8c02a10b7 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -12,14 +12,7 @@ class SystemHooksService hook.async_execute(data, 'system_hooks') end - # Execute external plugins - Gitlab::Plugin.all.each do |plugin| - begin - plugin.new.execute(data) - rescue => e - Rails.logger.warn("GitLab -> Plugins -> #{plugin.class.name} raised an axception during execution. #{e}") - end - end + Gitlab::Plugin.execute_all_async(data) end private diff --git a/app/workers/plugin_worker.rb b/app/workers/plugin_worker.rb new file mode 100644 index 00000000000..34a3c8d62ac --- /dev/null +++ b/app/workers/plugin_worker.rb @@ -0,0 +1,9 @@ +class PluginWorker + include ApplicationWorker + + sidekiq_options retry: false + + def perform(file_name, data) + Gitlab::Plugin.execute(file_name, data) + end +end diff --git a/lib/gitlab/plugin.rb b/lib/gitlab/plugin.rb index cbc57a5cce3..9604cac4b20 100644 --- a/lib/gitlab/plugin.rb +++ b/lib/gitlab/plugin.rb @@ -1,25 +1,23 @@ module Gitlab module Plugin - def self.all - files.map do |file| - file_name = File.basename(file, '.rb') - - # Just give sample data to method and expect it to not crash. - begin - klass = Object.const_get(file_name.classify) - klass.new.execute(Gitlab::DataBuilder::Push::SAMPLE_DATA) - rescue => e - Rails.logger.warn("GitLab -> Plugins -> #{file_name} raised an exception during boot check. #{e}") - next - else - Rails.logger.info "GitLab -> Plugins -> #{file_name} passed validation check" - klass - end - end - end - def self.files Dir.glob(Rails.root.join('plugins', '*_plugin.rb')) end + + def self.execute_all_async(data) + files.each do |file| + PluginWorker.perform_async(file, data) + end + end + + def self.execute(file, data) + # TODO: Implement + # + # Reuse some code from gitlab-shell https://gitlab.com/gitlab-org/gitlab-shell/blob/master/lib/gitlab_custom_hook.rb#L40 + # Pass data as STDIN (or JSON encode?) + # + # 1. Return true if 0 exit code + # 2. Return false if non-zero exit code + end end end diff --git a/lib/tasks/plugins.rake b/lib/tasks/plugins.rake index 8728e232c9c..9c9f1fece85 100644 --- a/lib/tasks/plugins.rake +++ b/lib/tasks/plugins.rake @@ -27,13 +27,13 @@ namespace :plugins do task validate: :environment do puts 'Validating plugins from /plugins directory' - Gitlab::Plugin.all.each do |plugin| - begin - plugin.new.execute(Gitlab::DataBuilder::Push::SAMPLE_DATA) - rescue => e - puts "- #{plugin} raised an exception during boot check. #{e}" + Gitlab::Plugin.files.each do |file| + result = Gitlab::Plugin.execute(file, Gitlab::DataBuilder::Push::SAMPLE_DATA) + + if result + puts "* #{file} succeed (zero exit code)" else - puts "- #{plugin} passed validation check" + puts "* #{file} failure (non-zero exit code)" end end end From 645dceb0a233fc523ac16611fa3fec317d29a7e1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 23 Feb 2018 15:58:57 +0200 Subject: [PATCH 07/24] Run plugins as separate process and pass data via STDIN Signed-off-by: Dmitriy Zaporozhets --- .gitignore | 2 +- config/application.rb | 1 - generator_templates/plugins/template.rb | 16 -------------- lib/gitlab/plugin.rb | 28 ++++++++++++++++++------- lib/tasks/plugins.rake | 24 --------------------- plugins/available/save_to_file.clj | 3 +++ plugins/available/save_to_file.rb | 3 +++ plugins/{ => enabled}/.gitkeep | 0 8 files changed, 27 insertions(+), 50 deletions(-) delete mode 100644 generator_templates/plugins/template.rb create mode 100755 plugins/available/save_to_file.clj create mode 100755 plugins/available/save_to_file.rb rename plugins/{ => enabled}/.gitkeep (100%) diff --git a/.gitignore b/.gitignore index fa39ae01ff0..35ca92b1a93 100644 --- a/.gitignore +++ b/.gitignore @@ -66,4 +66,4 @@ eslint-report.html /locale/**/LC_MESSAGES /locale/**/*.time_stamp /.rspec -/plugins/* +/plugins/enabled/* diff --git a/config/application.rb b/config/application.rb index f2fc6270748..918bd4d57cf 100644 --- a/config/application.rb +++ b/config/application.rb @@ -28,7 +28,6 @@ module Gitlab config.eager_load_paths.push(*%W[#{config.root}/lib #{config.root}/app/models/hooks #{config.root}/app/models/members - #{config.root}/plugins #{config.root}/app/models/project_services #{config.root}/app/workers/concerns #{config.root}/app/services/concerns diff --git a/generator_templates/plugins/template.rb b/generator_templates/plugins/template.rb deleted file mode 100644 index 16c87f2c2b2..00000000000 --- a/generator_templates/plugins/template.rb +++ /dev/null @@ -1,16 +0,0 @@ -# Requirements -# * File name must end with _s.rb. For example, jenkins_plugin.rb. -# * All code should be inside class. No code should be executed on file load. -# * Class name must be same as file name. -# If file name is jenkins_plugin.rb then class name must be JenkinsPlugin. -# -# Reccomendations -# * Code should not depend on or use GitLab classes and other code. -# * Consider contributing your plugin to GitLab source code so we can test it -# and make sure it will work in further version. -# -class $NAMEPlugin - def execute(data) - # TODO: Implement me - end -end diff --git a/lib/gitlab/plugin.rb b/lib/gitlab/plugin.rb index 9604cac4b20..1035d258907 100644 --- a/lib/gitlab/plugin.rb +++ b/lib/gitlab/plugin.rb @@ -1,23 +1,35 @@ module Gitlab module Plugin def self.files - Dir.glob(Rails.root.join('plugins', '*_plugin.rb')) + Dir.glob(Rails.root.join('plugins/enabled/*')) end def self.execute_all_async(data) files.each do |file| + puts file PluginWorker.perform_async(file, data) end end def self.execute(file, data) - # TODO: Implement - # - # Reuse some code from gitlab-shell https://gitlab.com/gitlab-org/gitlab-shell/blob/master/lib/gitlab_custom_hook.rb#L40 - # Pass data as STDIN (or JSON encode?) - # - # 1. Return true if 0 exit code - # 2. Return false if non-zero exit code + # Prepare the hook subprocess. Attach a pipe to its stdin, and merge + # both its stdout and stderr into our own stdout. + stdin_reader, stdin_writer = IO.pipe + hook_pid = spawn({}, file, in: stdin_reader, err: :out) + stdin_reader.close + + # Submit changes to the hook via its stdin. + begin + IO.copy_stream(StringIO.new(data.to_json), stdin_writer) + rescue Errno::EPIPE + # It is not an error if the hook does not consume all of its input. + end + + # Close the pipe to let the hook know there is no further input. + stdin_writer.close + + Process.wait(hook_pid) + $?.success? end end end diff --git a/lib/tasks/plugins.rake b/lib/tasks/plugins.rake index 9c9f1fece85..f4d7edb2eb2 100644 --- a/lib/tasks/plugins.rake +++ b/lib/tasks/plugins.rake @@ -1,28 +1,4 @@ namespace :plugins do - desc 'Generate skeleton for new plugin' - task generate: :environment do - ARGV.each { |a| task a.to_sym { } } - name = ARGV[1] - - unless name.present? - puts 'Error. You need to specify a name for the plugin' - exit 1 - end - - class_name = name.classify - param = name.underscore - file_path = Rails.root.join('plugins', param + '_plugin.rb') - template = File.read(Rails.root.join('generator_templates', 'plugins', 'template.rb')) - template.gsub!('$NAME', class_name) - - if File.write(file_path, template) - puts "Done. Your plugin saved under #{file_path}." - puts 'Feel free to edit it.' - else - puts "Failed to save #{file_path}." - end - end - desc 'Validate existing plugins' task validate: :environment do puts 'Validating plugins from /plugins directory' diff --git a/plugins/available/save_to_file.clj b/plugins/available/save_to_file.clj new file mode 100755 index 00000000000..a59d83749d3 --- /dev/null +++ b/plugins/available/save_to_file.clj @@ -0,0 +1,3 @@ +#!/usr/bin/env clojure +(let [in (slurp *in*)] + (spit "/tmp/clj-data.txt" in)) diff --git a/plugins/available/save_to_file.rb b/plugins/available/save_to_file.rb new file mode 100755 index 00000000000..61b0df9bfd6 --- /dev/null +++ b/plugins/available/save_to_file.rb @@ -0,0 +1,3 @@ +#!/usr/bin/env ruby +x = STDIN.read +File.write('/tmp/rb-data.txt', x) diff --git a/plugins/.gitkeep b/plugins/enabled/.gitkeep similarity index 100% rename from plugins/.gitkeep rename to plugins/enabled/.gitkeep From ba95015a09bc465533666b38609b4fb1e0177139 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Feb 2018 13:32:14 +0200 Subject: [PATCH 08/24] Reorganize plugins dir structure Signed-off-by: Dmitriy Zaporozhets --- lib/gitlab/plugin.rb | 5 +++-- plugins/enabled/.gitkeep | 0 plugins/{available => examples}/save_to_file.clj | 0 plugins/{available => examples}/save_to_file.rb | 0 4 files changed, 3 insertions(+), 2 deletions(-) delete mode 100644 plugins/enabled/.gitkeep rename plugins/{available => examples}/save_to_file.clj (100%) rename plugins/{available => examples}/save_to_file.rb (100%) diff --git a/lib/gitlab/plugin.rb b/lib/gitlab/plugin.rb index 1035d258907..be5d6d6b1c1 100644 --- a/lib/gitlab/plugin.rb +++ b/lib/gitlab/plugin.rb @@ -1,12 +1,13 @@ module Gitlab module Plugin def self.files - Dir.glob(Rails.root.join('plugins/enabled/*')) + Dir.glob(Rails.root.join('plugins/*')).select do |entry| + File.file?(entry) + end end def self.execute_all_async(data) files.each do |file| - puts file PluginWorker.perform_async(file, data) end end diff --git a/plugins/enabled/.gitkeep b/plugins/enabled/.gitkeep deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/plugins/available/save_to_file.clj b/plugins/examples/save_to_file.clj similarity index 100% rename from plugins/available/save_to_file.clj rename to plugins/examples/save_to_file.clj diff --git a/plugins/available/save_to_file.rb b/plugins/examples/save_to_file.rb similarity index 100% rename from plugins/available/save_to_file.rb rename to plugins/examples/save_to_file.rb From 84097def3c97f0e0d8993ff07375c89e4b91aea8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Feb 2018 13:32:35 +0200 Subject: [PATCH 09/24] Add /plugins to gitignore Signed-off-by: Dmitriy Zaporozhets --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 35ca92b1a93..fa39ae01ff0 100644 --- a/.gitignore +++ b/.gitignore @@ -66,4 +66,4 @@ eslint-report.html /locale/**/LC_MESSAGES /locale/**/*.time_stamp /.rspec -/plugins/enabled/* +/plugins/* From 3337130e015fba1d04a53e8e3a7098f966792f5f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Feb 2018 13:42:25 +0200 Subject: [PATCH 10/24] Use Gitlab::Popen instead of spawn [ci skip] Signed-off-by: Dmitriy Zaporozhets --- lib/gitlab/plugin.rb | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/lib/gitlab/plugin.rb b/lib/gitlab/plugin.rb index be5d6d6b1c1..5339c4dbbc8 100644 --- a/lib/gitlab/plugin.rb +++ b/lib/gitlab/plugin.rb @@ -13,24 +13,11 @@ module Gitlab end def self.execute(file, data) - # Prepare the hook subprocess. Attach a pipe to its stdin, and merge - # both its stdout and stderr into our own stdout. - stdin_reader, stdin_writer = IO.pipe - hook_pid = spawn({}, file, in: stdin_reader, err: :out) - stdin_reader.close - - # Submit changes to the hook via its stdin. - begin - IO.copy_stream(StringIO.new(data.to_json), stdin_writer) - rescue Errno::EPIPE - # It is not an error if the hook does not consume all of its input. + _output, exit_status = Gitlab::Popen.popen([file]) do |stdin| + stdin.write(data.to_json) end - # Close the pipe to let the hook know there is no further input. - stdin_writer.close - - Process.wait(hook_pid) - $?.success? + exit_status.zero? end end end From 4b998239a3fec56f93e24ca063d1196416aec938 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Feb 2018 14:15:51 +0200 Subject: [PATCH 11/24] Add plugin queue to sidekiq config [ci skip] Signed-off-by: Dmitriy Zaporozhets --- config/sidekiq_queues.yml | 1 + lib/gitlab/plugin.rb | 6 +++--- lib/tasks/plugins.rake | 5 +++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index f037e3d1221..4845dc28a4a 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -68,3 +68,4 @@ - [project_migrate_hashed_storage, 1] - [storage_migrator, 1] - [pages_domain_verification, 1] + - [plugin, 1] diff --git a/lib/gitlab/plugin.rb b/lib/gitlab/plugin.rb index 5339c4dbbc8..3e82f5cb0d8 100644 --- a/lib/gitlab/plugin.rb +++ b/lib/gitlab/plugin.rb @@ -7,9 +7,9 @@ module Gitlab end def self.execute_all_async(data) - files.each do |file| - PluginWorker.perform_async(file, data) - end + args = files.map { |file| [file, data] } + + PluginWorker.bulk_perform_async(args) end def self.execute(file, data) diff --git a/lib/tasks/plugins.rake b/lib/tasks/plugins.rake index f4d7edb2eb2..11c41f13614 100644 --- a/lib/tasks/plugins.rake +++ b/lib/tasks/plugins.rake @@ -13,4 +13,9 @@ namespace :plugins do end end end + + desc 'Validate existing plugins' + task validate_async: :environment do + Gitlab::Plugin.execute_all_async(Gitlab::DataBuilder::Push::SAMPLE_DATA) + end end From 3073f1aa6e8658825af739440f7288deeef9169d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Feb 2018 16:57:10 +0200 Subject: [PATCH 12/24] Handle excpetion in PluginWorker Signed-off-by: Dmitriy Zaporozhets --- app/workers/plugin_worker.rb | 2 ++ spec/workers/plugin_worker_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 spec/workers/plugin_worker_spec.rb diff --git a/app/workers/plugin_worker.rb b/app/workers/plugin_worker.rb index 34a3c8d62ac..cebdf8d0017 100644 --- a/app/workers/plugin_worker.rb +++ b/app/workers/plugin_worker.rb @@ -5,5 +5,7 @@ class PluginWorker def perform(file_name, data) Gitlab::Plugin.execute(file_name, data) + rescue => e + Rails.logger.error("#{self.class}: #{e.message}") end end diff --git a/spec/workers/plugin_worker_spec.rb b/spec/workers/plugin_worker_spec.rb new file mode 100644 index 00000000000..29c55482321 --- /dev/null +++ b/spec/workers/plugin_worker_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe PluginWorker do + include RepoHelpers + + subject { described_class.new } + + let(:filename) { 'my_plugin.rb' } + + 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 + + it 'handles exception well' do + data = { 'event_name' => 'project_create' } + + allow(Gitlab::Plugin).to receive(:execute).with(filename, data).and_raise('Permission denied') + + expect { subject.perform(filename, data) }.to_not raise_error + end + end +end From 7424e0e5b4fd875588a2dd5a15ee5de1559c13db Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Feb 2018 17:33:19 +0200 Subject: [PATCH 13/24] Add specs for Gitlab::Plugin [ci skip] Signed-off-by: Dmitriy Zaporozhets --- spec/lib/gitlab/plugin_spec.rb | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 spec/lib/gitlab/plugin_spec.rb diff --git a/spec/lib/gitlab/plugin_spec.rb b/spec/lib/gitlab/plugin_spec.rb new file mode 100644 index 00000000000..1dc508f86ce --- /dev/null +++ b/spec/lib/gitlab/plugin_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe Gitlab::Plugin do + 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').path } + + before do + File.write(plugin, plugin_source) + File.chmod(0o777, plugin) + end + + after do + FileUtils.rm(plugin) + FileUtils.rm(tmp_file) + end + + subject { described_class.execute(plugin.to_s, data) } + + it { is_expected.to be true } + + it 'ensures plugin received data via stdin' do + subject + + expect(File.read(tmp_file)).to eq(data.to_json) + end + end + + private + + def plugin_source + <<-EOS +#!/usr/bin/env ruby +x = STDIN.read +File.write('#{tmp_file}', x) + EOS + end +end From 98831a4867f60f7b7ca02bebd8771ca23940e9c5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Feb 2018 17:33:38 +0200 Subject: [PATCH 14/24] Exclude plugins dir from rubocop and codeclimate config Signed-off-by: Dmitriy Zaporozhets --- .codeclimate.yml | 1 + .rubocop.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.codeclimate.yml b/.codeclimate.yml index b02fe54a4ff..d80f57c9947 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -45,3 +45,4 @@ exclude_paths: - log/ - backups/ - coverage-javascript/ +- plugins/ diff --git a/.rubocop.yml b/.rubocop.yml index 24edb641657..293f61fb725 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,6 +17,7 @@ AllCops: - 'bin/**/*' - 'generator_templates/**/*' - 'builds/**/*' + - 'plugins/**/*' CacheRootDirectory: tmp # This cop checks whether some constant value isn't a From 79d911204ca92c759b696d0b8db8e8d54af6e2f9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 26 Feb 2018 18:17:17 +0200 Subject: [PATCH 15/24] Add plugin queue to all_queues.yml Signed-off-by: Dmitriy Zaporozhets --- app/workers/all_queues.yml | 1 + spec/workers/plugin_worker_spec.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 28a5e5da037..a9415410f8a 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -84,6 +84,7 @@ - new_note - pages - pages_domain_verification +- plugin - post_receive - process_commit - project_cache diff --git a/spec/workers/plugin_worker_spec.rb b/spec/workers/plugin_worker_spec.rb index 29c55482321..d64de05f211 100644 --- a/spec/workers/plugin_worker_spec.rb +++ b/spec/workers/plugin_worker_spec.rb @@ -21,7 +21,7 @@ describe PluginWorker do allow(Gitlab::Plugin).to receive(:execute).with(filename, data).and_raise('Permission denied') - expect { subject.perform(filename, data) }.to_not raise_error + expect { subject.perform(filename, data) }.not_to raise_error end end end From ac8a0fa061283f2eb25f1e673ab244f1009a71a4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 27 Feb 2018 14:33:32 +0200 Subject: [PATCH 16/24] Few code improvements for spec/lib/gitlab/plugin_spec.rb Signed-off-by: Dmitriy Zaporozhets --- spec/lib/gitlab/plugin_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/lib/gitlab/plugin_spec.rb b/spec/lib/gitlab/plugin_spec.rb index 1dc508f86ce..065b18858be 100644 --- a/spec/lib/gitlab/plugin_spec.rb +++ b/spec/lib/gitlab/plugin_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Plugin do 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').path } + let(:tmp_file) { Tempfile.new('plugin-dump') } before do File.write(plugin, plugin_source) @@ -13,7 +13,7 @@ describe Gitlab::Plugin do after do FileUtils.rm(plugin) - FileUtils.rm(tmp_file) + tmp_file.close! end subject { described_class.execute(plugin.to_s, data) } @@ -23,17 +23,17 @@ describe Gitlab::Plugin do it 'ensures plugin received data via stdin' do subject - expect(File.read(tmp_file)).to eq(data.to_json) + expect(File.read(tmp_file.path)).to eq(data.to_json) end end private def plugin_source - <<-EOS -#!/usr/bin/env ruby -x = STDIN.read -File.write('#{tmp_file}', x) + <<~EOS + #!/usr/bin/env ruby + x = STDIN.read + File.write('#{tmp_file.path}', x) EOS end end From 865c9abf89e66d5bf71e437c3870174000c4205b Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 27 Feb 2018 14:34:52 +0200 Subject: [PATCH 17/24] Add plugin feature to changelog [ci skip] Signed-off-by: Dmitriy Zaporozhets --- changelogs/unreleased/dz-system-hooks-plugins.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/dz-system-hooks-plugins.yml diff --git a/changelogs/unreleased/dz-system-hooks-plugins.yml b/changelogs/unreleased/dz-system-hooks-plugins.yml new file mode 100644 index 00000000000..e6eb1dfb03b --- /dev/null +++ b/changelogs/unreleased/dz-system-hooks-plugins.yml @@ -0,0 +1,5 @@ +--- +title: Add ability to use external plugins as an alternative to system hooks +merge_request: 17003 +author: +type: added From 8c9e4d3a9610fba98dc707178bf4a79473429135 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 27 Feb 2018 16:01:17 +0200 Subject: [PATCH 18/24] Add documentation for plugins feature Signed-off-by: Dmitriy Zaporozhets --- doc/administration/plugins.md | 50 +++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 doc/administration/plugins.md diff --git a/doc/administration/plugins.md b/doc/administration/plugins.md new file mode 100644 index 00000000000..0fea1d2e3ac --- /dev/null +++ b/doc/administration/plugins.md @@ -0,0 +1,50 @@ +# Plugins + +**Note:** Plugins must be configured on the filesystem of the GitLab +server. Only GitLab server administrators will be able to complete these tasks. +Please explore [system hooks] or [webhooks] as an option if you do not +have filesystem access. + +Introduced in GitLab 10.6. + +Plugin will run on each event so it's up to you to filter events or projects within a plugin code. You can have as many plugins as you want. Each plugin will be triggered by GitLab asyncronously in case of event. For list of events please see [system hooks] documentation. + +## 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. + +Follow the steps below to set up a custom hook: + +1. On the GitLab server, navigate to the project's plugin directory. + For an installation from source the path is usually + `/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 sepcial characters. +1. Make the hook file executable and make sure it's owned by git. +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`. + +That's it! Assuming the plugin code is properly implemented the hook will fire +as appropriate. Plugins file list is updated on each event. There is no need to restart GitLab to apply a new plugin. + +## Validation + +Writing own plugin can be tricky and its easier if you can check it without altering the system. We provided a rake task you can use with staging enviromnent to test your plugin before using it in production. The rake task will use a sample data and execute each of plugins. By output you should be able to determine if system sees your plugin and if it was executed without errors. + +```bash +# Omnibus installations +sudo gitlab-rake plugins:validate + +# Installations from source +bundle exec rake plugins:validate RAILS_ENV=production +``` + + +[hooks]: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks#Server-Side-Hooks +[system hooks]: ../system_hooks/system_hooks.md +[webhooks]: ../user/project/integrations/webhooks.md +[5073]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5073 +[93]: https://gitlab.com/gitlab-org/gitlab-shell/merge_requests/93 + From 2150b2cde2700a48095db4364d02f91c9b9a1456 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 27 Feb 2018 16:02:42 +0200 Subject: [PATCH 19/24] [ci skip] Add example of rake plugins:validate output to the doc Signed-off-by: Dmitriy Zaporozhets --- doc/administration/plugins.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/administration/plugins.md b/doc/administration/plugins.md index 0fea1d2e3ac..f71cecafb5f 100644 --- a/doc/administration/plugins.md +++ b/doc/administration/plugins.md @@ -41,6 +41,14 @@ sudo gitlab-rake plugins:validate bundle exec rake plugins:validate RAILS_ENV=production ``` +Example of output can be next: + +``` +-> bundle exec rake plugins:validate RAILS_ENV=production +Validating plugins from /plugins directory +* /home/git/gitlab/plugins/save_to_file.clj succeed (zero exit code) +* /home/git/gitlab/plugins/save_to_file.rb failure (non-zero exit code) +``` [hooks]: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks#Server-Side-Hooks [system hooks]: ../system_hooks/system_hooks.md From f0a64399c1171d2757c8ce70a36517d556cd9233 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 27 Feb 2018 18:08:38 +0200 Subject: [PATCH 20/24] Refactor plugin execution method Signed-off-by: Dmitriy Zaporozhets --- app/workers/plugin_worker.rb | 2 -- lib/gitlab/plugin.rb | 11 +++++- spec/lib/gitlab/plugin_spec.rb | 57 +++++++++++++++++++++--------- spec/workers/plugin_worker_spec.rb | 8 ----- 4 files changed, 51 insertions(+), 27 deletions(-) diff --git a/app/workers/plugin_worker.rb b/app/workers/plugin_worker.rb index cebdf8d0017..34a3c8d62ac 100644 --- a/app/workers/plugin_worker.rb +++ b/app/workers/plugin_worker.rb @@ -5,7 +5,5 @@ class PluginWorker def perform(file_name, data) Gitlab::Plugin.execute(file_name, data) - rescue => e - Rails.logger.error("#{self.class}: #{e.message}") end end diff --git a/lib/gitlab/plugin.rb b/lib/gitlab/plugin.rb index 3e82f5cb0d8..96332362a4c 100644 --- a/lib/gitlab/plugin.rb +++ b/lib/gitlab/plugin.rb @@ -13,11 +13,20 @@ module Gitlab end def self.execute(file, data) - _output, exit_status = Gitlab::Popen.popen([file]) do |stdin| + result = Gitlab::Popen.popen_with_detail([file]) do |stdin| stdin.write(data.to_json) end + exit_status = result.status&.exitstatus + + unless exit_status.zero? + Rails.logger.error("Plugin Error => #{file}: #{result.stderr}") + end + exit_status.zero? + rescue => e + Rails.logger.error("Plugin Error => #{file}: #{e.message}") + false end end end diff --git a/spec/lib/gitlab/plugin_spec.rb b/spec/lib/gitlab/plugin_spec.rb index 065b18858be..a01e1383e3b 100644 --- a/spec/lib/gitlab/plugin_spec.rb +++ b/spec/lib/gitlab/plugin_spec.rb @@ -6,34 +6,59 @@ 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 + before do File.write(plugin, plugin_source) - File.chmod(0o777, plugin) end after do FileUtils.rm(plugin) - tmp_file.close! end subject { described_class.execute(plugin.to_s, data) } - it { is_expected.to be true } + context 'successful execution' do + before do + File.chmod(0o777, plugin) + end - it 'ensures plugin received data via stdin' do - subject + after do + tmp_file.close! + end - expect(File.read(tmp_file.path)).to eq(data.to_json) + it { is_expected.to be true } + + it 'ensures plugin received data via stdin' do + subject + + expect(File.read(tmp_file.path)).to eq(data.to_json) + end + end + + context 'non-executable' do + it { is_expected.to be false } + end + + context 'non-zero exit' do + let(:plugin_source) do + <<~EOS + #!/usr/bin/env ruby + exit 1 + EOS + end + + before do + File.chmod(0o777, plugin) + end + + it { is_expected.to be false } end end - - private - - def plugin_source - <<~EOS - #!/usr/bin/env ruby - x = STDIN.read - File.write('#{tmp_file.path}', x) - EOS - end end diff --git a/spec/workers/plugin_worker_spec.rb b/spec/workers/plugin_worker_spec.rb index d64de05f211..60631def8f3 100644 --- a/spec/workers/plugin_worker_spec.rb +++ b/spec/workers/plugin_worker_spec.rb @@ -15,13 +15,5 @@ describe PluginWorker do expect(subject.perform(filename, data)).to be_truthy end - - it 'handles exception well' do - data = { 'event_name' => 'project_create' } - - allow(Gitlab::Plugin).to receive(:execute).with(filename, data).and_raise('Permission denied') - - expect { subject.perform(filename, data) }.not_to raise_error - end end end From 4eed9a12216296709306ce29faf607d8aed2a913 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 27 Feb 2018 18:22:50 +0200 Subject: [PATCH 21/24] Fix few typos in plugins doc Signed-off-by: Dmitriy Zaporozhets --- doc/administration/plugins.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/administration/plugins.md b/doc/administration/plugins.md index f71cecafb5f..89ab0b9c8ea 100644 --- a/doc/administration/plugins.md +++ b/doc/administration/plugins.md @@ -7,7 +7,7 @@ have filesystem access. Introduced in GitLab 10.6. -Plugin will run on each event so it's up to you to filter events or projects within a plugin code. You can have as many plugins as you want. Each plugin will be triggered by GitLab asyncronously in case of event. For list of events please see [system hooks] documentation. +A plugin will run on each event so it's up to you to filter events or projects within a plugin code. You can have as many plugins as you want. Each plugin will be triggered by GitLab asynchronously in case of an event. For a list of events please see [system hooks] documentation. ## Setup @@ -19,7 +19,7 @@ Follow the steps below to set up a custom hook: For an installation from source the path is usually `/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 sepcial characters. +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. 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 @@ -27,11 +27,11 @@ Follow the steps below to set up a custom hook: `#!/usr/bin/env ruby`. That's it! Assuming the plugin code is properly implemented the hook will fire -as appropriate. Plugins file list is updated on each event. There is no need to restart GitLab to apply a new plugin. +as appropriate. Plugins file list is updated for each event. There is no need to restart GitLab to apply a new plugin. ## Validation -Writing own plugin can be tricky and its easier if you can check it without altering the system. We provided a rake task you can use with staging enviromnent to test your plugin before using it in production. The rake task will use a sample data and execute each of plugins. By output you should be able to determine if system sees your plugin and if it was executed without errors. +Writing own plugin can be tricky and its easier if you can check it without altering the system. We provided a rake task you can use with staging environment to test your plugin before using it in production. The rake task will use a sample data and execute each of plugins. By output you should be able to determine if system sees your plugin and if it was executed without errors. ```bash # Omnibus installations From 2577ff7fd656f301c4b1909f0a1c358c8c30a614 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 28 Feb 2018 12:16:23 +0200 Subject: [PATCH 22/24] Refactor plugins feature and make some doc improvements Signed-off-by: Dmitriy Zaporozhets --- app/workers/plugin_worker.rb | 8 +++++++- doc/administration/plugins.md | 5 +++-- lib/gitlab/plugin.rb | 10 ++-------- lib/gitlab/plugin_logger.rb | 8 ++++++++ lib/tasks/plugins.rake | 8 ++++---- spec/lib/gitlab/plugin_spec.rb | 16 ++++++++++------ spec/workers/plugin_worker_spec.rb | 14 ++++++++++---- 7 files changed, 44 insertions(+), 25 deletions(-) create mode 100644 lib/gitlab/plugin_logger.rb 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, '']) - allow(Gitlab::Plugin).to receive(:execute).with(filename, data).and_return(true) + expect(subject.perform(filename, data)).to be_truthy + end + 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 From 886d442b405a0db3dccc1b24e050244f65099826 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 28 Feb 2018 12:31:19 +0200 Subject: [PATCH 23/24] Improve plugins documentation and remove unnecessary rake task Signed-off-by: Dmitriy Zaporozhets --- doc/administration/plugins.md | 11 +++++++++-- lib/tasks/plugins.rake | 5 ----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/doc/administration/plugins.md b/doc/administration/plugins.md index ed1a3480ffc..c91ac3012b9 100644 --- a/doc/administration/plugins.md +++ b/doc/administration/plugins.md @@ -11,7 +11,8 @@ 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 inside `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: @@ -30,9 +31,15 @@ Follow the steps below to set up a custom hook: 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. +If a plugin executes with non-zero exit code or GitLab fails to execute it, a +message will be logged to `plugin.log`. + ## Validation -Writing own plugin can be tricky and its easier if you can check it without altering the system. We provided a rake task you can use with staging environment to test your plugin before using it in production. The rake task will use a sample data and execute each of plugins. By output you should be able to determine if system sees your plugin and if it was executed without errors. +Writing own plugin can be tricky and its easier if you can check it without altering the system. +We provided a rake task you can use with staging environment to test your plugin before using it in production. +The rake task will use a sample data and execute each of plugins. By output you should be able to determine if +system sees your plugin and if it was executed without errors. ```bash # Omnibus installations diff --git a/lib/tasks/plugins.rake b/lib/tasks/plugins.rake index 7a9de3afbec..e73dd7e68df 100644 --- a/lib/tasks/plugins.rake +++ b/lib/tasks/plugins.rake @@ -13,9 +13,4 @@ namespace :plugins do end end end - - desc 'Validate existing plugins' - task validate_async: :environment do - Gitlab::Plugin.execute_all_async(Gitlab::DataBuilder::Push::SAMPLE_DATA) - end end From a96ba41f229bd3606696e8e3a6500730e6cb8f63 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 28 Feb 2018 13:17:44 +0200 Subject: [PATCH 24/24] Remove trailing line from plugin logger Signed-off-by: Dmitriy Zaporozhets --- lib/gitlab/plugin_logger.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/gitlab/plugin_logger.rb b/lib/gitlab/plugin_logger.rb index a106a2677ed..c4f6ec3e21d 100644 --- a/lib/gitlab/plugin_logger.rb +++ b/lib/gitlab/plugin_logger.rb @@ -5,4 +5,3 @@ module Gitlab end end end -