diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-07-14 18:08:31 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-07-14 18:08:31 +0000 |
| commit | 06ac12d53c3f0b7cee2755a1254bf1af05d55044 (patch) | |
| tree | 95d0be0bd751a22d6135f496c425c44d774fbe54 | |
| parent | b689f371350fbf1b71f266764ee018befc9b91f7 (diff) | |
| download | gitlab-ce-06ac12d53c3f0b7cee2755a1254bf1af05d55044.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
93 files changed, 658 insertions, 369 deletions
diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 75bace1fa22..c417ea92cc0 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -2443,17 +2443,6 @@ Gitlab/FeatureAvailableUsage: # WIP See https://gitlab.com/gitlab-org/gitlab/-/issues/327490 Style/RegexpLiteralMixedPreserve: Exclude: - - 'app/controllers/projects/repositories_controller.rb' - - 'app/helpers/ci/variables_helper.rb' - - 'app/models/alert_management/alert.rb' - - 'app/models/application_setting.rb' - - 'app/models/blob_viewer/go_mod.rb' - - 'app/models/concerns/ci/maskable.rb' - - 'app/models/operations/feature_flag.rb' - - 'app/models/packages/go/module.rb' - - 'app/services/packages/conan/search_service.rb' - - 'app/services/projects/update_remote_mirror_service.rb' - - 'config/initializers/rspec_profiling.rb' - 'ee/app/models/status_page/project_setting.rb' - 'ee/app/presenters/vulnerability_presenter.rb' - 'ee/lib/api/geo_nodes.rb' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 08bbe86fd1b..3116f541e87 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -40511f7a14ded77c826809d054d740a66e1c106f +9cde7b7d1ecc68f5ca3b68df5ad32e6b0bc9d661 diff --git a/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue b/app/assets/javascripts/admin/users/components/modals/delete_user_modal.vue index 413163c8536..413163c8536 100644 --- a/app/assets/javascripts/pages/admin/users/components/delete_user_modal.vue +++ b/app/assets/javascripts/admin/users/components/modals/delete_user_modal.vue diff --git a/app/assets/javascripts/pages/admin/users/components/user_modal_manager.vue b/app/assets/javascripts/admin/users/components/modals/user_modal_manager.vue index 1dfea3f1e7b..1dfea3f1e7b 100644 --- a/app/assets/javascripts/pages/admin/users/components/user_modal_manager.vue +++ b/app/assets/javascripts/admin/users/components/modals/user_modal_manager.vue diff --git a/app/assets/javascripts/admin/users/constants.js b/app/assets/javascripts/admin/users/constants.js index d0421413132..33ee7e1cb0d 100644 --- a/app/assets/javascripts/admin/users/constants.js +++ b/app/assets/javascripts/admin/users/constants.js @@ -20,3 +20,9 @@ export const I18N_USER_ACTIONS = { ban: s__('AdminUsers|Ban user'), unban: s__('AdminUsers|Unban user'), }; + +export const CONFIRM_DELETE_BUTTON_SELECTOR = '.js-delete-user-modal-button'; + +export const MODAL_TEXTS_CONTAINER_SELECTOR = '#js-modal-texts'; + +export const MODAL_MANAGER_SELECTOR = '#js-delete-user-modal'; diff --git a/app/assets/javascripts/admin/users/index.js b/app/assets/javascripts/admin/users/index.js index 54c8edc080b..05f8469e61a 100644 --- a/app/assets/javascripts/admin/users/index.js +++ b/app/assets/javascripts/admin/users/index.js @@ -2,7 +2,14 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; +import csrf from '~/lib/utils/csrf'; import AdminUsersApp from './components/app.vue'; +import ModalManager from './components/modals/user_modal_manager.vue'; +import { + CONFIRM_DELETE_BUTTON_SELECTOR, + MODAL_TEXTS_CONTAINER_SELECTOR, + MODAL_MANAGER_SELECTOR, +} from './constants'; Vue.use(VueApollo); @@ -29,3 +36,45 @@ export const initAdminUsersApp = (el = document.querySelector('#js-admin-users-a }), }); }; + +export const initDeleteUserModals = () => { + const modalsMountElement = document.querySelector(MODAL_TEXTS_CONTAINER_SELECTOR); + + if (!modalsMountElement) { + return; + } + + const modalConfiguration = Array.from(modalsMountElement.children).reduce((accumulator, node) => { + const { modal, ...config } = node.dataset; + + return { + ...accumulator, + [modal]: { + title: node.dataset.title, + ...config, + content: node.innerHTML, + }, + }; + }, {}); + + // eslint-disable-next-line no-new + new Vue({ + el: MODAL_MANAGER_SELECTOR, + functional: true, + methods: { + show(...args) { + this.$refs.manager.show(...args); + }, + }, + render(h) { + return h(ModalManager, { + ref: 'manager', + props: { + selector: CONFIRM_DELETE_BUTTON_SELECTOR, + modalConfiguration, + csrfToken: csrf.token, + }, + }); + }, + }); +}; diff --git a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue b/app/assets/javascripts/issuable_list/components/issuable_list_root.vue index a19c76cfe3f..87066a0a0b6 100644 --- a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue +++ b/app/assets/javascripts/issuable_list/components/issuable_list_root.vue @@ -134,7 +134,7 @@ export default { labelFilterParam: { type: String, required: false, - default: null, + default: undefined, }, isManualOrdering: { type: Boolean, diff --git a/app/assets/javascripts/pages/admin/users/keys/index.js b/app/assets/javascripts/pages/admin/identities/index.js index 868c8e33077..868c8e33077 100644 --- a/app/assets/javascripts/pages/admin/users/keys/index.js +++ b/app/assets/javascripts/pages/admin/identities/index.js diff --git a/app/assets/javascripts/pages/admin/impersonation_tokens/index.js b/app/assets/javascripts/pages/admin/impersonation_tokens/index.js index dc1bb88bf4b..3f9e4e9d591 100644 --- a/app/assets/javascripts/pages/admin/impersonation_tokens/index.js +++ b/app/assets/javascripts/pages/admin/impersonation_tokens/index.js @@ -1,3 +1,5 @@ import { initExpiresAtField } from '~/access_tokens'; +import initConfirmModal from '~/confirm_modal'; initExpiresAtField(); +initConfirmModal(); diff --git a/app/assets/javascripts/pages/admin/users/index.js b/app/assets/javascripts/pages/admin/users/index.js index 9a8b0c9990f..99bf842ef2d 100644 --- a/app/assets/javascripts/pages/admin/users/index.js +++ b/app/assets/javascripts/pages/admin/users/index.js @@ -1,64 +1,6 @@ -import Vue from 'vue'; - -import { initAdminUsersApp } from '~/admin/users'; +import { initAdminUsersApp, initDeleteUserModals } from '~/admin/users'; import initConfirmModal from '~/confirm_modal'; -import csrf from '~/lib/utils/csrf'; -import Translate from '~/vue_shared/translate'; -import ModalManager from './components/user_modal_manager.vue'; - -const CONFIRM_DELETE_BUTTON_SELECTOR = '.js-delete-user-modal-button'; -const MODAL_TEXTS_CONTAINER_SELECTOR = '#js-modal-texts'; -const MODAL_MANAGER_SELECTOR = '#js-delete-user-modal'; - -function loadModalsConfigurationFromHtml(modalsElement) { - const modalsConfiguration = {}; - - if (!modalsElement) { - /* eslint-disable-next-line @gitlab/require-i18n-strings */ - throw new Error('Modals content element not found!'); - } - - Array.from(modalsElement.children).forEach((node) => { - const { modal, ...config } = node.dataset; - modalsConfiguration[modal] = { - title: node.dataset.title, - ...config, - content: node.innerHTML, - }; - }); - - return modalsConfiguration; -} - -document.addEventListener('DOMContentLoaded', () => { - Vue.use(Translate); - - initAdminUsersApp(); - - const modalConfiguration = loadModalsConfigurationFromHtml( - document.querySelector(MODAL_TEXTS_CONTAINER_SELECTOR), - ); - - // eslint-disable-next-line no-new - new Vue({ - el: MODAL_MANAGER_SELECTOR, - functional: true, - methods: { - show(...args) { - this.$refs.manager.show(...args); - }, - }, - render(h) { - return h(ModalManager, { - ref: 'manager', - props: { - selector: CONFIRM_DELETE_BUTTON_SELECTOR, - modalConfiguration, - csrfToken: csrf.token, - }, - }); - }, - }); - initConfirmModal(); -}); +initAdminUsersApp(); +initDeleteUserModals(); +initConfirmModal(); diff --git a/app/assets/javascripts/repository/components/table/index.vue b/app/assets/javascripts/repository/components/table/index.vue index ca5711de49c..69eefc807d7 100644 --- a/app/assets/javascripts/repository/components/table/index.vue +++ b/app/assets/javascripts/repository/components/table/index.vue @@ -70,7 +70,7 @@ export default { ); }, showParentRow() { - return !this.isLoading && ['', '/'].indexOf(this.path) === -1; + return ['', '/'].indexOf(this.path) === -1; }, }, methods: { diff --git a/app/assets/javascripts/search/store/actions.js b/app/assets/javascripts/search/store/actions.js index eaf164d5591..b53557c0ec5 100644 --- a/app/assets/javascripts/search/store/actions.js +++ b/app/assets/javascripts/search/store/actions.js @@ -32,7 +32,12 @@ export const fetchProjects = ({ commit, state }, search) => { if (groupId) { // TODO (https://gitlab.com/gitlab-org/gitlab/-/issues/323331): For errors `createFlash` is called twice; in `callback` and in `Api.groupProjects` - Api.groupProjects(groupId, search, { order_by: 'similarity' }, callback); + Api.groupProjects( + groupId, + search, + { order_by: 'similarity', with_shared: false, include_subgroups: true }, + callback, + ); } else { // The .catch() is due to the API method not handling a rejection properly Api.projects(search, { order_by: 'id' }, callback).catch(() => { diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index 8f64a8aa1d3..8beebb52980 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -117,7 +117,7 @@ class Projects::RepositoriesController < Projects::ApplicationController # from Redis. def extract_ref_and_filename(id) path = id.strip - data = path.match(/(.*)\/(.*)/) + data = path.match(%r{(.*)/(.*)}) if data [data[1], data[2]] diff --git a/app/controllers/projects/service_hook_logs_controller.rb b/app/controllers/projects/service_hook_logs_controller.rb index 93ba3eedddb..88de0b7ba0d 100644 --- a/app/controllers/projects/service_hook_logs_controller.rb +++ b/app/controllers/projects/service_hook_logs_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Projects::ServiceHookLogsController < Projects::HookLogsController + extend Gitlab::Utils::Override + before_action :integration, only: [:show, :retry] def retry @@ -10,11 +12,12 @@ class Projects::ServiceHookLogsController < Projects::HookLogsController private - def hook - @hook ||= integration.service_hook - end - def integration @integration ||= @project.find_or_initialize_integration(params[:service_id]) end + + override :hook + def hook + @hook ||= integration.service_hook || not_found + end end diff --git a/app/helpers/ci/variables_helper.rb b/app/helpers/ci/variables_helper.rb index b20390d58e9..84572363a8d 100644 --- a/app/helpers/ci/variables_helper.rb +++ b/app/helpers/ci/variables_helper.rb @@ -48,7 +48,7 @@ module Ci end def ci_variable_maskable_regex - Ci::Maskable::REGEX.inspect.sub('\\A', '^').sub('\\z', '$').sub(/^\//, '').sub(/\/[a-z]*$/, '').gsub('\/', '/') + Ci::Maskable::REGEX.inspect.sub('\\A', '^').sub('\\z', '$').sub(%r{^/}, '').sub(%r{/[a-z]*$}, '').gsub('\/', '/') end end end diff --git a/app/helpers/routing/snippets_helper.rb b/app/helpers/routing/snippets_helper.rb index 2e81c5adc42..19450c1d033 100644 --- a/app/helpers/routing/snippets_helper.rb +++ b/app/helpers/routing/snippets_helper.rb @@ -49,7 +49,7 @@ module Routing def gitlab_raw_snippet_blob_url(snippet, path, ref = nil, **options) params = { snippet_id: snippet, - ref: ref || snippet.repository.root_ref, + ref: ref || snippet.default_branch, path: path } diff --git a/app/models/alert_management/alert.rb b/app/models/alert_management/alert.rb index 0cd89f91b1a..679406e68d7 100644 --- a/app/models/alert_management/alert.rb +++ b/app/models/alert_management/alert.rb @@ -210,7 +210,7 @@ module AlertManagement end def self.link_reference_pattern - @link_reference_pattern ||= super("alert_management", /(?<alert>\d+)\/details(\#)?/) + @link_reference_pattern ||= super("alert_management", %r{(?<alert>\d+)/details(\#)?}) end def self.reference_valid?(reference) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index cbde2db803a..a7140cc0718 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -293,7 +293,7 @@ class ApplicationSetting < ApplicationRecord validates :user_default_internal_regex, js_regex: true, allow_nil: true validates :personal_access_token_prefix, - format: { with: /\A[a-zA-Z0-9_+=\/@:.-]+\z/, + format: { with: %r{\A[a-zA-Z0-9_+=/@:.-]+\z}, message: _("can contain only letters of the Base64 alphabet (RFC4648) with the addition of '@', ':' and '.'") }, length: { maximum: 20, message: _('is too long (maximum is %{count} characters)') }, allow_blank: true @@ -590,7 +590,7 @@ class ApplicationSetting < ApplicationRecord end def sourcegraph_url_is_com? - !!(sourcegraph_url =~ /\Ahttps:\/\/(www\.)?sourcegraph\.com/) + !!(sourcegraph_url =~ %r{\Ahttps://(www\.)?sourcegraph\.com}) end def instance_review_permitted? diff --git a/app/models/blob_viewer/go_mod.rb b/app/models/blob_viewer/go_mod.rb index ae57e2c0526..d4d117f899c 100644 --- a/app/models/blob_viewer/go_mod.rb +++ b/app/models/blob_viewer/go_mod.rb @@ -5,13 +5,13 @@ module BlobViewer include ServerSide include Gitlab::Utils::StrongMemoize - MODULE_REGEX = / + MODULE_REGEX = %r{ \A (?# beginning of file) module\s+ (?# module directive) (?<name>.*?) (?# module name) - \s*(?:\/\/.*)? (?# comment) + \s*(?://.*)? (?# comment) (?:\n|\z) (?# newline or end of file) - /x.freeze + }x.freeze self.file_types = %i(go_mod go_sum) diff --git a/app/models/concerns/ci/maskable.rb b/app/models/concerns/ci/maskable.rb index e1ef4531845..62be0150ee0 100644 --- a/app/models/concerns/ci/maskable.rb +++ b/app/models/concerns/ci/maskable.rb @@ -11,7 +11,7 @@ module Ci # * Minimal length of 8 characters # * Characters must be from the Base64 alphabet (RFC4648) with the addition of '@', ':', '.', and '~' # * Absolutely no fun is allowed - REGEX = /\A[a-zA-Z0-9_+=\/@:.~-]{8,}\z/.freeze + REGEX = %r{\A[a-zA-Z0-9_+=/@:.~-]{8,}\z}.freeze included do validates :masked, inclusion: { in: [true, false] } diff --git a/app/models/concerns/integrations/has_web_hook.rb b/app/models/concerns/integrations/has_web_hook.rb new file mode 100644 index 00000000000..dabe7152b18 --- /dev/null +++ b/app/models/concerns/integrations/has_web_hook.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Integrations + module HasWebHook + extend ActiveSupport::Concern + + included do + after_save :update_web_hook!, if: :activated? + end + + # Return the URL to be used for the webhook. + def hook_url + raise NotImplementedError + end + + # Return whether the webhook should use SSL verification. + def hook_ssl_verification + true + end + + # Create or update the webhook, raising an exception if it cannot be saved. + def update_web_hook! + hook = service_hook || build_service_hook + hook.url = hook_url if hook.url != hook_url # avoid reencryption + hook.enable_ssl_verification = hook_ssl_verification + hook.save! if hook.changed? + hook + end + + # Execute the webhook, creating it if necessary. + def execute_web_hook!(*args) + update_web_hook! + service_hook.execute(*args) + end + end +end diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index fef2774c593..590be52151c 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -18,14 +18,8 @@ module Integrations attr_accessor :response - after_save :compose_service_hook, if: :activated? before_update :reset_password - def compose_service_hook - hook = service_hook || build_service_hook - hook.save - end - def reset_password if bamboo_url_changed? && !password_touched? self.password = nil diff --git a/app/models/integrations/buildkite.rb b/app/models/integrations/buildkite.rb index 4f28c961916..94a37f0c4f2 100644 --- a/app/models/integrations/buildkite.rb +++ b/app/models/integrations/buildkite.rb @@ -4,7 +4,9 @@ require "addressable/uri" module Integrations class Buildkite < BaseCi + include HasWebHook include ReactiveService + extend Gitlab::Utils::Override ENDPOINT = "https://buildkite.com" @@ -13,8 +15,6 @@ module Integrations validates :project_url, presence: true, public_url: true, if: :activated? validates :token, presence: true, if: :activated? - after_save :compose_service_hook, if: :activated? - def self.supported_events %w(push merge_request tag_push) end @@ -35,21 +35,15 @@ module Integrations self.properties.delete('enable_ssl_verification') # Remove unused key end - def webhook_url + override :hook_url + def hook_url "#{buildkite_endpoint('webhook')}/deliver/#{webhook_token}" end - def compose_service_hook - hook = service_hook || build_service_hook - hook.url = webhook_url - hook.enable_ssl_verification = true - hook.save - end - def execute(data) return unless supported_events.include?(data[:object_kind]) - service_hook.execute(data) + execute_web_hook!(data) end def commit_status(sha, ref) diff --git a/app/models/integrations/datadog.rb b/app/models/integrations/datadog.rb index d7d472f1d89..27c2fcf266b 100644 --- a/app/models/integrations/datadog.rb +++ b/app/models/integrations/datadog.rb @@ -2,6 +2,9 @@ module Integrations class Datadog < Integration + include HasWebHook + extend Gitlab::Utils::Override + DEFAULT_DOMAIN = 'datadoghq.com' URL_TEMPLATE = 'https://webhooks-http-intake.logs.%{datadog_domain}/api/v2/webhook' URL_TEMPLATE_API_KEYS = 'https://app.%{datadog_domain}/account/settings#api' @@ -21,8 +24,6 @@ module Integrations validates :api_url, presence: true, unless: -> (obj) { obj.datadog_site.present? } end - after_save :compose_service_hook, if: :activated? - def initialize_properties super @@ -98,12 +99,7 @@ module Integrations ] end - def compose_service_hook - hook = service_hook || build_service_hook - hook.url = hook_url - hook.save - end - + override :hook_url def hook_url url = api_url.presence || sprintf(URL_TEMPLATE, datadog_domain: datadog_domain) url = URI.parse(url) @@ -127,7 +123,7 @@ module Integrations object_kind = 'job' if object_kind == 'build' return unless supported_events.include?(object_kind) - service_hook.execute(data, "#{object_kind} hook") + execute_web_hook!(data, "#{object_kind} hook") end def test(data) diff --git a/app/models/integrations/drone_ci.rb b/app/models/integrations/drone_ci.rb index 6d60ebac8b7..c93ae432fe9 100644 --- a/app/models/integrations/drone_ci.rb +++ b/app/models/integrations/drone_ci.rb @@ -2,8 +2,10 @@ module Integrations class DroneCi < BaseCi + include HasWebHook include ReactiveService include ServicePushDataValidations + extend Gitlab::Utils::Override prop_accessor :drone_url, :token boolean_accessor :enable_ssl_verification @@ -11,24 +13,16 @@ module Integrations validates :drone_url, presence: true, public_url: true, if: :activated? validates :token, presence: true, if: :activated? - after_save :compose_service_hook, if: :activated? - - def compose_service_hook - hook = service_hook || build_service_hook - # If using a service template, project may not be available - hook.url = [drone_url, "/hook", "?owner=#{project.namespace.full_path}", "&name=#{project.path}", "&access_token=#{token}"].join if project - hook.enable_ssl_verification = !!enable_ssl_verification - hook.save - end - def execute(data) + return unless project + case data[:object_kind] when 'push' - service_hook.execute(data) if push_valid?(data) + execute_web_hook!(data) if push_valid?(data) when 'merge_request' - service_hook.execute(data) if merge_request_valid?(data) + execute_web_hook!(data) if merge_request_valid?(data) when 'tag_push' - service_hook.execute(data) if tag_push_valid?(data) + execute_web_hook!(data) if tag_push_valid?(data) end end @@ -105,5 +99,21 @@ module Integrations { type: 'checkbox', name: 'enable_ssl_verification', title: "Enable SSL verification" } ] end + + override :hook_url + def hook_url + [drone_url, "/hook", "?owner=#{project.namespace.full_path}", "&name=#{project.path}", "&access_token=#{token}"].join + end + + override :hook_ssl_verification + def hook_ssl_verification + !!enable_ssl_verification + end + + override :update_web_hook! + def update_web_hook! + # If using a service template, project may not be available + super if project + end end end diff --git a/app/models/integrations/jenkins.rb b/app/models/integrations/jenkins.rb index 264bbd0b6ed..55fc60990f3 100644 --- a/app/models/integrations/jenkins.rb +++ b/app/models/integrations/jenkins.rb @@ -2,7 +2,9 @@ module Integrations class Jenkins < BaseCi + include HasWebHook include ActionView::Helpers::UrlHelper + extend Gitlab::Utils::Override prop_accessor :jenkins_url, :project_name, :username, :password @@ -16,8 +18,6 @@ module Integrations default_value_for :merge_requests_events, false default_value_for :tag_push_events, false - after_save :compose_service_hook, if: :activated? - def reset_password # don't reset the password if a new one is provided if (jenkins_url_changed? || username.blank?) && !password_touched? @@ -25,16 +25,10 @@ module Integrations end end - def compose_service_hook - hook = service_hook || build_service_hook - hook.url = hook_url - hook.save - end - def execute(data) return unless supported_events.include?(data[:object_kind]) - service_hook.execute(data, "#{data[:object_kind]}_hook") + execute_web_hook!(data, "#{data[:object_kind]}_hook") end def test(data) @@ -48,6 +42,7 @@ module Integrations { success: true, result: result[:message] } end + override :hook_url def hook_url url = URI.parse(jenkins_url) url.path = File.join(url.path || '/', "project/#{project_name}") diff --git a/app/models/integrations/packagist.rb b/app/models/integrations/packagist.rb index b597bd11175..fb0917db02b 100644 --- a/app/models/integrations/packagist.rb +++ b/app/models/integrations/packagist.rb @@ -2,6 +2,9 @@ module Integrations class Packagist < Integration + include HasWebHook + extend Gitlab::Utils::Override + prop_accessor :username, :token, :server validates :username, presence: true, if: :activated? @@ -10,8 +13,6 @@ module Integrations default_value_for :push_events, true default_value_for :tag_push_events, true - after_save :compose_service_hook, if: :activated? - def title 'Packagist' end @@ -39,7 +40,7 @@ module Integrations def execute(data) return unless supported_events.include?(data[:object_kind]) - service_hook.execute(data) + execute_web_hook!(data) end def test(data) @@ -53,12 +54,7 @@ module Integrations { success: true, result: result[:message] } end - def compose_service_hook - hook = service_hook || build_service_hook - hook.url = hook_url - hook.save - end - + override :hook_url def hook_url base_url = server.presence || 'https://packagist.org' "#{base_url}/api/update-package?username=#{username}&apiToken=#{token}" diff --git a/app/models/integrations/teamcity.rb b/app/models/integrations/teamcity.rb index 403271aa6a9..135c304b57e 100644 --- a/app/models/integrations/teamcity.rb +++ b/app/models/integrations/teamcity.rb @@ -18,7 +18,6 @@ module Integrations attr_accessor :response - after_save :compose_service_hook, if: :activated? before_update :reset_password class << self @@ -31,11 +30,6 @@ module Integrations end end - def compose_service_hook - hook = service_hook || build_service_hook - hook.save - end - def reset_password if teamcity_url_changed? && !password_touched? self.password = nil diff --git a/app/models/operations/feature_flag.rb b/app/models/operations/feature_flag.rb index 8b052f80395..450a5970ad8 100644 --- a/app/models/operations/feature_flag.rb +++ b/app/models/operations/feature_flag.rb @@ -80,7 +80,7 @@ module Operations end def link_reference_pattern - @link_reference_pattern ||= super("feature_flags", /(?<feature_flag>\d+)\/edit/) + @link_reference_pattern ||= super("feature_flags", %r{(?<feature_flag>\d+)/edit}) end def reference_postfix diff --git a/app/models/packages/go/module.rb b/app/models/packages/go/module.rb index 00d51c21881..a029437c82d 100644 --- a/app/models/packages/go/module.rb +++ b/app/models/packages/go/module.rb @@ -33,7 +33,7 @@ module Packages end def path_valid?(major) - m = /\/v(\d+)$/i.match(@name) + m = %r{/v(\d+)$}i.match(@name) case major when 0, 1 diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 5fde346c4ab..d42dcb2fd00 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -115,7 +115,25 @@ module Auth # ensure_container_repository!(path, authorized_actions) - { type: type, name: path.to_s, actions: authorized_actions } + { + type: type, + name: path.to_s, + actions: authorized_actions, + migration_eligible: migration_eligible(requested_project, authorized_actions) + }.compact + end + + def migration_eligible(project, actions) + return unless actions.include?('push') + return unless Feature.enabled?(:container_registry_migration_phase1) + + # The migration process will start by allowing only specific test and gitlab-org projects using the + # `container_registry_migration_phase1_allow` FF. We'll then move on to a percentage rollout using this same FF. + # To remove the risk of impacting enterprise customers that rely heavily on the registry during the percentage + # rollout, we'll add their top-level group/namespace to the `container_registry_migration_phase1_deny` FF. Later, + # we'll remove them manually from this deny list, and their new repositories will become eligible. + Feature.disabled?(:container_registry_migration_phase1_deny, project.root_ancestor) && + Feature.enabled?(:container_registry_migration_phase1_allow, project) end ## diff --git a/app/services/packages/conan/search_service.rb b/app/services/packages/conan/search_service.rb index 143fd8a627b..31ee9bea084 100644 --- a/app/services/packages/conan/search_service.rb +++ b/app/services/packages/conan/search_service.rb @@ -41,7 +41,7 @@ module Packages end def search_for_single_package(query) - name, version, username, _ = query.split(/[@\/]/) + name, version, username, _ = query.split(%r{[@/]}) full_path = Packages::Conan::Metadatum.full_path_from(package_username: username) project = Project.find_by_full_path(full_path) return unless Ability.allowed?(current_user, :read_package, project) diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index eac84337967..6c29ba81910 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -65,7 +65,7 @@ module Projects # TODO: Support LFS sync over SSH # https://gitlab.com/gitlab-org/gitlab/-/issues/249587 - return unless remote_mirror.url =~ /\Ahttps?:\/\//i + return unless remote_mirror.url =~ %r{\Ahttps?://}i return unless remote_mirror.password_auth? Lfs::PushService.new( diff --git a/app/workers/project_service_worker.rb b/app/workers/project_service_worker.rb index 967be3b3e81..da38d2fc0cd 100644 --- a/app/workers/project_service_worker.rb +++ b/app/workers/project_service_worker.rb @@ -15,6 +15,6 @@ class ProjectServiceWorker # rubocop:disable Scalability/IdempotentWorker integration.execute(data) rescue StandardError => error integration_class = integration&.class&.name || "Not Found" - logger.error class: self.class.name, service_class: integration_class, message: error.message + Gitlab::ErrorTracking.log_exception(error, integration_class: integration_class) end end diff --git a/config/feature_flags/development/container_registry_migration_phase1.yml b/config/feature_flags/development/container_registry_migration_phase1.yml new file mode 100644 index 00000000000..85fbdcfab01 --- /dev/null +++ b/config/feature_flags/development/container_registry_migration_phase1.yml @@ -0,0 +1,8 @@ +--- +name: container_registry_migration_phase1 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63907 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335703 +milestone: '14.1' +type: development +group: group::package +default_enabled: false diff --git a/config/feature_flags/development/container_registry_migration_phase1_allow.yml b/config/feature_flags/development/container_registry_migration_phase1_allow.yml new file mode 100644 index 00000000000..1e8d260c93b --- /dev/null +++ b/config/feature_flags/development/container_registry_migration_phase1_allow.yml @@ -0,0 +1,8 @@ +--- +name: container_registry_migration_phase1_allow +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63907 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335705 +milestone: '14.1' +type: development +group: group::package +default_enabled: false diff --git a/config/feature_flags/development/container_registry_migration_phase1_deny.yml b/config/feature_flags/development/container_registry_migration_phase1_deny.yml new file mode 100644 index 00000000000..1aa66059045 --- /dev/null +++ b/config/feature_flags/development/container_registry_migration_phase1_deny.yml @@ -0,0 +1,8 @@ +--- +name: container_registry_migration_phase1_deny +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63907 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335706 +milestone: '14.1' +type: development +group: group::package +default_enabled: false diff --git a/config/initializers/rspec_profiling.rb b/config/initializers/rspec_profiling.rb index 1288fad8a94..5edea6489ed 100644 --- a/config/initializers/rspec_profiling.rb +++ b/config/initializers/rspec_profiling.rb @@ -61,7 +61,7 @@ RspecProfiling.configure do |config| RspecProfiling::Run.prepend(RspecProfilingExt::Run) config.collector = RspecProfilingExt::Collectors::CSVWithTimestamps config.csv_path = -> do - prefix = "#{ENV['CI_JOB_NAME']}-".gsub(/[ \/]/, '-') if ENV['CI_JOB_NAME'] + prefix = "#{ENV['CI_JOB_NAME']}-".gsub(%r{[ /]}, '-') if ENV['CI_JOB_NAME'] "rspec_profiling/#{prefix}#{Time.now.to_i}-#{SecureRandom.hex(8)}-rspec-data.csv" end end diff --git a/doc/administration/database_load_balancing.md b/doc/administration/database_load_balancing.md index 1aa01ae1f64..7d17b22a4d7 100644 --- a/doc/administration/database_load_balancing.md +++ b/doc/administration/database_load_balancing.md @@ -104,32 +104,19 @@ the following. This balances the load between `host1.example.com` and 1. Save the file and [restart GitLab](restart_gitlab.md#installations-from-source) for the changes to take effect. -### Enable the load balancer for Sidekiq +### Load balancing for Sidekiq -Sidekiq mostly writes to the database, which means that most of its traffic hits the -primary database. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/334494) in GitLab 14.1, load balancing for Sidekick is enabled by default. -Some background jobs can use database replicas to read application state. +Sidekiq jobs mostly write to the primary database, but there are read-only jobs that can benefit +from the use of Sidekiq load balancing. +These jobs can use load balancing and database replicas to read the application state. This allows to offload the primary database. -Load balancing is disabled by default in Sidekiq. When enabled, we can define -[the data consistency](../development/sidekiq_style_guide.md#job-data-consistency-strategies) +For Sidekiq, we can define +[data consistency](../development/sidekiq_style_guide.md#job-data-consistency-strategies) requirements for a specific job. -To enable it, define the `ENABLE_LOAD_BALANCING_FOR_SIDEKIQ` variable to the environment, as shown below. - -For Omnibus installations: - -```ruby -gitlab_rails['env'] = {"ENABLE_LOAD_BALANCING_FOR_SIDEKIQ" => "true"} -``` - -For installations from source: - -```shell -export ENABLE_LOAD_BALANCING_FOR_SIDEKIQ="true" -``` - ## Service Discovery > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/5883) in [GitLab Premium](https://about.gitlab.com/pricing/) 11.0. diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index db5a3407bdd..8d531e657c1 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -215,6 +215,24 @@ NOTE: `inplace_chroot` option might not work with the other features, such as [Pages Access Control](#access-control). The [GitLab Pages README](https://gitlab.com/gitlab-org/gitlab-pages#caveats) has more information about caveats and workarounds. +### Jailing mechanism disabled by default for API-based configuration + +Starting from GitLab 14.1 the [jailing/chroot mechanism is disabled by default](https://gitlab.com/gitlab-org/gitlab-pages/-/issues/589). +If you are using API-based configuration and the new [Zip storage architecture](#zip-storage) +there is nothing you need to do. + +If you run into any problems, [open a new issue](https://gitlab.com/gitlab-org/gitlab-pages/-/issues/new) +and enable the jail again by setting the environment variable: + +1. Edit `/etc/gitlab/gitlab.rb`. +1. Set the `DAEMON_ENABLE_JAIL` environment variable to `true` for GitLab Pages: + + ```ruby + gitlab_pages['env']['DAEMON_ENABLE_JAIL'] = "true" + ``` + +1. [Reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure). + ### Global settings Below is a table of all configuration settings known to Pages in Omnibus GitLab, @@ -1366,6 +1384,10 @@ GitLab 14.0 introduces a number of changes to GitLab Pages which may require man The most common problem is when using [`inplace_chroot`](#dial-tcp-lookup-gitlabexamplecom-and-x509-certificate-signed-by-unknown-authority). +NOTE: +Starting from 14.1, the chroot/jailing mechanism is +[disabled by default for API-based configuration](#jailing-mechanism-disabled-by-default-for-api-based-configuration). + WARNING: As the last resort you can temporarily enable legacy storage and configuration mechanisms. Support for them [will be removed in GitLab 14.3](https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6166), so GitLab Pages will stop working if don't resolve the underlying issue. diff --git a/doc/development/database/not_null_constraints.md b/doc/development/database/not_null_constraints.md index 9d1850f5504..48b198b46bd 100644 --- a/doc/development/database/not_null_constraints.md +++ b/doc/development/database/not_null_constraints.md @@ -175,7 +175,7 @@ class CleanupEpicsWithNullDescription < ActiveRecord::Migration[6.0] end ``` -#### Validate the text limit (next release) +#### Validate the `NOT NULL` constraint (next release) Validating the `NOT NULL` constraint will scan the whole table and make sure that each record is correct. @@ -201,7 +201,7 @@ end ## `NOT NULL` constraints on large tables -If you have to clean up a text column for a [high-traffic table](../migration_style_guide.md#high-traffic-tables) +If you have to clean up a nullable column for a [high-traffic table](../migration_style_guide.md#high-traffic-tables) (for example, the `artifacts` in `ci_builds`), your background migration will go on for a while and it will need an additional [background migration cleaning up](../background_migrations.md#cleaning-up) in the release after adding the data migration. diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index bb61edba9a1..277d427ce9c 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -478,7 +478,7 @@ of reading a stale record is non-zero due to replicas potentially lagging behind When the number of jobs that rely on the database increases, ensuring immediate data consistency can put unsustainable load on the primary database server. We therefore added the ability to use -[database load-balancing in Sidekiq workers](../administration/database_load_balancing.md#enable-the-load-balancer-for-sidekiq). +[database load balancing for Sidekiq workers](../administration/database_load_balancing.md#load-balancing-for-sidekiq). By configuring a worker's `data_consistency` field, we can then allow the scheduler to target read replicas under several strategies outlined below. diff --git a/doc/install/installation.md b/doc/install/installation.md index 572c6de18d0..9db8631a6a5 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -1004,6 +1004,17 @@ Using Sidekiq directly is still supported until 14.0. So if you're experiencing 1. Restart GitLab. 1. [Create an issue](https://gitlab.com/gitlab-org/gitlab/-/issues/-/new) describing the problem. +### Prometheus server setup + +You can configure the Prometheus server in `config/gitlab.yml`: + +```yaml +# example +prometheus: + enabled: true + server_address: '10.1.2.3:9090' +``` + ## Troubleshooting ### "You appear to have cloned an empty repository." diff --git a/doc/operations/incident_management/escalation_policies.md b/doc/operations/incident_management/escalation_policies.md new file mode 100644 index 00000000000..9df8f0dbf7f --- /dev/null +++ b/doc/operations/incident_management/escalation_policies.md @@ -0,0 +1,43 @@ +--- +stage: Monitor +group: Monitor +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Escalation Policies **(PREMIUM)** + +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4638) in [GitLab Premium](https://about.gitlab.com/pricing/) 14.1. + +Escalation Policies protect your company from missed critical alerts. Escalation Policies contain +time-boxed steps that automatically page the next responder in the escalation step if the responder +in the previous step has not responded. You can create an escalation policy in the GitLab project +where you manage [On-call schedules](oncall_schedules.md). + +## Add an escalation policy + +If you have at least Maintainer [permissions](../../user/permissions.md), +you can create an escalation policy: + +1. Go to **Operations > Escalation Policies** and select **Add an escalation policy**. +1. In the **Add escalation policy** form, enter the policy's name and description, and create + escalation rules to follow when a primary responder misses an alert. +1. Select **Add escalation policy**. + + + +### Edit an escalation policy + +Follow these steps to update an escalation policy: + +1. Go to **Operations > Escalation Policies** and select the **Pencil** icon on the top right of the + policy card, across from the policy name. +1. In the **Edit policy** form, edit the information you wish to update. +1. Select the **Edit policy** button to save your changes. + +### Delete an escalation policy + +Follow these steps to delete a policy: + +1. Go to **Operations > Escalation Policies** and select the **Trash Can** icon on the top right of + the policy card. +1. In the **Delete escalation policy** window, select the **Delete escalation policy** button. diff --git a/doc/operations/incident_management/img/escalation_policy_v14_1.png b/doc/operations/incident_management/img/escalation_policy_v14_1.png Binary files differnew file mode 100644 index 00000000000..89cd4318fcb --- /dev/null +++ b/doc/operations/incident_management/img/escalation_policy_v14_1.png diff --git a/doc/update/index.md b/doc/update/index.md index b23ebb58d35..fd6c9ca45cd 100644 --- a/doc/update/index.md +++ b/doc/update/index.md @@ -201,7 +201,7 @@ upgrade paths. | Target version | Your version | Supported upgrade path | Note | | --------------------- | ------------ | ------------------------ | ---- | -| `14.1.0` | `13.9.2` | `13.9.2` -> `13.12.6` -> `14.0.3` -> `14.1.0` | Two intermediate versions are required: `13.12` and `14.0`, then `14.1.0`. | +| `14.1.0` | `13.9.2` | `13.9.2` -> `13.12.6` -> `14.0.5` -> `14.1.0` | Two intermediate versions are required: `13.12` and `14.0`, then `14.1.0`. | | `13.5.4` | `12.9.2` | `12.9.2` -> `12.10.14` -> `13.0.14` -> `13.1.11` -> `13.5.4` | Three intermediate versions are required: `12.10`, `13.0` and `13.1`, then `13.5.4`. | | `13.2.10` | `11.5.0` | `11.5.0` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.10.14` -> `13.0.14` -> `13.1.11` -> `13.2.10` | Six intermediate versions are required: `11.11`, `12.0`, `12.1`, `12.10`, `13.0` and `13.1`, then `13.2.10`. | | `12.10.14` | `11.3.4` | `11.3.4` -> `11.11.8` -> `12.0.12` -> `12.1.17` -> `12.10.14` | Three intermediate versions are required: `11.11`, `12.0` and `12.1`, then `12.10.14`. | diff --git a/doc/user/admin_area/monitoring/background_migrations.md b/doc/user/admin_area/monitoring/background_migrations.md index eeedfa25469..cbaa4b30cb7 100644 --- a/doc/user/admin_area/monitoring/background_migrations.md +++ b/doc/user/admin_area/monitoring/background_migrations.md @@ -98,7 +98,7 @@ Expected batched background migration for the given configuration to be marked a To fix this error: -1. Update to either 14.0.3 or 14.1. +1. Update to either 14.0.5 or 14.1. 1. [Check the status](#check-the-status-of-background-migrations) of the batched background migration from the error message, and make sure it is listed as finished. If it is still active, either wait until it is done, or finalize it manually using the command suggested in the error, for example: ```shell diff --git a/doc/user/packages/helm_repository/index.md b/doc/user/packages/helm_repository/index.md index 99fde6a97eb..26d8bf76cd6 100644 --- a/doc/user/packages/helm_repository/index.md +++ b/doc/user/packages/helm_repository/index.md @@ -38,7 +38,7 @@ Once built, a chart can be uploaded to the `stable` channel with `curl` or `helm ```shell curl --request POST \ - --form 'chart=@mychart.tgz' \ + --form 'chart=@mychart-0.1.0.tgz' \ --user <username>:<personal_access_token> \ https://gitlab.example.com/api/v4/projects/1/packages/helm/api/stable/charts ``` @@ -50,6 +50,25 @@ Once built, a chart can be uploaded to the `stable` channel with `curl` or `helm helm push mychart-0.1.0.tgz project-1 ``` +## Use CI/CD to publish a Helm package + +To publish a Helm package automated through [GitLab CI/CD](../../../ci/index.md), you can use +`CI_JOB_TOKEN` in place of the personal access token in your commands. + +For example: + +```yaml +image: curlimages/curl:latest + +stages: + - upload + +upload: + stage: upload + script: + - 'curl --request POST --user gitlab-ci-token:$CI_JOB_TOKEN --form "chart=@mychart-0.1.0.tgz" "${CI_API_V4_URL}/projects/${CI_PROJECT_ID}/packages/helm/api/stable/charts"' +``` + ## Install a package To install the latest version of a chart, use the following command: diff --git a/lib/gitlab/database/load_balancing.rb b/lib/gitlab/database/load_balancing.rb index 88743cd2e75..31d41a6d6c0 100644 --- a/lib/gitlab/database/load_balancing.rb +++ b/lib/gitlab/database/load_balancing.rb @@ -85,7 +85,6 @@ module Gitlab # Returns true if load balancing is to be enabled. def self.enable? return false if Gitlab::Runtime.rake? - return false if Gitlab::Runtime.sidekiq? && !Gitlab::Utils.to_boolean(ENV['ENABLE_LOAD_BALANCING_FOR_SIDEKIQ'], default: false) return false unless self.configured? true diff --git a/qa/qa/resource/project_imported_from_github.rb b/qa/qa/resource/project_imported_from_github.rb index 96a71eee812..32182774508 100644 --- a/qa/qa/resource/project_imported_from_github.rb +++ b/qa/qa/resource/project_imported_from_github.rb @@ -16,10 +16,7 @@ module QA Page::Main::Menu.perform(&:go_to_create_project) - Page::Project::New.perform do |project_page| - project_page.click_import_project - project_page.click_github_link - end + go_to_import_page Page::Project::Import::Github.perform do |import_page| import_page.add_personal_access_token(github_personal_access_token) @@ -28,6 +25,13 @@ module QA end end + def go_to_import_page + Page::Project::New.perform do |project_page| + project_page.click_import_project + project_page.click_github_link + end + end + def fabricate_via_api! super rescue ResourceURLMissingError diff --git a/qa/qa/specs/features/browser_ui/4_verify/testing/view_code_coverage_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/testing/view_code_coverage_spec.rb index d6d8729114d..07484feb686 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/testing/view_code_coverage_spec.rb +++ b/qa/qa/specs/features/browser_ui/4_verify/testing/view_code_coverage_spec.rb @@ -40,7 +40,7 @@ module QA Page::MergeRequest::Show.perform do |mr_widget| Support::Retrier.retry_until(max_attempts: 5, sleep_interval: 5) do - mr_widget.has_pipeline_status?(/Pipeline #\d+ passed/) + mr_widget.has_pipeline_status?('passed') end expect(mr_widget).to have_content('Test coverage 66.67%') end diff --git a/spec/controllers/projects/service_hook_logs_controller_spec.rb b/spec/controllers/projects/service_hook_logs_controller_spec.rb index 040e59fc822..9caa4a06b44 100644 --- a/spec/controllers/projects/service_hook_logs_controller_spec.rb +++ b/spec/controllers/projects/service_hook_logs_controller_spec.rb @@ -27,6 +27,15 @@ RSpec.describe Projects::ServiceHookLogsController do specify do expect(response).to be_successful end + + it 'renders a 404 if the hook does not exist' do + log_params + integration.service_hook.destroy! + + subject + + expect(response).to have_gitlab_http_status(:not_found) + end end describe 'POST #retry' do @@ -37,5 +46,14 @@ RSpec.describe Projects::ServiceHookLogsController do expect_any_instance_of(described_class).to receive(:set_hook_execution_notice) expect(subject).to redirect_to(edit_project_service_path(project, integration)) end + + it 'renders a 404 if the hook does not exist' do + log_params + integration.service_hook.destroy! + + subject + + expect(response).to have_gitlab_http_status(:not_found) + end end end diff --git a/spec/features/admin/users/user_spec.rb b/spec/features/admin/users/user_spec.rb index 3599658ee56..9766bd48a8f 100644 --- a/spec/features/admin/users/user_spec.rb +++ b/spec/features/admin/users/user_spec.rb @@ -372,4 +372,36 @@ RSpec.describe 'Admin::Users::User' do end end end + + context 'when user has an unconfirmed email', :js do + let(:unconfirmed_user) { create(:user, :unconfirmed) } + + where(:path_helper) do + [ + [-> (user) { admin_user_path(user) }], + [-> (user) { projects_admin_user_path(user) }], + [-> (user) { keys_admin_user_path(user) }], + [-> (user) { admin_user_identities_path(user) }], + [-> (user) { admin_user_impersonation_tokens_path(user) }] + ] + end + + with_them do + it "allows an admin to force confirmation of the user's email", :aggregate_failures do + visit path_helper.call(unconfirmed_user) + + click_button 'Confirm user' + + page.within('[role="dialog"]') do + expect(page).to have_content("Confirm user #{unconfirmed_user.name}?") + expect(page).to have_content('This user has an unconfirmed email address. You may force a confirmation.') + + click_button 'Confirm user' + end + + expect(page).to have_content('Successfully confirmed') + expect(page).not_to have_button('Confirm user') + end + end + end end diff --git a/spec/frontend/pages/admin/users/components/__snapshots__/delete_user_modal_spec.js.snap b/spec/frontend/admin/users/components/modals/__snapshots__/delete_user_modal_spec.js.snap index 5e367891337..5e367891337 100644 --- a/spec/frontend/pages/admin/users/components/__snapshots__/delete_user_modal_spec.js.snap +++ b/spec/frontend/admin/users/components/modals/__snapshots__/delete_user_modal_spec.js.snap diff --git a/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js b/spec/frontend/admin/users/components/modals/delete_user_modal_spec.js index 93d9ee43179..fee74764645 100644 --- a/spec/frontend/pages/admin/users/components/delete_user_modal_spec.js +++ b/spec/frontend/admin/users/components/modals/delete_user_modal_spec.js @@ -1,6 +1,6 @@ import { GlButton, GlFormInput } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; -import DeleteUserModal from '~/pages/admin/users/components/delete_user_modal.vue'; +import DeleteUserModal from '~/admin/users/components/modals/delete_user_modal.vue'; import OncallSchedulesList from '~/vue_shared/components/oncall_schedules_list.vue'; import ModalStub from './stubs/modal_stub'; diff --git a/spec/frontend/pages/admin/users/components/stubs/modal_stub.js b/spec/frontend/admin/users/components/modals/stubs/modal_stub.js index 4dc55e909a0..4dc55e909a0 100644 --- a/spec/frontend/pages/admin/users/components/stubs/modal_stub.js +++ b/spec/frontend/admin/users/components/modals/stubs/modal_stub.js diff --git a/spec/frontend/pages/admin/users/components/user_modal_manager_spec.js b/spec/frontend/admin/users/components/modals/user_modal_manager_spec.js index 3669bc40d7e..65ce242662b 100644 --- a/spec/frontend/pages/admin/users/components/user_modal_manager_spec.js +++ b/spec/frontend/admin/users/components/modals/user_modal_manager_spec.js @@ -1,5 +1,5 @@ import { mount } from '@vue/test-utils'; -import UserModalManager from '~/pages/admin/users/components/user_modal_manager.vue'; +import UserModalManager from '~/admin/users/components/modals/user_modal_manager.vue'; import ModalStub from './stubs/modal_stub'; describe('Users admin page Modal Manager', () => { diff --git a/spec/frontend/search/store/actions_spec.js b/spec/frontend/search/store/actions_spec.js index 3dbf0412c79..3755f8ffae7 100644 --- a/spec/frontend/search/store/actions_spec.js +++ b/spec/frontend/search/store/actions_spec.js @@ -115,7 +115,7 @@ describe('Global Search Store Actions', () => { }); describe('when groupId is set', () => { - it('calls Api.groupProjects', () => { + it('calls Api.groupProjects with expected parameters', () => { actions.fetchProjects({ commit: mockCommit, state }); expect(Api.groupProjects).toHaveBeenCalledWith( @@ -123,6 +123,8 @@ describe('Global Search Store Actions', () => { state.query.search, { order_by: 'similarity', + include_subgroups: true, + with_shared: false, }, expect.any(Function), ); diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb index 40faf994ad2..a3f2b8fafa0 100644 --- a/spec/helpers/gitlab_routing_helper_spec.rb +++ b/spec/helpers/gitlab_routing_helper_spec.rb @@ -239,8 +239,9 @@ RSpec.describe GitlabRoutingHelper do let(:blob) { snippet.blobs.first } let(:ref) { 'snippet-test-ref' } let(:args) { {} } + let(:path) { blob.path } - subject { gitlab_raw_snippet_blob_url(snippet, blob.path, ref, **args) } + subject { gitlab_raw_snippet_blob_url(snippet, path, ref, **args) } it_behaves_like 'snippet blob raw url' @@ -248,7 +249,7 @@ RSpec.describe GitlabRoutingHelper do let(:args) { { inline: true } } let(:snippet) { personal_snippet } - it { expect(subject).to eq("http://test.host/-/snippets/#{snippet.id}/raw/#{ref}/#{blob.path}?inline=true") } + it { expect(subject).to eq("http://test.host/-/snippets/#{snippet.id}/raw/#{ref}/#{path}?inline=true") } end context 'without a ref' do @@ -257,7 +258,17 @@ RSpec.describe GitlabRoutingHelper do let(:expected_ref) { snippet.repository.root_ref } it 'uses the root ref' do - expect(subject).to eq("http://test.host/-/snippets/#{snippet.id}/raw/#{expected_ref}/#{blob.path}") + expect(subject).to eq("http://test.host/-/snippets/#{snippet.id}/raw/#{expected_ref}/#{path}") + end + + context 'when snippet does not have a repository' do + let(:snippet) { create(:personal_snippet) } + let(:path) { 'example' } + let(:expected_ref) { Gitlab::DefaultBranch.value } + + it 'uses the instance deafult branch' do + expect(subject).to eq("http://test.host/-/snippets/#{snippet.id}/raw/#{expected_ref}/#{path}") + end end end end diff --git a/spec/lib/gitlab/database/load_balancing_spec.rb b/spec/lib/gitlab/database/load_balancing_spec.rb index e7de7f2b43b..94717a10492 100644 --- a/spec/lib/gitlab/database/load_balancing_spec.rb +++ b/spec/lib/gitlab/database/load_balancing_spec.rb @@ -142,10 +142,10 @@ RSpec.describe Gitlab::Database::LoadBalancing do expect(described_class.enable?).to eq(false) end - it 'returns false when Sidekiq is being used' do + it 'returns true when Sidekiq is being used' do allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) - expect(described_class.enable?).to eq(false) + expect(described_class.enable?).to eq(true) end it 'returns false when running inside a Rake task' do @@ -170,18 +170,6 @@ RSpec.describe Gitlab::Database::LoadBalancing do expect(described_class.enable?).to eq(true) end - - context 'when ENABLE_LOAD_BALANCING_FOR_SIDEKIQ environment variable is set' do - before do - stub_env('ENABLE_LOAD_BALANCING_FOR_SIDEKIQ', 'true') - end - - it 'returns true when Sidekiq is being used' do - allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) - - expect(described_class.enable?).to eq(true) - end - end end describe '.configured?' do diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 51bf6cf1a81..ab4027170b2 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -9,11 +9,12 @@ RSpec.describe Integration do let_it_be(:project) { create(:project, group: group) } describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to belong_to :group } - it { is_expected.to have_one :service_hook } - it { is_expected.to have_one :jira_tracker_data } - it { is_expected.to have_one :issue_tracker_data } + it { is_expected.to belong_to(:project).inverse_of(:integrations) } + it { is_expected.to belong_to(:group).inverse_of(:integrations) } + it { is_expected.to have_one(:service_hook).inverse_of(:integration).with_foreign_key(:service_id) } + it { is_expected.to have_one(:issue_tracker_data).autosave(true).inverse_of(:integration).with_foreign_key(:service_id).class_name('Integrations::IssueTrackerData') } + it { is_expected.to have_one(:jira_tracker_data).autosave(true).inverse_of(:integration).with_foreign_key(:service_id).class_name('Integrations::JiraTrackerData') } + it { is_expected.to have_one(:open_project_tracker_data).autosave(true).inverse_of(:integration).with_foreign_key(:service_id).class_name('Integrations::OpenProjectTrackerData') } end describe 'validations' do diff --git a/spec/models/integrations/asana_spec.rb b/spec/models/integrations/asana_spec.rb index 6c9ee8ba08d..f7e7eb1b0ae 100644 --- a/spec/models/integrations/asana_spec.rb +++ b/spec/models/integrations/asana_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Asana do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'active' do before do @@ -42,7 +37,6 @@ RSpec.describe Integrations::Asana do allow(@asana).to receive_messages( project: project, project_id: project.id, - service_hook: true, api_key: 'verySecret', restrict_to_branch: 'master' ) diff --git a/spec/models/integrations/assembla_spec.rb b/spec/models/integrations/assembla_spec.rb index e5972bce95d..960dfea3dc4 100644 --- a/spec/models/integrations/assembla_spec.rb +++ b/spec/models/integrations/assembla_spec.rb @@ -5,11 +5,6 @@ require 'spec_helper' RSpec.describe Integrations::Assembla do include StubRequests - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project, :repository) } @@ -19,7 +14,6 @@ RSpec.describe Integrations::Assembla do allow(@assembla_integration).to receive_messages( project_id: project.id, project: project, - service_hook: true, token: 'verySecret', subdomain: 'project_name' ) diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb index b5a48cb235c..73ebf404828 100644 --- a/spec/models/integrations/bamboo_spec.rb +++ b/spec/models/integrations/bamboo_spec.rb @@ -22,11 +22,6 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do ) end - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when active' do before do diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index 89aabb05216..ac4031a9b7d 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -28,7 +28,6 @@ RSpec.describe Integrations::BaseChatNotification do allow(chat_integration).to receive_messages( project: project, project_id: project.id, - service_hook: true, webhook: webhook_url ) diff --git a/spec/models/integrations/bugzilla_spec.rb b/spec/models/integrations/bugzilla_spec.rb index d44fce6bcfe..432306c8fa8 100644 --- a/spec/models/integrations/bugzilla_spec.rb +++ b/spec/models/integrations/bugzilla_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Bugzilla do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/models/integrations/buildkite_spec.rb b/spec/models/integrations/buildkite_spec.rb index 3ed68c67b81..4207ae0d555 100644 --- a/spec/models/integrations/buildkite_spec.rb +++ b/spec/models/integrations/buildkite_spec.rb @@ -12,16 +12,14 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do described_class.create!( project: project, properties: { - service_hook: true, project_url: 'https://buildkite.com/organization-name/example-pipeline', token: 'secret-sauce-webhook-token:secret-sauce-status-token' } ) end - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } + it_behaves_like Integrations::HasWebHook do + let(:hook_url) { 'https://webhook.buildkite.com/deliver/secret-sauce-webhook-token' } end describe 'Validations' do @@ -66,9 +64,9 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do .to change { integration.service_hook.enable_ssl_verification }.from(false).to(true) end - describe '#webhook_url' do + describe '#hook_url' do it 'returns the webhook url' do - expect(integration.webhook_url).to eq( + expect(integration.hook_url).to eq( 'https://webhook.buildkite.com/deliver/secret-sauce-webhook-token' ) end diff --git a/spec/models/integrations/campfire_spec.rb b/spec/models/integrations/campfire_spec.rb index 54c712015da..0044e6fae21 100644 --- a/spec/models/integrations/campfire_spec.rb +++ b/spec/models/integrations/campfire_spec.rb @@ -5,11 +5,6 @@ require 'spec_helper' RSpec.describe Integrations::Campfire do include StubRequests - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do @@ -37,7 +32,6 @@ RSpec.describe Integrations::Campfire do allow(@campfire_integration).to receive_messages( project_id: project.id, project: project, - service_hook: true, token: 'verySecret', subdomain: 'project-name', room: 'test-room' diff --git a/spec/models/integrations/confluence_spec.rb b/spec/models/integrations/confluence_spec.rb index 88ee029408b..e2f9316bc95 100644 --- a/spec/models/integrations/confluence_spec.rb +++ b/spec/models/integrations/confluence_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Confluence do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do before do subject.active = active diff --git a/spec/models/integrations/custom_issue_tracker_spec.rb b/spec/models/integrations/custom_issue_tracker_spec.rb index a050343228e..e1ffe7a74f0 100644 --- a/spec/models/integrations/custom_issue_tracker_spec.rb +++ b/spec/models/integrations/custom_issue_tracker_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::CustomIssueTracker do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index 00b7a19ab05..e2749ab1bc1 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -38,9 +38,9 @@ RSpec.describe Integrations::Datadog do let(:pipeline_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } let(:build_data) { Gitlab::DataBuilder::Build.build(build) } - describe 'associations' do - it { is_expected.to belong_to(:project) } - it { is_expected.to have_one(:service_hook) } + it_behaves_like Integrations::HasWebHook do + let(:integration) { instance } + let(:hook_url) { "#{described_class::URL_TEMPLATE % { datadog_domain: dd_site }}?dd-api-key=#{api_key}&env=#{dd_env}&service=#{dd_service}" } end describe 'validations' do diff --git a/spec/models/integrations/discord_spec.rb b/spec/models/integrations/discord_spec.rb index 6d6ee4d2f1d..b85620782c1 100644 --- a/spec/models/integrations/discord_spec.rb +++ b/spec/models/integrations/discord_spec.rb @@ -35,7 +35,6 @@ RSpec.describe Integrations::Discord do allow(subject).to receive_messages( project: project, project_id: project.id, - service_hook: true, webhook: webhook_url ) diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index 037d9681e0e..062e23d628e 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -5,11 +5,6 @@ require 'spec_helper' RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers - describe 'associations' do - it { is_expected.to belong_to(:project) } - it { is_expected.to have_one(:service_hook) } - end - describe 'validations' do context 'active' do before do @@ -32,7 +27,15 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end shared_context :drone_ci_integration do - let(:drone) { described_class.new } + subject(:drone) do + described_class.new( + project: project, + active: true, + drone_url: drone_url, + token: token + ) + end + let(:project) { create(:project, :repository, name: 'project') } let(:path) { project.full_path } let(:drone_url) { 'http://drone.example.com' } @@ -45,16 +48,6 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do let(:build_page) { "#{drone_url}/gitlab/#{path}/redirect/commits/#{sha}?branch=#{branch}" } let(:commit_status_path) { "#{drone_url}/gitlab/#{path}/commits/#{sha}?branch=#{branch}&access_token=#{token}" } - before do - allow(drone).to receive_messages( - project_id: project.id, - project: project, - active: true, - drone_url: drone_url, - token: token - ) - end - def stub_request(status: 200, body: nil) body ||= %q({"status":"success"}) @@ -66,6 +59,20 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do end end + it_behaves_like Integrations::HasWebHook do + include_context :drone_ci_integration + + let(:integration) { drone } + let(:hook_url) { "#{drone_url}/hook?owner=#{project.namespace.full_path}&name=#{project.path}&access_token=#{token}" } + + it 'does not create a hook if project is not present' do + integration.project = nil + integration.instance = true + + expect { integration.save! }.not_to change(ServiceHook, :count) + end + end + describe "integration page/path methods" do include_context :drone_ci_integration @@ -137,10 +144,17 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do Gitlab::DataBuilder::Push.build_sample(project, user) end - it do - service_hook = double - expect(service_hook).to receive(:execute) - expect(drone).to receive(:service_hook).and_return(service_hook) + it 'executes the webhook' do + expect(drone).to receive(:execute_web_hook!).with(push_sample_data) + + drone.execute(push_sample_data) + end + + it 'does not try to execute the webhook if the integration is not in a project' do + drone.project = nil + drone.instance = true + + expect(drone).not_to receive(:execute_web_hook!) drone.execute(push_sample_data) end diff --git a/spec/models/integrations/ewm_spec.rb b/spec/models/integrations/ewm_spec.rb index 2baf670f4ae..49681fefe55 100644 --- a/spec/models/integrations/ewm_spec.rb +++ b/spec/models/integrations/ewm_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Ewm do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/models/integrations/external_wiki_spec.rb b/spec/models/integrations/external_wiki_spec.rb index ed82e9543ab..e4d6a1c7c84 100644 --- a/spec/models/integrations/external_wiki_spec.rb +++ b/spec/models/integrations/external_wiki_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::ExternalWiki do - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/models/integrations/flowdock_spec.rb b/spec/models/integrations/flowdock_spec.rb index 34164736f98..daafb1b3958 100644 --- a/spec/models/integrations/flowdock_spec.rb +++ b/spec/models/integrations/flowdock_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Flowdock do - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do @@ -38,7 +33,6 @@ RSpec.describe Integrations::Flowdock do allow(flowdock_integration).to receive_messages( project_id: project.id, project: project, - service_hook: true, token: 'verySecret' ) WebMock.stub_request(:post, api_url) diff --git a/spec/models/integrations/irker_spec.rb b/spec/models/integrations/irker_spec.rb index 127d9028774..8b207e8b43e 100644 --- a/spec/models/integrations/irker_spec.rb +++ b/spec/models/integrations/irker_spec.rb @@ -5,11 +5,6 @@ require 'socket' require 'json' RSpec.describe Integrations::Irker do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do @@ -46,7 +41,6 @@ RSpec.describe Integrations::Irker do active: true, project: project, project_id: project.id, - service_hook: true, server_host: @irker_server.addr[2], server_port: @irker_server.addr[1], default_irc_uri: 'irc://chat.freenode.net/', diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index e0a9ce590b8..9eb2a7fc098 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -24,9 +24,9 @@ RSpec.describe Integrations::Jenkins do let(:jenkins_authorization) { "Basic " + ::Base64.strict_encode64(jenkins_username + ':' + jenkins_password) } - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } + it_behaves_like Integrations::HasWebHook do + let(:integration) { described_class.new(jenkins_params) } + let(:hook_url) { "http://#{ERB::Util.url_encode jenkins_username}:#{ERB::Util.url_encode jenkins_password}@jenkins.example.com/project/my_project" } end describe 'username validation' do diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 90a9ef59efc..6ca72d68bbb 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -109,11 +109,6 @@ RSpec.describe Integrations::Jira do end end - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe '.reference_pattern' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/integrations/microsoft_teams_spec.rb b/spec/models/integrations/microsoft_teams_spec.rb index ab2846d8ccb..21b9a005746 100644 --- a/spec/models/integrations/microsoft_teams_spec.rb +++ b/spec/models/integrations/microsoft_teams_spec.rb @@ -6,11 +6,6 @@ RSpec.describe Integrations::MicrosoftTeams do let(:chat_integration) { described_class.new } let(:webhook_url) { 'https://example.gitlab.com/' } - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do @@ -45,7 +40,6 @@ RSpec.describe Integrations::MicrosoftTeams do allow(chat_integration).to receive_messages( project: project, project_id: project.id, - service_hook: true, webhook: webhook_url ) @@ -142,7 +136,6 @@ RSpec.describe Integrations::MicrosoftTeams do allow(chat_integration).to receive_messages( project: project, project_id: project.id, - service_hook: true, webhook: webhook_url ) @@ -224,7 +217,6 @@ RSpec.describe Integrations::MicrosoftTeams do before do allow(chat_integration).to receive_messages( project: project, - service_hook: true, webhook: webhook_url ) end diff --git a/spec/models/integrations/open_project_spec.rb b/spec/models/integrations/open_project_spec.rb index 5084024a8e8..789911acae8 100644 --- a/spec/models/integrations/open_project_spec.rb +++ b/spec/models/integrations/open_project_spec.rb @@ -27,9 +27,4 @@ RSpec.describe Integrations::OpenProject do it { is_expected.not_to validate_presence_of(:project_identifier_code) } end end - - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end end diff --git a/spec/models/integrations/packagist_spec.rb b/spec/models/integrations/packagist_spec.rb index 868ad4f9747..dce96890522 100644 --- a/spec/models/integrations/packagist_spec.rb +++ b/spec/models/integrations/packagist_spec.rb @@ -24,9 +24,9 @@ RSpec.describe Integrations::Packagist do let(:packagist_server) { 'https://packagist.example.com' } let(:project) { create(:project) } - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } + it_behaves_like Integrations::HasWebHook do + let(:integration) { described_class.new(packagist_params) } + let(:hook_url) { "#{packagist_server}/api/update-package?username=#{packagist_username}&apiToken=#{packagist_token}" } end describe '#execute' do diff --git a/spec/models/integrations/pivotaltracker_spec.rb b/spec/models/integrations/pivotaltracker_spec.rb index 8da77f7c8f9..bf8458a376c 100644 --- a/spec/models/integrations/pivotaltracker_spec.rb +++ b/spec/models/integrations/pivotaltracker_spec.rb @@ -5,11 +5,6 @@ require 'spec_helper' RSpec.describe Integrations::Pivotaltracker do include StubRequests - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index 69917df7ddb..f6f242bf58e 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -12,10 +12,6 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, let(:integration) { project.prometheus_integration } - describe "Associations" do - it { is_expected.to belong_to :project } - end - context 'redirects' do it 'does not follow redirects' do redirect_to = 'https://redirected.example.com' diff --git a/spec/models/integrations/pushover_spec.rb b/spec/models/integrations/pushover_spec.rb index 989d7f61011..716a00c5bcf 100644 --- a/spec/models/integrations/pushover_spec.rb +++ b/spec/models/integrations/pushover_spec.rb @@ -5,11 +5,6 @@ require 'spec_helper' RSpec.describe Integrations::Pushover do include StubRequests - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do @@ -51,7 +46,6 @@ RSpec.describe Integrations::Pushover do allow(pushover).to receive_messages( project: project, project_id: project.id, - service_hook: true, api_key: api_key, user_key: user_key, device: device, diff --git a/spec/models/integrations/redmine_spec.rb b/spec/models/integrations/redmine_spec.rb index 1fc4a4970c1..59997d2b6f6 100644 --- a/spec/models/integrations/redmine_spec.rb +++ b/spec/models/integrations/redmine_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Redmine do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do # if redmine is set in setting the urls are set to defaults # therefore the validation passes as the values are not nil diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index 5ed65bf7f46..d425357aef0 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -22,11 +22,6 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do ) end - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/models/integrations/youtrack_spec.rb b/spec/models/integrations/youtrack_spec.rb index 0c4014dffc0..f6a9dd8ef37 100644 --- a/spec/models/integrations/youtrack_spec.rb +++ b/spec/models/integrations/youtrack_spec.rb @@ -3,11 +3,6 @@ require 'spec_helper' RSpec.describe Integrations::Youtrack do - describe 'Associations' do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - describe 'Validations' do context 'when integration is active' do before do diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index ba7acd3d3df..4124696ac08 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -6,4 +6,96 @@ RSpec.describe Auth::ContainerRegistryAuthenticationService do include AdminModeHelper it_behaves_like 'a container registry auth service' + + context 'when in migration mode' do + include_context 'container registry auth service context' + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project) } + + before do + project.add_developer(current_user) + end + + shared_examples 'an unmodified token' do + it_behaves_like 'a valid token' + it { expect(payload['access']).not_to include(have_key('migration_eligible')) } + end + + shared_examples 'a modified token with migration eligibility' do |eligible| + it_behaves_like 'a valid token' + it { expect(payload['access']).to include(include('migration_eligible' => eligible)) } + end + + shared_examples 'a modified token' do + context 'with a non eligible root ancestor and project' do + before do + stub_feature_flags(container_registry_migration_phase1_deny: project.root_ancestor) + stub_feature_flags(container_registry_migration_phase1_allow: false) + end + + it_behaves_like 'a modified token with migration eligibility', false + end + + context 'with a non eligible root ancestor and eligible project' do + before do + stub_feature_flags(container_registry_migration_phase1_deny: false) + stub_feature_flags(container_registry_migration_phase1_deny: project.root_ancestor) + stub_feature_flags(container_registry_migration_phase1_allow: project) + end + + it_behaves_like 'a modified token with migration eligibility', false + end + + context 'with an eligible root ancestor and non eligible project' do + before do + stub_feature_flags(container_registry_migration_phase1_deny: false) + stub_feature_flags(container_registry_migration_phase1_allow: false) + end + + it_behaves_like 'a modified token with migration eligibility', false + end + + context 'with an eligible root ancestor and project' do + before do + stub_feature_flags(container_registry_migration_phase1_deny: false) + stub_feature_flags(container_registry_migration_phase1_allow: project) + end + + it_behaves_like 'a modified token with migration eligibility', true + end + end + + context 'with pull action' do + let(:current_params) do + { scopes: ["repository:#{project.full_path}:pull"] } + end + + it_behaves_like 'an unmodified token' + end + + context 'with push action' do + let(:current_params) do + { scopes: ["repository:#{project.full_path}:push"] } + end + + it_behaves_like 'a modified token' + end + + context 'with multiple actions including push' do + let(:current_params) do + { scopes: ["repository:#{project.full_path}:pull,push,delete"] } + end + + it_behaves_like 'a modified token' + end + + context 'with multiple actions excluding push' do + let(:current_params) do + { scopes: ["repository:#{project.full_path}:pull,delete"] } + end + + it_behaves_like 'an unmodified token' + end + end end diff --git a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb index dd89a67548f..66f9d5f6c33 100644 --- a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb +++ b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb @@ -18,6 +18,8 @@ Integration.available_integration_names.each do |service| hash.merge!(k => 'https://example.atlassian.net/wiki') elsif service == 'datadog' && k == :datadog_site hash.merge!(k => 'datadoghq.com') + elsif service == 'packagist' && k == :server + hash.merge!(k => 'https://packagist.example.com') elsif k =~ /^(.*_url|url|webhook)/ hash.merge!(k => "http://example.com") elsif service_klass.method_defined?("#{k}?") diff --git a/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb b/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb new file mode 100644 index 00000000000..1fa340a0cf4 --- /dev/null +++ b/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +RSpec.shared_examples Integrations::HasWebHook do + include AfterNextHelpers + + describe 'callbacks' do + it 'calls #update_web_hook! when enabled' do + expect(integration).to receive(:update_web_hook!) + + integration.active = true + integration.save! + end + + it 'does not call #update_web_hook! when disabled' do + expect(integration).not_to receive(:update_web_hook!) + + integration.active = false + integration.save! + end + + it 'does not call #update_web_hook! when validation fails' do + expect(integration).not_to receive(:update_web_hook!) + + integration.active = true + integration.project = nil + expect(integration.save).to be(false) + end + end + + describe '#hook_url' do + it 'returns a string' do + expect(integration.hook_url).to be_a(String) + end + end + + describe '#hook_ssl_verification' do + it 'returns a boolean' do + expect(integration.hook_ssl_verification).to be_in([true, false]) + end + end + + describe '#update_web_hook!' do + def call + integration.update_web_hook! + end + + it 'creates or updates a service hook' do + expect { call }.to change(ServiceHook, :count).by(1) + expect(integration.service_hook.url).to eq(hook_url) + + integration.service_hook.update!(url: 'http://other.com') + + expect { call }.to change { integration.service_hook.reload.url }.from('http://other.com').to(hook_url) + end + + it 'raises an error if the service hook could not be saved' do + call + integration.service_hook.integration = nil + + expect { call }.to raise_error(ActiveRecord::RecordInvalid) + end + + it 'does not attempt to save the service hook if there are no changes' do + call + + expect(integration.service_hook).not_to receive(:save!) + + call + end + end + + describe '#execute_web_hook!' do + let(:args) { ['foo', [1, 2, 3]] } + + def call + integration.execute_web_hook!(*args) + end + + it 'creates the webhook if necessary and executes it' do + expect_next(ServiceHook).to receive(:execute).with(*args) + expect { call }.to change(ServiceHook, :count).by(1) + + expect(integration.service_hook).to receive(:execute).with(*args) + expect { call }.not_to change(ServiceHook, :count) + end + + it 'raises an error if the service hook could not be saved' do + expect_next(ServiceHook).to receive(:execute).with(*args) + + call + integration.service_hook.integration = nil + + expect(integration.service_hook).not_to receive(:execute) + expect { call }.to raise_error(ActiveRecord::RecordInvalid) + end + end +end diff --git a/spec/support/shared_examples/services/container_registry_auth_service_shared_examples.rb b/spec/support/shared_examples/services/container_registry_auth_service_shared_examples.rb index d8794ef2693..c98d3768748 100644 --- a/spec/support/shared_examples/services/container_registry_auth_service_shared_examples.rb +++ b/spec/support/shared_examples/services/container_registry_auth_service_shared_examples.rb @@ -157,6 +157,10 @@ end RSpec.shared_examples 'a container registry auth service' do include_context 'container registry auth service context' + before do + stub_feature_flags(container_registry_migration_phase1: false) + end + describe '#full_access_token' do let_it_be(:project) { create(:project) } diff --git a/spec/workers/project_service_worker_spec.rb b/spec/workers/project_service_worker_spec.rb index 9383e7ec5c4..7813d011274 100644 --- a/spec/workers/project_service_worker_spec.rb +++ b/spec/workers/project_service_worker_spec.rb @@ -3,22 +3,24 @@ require 'spec_helper' RSpec.describe ProjectServiceWorker, '#perform' do let(:worker) { described_class.new } - let(:service) { Integrations::Jira.new } + let(:integration) { Integrations::Jira.new } before do - allow(Integration).to receive(:find).and_return(service) + allow(Integration).to receive(:find).and_return(integration) end - it 'executes service with given data' do + it 'executes integration with given data' do data = { test: 'test' } - expect(service).to receive(:execute).with(data) + expect(integration).to receive(:execute).with(data) worker.perform(1, data) end it 'logs error messages' do - allow(service).to receive(:execute).and_raise(StandardError, 'invalid URL') - expect(Sidekiq.logger).to receive(:error).with({ class: described_class.name, service_class: service.class.name, message: "invalid URL" }) + error = StandardError.new('invalid URL') + allow(integration).to receive(:execute).and_raise(error) + + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(error, integration_class: 'Integrations::Jira') worker.perform(1, {}) end |
