diff options
106 files changed, 1289 insertions, 471 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 638553d7bf7..5ee22fa6c36 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -150,6 +150,7 @@ stages: # Trigger a package build on omnibus-gitlab repository build-package: + image: ruby:2.3-alpine before_script: [] services: [] variables: diff --git a/Gemfile.lock b/Gemfile.lock index 873cd8781ef..dd2c85052f3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -341,7 +341,7 @@ GEM grape-entity (0.6.0) activesupport multi_json (>= 1.3.2) - grpc (1.2.5) + grpc (1.3.4) google-protobuf (~> 3.1) googleauth (~> 0.5.1) haml (4.0.7) diff --git a/app/assets/javascripts/droplab/plugins/ajax_filter.js b/app/assets/javascripts/droplab/plugins/ajax_filter.js index cfd7e2ca189..a5427417031 100644 --- a/app/assets/javascripts/droplab/plugins/ajax_filter.js +++ b/app/assets/javascripts/droplab/plugins/ajax_filter.js @@ -1,4 +1,5 @@ /* eslint-disable */ +import AjaxCache from '../../lib/utils/ajax_cache'; const AjaxFilter = { init: function(hook) { @@ -58,50 +59,24 @@ const AjaxFilter = { this.loading = true; var params = config.params || {}; params[config.searchKey] = searchValue; - var self = this; - self.cache = self.cache || {}; var url = config.endpoint + this.buildParams(params); - var urlCachedData = self.cache[url]; - if (urlCachedData) { - self._loadData(urlCachedData, config, self); - } else { - this._loadUrlData(url) - .then(function(data) { - self._loadData(data, config, self); - }, config.onError).catch(config.onError); - } + return AjaxCache.retrieve(url) + .then((data) => { + this._loadData(data, config); + }) + .catch(config.onError); }, - _loadUrlData: function _loadUrlData(url) { - var self = this; - return new Promise(function(resolve, reject) { - var xhr = new XMLHttpRequest; - xhr.open('GET', url, true); - xhr.onreadystatechange = function () { - if(xhr.readyState === XMLHttpRequest.DONE) { - if (xhr.status === 200) { - var data = JSON.parse(xhr.responseText); - self.cache[url] = data; - return resolve(data); - } else { - return reject([xhr.responseText, xhr.status]); - } - } - }; - xhr.send(); - }); - }, - - _loadData: function _loadData(data, config, self) { - const list = self.hook.list; + _loadData(data, config) { + const list = this.hook.list; if (config.loadingTemplate && list.data === undefined || list.data.length === 0) { const dataLoadingTemplate = list.list.querySelector('[data-loading-template]'); if (dataLoadingTemplate) { - dataLoadingTemplate.outerHTML = self.listTemplate; + dataLoadingTemplate.outerHTML = this.listTemplate; } } - if (!self.destroyed) { + if (!this.destroyed) { var hookListChildren = list.list.children; var onlyDynamicList = hookListChildren.length === 1 && hookListChildren[0].hasAttribute('data-dynamic'); if (onlyDynamicList && data.length === 0) { @@ -109,7 +84,7 @@ const AjaxFilter = { } list.setData.call(list, data); } - self.notLoading(); + this.notLoading(); list.currentIndex = 0; }, diff --git a/app/assets/javascripts/raven/raven_config.js b/app/assets/javascripts/raven/raven_config.js index da3fb7a6744..ae54fa5f1a9 100644 --- a/app/assets/javascripts/raven/raven_config.js +++ b/app/assets/javascripts/raven/raven_config.js @@ -1,4 +1,5 @@ import Raven from 'raven-js'; +import $ from 'jquery'; const IGNORE_ERRORS = [ // Random plugins/extensions @@ -74,7 +75,7 @@ const RavenConfig = { }, bindRavenErrors() { - window.$(document).on('ajaxError.raven', this.handleRavenErrors); + $(document).on('ajaxError.raven', this.handleRavenErrors); }, handleRavenErrors(event, req, config, err) { diff --git a/app/assets/stylesheets/framework/awards.scss b/app/assets/stylesheets/framework/awards.scss index 0db3ac1a60e..d64b1237b2c 100644 --- a/app/assets/stylesheets/framework/awards.scss +++ b/app/assets/stylesheets/framework/awards.scss @@ -110,6 +110,7 @@ .award-control { margin: 0 5px 6px 0; outline: 0; + position: relative; &.disabled { cursor: default; @@ -227,8 +228,8 @@ .award-control-icon-positive, .award-control-icon-super-positive { position: absolute; - left: 11px; - bottom: 7px; + left: 10px; + bottom: 6px; opacity: 0; @include transition(opacity, transform); } 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/application_controller.rb b/app/controllers/application_controller.rb index 8ce9150e4a9..ab5aed24917 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -11,6 +11,7 @@ class ApplicationController < ActionController::Base include EnforcesTwoFactorAuthentication before_action :authenticate_user_from_private_token! + before_action :authenticate_user_from_rss_token! before_action :authenticate_user! before_action :validate_user_service_ticket! before_action :check_password_expiration @@ -72,13 +73,20 @@ class ApplicationController < ActionController::Base user = User.find_by_authentication_token(token) || User.find_by_personal_access_token(token) - if user && can?(user, :log_in) - # Notice we are passing store false, so the user is not - # actually stored in the session and a token is needed - # for every request. If you want the token to work as a - # sign in token, you can simply remove store: false. - sign_in user, store: false - end + sessionless_sign_in(user) + end + + # This filter handles authentication for atom request with an rss_token + def authenticate_user_from_rss_token! + return unless request.format.atom? + + token = params[:rss_token].presence + + return unless token.present? + + user = User.find_by_rss_token(token) + + sessionless_sign_in(user) end def log_exception(exception) @@ -282,4 +290,14 @@ class ApplicationController < ActionController::Base ensure Gitlab::I18n.reset_locale end + + def sessionless_sign_in(user) + if user && can?(user, :log_in) + # Notice we are passing store false, so the user is not + # actually stored in the session and a token is needed + # for every request. If you want the token to work as a + # sign in token, you can simply remove store: false. + sign_in user, store: false + end + 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/profiles_controller.rb b/app/controllers/profiles_controller.rb index 57e23cea00e..8cd1c47eb3f 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -40,6 +40,14 @@ class ProfilesController < Profiles::ApplicationController redirect_to profile_account_path end + def reset_rss_token + if current_user.reset_rss_token! + flash[:notice] = "RSS token was successfully reset" + end + + redirect_to profile_account_path + end + def audit_log @events = AuditEvent.where(entity_type: "User", entity_id: current_user.id). order("created_at DESC"). 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/helpers/rss_helper.rb b/app/helpers/rss_helper.rb index ea5d2932ef4..9ac4df88dc3 100644 --- a/app/helpers/rss_helper.rb +++ b/app/helpers/rss_helper.rb @@ -1,5 +1,5 @@ module RssHelper def rss_url_options - { format: :atom, private_token: current_user.try(:private_token) } + { format: :atom, rss_token: current_user.try(:rss_token) } end end 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/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index b2494a0be6e..8977a7cdafe 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -77,6 +77,14 @@ class KubernetesService < DeploymentService ] end + def actual_namespace + if namespace.present? + namespace + else + default_namespace + end + end + # Check we can connect to the Kubernetes API def test(*args) kubeclient = build_kubeclient! @@ -91,7 +99,7 @@ class KubernetesService < DeploymentService variables = [ { key: 'KUBE_URL', value: api_url, public: true }, { key: 'KUBE_TOKEN', value: token, public: false }, - { key: 'KUBE_NAMESPACE', value: namespace_variable, public: true } + { key: 'KUBE_NAMESPACE', value: actual_namespace, public: true } ] if ca_pem.present? @@ -110,7 +118,7 @@ class KubernetesService < DeploymentService with_reactive_cache do |data| pods = data.fetch(:pods, nil) filter_pods(pods, app: environment.slug). - flat_map { |pod| terminals_for_pod(api_url, namespace, pod) }. + flat_map { |pod| terminals_for_pod(api_url, actual_namespace, pod) }. each { |terminal| add_terminal_auth(terminal, terminal_auth) } end end @@ -124,7 +132,7 @@ class KubernetesService < DeploymentService # Store as hashes, rather than as third-party types pods = begin - kubeclient.get_pods(namespace: namespace).as_json + kubeclient.get_pods(namespace: actual_namespace).as_json rescue KubeException => err raise err unless err.error_code == 404 [] @@ -142,20 +150,12 @@ class KubernetesService < DeploymentService default_namespace || TEMPLATE_PLACEHOLDER end - def namespace_variable - if namespace.present? - namespace - else - default_namespace - end - end - def default_namespace "#{project.path}-#{project.id}" if project.present? end def build_kubeclient!(api_path: 'api', api_version: 'v1') - raise "Incomplete settings" unless api_url && namespace && token + raise "Incomplete settings" unless api_url && actual_namespace && token ::Kubeclient::Client.new( join_api_url(api_path), diff --git a/app/models/user.rb b/app/models/user.rb index 55614233230..9b0c1ebd7c5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -15,6 +15,7 @@ class User < ActiveRecord::Base add_authentication_token_field :authentication_token add_authentication_token_field :incoming_email_token + add_authentication_token_field :rss_token default_value_for :admin, false default_value_for(:external) { current_application_settings.user_default_external } @@ -1004,6 +1005,13 @@ class User < ActiveRecord::Base save end + # each existing user needs to have an `rss_token`. + # we do this on read since migrating all existing users is not a feasible + # solution. + def rss_token + ensure_rss_token! + end + protected # override, from Devise::Validatable 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/admin/requests_profiles/index.html.haml b/app/views/admin/requests_profiles/index.html.haml index ae918086a57..c7b63d9de98 100644 --- a/app/views/admin/requests_profiles/index.html.haml +++ b/app/views/admin/requests_profiles/index.html.haml @@ -20,7 +20,7 @@ %ul.content-list - profiles.each do |profile| %li - = link_to profile.time.to_s(:long), admin_requests_profile_path(profile), data: {no_turbolink: true} + = link_to profile.time.to_s(:long), admin_requests_profile_path(profile) - else %p No profiles found 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/profiles/accounts/_reset_token.html.haml b/app/views/profiles/accounts/_reset_token.html.haml new file mode 100644 index 00000000000..c31a4a8ecd4 --- /dev/null +++ b/app/views/profiles/accounts/_reset_token.html.haml @@ -0,0 +1,11 @@ +- name = label.parameterize +- attribute = name.underscore + +.reset-action + %p.cgray + = label_tag name, label, class: "label-light" + = text_field_tag name, current_user.send(attribute), class: 'form-control', readonly: true, onclick: 'this.select()' + %p.help-block + = help_text + .prepend-top-default + = link_to button_label, [:reset, attribute, :profile], method: :put, data: { confirm: 'Are you sure?' }, class: 'btn btn-default private-token' diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 73f33e69d68..a319b18e507 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -8,35 +8,17 @@ .row.prepend-top-default .col-lg-3.profile-settings-sidebar %h4.prepend-top-0 - = incoming_email_token_enabled? ? "Private Tokens" : "Private Token" + Private Tokens %p - Keep - = incoming_email_token_enabled? ? "these tokens" : "this token" - secret, anyone with access to them can interact with GitLab as if they were you. + Keep these tokens secret, anyone with access to them can interact with + GitLab as if they were you. .col-lg-9.private-tokens-reset - .reset-action - %p.cgray - - if current_user.private_token - = label_tag "private-token", "Private token", class: "label-light" - = text_field_tag "private-token", current_user.private_token, class: "form-control", readonly: true, onclick: "this.select()" - - else - %span You don't have one yet. Click generate to fix it. - %p.help-block - Your private token is used to access the API and Atom feeds without username/password authentication. - .prepend-top-default - - if current_user.private_token - = link_to 'Reset private token', reset_private_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default private-token" - - else - = f.submit 'Generate', class: "btn btn-default" + = render partial: 'reset_token', locals: { label: 'Private token', button_label: 'Reset private token', help_text: 'Your private token is used to access the API and Atom feeds without username/password authentication.' } + + = render partial: 'reset_token', locals: { label: 'RSS token', button_label: 'Reset RSS token', help_text: 'Your RSS token is used to create urls for personalized RSS feeds.' } + - if incoming_email_token_enabled? - .reset-action - %p.cgray - = label_tag "incoming-email-token", "Incoming Email Token", class: 'label-light' - = text_field_tag "incoming-email-token", current_user.incoming_email_token, class: "form-control", readonly: true, onclick: "this.select()" - %p.help-block - Your incoming email token is used to create new issues by email, and is included in your project-specific email addresses. - .prepend-top-default - = link_to 'Reset incoming email token', reset_incoming_email_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default incoming-email-token" + = render partial: 'reset_token', locals: { label: 'Incoming email token', button_label: 'Reset incoming email token', help_text: 'Your incoming email token is used to create new issues by email, and is included in your project-specific email addresses.' } %hr .row.prepend-top-default 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/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml index c9ecfc81266..8b095f4ca10 100644 --- a/app/views/projects/issues/_discussion.html.haml +++ b/app/views/projects/issues/_discussion.html.haml @@ -1,7 +1,7 @@ - content_for :note_actions do - if can?(current_user, :update_issue, @issue) - = link_to 'Reopen issue', issue_path(@issue, issue: {state_event: :reopen}, format: 'json'), data: {no_turbolink: true, original_text: "Reopen issue", alternative_text: "Comment & reopen issue"}, class: "btn btn-nr btn-reopen btn-comment js-note-target-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' - = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, format: 'json'), data: {no_turbolink: true, original_text: "Close issue", alternative_text: "Comment & close issue"}, class: "btn btn-nr btn-close btn-comment js-note-target-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' + = link_to 'Reopen issue', issue_path(@issue, issue: {state_event: :reopen}, format: 'json'), data: {original_text: "Reopen issue", alternative_text: "Comment & reopen issue"}, class: "btn btn-nr btn-reopen btn-comment js-note-target-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' + = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, format: 'json'), data: {original_text: "Close issue", alternative_text: "Comment & close issue"}, class: "btn btn-nr btn-close btn-comment js-note-target-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' #notes = render 'shared/notes/notes_with_form', :autocomplete => true diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 0e928bfbe6d..67403c36d7f 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -33,9 +33,9 @@ %li = link_to 'Edit', edit_namespace_project_issue_path(@project.namespace, @project, @issue) %li - = link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, format: 'json'), data: {no_turbolink: true}, class: "btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' + = link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, format: 'json'), class: "btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' %li - = link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, format: 'json'), data: {no_turbolink: true}, class: "btn-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' + = link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, format: 'json'), class: "btn-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' - if can_report_spam %li = link_to 'Submit as spam', mark_as_spam_namespace_project_issue_path(@project.namespace, @project, @issue), method: :post, class: 'btn-spam', title: 'Submit as spam' @@ -46,8 +46,8 @@ - if can_update_issue = link_to 'Edit', edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'hidden-xs hidden-sm btn btn-grouped issuable-edit' - = link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, format: 'json'), data: {no_turbolink: true}, class: "hidden-xs hidden-sm btn btn-grouped btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' - = link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, format: 'json'), data: {no_turbolink: true}, class: "hidden-xs hidden-sm btn btn-grouped btn-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' + = link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, format: 'json'), class: "hidden-xs hidden-sm btn btn-grouped btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' + = link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, format: 'json'), class: "hidden-xs hidden-sm btn btn-grouped btn-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue' - if can_report_spam = link_to 'Submit as spam', mark_as_spam_namespace_project_issue_path(@project.namespace, @project, @issue), method: :post, class: 'hidden-xs hidden-sm btn btn-grouped btn-spam', title: 'Submit as spam' = link_to new_namespace_project_issue_path(@project.namespace, @project), class: 'hidden-xs hidden-sm btn btn-grouped new-issue-link btn-new btn-inverted', title: 'New issue', id: 'new_issue_link' do 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/views/users/show.html.haml b/app/views/users/show.html.haml index 2b70d70e360..c587155bc4f 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -71,7 +71,7 @@ = @user.location - unless @user.organization.blank? .profile-link-holder.middle-dot-divider - = icon('building') + = icon('briefcase') = @user.organization - if @user.bio.present? 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 diff --git a/changelogs/unreleased/12614-fix-long-message-from-mr.yml b/changelogs/unreleased/12614-fix-long-message-from-mr.yml new file mode 100644 index 00000000000..30408ea4216 --- /dev/null +++ b/changelogs/unreleased/12614-fix-long-message-from-mr.yml @@ -0,0 +1,4 @@ +--- +title: Implement web hook logging +merge_request: 11027 +author: Alexander Randa diff --git a/changelogs/unreleased/32799-remove-no_turbolink-attribute-from-haml.yml b/changelogs/unreleased/32799-remove-no_turbolink-attribute-from-haml.yml new file mode 100644 index 00000000000..9c1c1fe77f2 --- /dev/null +++ b/changelogs/unreleased/32799-remove-no_turbolink-attribute-from-haml.yml @@ -0,0 +1,4 @@ +--- +title: Remove redundant data-turbolink attributes from links +merge_request: 11672 +author: blackst0ne diff --git a/changelogs/unreleased/32807-company-icon.yml b/changelogs/unreleased/32807-company-icon.yml new file mode 100644 index 00000000000..718108d3733 --- /dev/null +++ b/changelogs/unreleased/32807-company-icon.yml @@ -0,0 +1,4 @@ +--- +title: Use briefcase icon for company in profile page +merge_request: +author: diff --git a/changelogs/unreleased/feature-rss-scoped-token.yml b/changelogs/unreleased/feature-rss-scoped-token.yml new file mode 100644 index 00000000000..740d8778be2 --- /dev/null +++ b/changelogs/unreleased/feature-rss-scoped-token.yml @@ -0,0 +1,4 @@ +--- +title: Expose atom links with an RSS token instead of using the private token +merge_request: 11647 +author: Alexis Reigel diff --git a/changelogs/unreleased/fix-terminals-support-for-kubernetes-service.yml b/changelogs/unreleased/fix-terminals-support-for-kubernetes-service.yml new file mode 100644 index 00000000000..fb91da9510c --- /dev/null +++ b/changelogs/unreleased/fix-terminals-support-for-kubernetes-service.yml @@ -0,0 +1,4 @@ +--- +title: Fix terminals support for Kubernetes Service +merge_request: +author: diff --git a/config/application.rb b/config/application.rb index 95ba6774916..b0533759252 100644 --- a/config/application.rb +++ b/config/application.rb @@ -65,6 +65,7 @@ module Gitlab hook import_url incoming_email_token + rss_token key otp_attempt password diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 5a90830b5b3..4fb4baf631f 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -368,11 +368,14 @@ Settings.cron_jobs['gitlab_usage_ping_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['gitlab_usage_ping_worker']['cron'] ||= Settings.__send__(:cron_random_weekly_time) Settings.cron_jobs['gitlab_usage_ping_worker']['job_class'] = 'GitlabUsagePingWorker' -# Every day at 00:30 Settings.cron_jobs['schedule_update_user_activity_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['schedule_update_user_activity_worker']['cron'] ||= '30 0 * * *' Settings.cron_jobs['schedule_update_user_activity_worker']['job_class'] = 'ScheduleUpdateUserActivityWorker' +Settings.cron_jobs['remove_old_web_hook_logs_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['remove_old_web_hook_logs_worker']['cron'] ||= '40 0 * * *' +Settings.cron_jobs['remove_old_web_hook_logs_worker']['job_class'] = 'RemoveOldWebHookLogsWorker' + # # GitLab Shell # diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 6e34dd3a2eb..c20581b1333 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -54,6 +54,12 @@ namespace :admin do member do get :test end + + resources :hook_logs, only: [:show] do + member do + get :retry + end + end end resources :broadcast_messages, only: [:index, :edit, :create, :update, :destroy] do diff --git a/config/routes/profile.rb b/config/routes/profile.rb index 07c341999ea..3dc890e5785 100644 --- a/config/routes/profile.rb +++ b/config/routes/profile.rb @@ -5,6 +5,7 @@ resource :profile, only: [:show, :update] do put :reset_private_token put :reset_incoming_email_token + put :reset_rss_token put :update_username end diff --git a/config/routes/project.rb b/config/routes/project.rb index a2bc63a4734..bec1f04d1f9 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -216,6 +216,12 @@ constraints(ProjectUrlConstrainer.new) do member do get :test end + + resources :hook_logs, only: [:show] do + member do + get :retry + end + end end resources :container_registry, only: [:index, :destroy], diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 0ca1f565185..93df2d6f5ff 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -44,9 +44,8 @@ - [project_cache, 1] - [project_destroy, 1] - [project_export, 1] - - [project_web_hook, 1] + - [web_hook, 1] - [repository_check, 1] - - [system_hook, 1] - [git_garbage_collect, 1] - [reactive_caching, 1] - [cronjob, 1] diff --git a/db/migrate/20170427103502_create_web_hook_logs.rb b/db/migrate/20170427103502_create_web_hook_logs.rb new file mode 100644 index 00000000000..3643c52180c --- /dev/null +++ b/db/migrate/20170427103502_create_web_hook_logs.rb @@ -0,0 +1,22 @@ +# rubocop:disable all +class CreateWebHookLogs < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :web_hook_logs do |t| + t.references :web_hook, null: false, index: true, foreign_key: { on_delete: :cascade } + + t.string :trigger + t.string :url + t.text :request_headers + t.text :request_data + t.text :response_headers + t.text :response_body + t.string :response_status + t.float :execution_duration + t.string :internal_error_message + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20170523091700_add_rss_token_to_users.rb b/db/migrate/20170523091700_add_rss_token_to_users.rb new file mode 100644 index 00000000000..06a85f6ac3d --- /dev/null +++ b/db/migrate/20170523091700_add_rss_token_to_users.rb @@ -0,0 +1,19 @@ +class AddRssTokenToUsers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column :users, :rss_token, :string + + add_concurrent_index :users, :rss_token + end + + def down + remove_concurrent_index :users, :rss_token if index_exists? :users, :rss_token + + remove_column :users, :rss_token + end +end diff --git a/db/schema.rb b/db/schema.rb index 84e25427d7f..f6b513a5725 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170521184006) do +ActiveRecord::Schema.define(version: 20170523091700) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1362,6 +1362,7 @@ ActiveRecord::Schema.define(version: 20170521184006) do t.date "last_activity_on" t.boolean "notified_of_own_activity" t.string "preferred_language" + t.string "rss_token" end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree @@ -1375,6 +1376,7 @@ ActiveRecord::Schema.define(version: 20170521184006) do add_index "users", ["name"], name: "index_users_on_name", using: :btree add_index "users", ["name"], name: "index_users_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"} add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree + add_index "users", ["rss_token"], name: "index_users_on_rss_token", using: :btree add_index "users", ["state"], name: "index_users_on_state", using: :btree add_index "users", ["username"], name: "index_users_on_username", using: :btree add_index "users", ["username"], name: "index_users_on_username_trigram", using: :gin, opclasses: {"username"=>"gin_trgm_ops"} @@ -1389,6 +1391,23 @@ ActiveRecord::Schema.define(version: 20170521184006) do add_index "users_star_projects", ["project_id"], name: "index_users_star_projects_on_project_id", using: :btree add_index "users_star_projects", ["user_id", "project_id"], name: "index_users_star_projects_on_user_id_and_project_id", unique: true, using: :btree + create_table "web_hook_logs", force: :cascade do |t| + t.integer "web_hook_id", null: false + t.string "trigger" + t.string "url" + t.text "request_headers" + t.text "request_data" + t.text "response_headers" + t.text "response_body" + t.string "response_status" + t.float "execution_duration" + t.string "internal_error_message" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "web_hook_logs", ["web_hook_id"], name: "index_web_hook_logs_on_web_hook_id", using: :btree + create_table "web_hooks", force: :cascade do |t| t.string "url", limit: 2000 t.integer "project_id" @@ -1452,4 +1471,5 @@ ActiveRecord::Schema.define(version: 20170521184006) do add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade add_foreign_key "trending_projects", "projects", on_delete: :cascade add_foreign_key "u2f_registrations", "users" + add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade end diff --git a/doc/install/kubernetes/gitlab_chart.md b/doc/install/kubernetes/gitlab_chart.md index 39ff4f8c1b8..b4ffd57afbb 100644 --- a/doc/install/kubernetes/gitlab_chart.md +++ b/doc/install/kubernetes/gitlab_chart.md @@ -206,9 +206,43 @@ its class in an annotation. >**Note:** The Ingress alone doesn't expose GitLab externally. You need to have a Ingress controller setup to do that. -Setting up an Ingress controller can be as simple as installing the `nginx-ingress` helm chart. But be sure +Setting up an Ingress controller can be done by installing the `nginx-ingress` helm chart. But be sure to read the [documentation](https://github.com/kubernetes/charts/blob/master/stable/nginx-ingress/README.md) +#### Preserving Source IPs + +If you are using the `LoadBalancer` serviceType you may run into issues where user IP addresses in the GitLab +logs, and used in abuse throttling are not accurate. This is due to how Kubernetes uses source NATing on cluster nodes without endpoints. + +See the [Kubernetes documentation](https://kubernetes.io/docs/tutorials/services/source-ip/#source-ip-for-services-with-typeloadbalancer) for more information. + +To fix this you can add the following service annotation to your `values.yaml` + +```yaml +## For minikube, set this to NodePort, elsewhere use LoadBalancer +## ref: http://kubernetes.io/docs/user-guide/services/#publishing-services---service-types +## +serviceType: LoadBalancer + +## Optional annotations for gitlab service. +serviceAnnotations: + service.beta.kubernetes.io/external-traffic: "OnlyLocal" +``` + +>**Note:** +If you are using the ingress routing, you will likely also need to specify the annotation on the service for the ingress +controller. For `nginx-ingress` you can check the +[configuration documentation](https://github.com/kubernetes/charts/blob/master/stable/nginx-ingress/README.md#configuration) +on how to add the annotation to the `controller.service.annotations` array. + +>**Note:** +When using the `nginx-ingress` controller on Google Container Engine (GKE), and using the `external-traffic` annotation, +you will need to additionally set the `controller.kind` to be DaemonSet. Otherwise only pods running on the same node +as the nginx controller will be able to reach GitLab. This may result in pods within your cluster not being able to reach GitLab. +See the [Kubernetes documentation](https://kubernetes.io/docs/tutorials/services/source-ip/#source-ip-for-services-with-typeloadbalancer) and +[nginx-ingress configuration documentation](https://github.com/kubernetes/charts/blob/master/stable/nginx-ingress/README.md#configuration) +for more information. + ### External database You can configure the GitLab Helm chart to connect to an external PostgreSQL diff --git a/doc/user/project/integrations/img/webhook_logs.png b/doc/user/project/integrations/img/webhook_logs.png Binary files differnew file mode 100755 index 00000000000..917068d9398 --- /dev/null +++ b/doc/user/project/integrations/img/webhook_logs.png diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 48d49c5d40c..d0bb1cd11a8 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -1017,6 +1017,22 @@ X-Gitlab-Event: Build Hook } ``` +## Troubleshoot webhooks + +Gitlab stores each perform of the webhook. +You can find records for last 2 days in "Recent Deliveries" section on the edit page of each webhook. + +![Recent deliveries](img/webhook_logs.png) + +In this section you can see HTTP status code (green for 200-299 codes, red for the others, `internal error` for failed deliveries ), triggered event, a time when the event was called, elapsed time of the request. + +If you need more information about execution, you can click `View details` link. +On this page, you can see data that GitLab sends (request headers and body) and data that it received (response headers and body). + +From this page, you can repeat delivery with the same data by clicking `Resend Request` button. + +>**Note:** If URL or secret token of the webhook were updated, data will be delivered to the new address. + ## Example webhook receiver If you want to see GitLab's webhooks in action for testing purposes you can use diff --git a/doc/user/project/milestones/img/progress.png b/doc/user/project/milestones/img/progress.png Binary files differnew file mode 100644 index 00000000000..c85aecca729 --- /dev/null +++ b/doc/user/project/milestones/img/progress.png diff --git a/doc/user/project/milestones/index.md b/doc/user/project/milestones/index.md index a43a42a8fe8..99233ed5ae2 100644 --- a/doc/user/project/milestones/index.md +++ b/doc/user/project/milestones/index.md @@ -44,3 +44,11 @@ special options available when filtering by milestone: * **Started** - show issues or merge requests from any milestone with a start date less than today. Note that this can return results from several milestones in the same project. + +## Milestone progress statistics + +Milestone statistics can be viewed in the milestone sidebar. The milestone percentage statistic +is calculated as; closed and merged merge requests plus all closed issues divided by +total merge requests and issues. + +![Milestone statistics](img/progress.png) diff --git a/features/project/hooks.feature b/features/project/hooks.feature deleted file mode 100644 index 627738004c4..00000000000 --- a/features/project/hooks.feature +++ /dev/null @@ -1,37 +0,0 @@ -Feature: Project Hooks - Background: - Given I sign in as a user - And I own project "Shop" - - Scenario: I should see hook list - Given project has hook - When I visit project hooks page - Then I should see project hook - - Scenario: I add new hook - Given I visit project hooks page - When I submit new hook - Then I should see newly created hook - - Scenario: I add new hook with SSL verification enabled - Given I visit project hooks page - When I submit new hook with SSL verification enabled - Then I should see newly created hook with SSL verification enabled - - Scenario: I test hook - Given project has hook - And I visit project hooks page - When I click test hook button - Then hook should be triggered - - Scenario: I test a hook on empty project - Given I own empty project with hook - And I visit project hooks page - When I click test hook button - Then I should see hook error message - - Scenario: I test a hook on down URL - Given project has hook - And I visit project hooks page - When I click test hook button with invalid URL - Then I should see hook service down error message diff --git a/features/steps/project/hooks.rb b/features/steps/project/hooks.rb deleted file mode 100644 index 945d58a6458..00000000000 --- a/features/steps/project/hooks.rb +++ /dev/null @@ -1,75 +0,0 @@ -require 'webmock' - -class Spinach::Features::ProjectHooks < Spinach::FeatureSteps - include SharedAuthentication - include SharedProject - include SharedPaths - include RSpec::Matchers - include RSpec::Mocks::ExampleMethods - include WebMock::API - - step 'project has hook' do - @hook = create(:project_hook, project: current_project) - end - - step 'I own empty project with hook' do - @project = create(:empty_project, - name: 'Empty Project', namespace: @user.namespace) - @hook = create(:project_hook, project: current_project) - end - - step 'I should see project hook' do - expect(page).to have_content @hook.url - end - - step 'I submit new hook' do - @url = 'http://example.org/1' - fill_in "hook_url", with: @url - expect { click_button "Add webhook" }.to change(ProjectHook, :count).by(1) - end - - step 'I submit new hook with SSL verification enabled' do - @url = 'http://example.org/2' - fill_in "hook_url", with: @url - check "hook_enable_ssl_verification" - expect { click_button "Add webhook" }.to change(ProjectHook, :count).by(1) - end - - step 'I should see newly created hook' do - expect(current_path).to eq namespace_project_settings_integrations_path(current_project.namespace, current_project) - expect(page).to have_content(@url) - end - - step 'I should see newly created hook with SSL verification enabled' do - expect(current_path).to eq namespace_project_settings_integrations_path(current_project.namespace, current_project) - expect(page).to have_content(@url) - expect(page).to have_content("SSL Verification: enabled") - end - - step 'I click test hook button' do - stub_request(:post, @hook.url).to_return(status: 200) - click_link 'Test' - end - - step 'I click test hook button with invalid URL' do - stub_request(:post, @hook.url).to_raise(SocketError) - click_link 'Test' - end - - step 'hook should be triggered' do - expect(current_path).to eq namespace_project_settings_integrations_path(current_project.namespace, current_project) - expect(page).to have_selector '.flash-notice', - text: 'Hook executed successfully: HTTP 200' - end - - step 'I should see hook error message' do - expect(page).to have_selector '.flash-alert', - text: 'Hook execution failed. '\ - 'Ensure the project has commits.' - end - - step 'I should see hook service down error message' do - expect(page).to have_selector '.flash-alert', - text: 'Hook execution failed: Exception from' - end -end diff --git a/lib/tasks/tokens.rake b/lib/tasks/tokens.rake index 95735f43802..ad1818ff1fa 100644 --- a/lib/tasks/tokens.rake +++ b/lib/tasks/tokens.rake @@ -11,6 +11,11 @@ namespace :tokens do reset_all_users_token(:reset_incoming_email_token!) end + desc "Reset all GitLab RSS tokens" + task reset_all_rss: :environment do + reset_all_users_token(:reset_rss_token!) + end + def reset_all_users_token(reset_token_method) TmpUser.find_in_batches do |batch| puts "Processing batch starting with user ID: #{batch.first.id}" @@ -35,4 +40,9 @@ class TmpUser < ActiveRecord::Base write_new_token(:incoming_email_token) save!(validate: false) end + + def reset_rss_token! + write_new_token(:rss_token) + save!(validate: false) + end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index d40aae04fc3..3f99e2ff596 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -99,6 +99,42 @@ describe ApplicationController do end end + describe '#authenticate_user_from_rss_token' do + describe "authenticating a user from an RSS token" do + controller(described_class) do + def index + render text: 'authenticated' + end + end + + context "when the 'rss_token' param is populated with the RSS token" do + context 'when the request format is atom' do + it "logs the user in" do + get :index, rss_token: user.rss_token, format: :atom + expect(response).to have_http_status 200 + expect(response.body).to eq 'authenticated' + end + end + + context 'when the request format is not atom' do + it "doesn't log the user in" do + get :index, rss_token: user.rss_token + expect(response.status).not_to have_http_status 200 + expect(response.body).not_to eq 'authenticated' + end + end + end + + context "when the 'rss_token' param is populated with an invalid RSS token" do + it "doesn't log the user" do + get :index, rss_token: "token" + expect(response.status).not_to eq 200 + expect(response.body).not_to eq 'authenticated' + end + end + end + end + describe '#route_not_found' do it 'renders 404 if authenticated' do allow(controller).to receive(:current_user).and_return(user) diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 28ddd0da753..3fad4d2d658 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -20,7 +20,6 @@ FactoryGirl.define do project factory: :empty_project active true properties({ - namespace: 'somepath', api_url: 'https://kubernetes.example.com', token: 'a' * 40 }) diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 33fa80772ff..e60fe713bc3 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -8,6 +8,10 @@ FactoryGirl.define do confirmation_token { nil } can_create_group true + before(:create) do |user| + user.ensure_rss_token + end + trait :admin do admin true end diff --git a/spec/factories/web_hook_log.rb b/spec/factories/web_hook_log.rb new file mode 100644 index 00000000000..230b3f6b26e --- /dev/null +++ b/spec/factories/web_hook_log.rb @@ -0,0 +1,14 @@ +FactoryGirl.define do + factory :web_hook_log do + web_hook factory: :project_hook + trigger 'push_hooks' + url { generate(:url) } + request_headers {} + request_data {} + response_headers {} + response_body '' + response_status '200' + execution_duration 2.0 + internal_error_message nil + end +end diff --git a/spec/features/admin/admin_hook_logs_spec.rb b/spec/features/admin/admin_hook_logs_spec.rb new file mode 100644 index 00000000000..5b67f4de6ac --- /dev/null +++ b/spec/features/admin/admin_hook_logs_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +feature 'Admin::HookLogs', feature: true do + let(:project) { create(:project) } + let(:system_hook) { create(:system_hook) } + let(:hook_log) { create(:web_hook_log, web_hook: system_hook, internal_error_message: 'some error') } + + before do + login_as :admin + end + + scenario 'show list of hook logs' do + hook_log + visit edit_admin_hook_path(system_hook) + + expect(page).to have_content('Recent Deliveries') + expect(page).to have_content(hook_log.url) + end + + scenario 'show hook log details' do + hook_log + visit edit_admin_hook_path(system_hook) + click_link 'View details' + + expect(page).to have_content("POST #{hook_log.url}") + expect(page).to have_content(hook_log.internal_error_message) + expect(page).to have_content('Resend Request') + end + + scenario 'retry hook log' do + WebMock.stub_request(:post, system_hook.url) + + hook_log + visit edit_admin_hook_path(system_hook) + click_link 'View details' + click_link 'Resend Request' + + expect(current_path).to eq(edit_admin_hook_path(system_hook)) + end +end diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb index c5f24d412d7..80f7ec43c06 100644 --- a/spec/features/admin/admin_hooks_spec.rb +++ b/spec/features/admin/admin_hooks_spec.rb @@ -58,10 +58,19 @@ describe 'Admin::Hooks', feature: true do end describe 'Remove existing hook' do - it 'remove existing hook' do - visit admin_hooks_path + context 'removes existing hook' do + it 'from hooks list page' do + visit admin_hooks_path + + expect { click_link 'Remove' }.to change(SystemHook, :count).by(-1) + end - expect { click_link 'Remove' }.to change(SystemHook, :count).by(-1) + it 'from hook edit page' do + visit admin_hooks_path + click_link 'Edit' + + expect { click_link 'Remove' }.to change(SystemHook, :count).by(-1) + end end end diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb index 9ea325ab41b..711c8a710f3 100644 --- a/spec/features/atom/dashboard_issues_spec.rb +++ b/spec/features/atom/dashboard_issues_spec.rb @@ -20,13 +20,20 @@ describe "Dashboard Issues Feed", feature: true do expect(body).to have_selector('title', text: "#{user.name} issues") end + it "renders atom feed via RSS token" do + visit issues_dashboard_path(:atom, rss_token: user.rss_token) + + expect(response_headers['Content-Type']).to have_content('application/atom+xml') + expect(body).to have_selector('title', text: "#{user.name} issues") + end + it "renders atom feed with url parameters" do - visit issues_dashboard_path(:atom, private_token: user.private_token, state: 'opened', assignee_id: user.id) + visit issues_dashboard_path(:atom, rss_token: user.rss_token, state: 'opened', assignee_id: user.id) link = find('link[type="application/atom+xml"]') params = CGI.parse(URI.parse(link[:href]).query) - expect(params).to include('private_token' => [user.private_token]) + expect(params).to include('rss_token' => [user.rss_token]) expect(params).to include('state' => ['opened']) expect(params).to include('assignee_id' => [user.id.to_s]) end @@ -35,7 +42,7 @@ describe "Dashboard Issues Feed", feature: true do let!(:issue2) { create(:issue, author: user, assignees: [assignee], project: project2, description: 'test desc') } it "renders issue fields" do - visit issues_dashboard_path(:atom, private_token: user.private_token) + visit issues_dashboard_path(:atom, rss_token: user.rss_token) entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue2.title}')]") @@ -58,7 +65,7 @@ describe "Dashboard Issues Feed", feature: true do end it "renders issue label and milestone info" do - visit issues_dashboard_path(:atom, private_token: user.private_token) + visit issues_dashboard_path(:atom, rss_token: user.rss_token) entry = find(:xpath, "//feed/entry[contains(summary/text(),'#{issue1.title}')]") diff --git a/spec/features/atom/dashboard_spec.rb b/spec/features/atom/dashboard_spec.rb index 746df36bb25..1df058b023c 100644 --- a/spec/features/atom/dashboard_spec.rb +++ b/spec/features/atom/dashboard_spec.rb @@ -11,6 +11,13 @@ describe "Dashboard Feed", feature: true do end end + context "projects atom feed via RSS token" do + it "renders projects atom feed" do + visit dashboard_projects_path(:atom, rss_token: user.rss_token) + expect(body).to have_selector('feed title') + end + end + context 'feed content' do let(:project) { create(:project) } let(:issue) { create(:issue, project: project, author: user, description: '') } @@ -20,7 +27,7 @@ describe "Dashboard Feed", feature: true do project.team << [user, :master] issue_event(issue, user) note_event(note, user) - visit dashboard_projects_path(:atom, private_token: user.private_token) + visit dashboard_projects_path(:atom, rss_token: user.rss_token) end it "has issue opened event" do diff --git a/spec/features/atom/issues_spec.rb b/spec/features/atom/issues_spec.rb index 4f6754ad541..a61231ea254 100644 --- a/spec/features/atom/issues_spec.rb +++ b/spec/features/atom/issues_spec.rb @@ -43,25 +43,40 @@ describe 'Issues Feed', feature: true do end end + context 'when authenticated via RSS token' do + it 'renders atom feed' do + visit namespace_project_issues_path(project.namespace, project, :atom, + rss_token: user.rss_token) + + expect(response_headers['Content-Type']). + to have_content('application/atom+xml') + expect(body).to have_selector('title', text: "#{project.name} issues") + expect(body).to have_selector('author email', text: issue.author_public_email) + expect(body).to have_selector('assignees assignee email', text: issue.assignees.first.public_email) + expect(body).to have_selector('assignee email', text: issue.assignees.first.public_email) + expect(body).to have_selector('entry summary', text: issue.title) + end + end + it "renders atom feed with url parameters for project issues" do visit namespace_project_issues_path(project.namespace, project, - :atom, private_token: user.private_token, state: 'opened', assignee_id: user.id) + :atom, rss_token: user.rss_token, state: 'opened', assignee_id: user.id) link = find('link[type="application/atom+xml"]') params = CGI.parse(URI.parse(link[:href]).query) - expect(params).to include('private_token' => [user.private_token]) + expect(params).to include('rss_token' => [user.rss_token]) expect(params).to include('state' => ['opened']) expect(params).to include('assignee_id' => [user.id.to_s]) end it "renders atom feed with url parameters for group issues" do - visit issues_group_path(group, :atom, private_token: user.private_token, state: 'opened', assignee_id: user.id) + visit issues_group_path(group, :atom, rss_token: user.rss_token, state: 'opened', assignee_id: user.id) link = find('link[type="application/atom+xml"]') params = CGI.parse(URI.parse(link[:href]).query) - expect(params).to include('private_token' => [user.private_token]) + expect(params).to include('rss_token' => [user.rss_token]) expect(params).to include('state' => ['opened']) expect(params).to include('assignee_id' => [user.id.to_s]) end diff --git a/spec/features/atom/users_spec.rb b/spec/features/atom/users_spec.rb index 7a2987e815d..fae5aaa52bd 100644 --- a/spec/features/atom/users_spec.rb +++ b/spec/features/atom/users_spec.rb @@ -11,6 +11,13 @@ describe "User Feed", feature: true do end end + context 'user atom feed via RSS token' do + it "renders user atom feed" do + visit user_path(user, :atom, rss_token: user.rss_token) + expect(body).to have_selector('feed title') + end + end + context 'feed content' do let(:project) { create(:project) } let(:issue) do @@ -40,7 +47,7 @@ describe "User Feed", feature: true do issue_event(issue, user) note_event(note, user) merge_request_event(merge_request, user) - visit user_path(user, :atom, private_token: user.private_token) + visit user_path(user, :atom, rss_token: user.rss_token) end it 'has issue opened event' do diff --git a/spec/features/dashboard/activity_spec.rb b/spec/features/dashboard/activity_spec.rb index c977f266296..0764044260e 100644 --- a/spec/features/dashboard/activity_spec.rb +++ b/spec/features/dashboard/activity_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'Dashboard Activity', feature: true do login_as(create :user) visit activity_dashboard_path end - - it_behaves_like "it has an RSS button with current_user's private token" - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + + it_behaves_like "it has an RSS button with current_user's RSS token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end diff --git a/spec/features/dashboard/issues_spec.rb b/spec/features/dashboard/issues_spec.rb index 7a132dba1e9..2346a9ec2ed 100644 --- a/spec/features/dashboard/issues_spec.rb +++ b/spec/features/dashboard/issues_spec.rb @@ -62,6 +62,6 @@ RSpec.describe 'Dashboard Issues', feature: true do expect(page).to have_content(other_issue.title) end - it_behaves_like "it has an RSS button with current_user's private token" - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "it has an RSS button with current_user's RSS token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index f1789fc9d43..01351548a99 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/spec/features/dashboard/projects_spec.rb @@ -31,5 +31,5 @@ RSpec.describe 'Dashboard Projects', feature: true do end end - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb index ad60fb2c74f..1c53f6dff06 100644 --- a/spec/features/dashboard_issues_spec.rb +++ b/spec/features/dashboard_issues_spec.rb @@ -53,10 +53,10 @@ describe "Dashboard Issues filtering", feature: true, js: true do auto_discovery_link = find('link[type="application/atom+xml"]', visible: false) auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query) - expect(params).to include('private_token' => [user.private_token]) + expect(params).to include('rss_token' => [user.rss_token]) expect(params).to include('milestone_title' => ['']) expect(params).to include('assignee_id' => [user.id.to_s]) - expect(auto_discovery_params).to include('private_token' => [user.private_token]) + expect(auto_discovery_params).to include('rss_token' => [user.rss_token]) expect(auto_discovery_params).to include('milestone_title' => ['']) expect(auto_discovery_params).to include('assignee_id' => [user.id.to_s]) end diff --git a/spec/features/groups/activity_spec.rb b/spec/features/groups/activity_spec.rb index 3b481cba424..81f9c103e95 100644 --- a/spec/features/groups/activity_spec.rb +++ b/spec/features/groups/activity_spec.rb @@ -11,8 +11,8 @@ feature 'Group activity page', feature: true do visit path end - it_behaves_like "it has an RSS button with current_user's private token" - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "it has an RSS button with current_user's RSS token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end context 'when signed out' do @@ -20,7 +20,7 @@ feature 'Group activity page', feature: true do visit path end - it_behaves_like "it has an RSS button without a private token" - it_behaves_like "an autodiscoverable RSS feed without a private token" + it_behaves_like "it has an RSS button without an RSS token" + it_behaves_like "an autodiscoverable RSS feed without an RSS token" end end diff --git a/spec/features/groups/issues_spec.rb b/spec/features/groups/issues_spec.rb index aa2e9632d6c..d6b88542ef7 100644 --- a/spec/features/groups/issues_spec.rb +++ b/spec/features/groups/issues_spec.rb @@ -12,15 +12,15 @@ feature 'Group issues page', feature: true do context 'when signed in' do let(:user) { user_in_group } - it_behaves_like "it has an RSS button with current_user's private token" - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "it has an RSS button with current_user's RSS token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end context 'when signed out' do let(:user) { nil } - it_behaves_like "it has an RSS button without a private token" - it_behaves_like "an autodiscoverable RSS feed without a private token" + it_behaves_like "it has an RSS button without an RSS token" + it_behaves_like "an autodiscoverable RSS feed without an RSS token" end end diff --git a/spec/features/groups/show_spec.rb b/spec/features/groups/show_spec.rb index fb39693e8ca..d3c49c37374 100644 --- a/spec/features/groups/show_spec.rb +++ b/spec/features/groups/show_spec.rb @@ -11,7 +11,7 @@ feature 'Group show page', feature: true do visit path end - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end context 'when signed out' do @@ -19,6 +19,6 @@ feature 'Group show page', feature: true do visit path end - it_behaves_like "an autodiscoverable RSS feed without a private token" + it_behaves_like "an autodiscoverable RSS feed without an RSS token" end end diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index 03ff1cffb3f..7958ad7e24f 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -810,10 +810,10 @@ describe 'Filter issues', js: true, feature: true do auto_discovery_link = find('link[type="application/atom+xml"]', visible: false) auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query) - expect(params).to include('private_token' => [user.private_token]) + expect(params).to include('rss_token' => [user.rss_token]) expect(params).to include('milestone_title' => [milestone.title]) expect(params).to include('assignee_id' => [user.id.to_s]) - expect(auto_discovery_params).to include('private_token' => [user.private_token]) + expect(auto_discovery_params).to include('rss_token' => [user.rss_token]) expect(auto_discovery_params).to include('milestone_title' => [milestone.title]) expect(auto_discovery_params).to include('assignee_id' => [user.id.to_s]) end @@ -825,10 +825,10 @@ describe 'Filter issues', js: true, feature: true do auto_discovery_link = find('link[type="application/atom+xml"]', visible: false) auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query) - expect(params).to include('private_token' => [user.private_token]) + expect(params).to include('rss_token' => [user.rss_token]) expect(params).to include('milestone_title' => [milestone.title]) expect(params).to include('assignee_id' => [user.id.to_s]) - expect(auto_discovery_params).to include('private_token' => [user.private_token]) + expect(auto_discovery_params).to include('rss_token' => [user.rss_token]) expect(auto_discovery_params).to include('milestone_title' => [milestone.title]) expect(auto_discovery_params).to include('assignee_id' => [user.id.to_s]) end diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb index e63feb14b7e..7df628fd7a0 100644 --- a/spec/features/profile_spec.rb +++ b/spec/features/profile_spec.rb @@ -47,6 +47,21 @@ describe 'Profile account page', feature: true do end end + describe 'when I reset RSS token' do + before do + visit profile_account_path + end + + it 'resets RSS token' do + previous_token = find("#rss-token").value + + click_link('Reset RSS token') + + expect(page).to have_content 'RSS token was successfully reset' + expect(find('#rss-token').value).not_to eq(previous_token) + end + end + describe 'when I reset incoming email token' do before do allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true) diff --git a/spec/features/projects/activity/rss_spec.rb b/spec/features/projects/activity/rss_spec.rb index b47c6d431eb..3c1de5c09b2 100644 --- a/spec/features/projects/activity/rss_spec.rb +++ b/spec/features/projects/activity/rss_spec.rb @@ -16,7 +16,7 @@ feature 'Project Activity RSS' do visit path end - it_behaves_like "it has an RSS button with current_user's private token" + it_behaves_like "it has an RSS button with current_user's RSS token" end context 'when signed out' do @@ -24,6 +24,6 @@ feature 'Project Activity RSS' do visit path end - it_behaves_like "it has an RSS button without a private token" + it_behaves_like "it has an RSS button without an RSS token" end end diff --git a/spec/features/projects/commit/rss_spec.rb b/spec/features/projects/commit/rss_spec.rb index 6e0e1916f87..03b6d560c96 100644 --- a/spec/features/projects/commit/rss_spec.rb +++ b/spec/features/projects/commit/rss_spec.rb @@ -12,8 +12,8 @@ feature 'Project Commits RSS' do visit path end - it_behaves_like "it has an RSS button with current_user's private token" - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "it has an RSS button with current_user's RSS token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end context 'when signed out' do @@ -21,7 +21,7 @@ feature 'Project Commits RSS' do visit path end - it_behaves_like "it has an RSS button without a private token" - it_behaves_like "an autodiscoverable RSS feed without a private token" + it_behaves_like "it has an RSS button without an RSS token" + it_behaves_like "an autodiscoverable RSS feed without an RSS token" end end diff --git a/spec/features/projects/compare_spec.rb b/spec/features/projects/compare_spec.rb index 294a63a5c6d..4162f2579d1 100644 --- a/spec/features/projects/compare_spec.rb +++ b/spec/features/projects/compare_spec.rb @@ -52,8 +52,12 @@ describe "Compare", js: true do def select_using_dropdown(dropdown_type, selection) dropdown = find(".js-compare-#{dropdown_type}-dropdown") dropdown.find(".compare-dropdown-toggle").click + # find input before using to wait for the inputs visiblity + dropdown.find('.dropdown-menu') dropdown.fill_in("Filter by Git revision", with: selection) wait_for_requests - dropdown.find_all("a[data-ref=\"#{selection}\"]", visible: true).last.click + # find before all to wait for the items visiblity + dropdown.find("a[data-ref=\"#{selection}\"]", match: :first) + dropdown.all("a[data-ref=\"#{selection}\"]").last.click end end diff --git a/spec/features/projects/issues/rss_spec.rb b/spec/features/projects/issues/rss_spec.rb index 71429f00095..f6852192aef 100644 --- a/spec/features/projects/issues/rss_spec.rb +++ b/spec/features/projects/issues/rss_spec.rb @@ -16,8 +16,8 @@ feature 'Project Issues RSS' do visit path end - it_behaves_like "it has an RSS button with current_user's private token" - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "it has an RSS button with current_user's RSS token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end context 'when signed out' do @@ -25,7 +25,7 @@ feature 'Project Issues RSS' do visit path end - it_behaves_like "it has an RSS button without a private token" - it_behaves_like "an autodiscoverable RSS feed without a private token" + it_behaves_like "it has an RSS button without an RSS token" + it_behaves_like "an autodiscoverable RSS feed without an RSS token" end end diff --git a/spec/features/projects/main/rss_spec.rb b/spec/features/projects/main/rss_spec.rb index b1a3af612a1..53966229a2a 100644 --- a/spec/features/projects/main/rss_spec.rb +++ b/spec/features/projects/main/rss_spec.rb @@ -12,7 +12,7 @@ feature 'Project RSS' do visit path end - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end context 'when signed out' do @@ -20,6 +20,6 @@ feature 'Project RSS' do visit path end - it_behaves_like "an autodiscoverable RSS feed without a private token" + it_behaves_like "an autodiscoverable RSS feed without an RSS token" end end diff --git a/spec/features/projects/settings/integration_settings_spec.rb b/spec/features/projects/settings/integration_settings_spec.rb index d3232f0cc16..fbaea14a2be 100644 --- a/spec/features/projects/settings/integration_settings_spec.rb +++ b/spec/features/projects/settings/integration_settings_spec.rb @@ -85,11 +85,55 @@ feature 'Integration settings', feature: true do expect(current_path).to eq(integrations_path) end - scenario 'remove existing webhook' do - hook - visit integrations_path + context 'remove existing webhook' do + scenario 'from webhooks list page' do + hook + visit integrations_path + + expect { click_link 'Remove' }.to change(ProjectHook, :count).by(-1) + end + + scenario 'from webhook edit page' do + hook + visit integrations_path + click_link 'Edit' + + expect { click_link 'Remove' }.to change(ProjectHook, :count).by(-1) + end + end + end + + context 'Webhook logs' do + let(:hook) { create(:project_hook, project: project) } + let(:hook_log) { create(:web_hook_log, web_hook: hook, internal_error_message: 'some error') } + + scenario 'show list of hook logs' do + hook_log + visit edit_namespace_project_hook_path(project.namespace, project, hook) + + expect(page).to have_content('Recent Deliveries') + expect(page).to have_content(hook_log.url) + end + + scenario 'show hook log details' do + hook_log + visit edit_namespace_project_hook_path(project.namespace, project, hook) + click_link 'View details' + + expect(page).to have_content("POST #{hook_log.url}") + expect(page).to have_content(hook_log.internal_error_message) + expect(page).to have_content('Resend Request') + end + + scenario 'retry hook log' do + WebMock.stub_request(:post, hook.url) + + hook_log + visit edit_namespace_project_hook_path(project.namespace, project, hook) + click_link 'View details' + click_link 'Resend Request' - expect { click_link 'Remove' }.to change(ProjectHook, :count).by(-1) + expect(current_path).to eq(edit_namespace_project_hook_path(project.namespace, project, hook)) end end end diff --git a/spec/features/projects/tree/rss_spec.rb b/spec/features/projects/tree/rss_spec.rb index 9ac51997d65..9bf59c4139c 100644 --- a/spec/features/projects/tree/rss_spec.rb +++ b/spec/features/projects/tree/rss_spec.rb @@ -12,7 +12,7 @@ feature 'Project Tree RSS' do visit path end - it_behaves_like "an autodiscoverable RSS feed with current_user's private token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end context 'when signed out' do @@ -20,6 +20,6 @@ feature 'Project Tree RSS' do visit path end - it_behaves_like "an autodiscoverable RSS feed without a private token" + it_behaves_like "an autodiscoverable RSS feed without an RSS token" end end diff --git a/spec/features/users/rss_spec.rb b/spec/features/users/rss_spec.rb index 14564abb16d..dbd5f66b55e 100644 --- a/spec/features/users/rss_spec.rb +++ b/spec/features/users/rss_spec.rb @@ -9,7 +9,7 @@ feature 'User RSS' do visit path end - it_behaves_like "it has an RSS button with current_user's private token" + it_behaves_like "it has an RSS button with current_user's RSS token" end context 'when signed out' do @@ -17,6 +17,6 @@ feature 'User RSS' do visit path end - it_behaves_like "it has an RSS button without a private token" + it_behaves_like "it has an RSS button without an RSS token" end end diff --git a/spec/helpers/rss_helper_spec.rb b/spec/helpers/rss_helper_spec.rb index f3f174f3d14..269e1057e8d 100644 --- a/spec/helpers/rss_helper_spec.rb +++ b/spec/helpers/rss_helper_spec.rb @@ -3,17 +3,17 @@ require 'spec_helper' describe RssHelper do describe '#rss_url_options' do context 'when signed in' do - it "includes the current_user's private_token" do + it "includes the current_user's rss_token" do current_user = create(:user) allow(helper).to receive(:current_user).and_return(current_user) - expect(helper.rss_url_options).to include private_token: current_user.private_token + expect(helper.rss_url_options).to include rss_token: current_user.rss_token end end context 'when signed out' do - it "does not have a private_token" do + it "does not have an rss_token" do allow(helper).to receive(:current_user).and_return(nil) - expect(helper.rss_url_options[:private_token]).to be_nil + expect(helper.rss_url_options[:rss_token]).to be_nil end end end diff --git a/spec/javascripts/raven/raven_config_spec.js b/spec/javascripts/raven/raven_config_spec.js index b31a7c28ebe..c82658b9262 100644 --- a/spec/javascripts/raven/raven_config_spec.js +++ b/spec/javascripts/raven/raven_config_spec.js @@ -140,24 +140,6 @@ describe('RavenConfig', () => { }); }); - describe('bindRavenErrors', () => { - let $document; - let $; - - beforeEach(() => { - $document = jasmine.createSpyObj('$document', ['on']); - $ = jasmine.createSpy('$').and.returnValue($document); - - window.$ = $; - - RavenConfig.bindRavenErrors(); - }); - - it('should call .on', function () { - expect($document.on).toHaveBeenCalledWith('ajaxError.raven', RavenConfig.handleRavenErrors); - }); - }); - describe('handleRavenErrors', () => { let event; let req; diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 34f617e23a5..2e9646286df 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -131,6 +131,7 @@ services: - service_hook hooks: - project +- web_hook_logs protected_branches: - project - merge_access_levels diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 56b24ce62f3..c8023dc13b1 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -965,7 +965,7 @@ describe Ci::Pipeline, models: true do end before do - ProjectWebHookWorker.drain + WebHookWorker.drain end context 'with pipeline hooks enabled' do diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 1a83c836652..57454d2a773 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -1,36 +1,19 @@ -require "spec_helper" +require 'spec_helper' describe ServiceHook, models: true do - describe "Associations" do + describe 'associations' do it { is_expected.to belong_to :service } end - describe "execute" do - before(:each) do - @service_hook = create(:service_hook) - @data = { project_id: 1, data: {} } + describe 'execute' do + let(:hook) { build(:service_hook) } + let(:data) { { key: 'value' } } - WebMock.stub_request(:post, @service_hook.url) - end - - it "POSTs to the webhook URL" do - @service_hook.execute(@data) - expect(WebMock).to have_requested(:post, @service_hook.url).with( - headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Service Hook' } - ).once - end - - it "POSTs the data as JSON" do - @service_hook.execute(@data) - expect(WebMock).to have_requested(:post, @service_hook.url).with( - headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Service Hook' } - ).once - end - - it "catches exceptions" do - expect(WebHook).to receive(:post).and_raise("Some HTTP Post error") + it '#execute' do + expect(WebHookService).to receive(:new).with(hook, data, 'service_hook').and_call_original + expect_any_instance_of(WebHookService).to receive(:execute) - expect { @service_hook.execute(@data) }.to raise_error(RuntimeError) + hook.execute(data) end end end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 4340170888d..0d2b622132e 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -126,4 +126,26 @@ describe SystemHook, models: true do expect(SystemHook.repository_update_hooks).to eq([hook]) end end + + describe 'execute WebHookService' do + let(:hook) { build(:system_hook) } + let(:data) { { key: 'value' } } + let(:hook_name) { 'system_hook' } + + before do + expect(WebHookService).to receive(:new).with(hook, data, hook_name).and_call_original + end + + it '#execute' do + expect_any_instance_of(WebHookService).to receive(:execute) + + hook.execute(data, hook_name) + end + + it '#async_execute' do + expect_any_instance_of(WebHookService).to receive(:async_execute) + + hook.async_execute(data, hook_name) + end + end end diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb new file mode 100644 index 00000000000..c649cf3b589 --- /dev/null +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +describe WebHookLog, models: true do + it { is_expected.to belong_to(:web_hook) } + + it { is_expected.to serialize(:request_headers).as(Hash) } + it { is_expected.to serialize(:request_data).as(Hash) } + it { is_expected.to serialize(:response_headers).as(Hash) } + + it { is_expected.to validate_presence_of(:web_hook) } + + describe '#success?' do + let(:web_hook_log) { build(:web_hook_log, response_status: status) } + + describe '2xx' do + let(:status) { '200' } + it { expect(web_hook_log.success?).to be_truthy } + end + + describe 'not 2xx' do + let(:status) { '500' } + it { expect(web_hook_log.success?).to be_falsey } + end + + describe 'internal erorr' do + let(:status) { 'internal error' } + it { expect(web_hook_log.success?).to be_falsey } + end + end +end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 9d4db1bfb52..53157c24477 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -1,89 +1,54 @@ require 'spec_helper' describe WebHook, models: true do - describe "Validations" do + let(:hook) { build(:project_hook) } + + describe 'associations' do + it { is_expected.to have_many(:web_hook_logs).dependent(:destroy) } + end + + describe 'validations' do it { is_expected.to validate_presence_of(:url) } describe 'url' do - it { is_expected.to allow_value("http://example.com").for(:url) } - it { is_expected.to allow_value("https://example.com").for(:url) } - it { is_expected.to allow_value(" https://example.com ").for(:url) } - it { is_expected.to allow_value("http://test.com/api").for(:url) } - it { is_expected.to allow_value("http://test.com/api?key=abc").for(:url) } - it { is_expected.to allow_value("http://test.com/api?key=abc&type=def").for(:url) } + it { is_expected.to allow_value('http://example.com').for(:url) } + it { is_expected.to allow_value('https://example.com').for(:url) } + it { is_expected.to allow_value(' https://example.com ').for(:url) } + it { is_expected.to allow_value('http://test.com/api').for(:url) } + it { is_expected.to allow_value('http://test.com/api?key=abc').for(:url) } + it { is_expected.to allow_value('http://test.com/api?key=abc&type=def').for(:url) } - it { is_expected.not_to allow_value("example.com").for(:url) } - it { is_expected.not_to allow_value("ftp://example.com").for(:url) } - it { is_expected.not_to allow_value("herp-and-derp").for(:url) } + it { is_expected.not_to allow_value('example.com').for(:url) } + it { is_expected.not_to allow_value('ftp://example.com').for(:url) } + it { is_expected.not_to allow_value('herp-and-derp').for(:url) } it 'strips :url before saving it' do - hook = create(:project_hook, url: ' https://example.com ') + hook.url = ' https://example.com ' + hook.save expect(hook.url).to eq('https://example.com') end end end - describe "execute" do - let(:project) { create(:empty_project) } - let(:project_hook) { create(:project_hook) } - - before(:each) do - project.hooks << [project_hook] - @data = { before: 'oldrev', after: 'newrev', ref: 'ref' } - - WebMock.stub_request(:post, project_hook.url) - end - - context 'when token is defined' do - let(:project_hook) { create(:project_hook, :token) } - - it 'POSTs to the webhook URL' do - project_hook.execute(@data, 'push_hooks') - expect(WebMock).to have_requested(:post, project_hook.url).with( - headers: { 'Content-Type' => 'application/json', - 'X-Gitlab-Event' => 'Push Hook', - 'X-Gitlab-Token' => project_hook.token } - ).once - end - end - - it "POSTs to the webhook URL" do - project_hook.execute(@data, 'push_hooks') - expect(WebMock).to have_requested(:post, project_hook.url).with( - headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Push Hook' } - ).once - end - - it "POSTs the data as JSON" do - project_hook.execute(@data, 'push_hooks') - expect(WebMock).to have_requested(:post, project_hook.url).with( - headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Push Hook' } - ).once - end - - it "catches exceptions" do - expect(WebHook).to receive(:post).and_raise("Some HTTP Post error") - - expect { project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError) - end - - it "handles SSL exceptions" do - expect(WebHook).to receive(:post).and_raise(OpenSSL::SSL::SSLError.new('SSL error')) + describe 'execute' do + let(:data) { { key: 'value' } } + let(:hook_name) { 'project hook' } - expect(project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error']) + before do + expect(WebHookService).to receive(:new).with(hook, data, hook_name).and_call_original end - it "handles 200 status code" do - WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: "Success") + it '#execute' do + expect_any_instance_of(WebHookService).to receive(:execute) - expect(project_hook.execute(@data, 'push_hooks')).to eq([200, 'Success']) + hook.execute(data, hook_name) end - it "handles 2xx status codes" do - WebMock.stub_request(:post, project_hook.url).to_return(status: 201, body: "Success") + it '#async_execute' do + expect_any_instance_of(WebHookService).to receive(:async_execute) - expect(project_hook.execute(@data, 'push_hooks')).to eq([201, 'Success']) + hook.async_execute(data, hook_name) end end end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 8c0a8f657cb..0dcf4a4b5d6 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -13,7 +13,7 @@ describe KubernetesService, models: true, caching: true do let(:discovery_url) { service.api_url + '/api/v1' } let(:discovery_response) { { body: kube_discovery_body.to_json } } - let(:pods_url) { service.api_url + "/api/v1/namespaces/#{service.namespace}/pods" } + let(:pods_url) { service.api_url + "/api/v1/namespaces/#{service.actual_namespace}/pods" } let(:pods_response) { { body: kube_pods_body(kube_pod).to_json } } def stub_kubeclient_discover @@ -105,6 +105,34 @@ describe KubernetesService, models: true, caching: true do end end + describe '#actual_namespace' do + subject { service.actual_namespace } + + it "returns the default namespace" do + is_expected.to eq(service.send(:default_namespace)) + end + + context 'when namespace is specified' do + before do + service.namespace = 'my-namespace' + end + + it "returns the user-namespace" do + is_expected.to eq('my-namespace') + end + end + + context 'when service is not assigned to project' do + before do + service.project = nil + end + + it "does not return namespace" do + is_expected.to be_nil + end + end + end + describe '#test' do before do stub_kubeclient_discover @@ -194,6 +222,7 @@ describe KubernetesService, models: true, caching: true do describe '#terminals' do let(:environment) { build(:environment, project: project, name: "env", slug: "env-000000") } + subject { service.terminals(environment) } context 'with invalid pods' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6a15830a15c..aabdac4bb75 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -440,6 +440,22 @@ describe User, models: true do end end + describe 'ensure incoming email token' do + it 'has incoming email token' do + user = create(:user) + expect(user.incoming_email_token).not_to be_blank + end + end + + describe 'rss token' do + it 'ensures an rss token on read' do + user = create(:user, rss_token: nil) + rss_token = user.rss_token + expect(rss_token).not_to be_blank + expect(user.reload.rss_token).to eq rss_token + end + end + describe '#recently_sent_password_reset?' do it 'is false when reset_password_sent_at is nil' do user = build_stubbed(:user, reset_password_sent_at: nil) diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb index e5fc0b676af..179fc9733ad 100644 --- a/spec/routing/admin_routing_spec.rb +++ b/spec/routing/admin_routing_spec.rb @@ -103,6 +103,18 @@ describe Admin::HooksController, "routing" do end end +# admin_hook_hook_log_retry GET /admin/hooks/:hook_id/hook_logs/:id/retry(.:format) admin/hook_logs#retry +# admin_hook_hook_log GET /admin/hooks/:hook_id/hook_logs/:id(.:format) admin/hook_logs#show +describe Admin::HookLogsController, 'routing' do + it 'to #retry' do + expect(get('/admin/hooks/1/hook_logs/1/retry')).to route_to('admin/hook_logs#retry', hook_id: '1', id: '1') + end + + it 'to #show' do + expect(get('/admin/hooks/1/hook_logs/1')).to route_to('admin/hook_logs#show', hook_id: '1', id: '1') + end +end + # admin_logs GET /admin/logs(.:format) admin/logs#show describe Admin::LogsController, "routing" do it "to #show" do diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index a391c046f92..54417f6b3e1 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -349,6 +349,18 @@ describe 'project routing' do end end + # retry_namespace_project_hook_hook_log GET /:project_id/hooks/:hook_id/hook_logs/:id/retry(.:format) projects/hook_logs#retry + # namespace_project_hook_hook_log GET /:project_id/hooks/:hook_id/hook_logs/:id(.:format) projects/hook_logs#show + describe Projects::HookLogsController, 'routing' do + it 'to #retry' do + expect(get('/gitlab/gitlabhq/hooks/1/hook_logs/1/retry')).to route_to('projects/hook_logs#retry', namespace_id: 'gitlab', project_id: 'gitlabhq', hook_id: '1', id: '1') + end + + it 'to #show' do + expect(get('/gitlab/gitlabhq/hooks/1/hook_logs/1')).to route_to('projects/hook_logs#show', namespace_id: 'gitlab', project_id: 'gitlabhq', hook_id: '1', id: '1') + end + end + # project_commit GET /:project_id/commit/:id(.:format) commit#show {id: /\h{7,40}/, project_id: /[^\/]+/} describe Projects::CommitController, 'routing' do it 'to #show' do diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index abacc50a371..a62af13cf0c 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -151,6 +151,10 @@ describe ProfilesController, "routing" do expect(put("/profile/reset_private_token")).to route_to('profiles#reset_private_token') end + it "to #reset_rss_token" do + expect(put("/profile/reset_rss_token")).to route_to('profiles#reset_rss_token') + end + it "to #show" do expect(get("/profile")).to route_to('profiles#show') end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb new file mode 100644 index 00000000000..b5abc46e80c --- /dev/null +++ b/spec/services/web_hook_service_spec.rb @@ -0,0 +1,137 @@ +require 'spec_helper' + +describe WebHookService, services: true do + let(:project) { create(:empty_project) } + let(:project_hook) { create(:project_hook) } + let(:headers) do + { + 'Content-Type' => 'application/json', + 'X-Gitlab-Event' => 'Push Hook' + } + end + let(:data) do + { before: 'oldrev', after: 'newrev', ref: 'ref' } + end + let(:service_instance) { WebHookService.new(project_hook, data, 'push_hooks') } + + describe '#execute' do + before(:each) do + project.hooks << [project_hook] + + WebMock.stub_request(:post, project_hook.url) + end + + context 'when token is defined' do + let(:project_hook) { create(:project_hook, :token) } + + it 'POSTs to the webhook URL' do + service_instance.execute + expect(WebMock).to have_requested(:post, project_hook.url).with( + headers: headers.merge({ 'X-Gitlab-Token' => project_hook.token }) + ).once + end + end + + it 'POSTs to the webhook URL' do + service_instance.execute + expect(WebMock).to have_requested(:post, project_hook.url).with( + headers: headers + ).once + end + + it 'POSTs the data as JSON' do + service_instance.execute + expect(WebMock).to have_requested(:post, project_hook.url).with( + headers: headers + ).once + end + + it 'catches exceptions' do + WebMock.stub_request(:post, project_hook.url).to_raise(StandardError.new('Some error')) + + expect { service_instance.execute }.to raise_error(StandardError) + end + + it 'handles exceptions' do + exceptions = [SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout] + exceptions.each do |exception_class| + exception = exception_class.new('Exception message') + + WebMock.stub_request(:post, project_hook.url).to_raise(exception) + expect(service_instance.execute).to eq([nil, exception.message]) + expect { service_instance.execute }.not_to raise_error + end + end + + it 'handles 200 status code' do + WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: 'Success') + + expect(service_instance.execute).to eq([200, 'Success']) + end + + it 'handles 2xx status codes' do + WebMock.stub_request(:post, project_hook.url).to_return(status: 201, body: 'Success') + + expect(service_instance.execute).to eq([201, 'Success']) + end + + context 'execution logging' do + let(:hook_log) { project_hook.web_hook_logs.last } + + context 'with success' do + before do + WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: 'Success') + service_instance.execute + end + + it 'log successful execution' do + expect(hook_log.trigger).to eq('push_hooks') + expect(hook_log.url).to eq(project_hook.url) + expect(hook_log.request_headers).to eq(headers) + expect(hook_log.response_body).to eq('Success') + expect(hook_log.response_status).to eq('200') + expect(hook_log.execution_duration).to be > 0 + expect(hook_log.internal_error_message).to be_nil + end + end + + context 'with exception' do + before do + WebMock.stub_request(:post, project_hook.url).to_raise(SocketError.new('Some HTTP Post error')) + service_instance.execute + end + + it 'log failed execution' do + expect(hook_log.trigger).to eq('push_hooks') + expect(hook_log.url).to eq(project_hook.url) + expect(hook_log.request_headers).to eq(headers) + expect(hook_log.response_body).to eq('') + expect(hook_log.response_status).to eq('internal error') + expect(hook_log.execution_duration).to be > 0 + expect(hook_log.internal_error_message).to eq('Some HTTP Post error') + end + end + + context 'should not log ServiceHooks' do + let(:service_hook) { create(:service_hook) } + let(:service_instance) { WebHookService.new(service_hook, data, 'service_hook') } + + before do + WebMock.stub_request(:post, service_hook.url).to_return(status: 200, body: 'Success') + end + + it { expect { service_instance.execute }.not_to change(WebHookLog, :count) } + end + end + end + + describe '#async_execute' do + let(:system_hook) { create(:system_hook) } + + it 'enqueue WebHookWorker' do + expect(Sidekiq::Client).to receive(:enqueue).with(WebHookWorker, project_hook.id, data, 'push_hooks') + + WebHookService.new(project_hook, data, 'push_hooks').async_execute + end + end +end diff --git a/spec/support/features/rss_shared_examples.rb b/spec/support/features/rss_shared_examples.rb index 9a3b0a731ad..1cbb4134995 100644 --- a/spec/support/features/rss_shared_examples.rb +++ b/spec/support/features/rss_shared_examples.rb @@ -1,23 +1,23 @@ -shared_examples "an autodiscoverable RSS feed with current_user's private token" do - it "has an RSS autodiscovery link tag with current_user's private token" do - expect(page).to have_css("link[type*='atom+xml'][href*='private_token=#{Thread.current[:current_user].private_token}']", visible: false) +shared_examples "an autodiscoverable RSS feed with current_user's RSS token" do + it "has an RSS autodiscovery link tag with current_user's RSS token" do + expect(page).to have_css("link[type*='atom+xml'][href*='rss_token=#{Thread.current[:current_user].rss_token}']", visible: false) end end -shared_examples "it has an RSS button with current_user's private token" do - it "shows the RSS button with current_user's private token" do - expect(page).to have_css("a:has(.fa-rss)[href*='private_token=#{Thread.current[:current_user].private_token}']") +shared_examples "it has an RSS button with current_user's RSS token" do + it "shows the RSS button with current_user's RSS token" do + expect(page).to have_css("a:has(.fa-rss)[href*='rss_token=#{Thread.current[:current_user].rss_token}']") end end -shared_examples "an autodiscoverable RSS feed without a private token" do - it "has an RSS autodiscovery link tag without a private token" do - expect(page).to have_css("link[type*='atom+xml']:not([href*='private_token'])", visible: false) +shared_examples "an autodiscoverable RSS feed without an RSS token" do + it "has an RSS autodiscovery link tag without an RSS token" do + expect(page).to have_css("link[type*='atom+xml']:not([href*='rss_token'])", visible: false) end end -shared_examples "it has an RSS button without a private token" do - it "shows the RSS button without a private token" do - expect(page).to have_css("a:has(.fa-rss):not([href*='private_token'])") +shared_examples "it has an RSS button without an RSS token" do + it "shows the RSS button without an RSS token" do + expect(page).to have_css("a:has(.fa-rss):not([href*='rss_token'])") end end diff --git a/spec/support/kubernetes_helpers.rb b/spec/support/kubernetes_helpers.rb index d2a1ded57ff..9280fad4ace 100644 --- a/spec/support/kubernetes_helpers.rb +++ b/spec/support/kubernetes_helpers.rb @@ -41,7 +41,7 @@ module KubernetesHelpers containers.map do |container| terminal = { selectors: { pod: pod_name, container: container['name'] }, - url: container_exec_url(service.api_url, service.namespace, pod_name, container['name']), + url: container_exec_url(service.api_url, service.actual_namespace, pod_name, container['name']), subprotocols: ['channel.k8s.io'], headers: { 'Authorization' => ["Bearer #{service.token}"] }, created_at: DateTime.parse(pod['metadata']['creationTimestamp']), diff --git a/spec/tasks/tokens_spec.rb b/spec/tasks/tokens_spec.rb index 19036c7677c..b84137eb365 100644 --- a/spec/tasks/tokens_spec.rb +++ b/spec/tasks/tokens_spec.rb @@ -18,4 +18,10 @@ describe 'tokens rake tasks' do expect { run_rake_task('tokens:reset_all_email') }.to change { user.reload.incoming_email_token } end end + + describe 'reset_all_rss task' do + it 'invokes create_hooks task' do + expect { run_rake_task('tokens:reset_all_rss') }.to change { user.reload.rss_token } + end + end end diff --git a/spec/workers/remove_old_web_hook_logs_worker_spec.rb b/spec/workers/remove_old_web_hook_logs_worker_spec.rb new file mode 100644 index 00000000000..6d26ba5dfa0 --- /dev/null +++ b/spec/workers/remove_old_web_hook_logs_worker_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe RemoveOldWebHookLogsWorker do + subject { described_class.new } + + describe '#perform' do + let!(:week_old_record) { create(:web_hook_log, created_at: Time.now - 1.week) } + let!(:three_days_old_record) { create(:web_hook_log, created_at: Time.now - 3.days) } + let!(:one_day_old_record) { create(:web_hook_log, created_at: Time.now - 1.day) } + + it 'removes web hook logs older than 2 days' do + subject.perform + + expect(WebHookLog.all).to include(one_day_old_record) + expect(WebHookLog.all).not_to include(week_old_record, three_days_old_record) + end + end +end |