diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2017-05-25 13:26:12 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2017-05-25 13:26:12 +0000 |
commit | 1ed7b7629a2c0fbcdfeeda386ec6292e57728e74 (patch) | |
tree | 4cd07f47a698bedf1a5811480482bfaa1582a5ae /app | |
parent | 78de1c059ac588df4ba1ef352b28e5b1c6102804 (diff) | |
parent | 330789c23c777d8ca646eba7c25f39cb7342cdee (diff) | |
download | gitlab-ce-1ed7b7629a2c0fbcdfeeda386ec6292e57728e74.tar.gz |
Merge branch '17848-web-hook-logging' into 'master'
Implement Web Hooks calls logging
Closes #17848
See merge request !11027
Diffstat (limited to 'app')
23 files changed, 418 insertions, 76 deletions
diff --git a/app/controllers/admin/hook_logs_controller.rb b/app/controllers/admin/hook_logs_controller.rb new file mode 100644 index 00000000000..aa069b89563 --- /dev/null +++ b/app/controllers/admin/hook_logs_controller.rb @@ -0,0 +1,29 @@ +class Admin::HookLogsController < Admin::ApplicationController + include HooksExecution + + before_action :hook, only: [:show, :retry] + before_action :hook_log, only: [:show, :retry] + + respond_to :html + + def show + end + + def retry + status, message = hook.execute(hook_log.request_data, hook_log.trigger) + + set_hook_execution_notice(status, message) + + redirect_to edit_admin_hook_path(@hook) + end + + private + + def hook + @hook ||= SystemHook.find(params[:hook_id]) + end + + def hook_log + @hook_log ||= hook.web_hook_logs.find(params[:id]) + end +end diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb index ccfe553c89e..b9251e140f8 100644 --- a/app/controllers/admin/hooks_controller.rb +++ b/app/controllers/admin/hooks_controller.rb @@ -1,5 +1,7 @@ class Admin::HooksController < Admin::ApplicationController - before_action :hook, only: :edit + include HooksExecution + + before_action :hook_logs, only: :edit def index @hooks = SystemHook.all @@ -36,15 +38,9 @@ class Admin::HooksController < Admin::ApplicationController end def test - data = { - event_name: "project_create", - name: "Ruby", - path: "ruby", - project_id: 1, - owner_name: "Someone", - owner_email: "example@gitlabhq.com" - } - hook.execute(data, 'system_hooks') + status, message = hook.execute(sample_hook_data, 'system_hooks') + + set_hook_execution_notice(status, message) redirect_back_or_default end @@ -55,6 +51,11 @@ class Admin::HooksController < Admin::ApplicationController @hook ||= SystemHook.find(params[:id]) end + def hook_logs + @hook_logs ||= + Kaminari.paginate_array(hook.web_hook_logs.order(created_at: :desc)).page(params[:page]) + end + def hook_params params.require(:hook).permit( :enable_ssl_verification, @@ -65,4 +66,15 @@ class Admin::HooksController < Admin::ApplicationController :url ) end + + def sample_hook_data + { + event_name: "project_create", + name: "Ruby", + path: "ruby", + project_id: 1, + owner_name: "Someone", + owner_email: "example@gitlabhq.com" + } + end end diff --git a/app/controllers/concerns/hooks_execution.rb b/app/controllers/concerns/hooks_execution.rb new file mode 100644 index 00000000000..846cd60518f --- /dev/null +++ b/app/controllers/concerns/hooks_execution.rb @@ -0,0 +1,15 @@ +module HooksExecution + extend ActiveSupport::Concern + + private + + def set_hook_execution_notice(status, message) + if status && status >= 200 && status < 400 + flash[:notice] = "Hook executed successfully: HTTP #{status}" + elsif status + flash[:alert] = "Hook executed successfully but returned HTTP #{status} #{message}" + else + flash[:alert] = "Hook execution failed: #{message}" + end + end +end diff --git a/app/controllers/projects/hook_logs_controller.rb b/app/controllers/projects/hook_logs_controller.rb new file mode 100644 index 00000000000..354f0d6db3a --- /dev/null +++ b/app/controllers/projects/hook_logs_controller.rb @@ -0,0 +1,33 @@ +class Projects::HookLogsController < Projects::ApplicationController + include HooksExecution + + before_action :authorize_admin_project! + + before_action :hook, only: [:show, :retry] + before_action :hook_log, only: [:show, :retry] + + respond_to :html + + layout 'project_settings' + + def show + end + + def retry + status, message = hook.execute(hook_log.request_data, hook_log.trigger) + + set_hook_execution_notice(status, message) + + redirect_to edit_namespace_project_hook_path(@project.namespace, @project, @hook) + end + + private + + def hook + @hook ||= @project.hooks.find(params[:hook_id]) + end + + def hook_log + @hook_log ||= hook.web_hook_logs.find(params[:id]) + end +end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 86d13a0d222..38bd82841dc 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -1,7 +1,9 @@ class Projects::HooksController < Projects::ApplicationController + include HooksExecution + # Authorize before_action :authorize_admin_project! - before_action :hook, only: :edit + before_action :hook_logs, only: :edit respond_to :html @@ -34,13 +36,7 @@ class Projects::HooksController < Projects::ApplicationController if !@project.empty_repo? status, message = TestHookService.new.execute(hook, current_user) - if status && status >= 200 && status < 400 - flash[:notice] = "Hook executed successfully: HTTP #{status}" - elsif status - flash[:alert] = "Hook executed successfully but returned HTTP #{status} #{message}" - else - flash[:alert] = "Hook execution failed: #{message}" - end + set_hook_execution_notice(status, message) else flash[:alert] = 'Hook execution failed. Ensure the project has commits.' end @@ -60,6 +56,11 @@ class Projects::HooksController < Projects::ApplicationController @hook ||= @project.hooks.find(params[:id]) end + def hook_logs + @hook_logs ||= + Kaminari.paginate_array(hook.web_hook_logs.order(created_at: :desc)).page(params[:page]) + end + def hook_params params.require(:hook).permit( :job_events, diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb index eef24052a06..40e43c27f91 100644 --- a/app/models/hooks/service_hook.rb +++ b/app/models/hooks/service_hook.rb @@ -2,6 +2,6 @@ class ServiceHook < WebHook belongs_to :service def execute(data) - super(data, 'service_hook') + WebHookService.new(self, data, 'service_hook').execute end end diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index c645805c6da..1584235ab00 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -3,8 +3,4 @@ class SystemHook < WebHook default_value_for :push_events, false default_value_for :repository_update_events, true - - def async_execute(data, hook_name) - Sidekiq::Client.enqueue(SystemHookWorker, id, data, hook_name) - end end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index a165fdc312f..7503f3739c3 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -1,6 +1,5 @@ class WebHook < ActiveRecord::Base include Sortable - include HTTParty default_value_for :push_events, true default_value_for :issues_events, false @@ -13,52 +12,18 @@ class WebHook < ActiveRecord::Base default_value_for :repository_update_events, false default_value_for :enable_ssl_verification, true + has_many :web_hook_logs, dependent: :destroy + scope :push_hooks, -> { where(push_events: true) } scope :tag_push_hooks, -> { where(tag_push_events: true) } - # HTTParty timeout - default_timeout Gitlab.config.gitlab.webhook_timeout - validates :url, presence: true, url: true def execute(data, hook_name) - parsed_url = URI.parse(url) - if parsed_url.userinfo.blank? - response = WebHook.post(url, - body: data.to_json, - headers: build_headers(hook_name), - verify: enable_ssl_verification) - else - post_url = url.gsub("#{parsed_url.userinfo}@", '') - auth = { - username: CGI.unescape(parsed_url.user), - password: CGI.unescape(parsed_url.password) - } - response = WebHook.post(post_url, - body: data.to_json, - headers: build_headers(hook_name), - verify: enable_ssl_verification, - basic_auth: auth) - end - - [response.code, response.to_s] - rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e - logger.error("WebHook Error => #{e}") - [false, e.to_s] + WebHookService.new(self, data, hook_name).execute end def async_execute(data, hook_name) - Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data, hook_name) - end - - private - - def build_headers(hook_name) - headers = { - 'Content-Type' => 'application/json', - 'X-Gitlab-Event' => hook_name.singularize.titleize - } - headers['X-Gitlab-Token'] = token if token.present? - headers + WebHookService.new(self, data, hook_name).async_execute end end diff --git a/app/models/hooks/web_hook_log.rb b/app/models/hooks/web_hook_log.rb new file mode 100644 index 00000000000..2738b229d84 --- /dev/null +++ b/app/models/hooks/web_hook_log.rb @@ -0,0 +1,13 @@ +class WebHookLog < ActiveRecord::Base + belongs_to :web_hook + + serialize :request_headers, Hash + serialize :request_data, Hash + serialize :response_headers, Hash + + validates :web_hook, presence: true + + def success? + response_status =~ /^2/ + end +end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb new file mode 100644 index 00000000000..4241b912d5b --- /dev/null +++ b/app/services/web_hook_service.rb @@ -0,0 +1,120 @@ +class WebHookService + class InternalErrorResponse + attr_reader :body, :headers, :code + + def initialize + @headers = HTTParty::Response::Headers.new({}) + @body = '' + @code = 'internal error' + end + end + + include HTTParty + + # HTTParty timeout + default_timeout Gitlab.config.gitlab.webhook_timeout + + attr_accessor :hook, :data, :hook_name + + def initialize(hook, data, hook_name) + @hook = hook + @data = data + @hook_name = hook_name + end + + def execute + start_time = Time.now + + response = if parsed_url.userinfo.blank? + make_request(hook.url) + else + make_request_with_auth + end + + log_execution( + trigger: hook_name, + url: hook.url, + request_data: data, + response: response, + execution_duration: Time.now - start_time + ) + + [response.code, response.to_s] + rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e + log_execution( + trigger: hook_name, + url: hook.url, + request_data: data, + response: InternalErrorResponse.new, + execution_duration: Time.now - start_time, + error_message: e.to_s + ) + + Rails.logger.error("WebHook Error => #{e}") + + [nil, e.to_s] + end + + def async_execute + Sidekiq::Client.enqueue(WebHookWorker, hook.id, data, hook_name) + end + + private + + def parsed_url + @parsed_url ||= URI.parse(hook.url) + end + + def make_request(url, basic_auth = false) + self.class.post(url, + body: data.to_json, + headers: build_headers(hook_name), + verify: hook.enable_ssl_verification, + basic_auth: basic_auth) + end + + def make_request_with_auth + post_url = hook.url.gsub("#{parsed_url.userinfo}@", '') + basic_auth = { + username: CGI.unescape(parsed_url.user), + password: CGI.unescape(parsed_url.password) + } + make_request(post_url, basic_auth) + end + + def log_execution(trigger:, url:, request_data:, response:, execution_duration:, error_message: nil) + # logging for ServiceHook's is not available + return if hook.is_a?(ServiceHook) + + WebHookLog.create( + web_hook: hook, + trigger: trigger, + url: url, + execution_duration: execution_duration, + request_headers: build_headers(hook_name), + request_data: request_data, + response_headers: format_response_headers(response), + response_body: response.body, + response_status: response.code, + internal_error_message: error_message + ) + end + + def build_headers(hook_name) + @headers ||= begin + { + 'Content-Type' => 'application/json', + 'X-Gitlab-Event' => hook_name.singularize.titleize + }.tap do |hash| + hash['X-Gitlab-Token'] = hook.token if hook.token.present? + end + end + end + + # Make response headers more stylish + # Net::HTTPHeader has downcased hash with arrays: { 'content-type' => ['text/html; charset=utf-8'] } + # This method format response to capitalized hash with strings: { 'Content-Type' => 'text/html; charset=utf-8' } + def format_response_headers(response) + response.headers.each_capitalized.to_h + end +end diff --git a/app/views/admin/hook_logs/_index.html.haml b/app/views/admin/hook_logs/_index.html.haml new file mode 100644 index 00000000000..7dd9943190f --- /dev/null +++ b/app/views/admin/hook_logs/_index.html.haml @@ -0,0 +1,37 @@ +.row.prepend-top-default.append-bottom-default + .col-lg-3 + %h4.prepend-top-0 + Recent Deliveries + %p When an event in GitLab triggers a webhook, you can use the request details to figure out if something went wrong. + .col-lg-9 + - if hook_logs.any? + %table.table + %thead + %tr + %th Status + %th Trigger + %th URL + %th Elapsed time + %th Request time + %th + - hook_logs.each do |hook_log| + %tr + %td + = render partial: 'shared/hook_logs/status_label', locals: { hook_log: hook_log } + %td.hidden-xs + %span.label.label-gray.deploy-project-label + = hook_log.trigger.singularize.titleize + %td + = truncate(hook_log.url, length: 50) + %td.light + #{number_with_precision(hook_log.execution_duration, precision: 2)} ms + %td.light + = time_ago_with_tooltip(hook_log.created_at) + %td + = link_to 'View details', admin_hook_hook_log_path(hook, hook_log) + + = paginate hook_logs, theme: 'gitlab' + + - else + .settings-message.text-center + You don't have any webhooks deliveries diff --git a/app/views/admin/hook_logs/show.html.haml b/app/views/admin/hook_logs/show.html.haml new file mode 100644 index 00000000000..56127bacda2 --- /dev/null +++ b/app/views/admin/hook_logs/show.html.haml @@ -0,0 +1,10 @@ +- page_title 'Request details' +%h3.page-title + Request details + +%hr + += link_to 'Resend Request', retry_admin_hook_hook_log_path(@hook, @hook_log), class: "btn btn-default pull-right prepend-left-10" + += render partial: 'shared/hook_logs/content', locals: { hook_log: @hook_log } + diff --git a/app/views/admin/hooks/edit.html.haml b/app/views/admin/hooks/edit.html.haml index 0777f5e2629..0e35a1905bf 100644 --- a/app/views/admin/hooks/edit.html.haml +++ b/app/views/admin/hooks/edit.html.haml @@ -12,3 +12,9 @@ = render partial: 'form', locals: { form: f, hook: @hook } .form-actions = f.submit 'Save changes', class: 'btn btn-create' + = link_to 'Test hook', test_admin_hook_path(@hook), class: 'btn btn-default' + = link_to 'Remove', admin_hook_path(@hook), method: :delete, class: 'btn btn-remove pull-right', data: { confirm: 'Are you sure?' } + +%hr + += render partial: 'admin/hook_logs/index', locals: { hook: @hook, hook_logs: @hook_logs } diff --git a/app/views/layouts/nav/_admin.html.haml b/app/views/layouts/nav/_admin.html.haml index d068c895fa3..f6132464910 100644 --- a/app/views/layouts/nav/_admin.html.haml +++ b/app/views/layouts/nav/_admin.html.haml @@ -17,7 +17,7 @@ = link_to admin_broadcast_messages_path, title: 'Messages' do %span Messages - = nav_link(controller: :hooks) do + = nav_link(controller: [:hooks, :hook_logs]) do = link_to admin_hooks_path, title: 'Hooks' do %span System Hooks diff --git a/app/views/projects/hook_logs/_index.html.haml b/app/views/projects/hook_logs/_index.html.haml new file mode 100644 index 00000000000..6962b223451 --- /dev/null +++ b/app/views/projects/hook_logs/_index.html.haml @@ -0,0 +1,37 @@ +.row.prepend-top-default.append-bottom-default + .col-lg-3 + %h4.prepend-top-0 + Recent Deliveries + %p When an event in GitLab triggers a webhook, you can use the request details to figure out if something went wrong. + .col-lg-9 + - if hook_logs.any? + %table.table + %thead + %tr + %th Status + %th Trigger + %th URL + %th Elapsed time + %th Request time + %th + - hook_logs.each do |hook_log| + %tr + %td + = render partial: 'shared/hook_logs/status_label', locals: { hook_log: hook_log } + %td.hidden-xs + %span.label.label-gray.deploy-project-label + = hook_log.trigger.singularize.titleize + %td + = truncate(hook_log.url, length: 50) + %td.light + #{number_with_precision(hook_log.execution_duration, precision: 2)} ms + %td.light + = time_ago_with_tooltip(hook_log.created_at) + %td + = link_to 'View details', namespace_project_hook_hook_log_path(project.namespace, project, hook, hook_log) + + = paginate hook_logs, theme: 'gitlab' + + - else + .settings-message.text-center + You don't have any webhooks deliveries diff --git a/app/views/projects/hook_logs/show.html.haml b/app/views/projects/hook_logs/show.html.haml new file mode 100644 index 00000000000..2eabe92f8eb --- /dev/null +++ b/app/views/projects/hook_logs/show.html.haml @@ -0,0 +1,11 @@ += render 'projects/settings/head' + +.row.prepend-top-default.append-bottom-default + .col-lg-3 + %h4.prepend-top-0 + Request details + .col-lg-9 + + = link_to 'Resend Request', retry_namespace_project_hook_hook_log_path(@project.namespace, @project, @hook, @hook_log), class: "btn btn-default pull-right prepend-left-10" + + = render partial: 'shared/hook_logs/content', locals: { hook_log: @hook_log } diff --git a/app/views/projects/hooks/edit.html.haml b/app/views/projects/hooks/edit.html.haml index 7998713be1f..fd382c1d63f 100644 --- a/app/views/projects/hooks/edit.html.haml +++ b/app/views/projects/hooks/edit.html.haml @@ -1,3 +1,4 @@ +- page_title 'Integrations' = render 'projects/settings/head' .row.prepend-top-default @@ -10,5 +11,12 @@ .col-lg-9.append-bottom-default = form_for [@project.namespace.becomes(Namespace), @project, @hook], as: :hook, url: namespace_project_hook_path do |f| = render partial: 'shared/web_hooks/form', locals: { form: f, hook: @hook } + = f.submit 'Save changes', class: 'btn btn-create' + = link_to 'Test hook', test_namespace_project_hook_path(@project.namespace, @project, @hook), class: 'btn btn-default' + = link_to 'Remove', namespace_project_hook_path(@project.namespace, @project, @hook), method: :delete, class: 'btn btn-remove pull-right', data: { confirm: 'Are you sure?' } + +%hr + += render partial: 'projects/hook_logs/index', locals: { hook: @hook, hook_logs: @hook_logs, project: @project } diff --git a/app/views/projects/settings/_head.html.haml b/app/views/projects/settings/_head.html.haml index faed65d6588..00bd563999f 100644 --- a/app/views/projects/settings/_head.html.haml +++ b/app/views/projects/settings/_head.html.haml @@ -14,7 +14,7 @@ %span Members - if can_edit - = nav_link(controller: [:integrations, :services, :hooks]) do + = nav_link(controller: [:integrations, :services, :hooks, :hook_logs]) do = link_to project_settings_integrations_path(@project), title: 'Integrations' do %span Integrations diff --git a/app/views/shared/hook_logs/_content.html.haml b/app/views/shared/hook_logs/_content.html.haml new file mode 100644 index 00000000000..af6a499fadb --- /dev/null +++ b/app/views/shared/hook_logs/_content.html.haml @@ -0,0 +1,44 @@ +%p + %strong Request URL: + POST + = hook_log.url + = render partial: 'shared/hook_logs/status_label', locals: { hook_log: hook_log } + +%p + %strong Trigger: + %td.hidden-xs + %span.label.label-gray.deploy-project-label + = hook_log.trigger.singularize.titleize +%p + %strong Elapsed time: + #{number_with_precision(hook_log.execution_duration, precision: 2)} ms +%p + %strong Request time: + = time_ago_with_tooltip(hook_log.created_at) + +%hr + +- if hook_log.internal_error_message.present? + .bs-callout.bs-callout-danger + = hook_log.internal_error_message + +%h5 Request headers: +%pre + - hook_log.request_headers.each do |k,v| + <strong>#{k}:</strong> #{v} + %br + +%h5 Request body: +%pre + :plain + #{JSON.pretty_generate(hook_log.request_data)} +%h5 Response headers: +%pre + - hook_log.response_headers.each do |k,v| + <strong>#{k}:</strong> #{v} + %br + +%h5 Response body: +%pre + :plain + #{hook_log.response_body} diff --git a/app/views/shared/hook_logs/_status_label.html.haml b/app/views/shared/hook_logs/_status_label.html.haml new file mode 100644 index 00000000000..b4ea8e6f952 --- /dev/null +++ b/app/views/shared/hook_logs/_status_label.html.haml @@ -0,0 +1,3 @@ +- label_status = hook_log.success? ? 'label-success' : 'label-danger' +%span{ class: "label #{label_status}" } + = hook_log.response_status diff --git a/app/workers/remove_old_web_hook_logs_worker.rb b/app/workers/remove_old_web_hook_logs_worker.rb new file mode 100644 index 00000000000..555e1bb8691 --- /dev/null +++ b/app/workers/remove_old_web_hook_logs_worker.rb @@ -0,0 +1,10 @@ +class RemoveOldWebHookLogsWorker + include Sidekiq::Worker + include CronjobQueue + + WEB_HOOK_LOG_LIFETIME = 2.days + + def perform + WebHookLog.destroy_all(['created_at < ?', Time.now - WEB_HOOK_LOG_LIFETIME]) + end +end diff --git a/app/workers/system_hook_worker.rb b/app/workers/system_hook_worker.rb deleted file mode 100644 index 55d4e7d6dab..00000000000 --- a/app/workers/system_hook_worker.rb +++ /dev/null @@ -1,10 +0,0 @@ -class SystemHookWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue - - sidekiq_options retry: 4 - - def perform(hook_id, data, hook_name) - SystemHook.find(hook_id).execute(data, hook_name) - end -end diff --git a/app/workers/project_web_hook_worker.rb b/app/workers/web_hook_worker.rb index d973e662ff2..ad5ddf02a12 100644 --- a/app/workers/project_web_hook_worker.rb +++ b/app/workers/web_hook_worker.rb @@ -1,11 +1,13 @@ -class ProjectWebHookWorker +class WebHookWorker include Sidekiq::Worker include DedicatedSidekiqQueue sidekiq_options retry: 4 def perform(hook_id, data, hook_name) + hook = WebHook.find(hook_id) data = data.with_indifferent_access - WebHook.find(hook_id).execute(data, hook_name) + + WebHookService.new(hook, data, hook_name).execute end end |