diff options
90 files changed, 1340 insertions, 788 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index b978278f05f..372cf402c73 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.43.0 +1.44.0 diff --git a/app/assets/javascripts/autosave.js b/app/assets/javascripts/autosave.js index 0a05e0d44ce..8381dcec9c3 100644 --- a/app/assets/javascripts/autosave.js +++ b/app/assets/javascripts/autosave.js @@ -6,7 +6,7 @@ export default class Autosave { constructor(field, key, fallbackKey, lockVersion) { this.field = field; - this.isLocalStorageAvailable = AccessorUtilities.isLocalStorageAccessSafe(); + this.isLocalStorageAvailable = AccessorUtilities.canUseLocalStorage(); if (key.join != null) { key = key.join('/'); } diff --git a/app/assets/javascripts/behaviors/shortcuts/keybindings.js b/app/assets/javascripts/behaviors/shortcuts/keybindings.js index 005ef103ded..ebf2ab0381e 100644 --- a/app/assets/javascripts/behaviors/shortcuts/keybindings.js +++ b/app/assets/javascripts/behaviors/shortcuts/keybindings.js @@ -19,7 +19,7 @@ export const LOCAL_STORAGE_KEY = 'gl-keyboard-shortcuts-customizations'; */ export const getCustomizations = memoize(() => { let parsedCustomizations = {}; - const localStorageIsSafe = AccessorUtilities.isLocalStorageAccessSafe(); + const localStorageIsSafe = AccessorUtilities.canUseLocalStorage(); if (localStorageIsSafe) { try { diff --git a/app/assets/javascripts/behaviors/shortcuts/shortcuts_toggle.vue b/app/assets/javascripts/behaviors/shortcuts/shortcuts_toggle.vue index 8f1518a1c9c..cf7a71d4206 100644 --- a/app/assets/javascripts/behaviors/shortcuts/shortcuts_toggle.vue +++ b/app/assets/javascripts/behaviors/shortcuts/shortcuts_toggle.vue @@ -13,7 +13,7 @@ export default { }, data() { return { - localStorageUsable: AccessorUtilities.isLocalStorageAccessSafe(), + localStorageUsable: AccessorUtilities.canUseLocalStorage(), shortcutsEnabled: !shouldDisableShortcuts(), }; }, diff --git a/app/assets/javascripts/boards/components/board_list_header.vue b/app/assets/javascripts/boards/components/board_list_header.vue index 8d5f0f7eb89..dc5313b1bf6 100644 --- a/app/assets/javascripts/boards/components/board_list_header.vue +++ b/app/assets/javascripts/boards/components/board_list_header.vue @@ -201,7 +201,7 @@ export default { }); }, addToLocalStorage() { - if (AccessorUtilities.isLocalStorageAccessSafe()) { + if (AccessorUtilities.canUseLocalStorage()) { localStorage.setItem(`${this.uniqueKey}.collapsed`, this.list.collapsed); } }, diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index c2c035963f4..8dcab55ac61 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -218,14 +218,14 @@ export default class Clusters { } setBannerDismissedState(status, isDismissed) { - if (AccessorUtilities.isLocalStorageAccessSafe()) { + if (AccessorUtilities.canUseLocalStorage()) { window.localStorage.setItem(this.clusterBannerDismissedKey, `${status}_${isDismissed}`); } } isBannerDismissed(status) { let bannerState; - if (AccessorUtilities.isLocalStorageAccessSafe()) { + if (AccessorUtilities.canUseLocalStorage()) { bannerState = window.localStorage.getItem(this.clusterBannerDismissedKey); } @@ -233,7 +233,7 @@ export default class Clusters { } setClusterNewlyCreated(state) { - if (AccessorUtilities.isLocalStorageAccessSafe()) { + if (AccessorUtilities.canUseLocalStorage()) { window.localStorage.setItem(this.clusterNewlyCreatedKey, Boolean(state)); } } @@ -242,7 +242,7 @@ export default class Clusters { // once this is true, it will always be true for a given page load if (!this.isNewlyCreated) { let newlyCreated; - if (AccessorUtilities.isLocalStorageAccessSafe()) { + if (AccessorUtilities.canUseLocalStorage()) { newlyCreated = window.localStorage.getItem(this.clusterNewlyCreatedKey); } diff --git a/app/assets/javascripts/emoji/index.js b/app/assets/javascripts/emoji/index.js index dcb9dc72976..7672151af2a 100644 --- a/app/assets/javascripts/emoji/index.js +++ b/app/assets/javascripts/emoji/index.js @@ -11,7 +11,7 @@ export const FALLBACK_EMOJI_KEY = 'grey_question'; export const EMOJI_VERSION = '1'; -const isLocalStorageAvailable = AccessorUtilities.isLocalStorageAccessSafe(); +const isLocalStorageAvailable = AccessorUtilities.canUseLocalStorage(); async function loadEmoji() { if ( diff --git a/app/assets/javascripts/emoji/support/unicode_support_map.js b/app/assets/javascripts/emoji/support/unicode_support_map.js index fe3bc75f9fd..d90a774c293 100644 --- a/app/assets/javascripts/emoji/support/unicode_support_map.js +++ b/app/assets/javascripts/emoji/support/unicode_support_map.js @@ -141,7 +141,7 @@ function generateUnicodeSupportMap(testMap) { } export default function getUnicodeSupportMap() { - const isLocalStorageAvailable = AccessorUtilities.isLocalStorageAccessSafe(); + const isLocalStorageAvailable = AccessorUtilities.canUseLocalStorage(); let glEmojiVersionFromCache; let userAgentFromCache; diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue index 2e27f51b71f..5db8c8cf8d3 100644 --- a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue +++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue @@ -118,7 +118,7 @@ export default { required: true, }, }, - hasLocalStorage: AccessorUtils.isLocalStorageAccessSafe(), + hasLocalStorage: AccessorUtils.canUseLocalStorage(), data() { return { errorSearchQuery: '', diff --git a/app/assets/javascripts/error_tracking/store/list/mutations.js b/app/assets/javascripts/error_tracking/store/list/mutations.js index d92a64947ad..523861363d7 100644 --- a/app/assets/javascripts/error_tracking/store/list/mutations.js +++ b/app/assets/javascripts/error_tracking/store/list/mutations.js @@ -22,7 +22,7 @@ export default { // only keep the last 5 state.recentSearches = recentSearches.slice(0, 5); - if (AccessorUtils.isLocalStorageAccessSafe()) { + if (AccessorUtils.canUseLocalStorage()) { localStorage.setItem( `recent-searches${state.indexPath}`, JSON.stringify(state.recentSearches), @@ -31,7 +31,7 @@ export default { }, [types.CLEAR_RECENT_SEARCHES](state) { state.recentSearches = []; - if (AccessorUtils.isLocalStorageAccessSafe()) { + if (AccessorUtils.canUseLocalStorage()) { localStorage.removeItem(`recent-searches${state.indexPath}`); } }, diff --git a/app/assets/javascripts/filtered_search/services/recent_searches_service.js b/app/assets/javascripts/filtered_search/services/recent_searches_service.js index 56824977a43..c3514198ad9 100644 --- a/app/assets/javascripts/filtered_search/services/recent_searches_service.js +++ b/app/assets/javascripts/filtered_search/services/recent_searches_service.js @@ -33,7 +33,7 @@ class RecentSearchesService { } static isAvailable() { - return AccessorUtilities.isLocalStorageAccessSafe(); + return AccessorUtilities.canUseLocalStorage(); } } diff --git a/app/assets/javascripts/frequent_items/components/app.vue b/app/assets/javascripts/frequent_items/components/app.vue index dd405893e43..8ad9eeaa266 100644 --- a/app/assets/javascripts/frequent_items/components/app.vue +++ b/app/assets/javascripts/frequent_items/components/app.vue @@ -84,7 +84,7 @@ export default { logItemAccess(storageKey, unsanitizedItem) { const item = sanitizeItem(unsanitizedItem); - if (!AccessorUtilities.isLocalStorageAccessSafe()) { + if (!AccessorUtilities.canUseLocalStorage()) { return false; } diff --git a/app/assets/javascripts/frequent_items/store/actions.js b/app/assets/javascripts/frequent_items/store/actions.js index 65a762f54ad..babc2ef2e32 100644 --- a/app/assets/javascripts/frequent_items/store/actions.js +++ b/app/assets/javascripts/frequent_items/store/actions.js @@ -25,7 +25,7 @@ export const receiveFrequentItemsError = ({ commit }) => { export const fetchFrequentItems = ({ state, dispatch }) => { dispatch('requestFrequentItems'); - if (AccessorUtilities.isLocalStorageAccessSafe()) { + if (AccessorUtilities.canUseLocalStorage()) { const storedFrequentItems = JSON.parse(localStorage.getItem(state.storageKey)); dispatch( diff --git a/app/assets/javascripts/jira_connect/subscriptions/utils.js b/app/assets/javascripts/jira_connect/subscriptions/utils.js index ecd1a31339a..ed7a9484a81 100644 --- a/app/assets/javascripts/jira_connect/subscriptions/utils.js +++ b/app/assets/javascripts/jira_connect/subscriptions/utils.js @@ -7,7 +7,7 @@ const isFunction = (fn) => typeof fn === 'function'; * Persist alert data to localStorage. */ export const persistAlert = ({ title, message, linkUrl, variant } = {}) => { - if (!AccessorUtilities.isLocalStorageAccessSafe()) { + if (!AccessorUtilities.canUseLocalStorage()) { return; } @@ -19,7 +19,7 @@ export const persistAlert = ({ title, message, linkUrl, variant } = {}) => { * Return alert data from localStorage. */ export const retrieveAlert = () => { - if (!AccessorUtilities.isLocalStorageAccessSafe()) { + if (!AccessorUtilities.canUseLocalStorage()) { return null; } diff --git a/app/assets/javascripts/lib/utils/accessor.js b/app/assets/javascripts/lib/utils/accessor.js index 39cffedcac6..d4a6d70c62c 100644 --- a/app/assets/javascripts/lib/utils/accessor.js +++ b/app/assets/javascripts/lib/utils/accessor.js @@ -1,4 +1,4 @@ -function isPropertyAccessSafe(base, property) { +function canAccessProperty(base, property) { let safe; try { @@ -10,7 +10,7 @@ function isPropertyAccessSafe(base, property) { return safe; } -function isFunctionCallSafe(base, functionName, ...args) { +function canCallFunction(base, functionName, ...args) { let safe = true; try { @@ -22,16 +22,28 @@ function isFunctionCallSafe(base, functionName, ...args) { return safe; } -function isLocalStorageAccessSafe() { +/** + * Determines if `window.localStorage` is available and + * can be written to and read from. + * + * Important: This is not a guarantee that + * `localStorage.setItem` will work in all cases. + * + * `setItem` can still throw exceptions and should be + * surrounded with a try/catch where used. + * + * See: https://developer.mozilla.org/en-US/docs/Web/API/Storage/setItem#exceptions + */ +function canUseLocalStorage() { let safe; - const TEST_KEY = 'isLocalStorageAccessSafe'; + const TEST_KEY = 'canUseLocalStorage'; const TEST_VALUE = 'true'; - safe = isPropertyAccessSafe(window, 'localStorage'); + safe = canAccessProperty(window, 'localStorage'); if (!safe) return safe; - safe = isFunctionCallSafe(window.localStorage, 'setItem', TEST_KEY, TEST_VALUE); + safe = canCallFunction(window.localStorage, 'setItem', TEST_KEY, TEST_VALUE); if (safe) window.localStorage.removeItem(TEST_KEY); @@ -39,9 +51,7 @@ function isLocalStorageAccessSafe() { } const AccessorUtilities = { - isPropertyAccessSafe, - isFunctionCallSafe, - isLocalStorageAccessSafe, + canUseLocalStorage, }; export default AccessorUtilities; diff --git a/app/assets/javascripts/pages/sessions/new/signin_tabs_memoizer.js b/app/assets/javascripts/pages/sessions/new/signin_tabs_memoizer.js index 1e7c29aefaa..7e646125331 100644 --- a/app/assets/javascripts/pages/sessions/new/signin_tabs_memoizer.js +++ b/app/assets/javascripts/pages/sessions/new/signin_tabs_memoizer.js @@ -8,7 +8,7 @@ export default class SigninTabsMemoizer { constructor({ currentTabKey = 'current_signin_tab', tabSelector = 'ul.new-session-tabs' } = {}) { this.currentTabKey = currentTabKey; this.tabSelector = tabSelector; - this.isLocalStorageAvailable = AccessorUtilities.isLocalStorageAccessSafe(); + this.isLocalStorageAvailable = AccessorUtilities.canUseLocalStorage(); // sets selected tab if given as hash tag if (window.location.hash) { this.saveData(window.location.hash); diff --git a/app/assets/javascripts/project_select_combo_button.js b/app/assets/javascripts/project_select_combo_button.js index 4b14df21f05..fd45d643ecc 100644 --- a/app/assets/javascripts/project_select_combo_button.js +++ b/app/assets/javascripts/project_select_combo_button.js @@ -30,7 +30,7 @@ export default class ProjectSelectComboButton { } initLocalStorage() { - const localStorageIsSafe = AccessorUtilities.isLocalStorageAccessSafe(); + const localStorageIsSafe = AccessorUtilities.canUseLocalStorage(); if (localStorageIsSafe) { this.localStorageKey = [ diff --git a/app/assets/javascripts/protected_branches/protected_branch_create.js b/app/assets/javascripts/protected_branches/protected_branch_create.js index d0d2c1400a7..d4b52860261 100644 --- a/app/assets/javascripts/protected_branches/protected_branch_create.js +++ b/app/assets/javascripts/protected_branches/protected_branch_create.js @@ -12,7 +12,7 @@ export default class ProtectedBranchCreate { this.hasLicense = options.hasLicense; this.$form = $('.js-new-protected-branch'); - this.isLocalStorageAvailable = AccessorUtilities.isLocalStorageAccessSafe(); + this.isLocalStorageAvailable = AccessorUtilities.canUseLocalStorage(); this.currentProjectUserDefaults = {}; this.buildDropdowns(); this.$forcePushToggle = this.$form.find('.js-force-push-toggle'); diff --git a/app/assets/javascripts/search/store/utils.js b/app/assets/javascripts/search/store/utils.js index b7d97213594..b00b9bb0f2e 100644 --- a/app/assets/javascripts/search/store/utils.js +++ b/app/assets/javascripts/search/store/utils.js @@ -6,7 +6,7 @@ function extractKeys(object, keyList) { } export const loadDataFromLS = (key) => { - if (!AccessorUtilities.isLocalStorageAccessSafe()) { + if (!AccessorUtilities.canUseLocalStorage()) { return []; } @@ -20,7 +20,7 @@ export const loadDataFromLS = (key) => { }; export const setFrequentItemToLS = (key, data, itemData) => { - if (!AccessorUtilities.isLocalStorageAccessSafe()) { + if (!AccessorUtilities.canUseLocalStorage()) { return []; } diff --git a/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_utils.js b/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_utils.js index 79054954f1c..5cc96471aef 100644 --- a/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_utils.js +++ b/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_utils.js @@ -219,7 +219,7 @@ export function urlQueryToFilter(query = '', { filteredSearchTermKey, filterName */ export function getRecentlyUsedSuggestions(recentSuggestionsStorageKey) { let recentlyUsedSuggestions = []; - if (AccessorUtilities.isLocalStorageAccessSafe()) { + if (AccessorUtilities.canUseLocalStorage()) { recentlyUsedSuggestions = JSON.parse(localStorage.getItem(recentSuggestionsStorageKey)) || []; } return recentlyUsedSuggestions; @@ -237,7 +237,7 @@ export function setTokenValueToRecentlyUsed(recentSuggestionsStorageKey, tokenVa recentlyUsedSuggestions.splice(0, 0, { ...tokenValue }); - if (AccessorUtilities.isLocalStorageAccessSafe()) { + if (AccessorUtilities.canUseLocalStorage()) { localStorage.setItem( recentSuggestionsStorageKey, JSON.stringify(uniqWith(recentlyUsedSuggestions, isEqual).slice(0, MAX_RECENT_TOKENS_SIZE)), diff --git a/app/assets/stylesheets/startup/startup-dark.scss b/app/assets/stylesheets/startup/startup-dark.scss index feb7469fb62..b7958cdf4a3 100644 --- a/app/assets/stylesheets/startup/startup-dark.scss +++ b/app/assets/stylesheets/startup/startup-dark.scss @@ -1270,7 +1270,7 @@ input { .nav-sidebar-inner-scroll > div.context-header a .avatar-container { font-weight: 400; flex: none; - box-shadow: inset 0 0 0 1px rgba(255, 255, 255, 0.08); + box-shadow: inset 0 0 0 1px rgba(0, 0, 0, 0.08); } .nav-sidebar-inner-scroll > div.context-header a .avatar-container.rect-avatar { border-style: none; @@ -1280,7 +1280,7 @@ input { a .avatar-container.rect-avatar .avatar.s32 { - box-shadow: inset 0 0 0 1px rgba(255, 255, 255, 0.08); + box-shadow: inset 0 0 0 1px rgba(0, 0, 0, 0.08); } .sidebar-top-level-items { margin-top: 0.25rem; @@ -1294,7 +1294,7 @@ input { .sidebar-top-level-items .context-header a .avatar-container { font-weight: 400; flex: none; - box-shadow: inset 0 0 0 1px rgba(255, 255, 255, 0.08); + box-shadow: inset 0 0 0 1px rgba(0, 0, 0, 0.08); } .sidebar-top-level-items .context-header a .avatar-container.rect-avatar { border-style: none; @@ -1304,7 +1304,7 @@ input { a .avatar-container.rect-avatar .avatar.s32 { - box-shadow: inset 0 0 0 1px rgba(255, 255, 255, 0.08); + box-shadow: inset 0 0 0 1px rgba(0, 0, 0, 0.08); } .sidebar-top-level-items > li .badge.badge-pill { border-radius: 0.5rem; @@ -1515,7 +1515,7 @@ svg.s16 { float: left; margin-right: 16px; border-radius: 50%; - border: 1px solid rgba(255, 255, 255, 0.08); + border: 1px solid rgba(0, 0, 0, 0.08); } .avatar.s16, .avatar-container.s16 { diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 02c963e432f..a83458f3260 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -42,6 +42,7 @@ class ApplicationController < ActionController::Base # Make sure the `auth_user` is memoized so it can be logged, we do this after # all other before filters that could have set the user. before_action :auth_user + before_action :limit_session_time, if: -> { !current_user } prepend_around_action :set_current_context @@ -51,7 +52,7 @@ class ApplicationController < ActionController::Base around_action :set_current_admin after_action :set_page_title_header, if: :json_request? - after_action :limit_session_time, if: -> { !current_user } + after_action :ensure_authenticated_session_time, if: -> { current_user } protect_from_forgery with: :exception, prepend: true diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index dd26f8b020f..cf706a8f98e 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -41,7 +41,6 @@ class IssuableFinder include FinderMethods include CreatedAtFilter include Gitlab::Utils::StrongMemoize - prepend OptimizedIssuableLabelFilter requires_cross_project_access unless: -> { params.project? } @@ -149,7 +148,6 @@ class IssuableFinder # Negates all params found in `negatable_params` def filter_negated_items(items) - items = by_negated_label(items) items = by_negated_milestone(items) items = by_negated_release(items) items = by_negated_my_reaction_emoji(items) @@ -172,29 +170,19 @@ class IssuableFinder count_params = params.merge(state: nil, sort: nil, force_cte: true) finder = self.class.new(current_user, count_params) + state_counts = finder + .execute + .reorder(nil) + .group(:state_id) + .count + counts = Hash.new(0) - # Searching by label includes a GROUP BY in the query, but ours will be last - # because it is added last. Searching by multiple labels also includes a row - # per issuable, so we have to count those in Ruby - which is bad, but still - # better than performing multiple queries. - # - # This does not apply when we are using a CTE for the search, as the labels - # GROUP BY is inside the subquery in that case, so we set labels_count to 1. - # - # Groups and projects have separate feature flags to suggest the use - # of a CTE. The CTE will not be used if the sort doesn't support it, - # but will always be used for the counts here as we ignore sorting - # anyway. - labels_count = params.label_names.any? ? params.label_names.count : 1 - labels_count = 1 if use_cte_for_search? - - finder.execute.reorder(nil).group(:state_id).count.each do |key, value| - counts[count_key(key)] += value / labels_count + state_counts.each do |key, value| + counts[count_key(key)] += value end counts[:all] = counts.values.sum - counts.with_indifferent_access end # rubocop: enable CodeReuse/ActiveRecord @@ -360,7 +348,7 @@ class IssuableFinder def sort(items) # Ensure we always have an explicit sort order (instead of inheriting # multiple orders when combining ActiveRecord::Relation objects). - params[:sort] ? items.sort_by_attribute(params[:sort], excluded_labels: params.label_names) : items.reorder(id: :desc) + params[:sort] ? items.sort_by_attribute(params[:sort], excluded_labels: label_filter.label_names_excluded_from_priority_sort) : items.reorder(id: :desc) end # rubocop: enable CodeReuse/ActiveRecord @@ -384,6 +372,20 @@ class IssuableFinder end end + def by_label(items) + label_filter.filter(items) + end + + def label_filter + strong_memoize(:label_filter) do + Issuables::LabelFilter.new( + params: original_params, + project: params.project, + group: params.group + ) + end + end + # rubocop: disable CodeReuse/ActiveRecord def by_milestone(items) return items unless params.milestones? @@ -436,24 +438,6 @@ class IssuableFinder items.without_particular_release(not_params[:release_tag], not_params[:project_id]) end - def by_label(items) - return items unless params.labels? - - if params.filter_by_no_label? - items.without_label - elsif params.filter_by_any_label? - items.any_label(params[:sort]) - else - items.with_label(params.label_names, params[:sort]) - end - end - - def by_negated_label(items) - return items unless not_params.labels? - - items.without_particular_labels(not_params.label_names) - end - def by_my_reaction_emoji(items) return items unless params[:my_reaction_emoji] && current_user diff --git a/app/finders/issuable_finder/params.rb b/app/finders/issuable_finder/params.rb index 595f4e4cf8a..359a56bd39b 100644 --- a/app/finders/issuable_finder/params.rb +++ b/app/finders/issuable_finder/params.rb @@ -29,20 +29,6 @@ class IssuableFinder params.present? end - def filter_by_no_label? - downcased = label_names.map(&:downcase) - - downcased.include?(FILTER_NONE) - end - - def filter_by_any_label? - label_names.map(&:downcase).include?(FILTER_ANY) - end - - def labels? - params[:label_name].present? - end - def milestones? params[:milestone_title].present? || params[:milestone_wildcard_id].present? end @@ -160,24 +146,6 @@ class IssuableFinder end end - def label_names - if labels? - params[:label_name].is_a?(String) ? params[:label_name].split(',') : params[:label_name] - else - [] - end - end - - def labels - strong_memoize(:labels) do - if labels? && !filter_by_no_label? - LabelsFinder.new(current_user, project_ids: projects, title: label_names).execute(skip_authorization: true) # rubocop: disable CodeReuse/Finder - else - Label.none - end - end - end - def milestones strong_memoize(:milestones) do if milestones? diff --git a/app/finders/issuables/label_filter.rb b/app/finders/issuables/label_filter.rb new file mode 100644 index 00000000000..2bbc963aa90 --- /dev/null +++ b/app/finders/issuables/label_filter.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +module Issuables + class LabelFilter < BaseFilter + include Gitlab::Utils::StrongMemoize + extend Gitlab::Cache::RequestCache + + def initialize(project:, group:, **kwargs) + @project = project + @group = group + + super(**kwargs) + end + + def filter(issuables) + filtered = by_label(issuables) + by_negated_label(filtered) + end + + def label_names_excluded_from_priority_sort + label_names_from_params + end + + private + + # rubocop: disable CodeReuse/ActiveRecord + def by_label(issuables) + return issuables unless label_names_from_params.present? + + target_model = issuables.model + + if filter_by_no_label? + issuables.where(label_link_query(target_model).arel.exists.not) + elsif filter_by_any_label? + issuables.where(label_link_query(target_model).arel.exists) + else + issuables_with_selected_labels(issuables, label_names_from_params) + end + end + # rubocop: enable CodeReuse/ActiveRecord + + def by_negated_label(issuables) + return issuables unless label_names_from_not_params.present? + + issuables_without_selected_labels(issuables, label_names_from_not_params) + end + + def filter_by_no_label? + label_names_from_params.map(&:downcase).include?(FILTER_NONE) + end + + def filter_by_any_label? + label_names_from_params.map(&:downcase).include?(FILTER_ANY) + end + + # rubocop: disable CodeReuse/ActiveRecord + def issuables_with_selected_labels(issuables, label_names) + target_model = issuables.model + + if root_namespace + all_label_ids = find_label_ids(label_names) + # Found less labels in the DB than we were searching for. Return nothing. + return issuables.none if all_label_ids.size != label_names.size + + all_label_ids.each do |label_ids| + issuables = issuables.where(label_link_query(target_model, label_ids: label_ids).arel.exists) + end + else + label_names.each do |label_name| + issuables = issuables.where(label_link_query(target_model, label_names: label_name).arel.exists) + end + end + + issuables + end + # rubocop: enable CodeReuse/ActiveRecord + + # rubocop: disable CodeReuse/ActiveRecord + def issuables_without_selected_labels(issuables, label_names) + target_model = issuables.model + + if root_namespace + label_ids = find_label_ids(label_names).flatten(1) + + issuables.where(label_link_query(target_model, label_ids: label_ids).arel.exists.not) + else + issuables.where(label_link_query(target_model, label_names: label_names).arel.exists.not) + end + end + # rubocop: enable CodeReuse/ActiveRecord + + # rubocop: disable CodeReuse/ActiveRecord + def find_label_ids(label_names) + group_labels = Label + .where(project_id: nil) + .where(title: label_names) + .where(group_id: root_namespace.self_and_descendant_ids) + + project_labels = Label + .where(group_id: nil) + .where(title: label_names) + .where(project_id: Project.select(:id).where(namespace_id: root_namespace.self_and_descendant_ids)) + + Label + .from_union([group_labels, project_labels], remove_duplicates: false) + .reorder(nil) + .pluck(:title, :id) + .group_by(&:first) + .values + .map { |labels| labels.map(&:last) } + end + # Avoid repeating label queries times when the finder is instantiated multiple times during the request. + request_cache(:find_label_ids) { root_namespace.id } + # rubocop: enable CodeReuse/ActiveRecord + + # rubocop: disable CodeReuse/ActiveRecord + def label_link_query(target_model, label_ids: nil, label_names: nil) + relation = LabelLink + .where(target_type: target_model.name) + .where(LabelLink.arel_table['target_id'].eq(target_model.arel_table['id'])) + + relation = relation.where(label_id: label_ids) if label_ids + relation = relation.joins(:label).where(labels: { name: label_names }) if label_names + + relation + end + # rubocop: enable CodeReuse/ActiveRecord + + def label_names_from_params + return if params[:label_name].blank? + + strong_memoize(:label_names_from_params) do + split_label_names(params[:label_name]) + end + end + + def label_names_from_not_params + return if not_params.blank? || not_params[:label_name].blank? + + strong_memoize(:label_names_from_not_params) do + split_label_names(not_params[:label_name]) + end + end + + def split_label_names(label_name_param) + label_name_param.is_a?(String) ? label_name_param.split(',') : label_name_param + end + + def root_namespace + strong_memoize(:root_namespace) do + (@project || @group)&.root_ancestor + end + end + end +end diff --git a/app/graphql/types/ci/stage_type.rb b/app/graphql/types/ci/stage_type.rb index 00fbb01b730..c0d931b3d31 100644 --- a/app/graphql/types/ci/stage_type.rb +++ b/app/graphql/types/ci/stage_type.rb @@ -15,10 +15,8 @@ module Types description: 'Group of jobs for the stage.' field :detailed_status, Types::Ci::DetailedStatusType, null: true, description: 'Detailed status of the stage.' - field :jobs, Ci::JobType.connection_type, null: true, - description: 'Jobs for the stage.', - method: 'latest_statuses', - max_page_size: 200 + field :jobs, Types::Ci::JobType.connection_type, null: true, + description: 'Jobs for the stage.' field :status, GraphQL::Types::String, null: true, description: 'Status of the pipeline stage.' @@ -49,6 +47,13 @@ module Types end end + def jobs + GraphQL::Pagination::ActiveRecordRelationConnection.new( + object.latest_statuses, + max_page_size: Gitlab::CurrentSettings.current_application_settings.jobs_per_stage_page_size + ) + end + private # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index 117f662fec6..e9466a9e97e 100644 --- a/app/helpers/sessions_helper.rb +++ b/app/helpers/sessions_helper.rb @@ -22,11 +22,21 @@ module SessionsHelper # creates a new session after login, so the short TTL doesn't even need to # be extended. def limit_session_time + set_session_time(Settings.gitlab['unauthenticated_session_expire_delay']) + end + + def ensure_authenticated_session_time + set_session_time(nil) + end + + def set_session_time(expiry_s) # Rack sets this header, but not all tests may have it: https://github.com/rack/rack/blob/fdcd03a3c5a1c51d1f96fc97f9dfa1a9deac0c77/lib/rack/session/abstract/id.rb#L251-L259 return unless request.env['rack.session.options'] - # This works because Rack uses these options every time a request is handled: - # https://github.com/rack/rack/blob/fdcd03a3c5a1c51d1f96fc97f9dfa1a9deac0c77/lib/rack/session/abstract/id.rb#L342 - request.env['rack.session.options'][:expire_after] = Settings.gitlab['unauthenticated_session_expire_delay'] + # This works because Rack uses these options every time a request is handled, and redis-store + # uses the Rack setting first: + # 1. https://github.com/rack/rack/blob/fdcd03a3c5a1c51d1f96fc97f9dfa1a9deac0c77/lib/rack/session/abstract/id.rb#L342 + # 2. https://github.com/redis-store/redis-store/blob/3acfa95f4eb6260c714fdb00a3d84be8eedc13b2/lib/redis/store/ttl.rb#L32 + request.env['rack.session.options'][:expire_after] = expiry_s end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index ebb4ba39dd6..5f16b990d01 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -207,6 +207,10 @@ class ApplicationSetting < ApplicationRecord numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than: ::Gitlab::Pages::MAX_SIZE / 1.megabyte } + validates :jobs_per_stage_page_size, + presence: true, + numericality: { only_integer: true, greater_than_or_equal_to: 0 } + validates :default_artifacts_expire_in, presence: true, duration: true validates :container_expiration_policies_enable_historic_entries, diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 48acec58dc3..5c307158a9a 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -117,20 +117,6 @@ module Issuable end # rubocop:enable GitlabSecurity/SqlInjection - scope :without_particular_labels, ->(label_names) do - labels_table = Label.arel_table - label_links_table = LabelLink.arel_table - issuables_table = klass.arel_table - inner_query = label_links_table.project('true') - .join(labels_table, Arel::Nodes::InnerJoin).on(labels_table[:id].eq(label_links_table[:label_id])) - .where(label_links_table[:target_type].eq(name) - .and(label_links_table[:target_id].eq(issuables_table[:id])) - .and(labels_table[:title].in(label_names))) - .exists.not - - where(inner_query) - end - scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) } scope :with_label_ids, ->(label_ids) { joins(:label_links).where(label_links: { label_id: label_ids }) } scope :join_project, -> { joins(:project) } diff --git a/app/models/concerns/optimized_issuable_label_filter.rb b/app/models/concerns/optimized_issuable_label_filter.rb deleted file mode 100644 index 19d2ac620f3..00000000000 --- a/app/models/concerns/optimized_issuable_label_filter.rb +++ /dev/null @@ -1,121 +0,0 @@ -# frozen_string_literal: true - -module OptimizedIssuableLabelFilter - extend ActiveSupport::Concern - - prepended do - extend Gitlab::Cache::RequestCache - - # Avoid repeating label queries times when the finder is instantiated multiple times during the request. - request_cache(:find_label_ids) { [root_namespace.id, params.label_names] } - end - - def by_label(items) - return items unless params.labels? - - return super if Feature.disabled?(:optimized_issuable_label_filter, default_enabled: :yaml) - - target_model = items.model - - if params.filter_by_no_label? - items.where('NOT EXISTS (?)', optimized_any_label_query(target_model)) - elsif params.filter_by_any_label? - items.where('EXISTS (?)', optimized_any_label_query(target_model)) - else - issuables_with_selected_labels(items, target_model) - end - end - - # Taken from IssuableFinder - def count_by_state - return super if Feature.disabled?(:optimized_issuable_label_filter, default_enabled: :yaml) - - count_params = params.merge(state: nil, sort: nil, force_cte: true) - finder = self.class.new(current_user, count_params) - - state_counts = finder - .execute - .reorder(nil) - .group(:state_id) - .count - - counts = Hash.new(0) - - state_counts.each do |key, value| - counts[count_key(key)] += value - end - - counts[:all] = counts.values.sum - counts.with_indifferent_access - end - - private - - def issuables_with_selected_labels(items, target_model) - if root_namespace - all_label_ids = find_label_ids - # Found less labels in the DB than we were searching for. Return nothing. - return items.none if all_label_ids.size != params.label_names.size - - all_label_ids.each do |label_ids| - items = items.where('EXISTS (?)', optimized_label_query_by_label_ids(target_model, label_ids)) - end - else - params.label_names.each do |label_name| - items = items.where('EXISTS (?)', optimized_label_query_by_label_name(target_model, label_name)) - end - end - - items - end - - def find_label_ids - group_labels = Label - .where(project_id: nil) - .where(title: params.label_names) - .where(group_id: root_namespace.self_and_descendants.select(:id)) - - project_labels = Label - .where(group_id: nil) - .where(title: params.label_names) - .where(project_id: Project.select(:id).where(namespace_id: root_namespace.self_and_descendants.select(:id))) - - Label - .from_union([group_labels, project_labels], remove_duplicates: false) - .reorder(nil) - .pluck(:title, :id) - .group_by(&:first) - .values - .map { |labels| labels.map(&:last) } - end - - def root_namespace - strong_memoize(:root_namespace) do - (params.project || params.group)&.root_ancestor - end - end - - def optimized_any_label_query(target_model) - LabelLink - .where(target_type: target_model.name) - .where(LabelLink.arel_table['target_id'].eq(target_model.arel_table['id'])) - .limit(1) - end - - def optimized_label_query_by_label_ids(target_model, label_ids) - LabelLink - .where(target_type: target_model.name) - .where(LabelLink.arel_table['target_id'].eq(target_model.arel_table['id'])) - .where(label_id: label_ids) - .limit(1) - end - - def optimized_label_query_by_label_name(target_model, label_name) - LabelLink - .joins(:label) - .where(target_type: target_model.name) - .where(LabelLink.arel_table['target_id'].eq(target_model.arel_table['id'])) - .where(labels: { name: label_name }) - .limit(1) - end -end diff --git a/app/models/concerns/partitioned_table.rb b/app/models/concerns/partitioned_table.rb index eab5d4c35bb..23d2d00b346 100644 --- a/app/models/concerns/partitioned_table.rb +++ b/app/models/concerns/partitioned_table.rb @@ -14,8 +14,6 @@ module PartitionedTable strategy_class = PARTITIONING_STRATEGIES[strategy.to_sym] || raise(ArgumentError, "Unknown partitioning strategy: #{strategy}") @partitioning_strategy = strategy_class.new(self, partitioning_key, **kwargs) - - Gitlab::Database::Partitioning::PartitionManager.register(self) end end end diff --git a/app/models/postgresql/detached_partition.rb b/app/models/postgresql/detached_partition.rb index 76b299ff9d4..12b48895e0c 100644 --- a/app/models/postgresql/detached_partition.rb +++ b/app/models/postgresql/detached_partition.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Postgresql - class DetachedPartition < ApplicationRecord + class DetachedPartition < ::Gitlab::Database::SharedModel scope :ready_to_drop, -> { where('drop_after < ?', Time.current) } end end diff --git a/app/presenters/ci/build_runner_presenter.rb b/app/presenters/ci/build_runner_presenter.rb index 0baee614568..b0066e2d7f0 100644 --- a/app/presenters/ci/build_runner_presenter.rb +++ b/app/presenters/ci/build_runner_presenter.rb @@ -33,7 +33,7 @@ module Ci end def runner_variables - if Feature.enabled?(:variable_inside_variable, project) + if Feature.enabled?(:variable_inside_variable, project, default_enabled: :yaml) variables.sort_and_expand_all(project, keep_undefined: true).to_runner_variables else variables.to_runner_variables diff --git a/app/workers/database/partition_management_worker.rb b/app/workers/database/partition_management_worker.rb index a203c76558a..5a1f139dc29 100644 --- a/app/workers/database/partition_management_worker.rb +++ b/app/workers/database/partition_management_worker.rb @@ -12,7 +12,7 @@ module Database idempotent! def perform - Gitlab::Database::Partitioning::PartitionManager.new.sync_partitions + Gitlab::Database::Partitioning.sync_partitions ensure Gitlab::Database::Partitioning::PartitionMonitoring.new.report_metrics end diff --git a/config/feature_flags/development/optimized_issuable_label_filter.yml b/config/feature_flags/development/optimized_issuable_label_filter.yml deleted file mode 100644 index cc4e8aa2fa9..00000000000 --- a/config/feature_flags/development/optimized_issuable_label_filter.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: optimized_issuable_label_filter -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34503 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/259719 -milestone: '13.4' -type: development -group: group::optimize -default_enabled: true diff --git a/config/feature_flags/development/variable_inside_variable.yml b/config/feature_flags/development/variable_inside_variable.yml index 2060958590f..fee4897b3f0 100644 --- a/config/feature_flags/development/variable_inside_variable.yml +++ b/config/feature_flags/development/variable_inside_variable.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/297382 milestone: '13.11' type: development group: group::runner -default_enabled: false +default_enabled: true diff --git a/config/initializers/postgres_partitioning.rb b/config/initializers/postgres_partitioning.rb index 67f155a6861..f2e2fba1559 100644 --- a/config/initializers/postgres_partitioning.rb +++ b/config/initializers/postgres_partitioning.rb @@ -1,19 +1,20 @@ # frozen_string_literal: true -# Make sure we have loaded partitioned models here -# (even with eager loading disabled). - -Gitlab::Database::Partitioning::PartitionManager.register(AuditEvent) -Gitlab::Database::Partitioning::PartitionManager.register(WebHookLog) -Gitlab::Database::Partitioning::PartitionManager.register(LooseForeignKeys::DeletedRecord) +Gitlab::Database::Partitioning.register_models([ + AuditEvent, + WebHookLog, + LooseForeignKeys::DeletedRecord +]) if Gitlab.ee? - Gitlab::Database::Partitioning::PartitionManager.register(IncidentManagement::PendingEscalations::Alert) - Gitlab::Database::Partitioning::PartitionManager.register(IncidentManagement::PendingEscalations::Issue) + Gitlab::Database::Partitioning.register_models([ + IncidentManagement::PendingEscalations::Alert, + IncidentManagement::PendingEscalations::Issue + ]) end begin - Gitlab::Database::Partitioning::PartitionManager.new.sync_partitions unless ENV['DISABLE_POSTGRES_PARTITION_CREATION_ON_STARTUP'] + Gitlab::Database::Partitioning.sync_partitions unless ENV['DISABLE_POSTGRES_PARTITION_CREATION_ON_STARTUP'] rescue ActiveRecord::ActiveRecordError, PG::Error # ignore - happens when Rake tasks yet have to create a database, e.g. for testing end diff --git a/db/migrate/20210908100810_add_jobs_per_stage_page_size_to_application_settings.rb b/db/migrate/20210908100810_add_jobs_per_stage_page_size_to_application_settings.rb new file mode 100644 index 00000000000..81a76ecb10a --- /dev/null +++ b/db/migrate/20210908100810_add_jobs_per_stage_page_size_to_application_settings.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddJobsPerStagePageSizeToApplicationSettings < Gitlab::Database::Migration[1.0] + def change + add_column :application_settings, :jobs_per_stage_page_size, :integer, default: 200, null: false + end +end diff --git a/db/schema_migrations/20210908100810 b/db/schema_migrations/20210908100810 new file mode 100644 index 00000000000..a8c9023a1cc --- /dev/null +++ b/db/schema_migrations/20210908100810 @@ -0,0 +1 @@ +06e45cf159cf1182b34e83f893bcff65e4722dded2bf4cbcf61fafd652158823
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 534a2bcef8d..de719052be4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10371,6 +10371,7 @@ CREATE TABLE application_settings ( throttle_unauthenticated_api_enabled boolean DEFAULT false NOT NULL, throttle_unauthenticated_api_requests_per_period integer DEFAULT 3600 NOT NULL, throttle_unauthenticated_api_period_in_seconds integer DEFAULT 3600 NOT NULL, + jobs_per_stage_page_size integer DEFAULT 200 NOT NULL, sidekiq_job_limiter_mode smallint DEFAULT 1 NOT NULL, sidekiq_job_limiter_compression_threshold_bytes integer DEFAULT 100000 NOT NULL, sidekiq_job_limiter_limit_bytes integer DEFAULT 0 NOT NULL, diff --git a/doc/administration/index.md b/doc/administration/index.md index d3e018c9769..9412994edb7 100644 --- a/doc/administration/index.md +++ b/doc/administration/index.md @@ -111,7 +111,7 @@ Learn how to install, configure, update, and maintain your GitLab instance. ### GitLab platform integrations -- [Mattermost](https://docs.gitlab.com/omnibus/gitlab-mattermost/): Integrate with [Mattermost](https://mattermost.com), an open source, private cloud workplace for web messaging. +- [Mattermost](../integration/mattermost/index.md): Integrate with [Mattermost](https://mattermost.com), an open source, private cloud workplace for web messaging. - [PlantUML](integration/plantuml.md): Create diagrams in AsciiDoc and Markdown documents created in snippets, wikis, and repositories. - [Web terminals](integration/terminal.md): Provide terminal access to your applications deployed to Kubernetes from GitLab CI/CD [environments](../ci/environments/index.md#web-terminals). diff --git a/doc/api/packages/nuget.md b/doc/api/packages/nuget.md index 9d73beac452..aee3a4fe542 100644 --- a/doc/api/packages/nuget.md +++ b/doc/api/packages/nuget.md @@ -73,7 +73,7 @@ curl --user <username>:<personal_access_token> "https://gitlab.example.com/api/v Write the output to a file: ```shell -curl --user <username>:<personal_access_token> "https://gitlab.example.com/api/v4/projects/1/packages/nuget/download/MyNuGetPkg/1.3.0.17/mynugetpkg.1.3.0.17.nupkg" >> MyNuGetPkg.1.3.0.17.nupkg +curl --user <username>:<personal_access_token> "https://gitlab.example.com/api/v4/projects/1/packages/nuget/download/MyNuGetPkg/1.3.0.17/mynugetpkg.1.3.0.17.nupkg" > MyNuGetPkg.1.3.0.17.nupkg ``` This writes the downloaded file to `MyNuGetPkg.1.3.0.17.nupkg` in the current directory. diff --git a/doc/ci/variables/where_variables_can_be_used.md b/doc/ci/variables/where_variables_can_be_used.md index c15e4c123dc..551033654f2 100644 --- a/doc/ci/variables/where_variables_can_be_used.md +++ b/doc/ci/variables/where_variables_can_be_used.md @@ -63,11 +63,12 @@ because the expansion is done in GitLab before any runner gets the job. #### Nested variable expansion -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48627) in GitLab 13.10. -> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default. -> - It can be enabled or disabled for a single project. -> - It's disabled on GitLab.com. -> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enabling-the-nested-variable-expansion-feature). **(FREE SELF)** +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48627) in GitLab 13.10. [Deployed behind the `variable_inside_variable` feature flag](../../user/feature_flags.md), disabled by default. +> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.3. +> - [Enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.3. + +FLAG: +On self-managed GitLab, by default this feature is available. To hide the feature per project or for your entire instance, ask an administrator to [disable the `variable_inside_variable` flag](../../administration/feature_flags.md). GitLab expands job variable values recursively before sending them to the runner. For example: @@ -86,29 +87,6 @@ References to unavailable variables are left intact. In this case, the runner [attempts to expand the variable value](#gitlab-runner-internal-variable-expansion-mechanism) at runtime. For example, a variable like `CI_BUILDS_DIR` is known by the runner only at runtime. -##### Enabling the nested variable expansion feature **(FREE SELF)** - -This feature comes with the `:variable_inside_variable` feature flag disabled by default. - -To enable this feature, ask a GitLab administrator with [Rails console access](../../administration/feature_flags.md#how-to-enable-and-disable-features-behind-flags) to run the -following command: - -```ruby -# For the instance -Feature.enable(:variable_inside_variable) -# For a single project -Feature.enable(:variable_inside_variable, Project.find(<project id>)) -``` - -To disable it: - -```ruby -# For the instance -Feature.disable(:variable_inside_variable) -# For a single project -Feature.disable(:variable_inside_variable, Project.find(<project id>)) -``` - ### GitLab Runner internal variable expansion mechanism - Supported: project/group variables, `.gitlab-ci.yml` variables, `config.toml` variables, and diff --git a/doc/development/architecture.md b/doc/development/architecture.md index 2b64dcbe5c1..fe2b621da29 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -610,7 +610,7 @@ to make sure we were logging responsibly. This is just a packaged version of the - [Project page](https://github.com/mattermost/mattermost-server/blob/master/README.md) - Configuration: - - [Omnibus](https://docs.gitlab.com/omnibus/gitlab-mattermost/) + - [Omnibus](../integration/mattermost/index.md) - [Charts](https://docs.mattermost.com/install/install-mmte-helm-gitlab-helm.html) - Layer: Core Service (Processor) - GitLab.com: [Mattermost](../user/project/integrations/mattermost.md) diff --git a/doc/integration/mattermost/gitlab-mattermost.msc b/doc/integration/mattermost/gitlab-mattermost.msc new file mode 100644 index 00000000000..f6d4bf7aa68 --- /dev/null +++ b/doc/integration/mattermost/gitlab-mattermost.msc @@ -0,0 +1,28 @@ +msc { + # Use https://mscgen.js.org or mscgen to convert this into PNG + hscale="1.5", + wordwraparcs=on; + + user [ label="User", textbgcolor="blue", textcolor="white" ], + mattermost [ label="Mattermost", textbgcolor="red", textcolor="white"], + gitlab [ label="GitLab", textbgcolor="indigo", textcolor="white"]; + + user=>mattermost [label="GET https://mm.domain.com"]; + mattermost note gitlab [label="Obtain access code", textcolor="green"]; + mattermost=>gitlab [label="GET https://gitlab.domain.com/oauth/authorize", textcolor="indigo"]; + gitlab rbox user [label="GitLab user logs in (if necessary)"]; + gitlab rbox gitlab [label="GitLab verifies client_id matches an OAuth application"]; + gitlab=>user [label="GitLab asks user to authorize Mattermost OAuth app"]; + user=>gitlab [label="User clicks 'Allow'"]; + gitlab rbox gitlab [label="GitLab verifies redirect_uri matches list of valid URLs"]; + gitlab=>user [label="302 Redirect: https://mm.domain.com/signup/gitlab/complete"]; + user=>mattermost [label="GET https://mm.domain.com/signup/gitlab/complete", textcolor="red"]; + mattermost note gitlab [label="Exchange access code for access token", textcolor="green"]; + mattermost=>gitlab [label="POST http://gitlab.domain.com/oauth/token", textcolor="indigo"]; + gitlab=>gitlab [label="Doorkeeper::TokensController#create"]; + gitlab=>mattermost [label="Access token", textcolor="red"]; + mattermost note gitlab [label="Mattermost looks up GitLab user", textcolor="green"]; + mattermost=>gitlab [label="GET https://gitlab.domain.com/api/v4/user", textcolor="indigo"]; + gitlab=>mattermost [label="User details", textcolor="red"]; + mattermost=>user [label="Mattermost/GitLab user ready"]; +} diff --git a/doc/integration/mattermost/img/gitlab-mattermost.png b/doc/integration/mattermost/img/gitlab-mattermost.png Binary files differnew file mode 100644 index 00000000000..1eedcb391b0 --- /dev/null +++ b/doc/integration/mattermost/img/gitlab-mattermost.png diff --git a/doc/integration/mattermost/index.md b/doc/integration/mattermost/index.md new file mode 100644 index 00000000000..4830f8ba84e --- /dev/null +++ b/doc/integration/mattermost/index.md @@ -0,0 +1,495 @@ +--- +stage: Enablement +group: Distribution +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/#designated-technical-writers +--- + +# GitLab Mattermost + +NOTE: +This document applies to GitLab 11.0 and later. + +You can run a [GitLab Mattermost](https://gitlab.com/gitlab-org/gitlab-mattermost) +service on your GitLab server. Mattermost is not part of the single application that GitLab is. There is a good integration between with GitLab, and our Omnibus installer allows you to easily install it. But it is a separate application from a separate company. + +## Prerequisites + +Each release of GitLab Mattermost is compiled and manually tested on an AMD 64 chipset for Linux. ARM chipsets and operating systems, like Raspberry Pi, are not supported. + +## Getting started + +GitLab Mattermost expects to run on its own virtual host. In your DNS settings you will need +two entries pointing to the same machine, e.g., `gitlab.example.com` and +`mattermost.example.com`. + +GitLab Mattermost is disabled by default. To enable it: + +1. Edit `/etc/gitlab/gitlab.rb` and add the Mattermost external URL: + + ```ruby + mattermost_external_url 'https://mattermost.example.com' + ``` + +1. Reconfigure GitLab: + + ```shell + sudo gitlab-ctl reconfigure + ``` + +1. Confirm that GitLab Mattermost is reachable at `https://mattermost.example.com` and authorized to connect to GitLab. Authorizing Mattermost with GitLab allows users to use GitLab as an SSO provider. + +The Omnibus GitLab package attempts to automatically authorize GitLab Mattermost with GitLab if the applications are running on the same server. + +Automatic authorization requires access to the GitLab database. If the GitLab database is not available +you will need to manually authorize GitLab Mattermost for access to GitLab using the process described in the [Authorize GitLab Mattermost section](#authorize-gitlab-mattermost). + +## Configuring Mattermost + +Starting in GitLab 11.0, Mattermost can be configured using the Mattermost System Console. An extensive list of +Mattermost settings and where they can be set is available [in the Mattermost documentation](https://docs.mattermost.com/administration/config-settings.html). + +While using the System Console is recommended, you can also configure Mattermost using one of the following options: + +1. Edit the Mattermost configuration directly through `/var/opt/gitlab/mattermost/config.json`. +1. Specify environment variables used to run Mattermost by changing the `mattermost['env']` setting in `gitlab.rb`. Any settings configured in this way will be disabled from the System Console and cannot be changed without restarting Mattermost. + +## Running GitLab Mattermost with HTTPS + +Place the SSL certificate and SSL certificate key inside `/etc/gitlab/ssl`. If the directory doesn't exist, create it: + +```shell +sudo mkdir -p /etc/gitlab/ssl +sudo chmod 755 /etc/gitlab/ssl +sudo cp mattermost.gitlab.example.key mattermost.gitlab.example.crt /etc/gitlab/ssl/ +``` + +In `/etc/gitlab/gitlab.rb` specify the following configuration: + +```ruby +mattermost_external_url 'https://mattermost.gitlab.example' +mattermost_nginx['redirect_http_to_https'] = true +``` + +If you haven't named your certificate and key `mattermost.gitlab.example.crt` +and `mattermost.gitlab.example.key` then you'll need to also add the full paths +as shown below. + +```ruby +mattermost_nginx['ssl_certificate'] = "/etc/gitlab/ssl/mattermost-nginx.crt" +mattermost_nginx['ssl_certificate_key'] = "/etc/gitlab/ssl/mattermost-nginx.key" +``` + +where `mattermost-nginx.crt` and `mattermost-nginx.key` are SSL cert and key, respectively. + +Once the configuration is set, run `sudo gitlab-ctl reconfigure` to apply the changes. + +## Running GitLab Mattermost on its own server + +If you want to run GitLab and GitLab Mattermost on two separate servers the GitLab services will still be set up on your GitLab Mattermost server, but they will not accept user requests or +consume system resources. You can use the following settings and configuration details on the GitLab Mattermost server to effectively disable the GitLab service bundled into the Omnibus package. + +```ruby +mattermost_external_url 'http://mattermost.example.com' + +# Shut down GitLab services on the Mattermost server +gitlab_rails['enable'] = false +redis['enable'] = false +postgres_exporter['enable'] = false +``` + +Then follow the appropriate steps in the [Authorize GitLab Mattermost section](#authorize-gitlab-mattermost). Last, to enable +integrations with GitLab add the following on the GitLab Server: + +```ruby +gitlab_rails['mattermost_host'] = "https://mattermost.example.com" +``` + +By default GitLab Mattermost requires all users to sign up with GitLab and disables the sign-up by email option. See Mattermost [documentation on GitLab SSO](https://docs.mattermost.com/deployment/sso-gitlab.html). + +## Manually (re)authorizing GitLab Mattermost with GitLab + +### Reauthorize GitLab Mattermost + +To reauthorize GitLab Mattermost, you first need to revoke the existing +authorization. This can be done in the **Settings > Applications** area of GitLab. Then follow the steps below to complete authorization. + +### Authorize GitLab Mattermost + +Navigate to the **Settings > Applications** area in GitLab. Create a new application and for the **Redirect URI** use the following (replace `http` with `https` if you use HTTPS): + +```plaintext +http://mattermost.example.com/signup/gitlab/complete +http://mattermost.example.com/login/gitlab/complete +``` + +Note that you do not need to select any options under **Scopes**. Choose **Save application**. + +Once the application is created you will be provided with an `Application ID` and `Secret`. One other piece of information needed is the URL of GitLab instance. +Return to the server running GitLab Mattermost and edit the `/etc/gitlab/gitlab.rb` configuration file as follows using the values you received above: + +```ruby +mattermost['gitlab_enable'] = true +mattermost['gitlab_id'] = "12345656" +mattermost['gitlab_secret'] = "123456789" +mattermost['gitlab_scope'] = "" +mattermost['gitlab_auth_endpoint'] = "http://gitlab.example.com/oauth/authorize" +mattermost['gitlab_token_endpoint'] = "http://gitlab.example.com/oauth/token" +mattermost['gitlab_user_api_endpoint'] = "http://gitlab.example.com/api/v4/user" +``` + +Save the changes and then run `sudo gitlab-ctl reconfigure`. If there are no errors your GitLab and GitLab Mattermost should be configured correctly. + +### Specify numeric user and group identifiers + +Omnibus GitLab creates a user and group `mattermost`. You can specify the +numeric identifiers for these users in `/etc/gitlab/gitlab.rb` as follows: + +```ruby +mattermost['uid'] = 1234 +mattermost['gid'] = 1234 +``` + +Run `sudo gitlab-ctl reconfigure` to apply the changes. + +### Setting custom environment variables + +If necessary you can set custom environment variables to be used by Mattermost +via `/etc/gitlab/gitlab.rb`. This can be useful if the Mattermost server +is operated behind a corporate internet proxy. In `/etc/gitlab/gitlab.rb` +supply a `mattermost['env']` with a hash value. For example: + +```ruby +mattermost['env'] = {"HTTP_PROXY" => "my_proxy", "HTTPS_PROXY" => "my_proxy", "NO_PROXY" => "my_no_proxy"} +``` + +Run `sudo gitlab-ctl reconfigure` to apply the changes. + +### Connecting to the bundled PostgreSQL database + +If you need to connect to the bundled PostgreSQL database and are using the default Omnibus GitLab database configuration, you can connect as +the PostgreSQL superuser: + +```shell +sudo gitlab-psql -d mattermost_production +``` + +### Back up GitLab Mattermost + +GitLab Mattermost is not included in the regular [Omnibus GitLab backup](../../raketasks/backup_restore.md#back-up-gitlab) Rake task. + +The general Mattermost [backup and disaster recovery](https://docs.mattermost.com/deploy/backup-disaster-recovery.html) documentation can be used as a guide +on what needs to be backed up. + +#### Back up the bundled PostgreSQL database + +If you need to back up the bundled PostgreSQL database and are using the default Omnibus GitLab database configuration, you can back up using this command: + +```shell +sudo -i -u gitlab-psql -- /opt/gitlab/embedded/bin/pg_dump -h /var/opt/gitlab/postgresql mattermost_production | gzip > mattermost_dbdump_$(date --rfc-3339=date).sql.gz +``` + +#### Back up the `data` directory and `config.json` + +Mattermost has a `data` directory and `config.json` file that will need to be backed up as well: + +```shell +sudo tar -zcvf mattermost_data_$(date --rfc-3339=date).gz -C /var/opt/gitlab/mattermost data config.json +``` + +### Restore GitLab Mattermost + +If you have previously [created a backup of GitLab Mattermost](#back-up-gitlab-mattermost), you can run the following commands to restore it: + +```shell +# Stop Mattermost so we don't have any open database connections +sudo gitlab-ctl stop mattermost + +# Drop the Mattermost database +sudo -u gitlab-psql /opt/gitlab/embedded/bin/dropdb -U gitlab-psql -h /var/opt/gitlab/postgresql -p 5432 mattermost_production + +# Create the Mattermost database +sudo -u gitlab-psql /opt/gitlab/embedded/bin/createdb -U gitlab-psql -h /var/opt/gitlab/postgresql -p 5432 mattermost_production + +# Perform the database restore +# Replace /tmp/mattermost_dbdump_2021-08-05.sql.gz with your backup +sudo -u mattermost sh -c "zcat /tmp/mattermost_dbdump_2021-08-05.sql.gz | /opt/gitlab/embedded/bin/psql -U gitlab_mattermost -h /var/opt/gitlab/postgresql -p 5432 mattermost_production" + +# Restore the data directory and config.json +# Replace /tmp/mattermost_data_2021-08-09.gz with your backup +sudo tar -xzvf /tmp/mattermost_data_2021-08-09.gz -C /var/opt/gitlab/mattermost + +# Fix permissions if required +sudo chown -R mattermost:mattermost /var/opt/gitlab/mattermost/data +sudo chown mattermost:mattermost /var/opt/gitlab/mattermost/config.json + +# Start Mattermost +sudo gitlab-ctl start mattermost +``` + +### Mattermost Command Line Tools (CLI) + +NOTE: +This CLI will be replaced in a future release with the new [`mmctl` Command Line Tool](https://docs.mattermost.com/administration/mmctl-cli-tool.html). + +To use the [Mattermost Command Line Tools (CLI)](https://docs.mattermost.com/administration/command-line-tools.html), ensure that you are in the `/opt/gitlab/embedded/service/mattermost` directory when you run the CLI commands and that you specify the location of the configuration file. The executable is `/opt/gitlab/embedded/bin/mattermost`. + +```shell +cd /opt/gitlab/embedded/service/mattermost + +sudo /opt/gitlab/embedded/bin/chpst -e /opt/gitlab/etc/mattermost/env -P -U mattermost:mattermost -u mattermost:mattermost /opt/gitlab/embedded/bin/mattermost --config=/var/opt/gitlab/mattermost/config.json version +``` + +Until [#4745](https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/4745) has been implemented, the command requires quite of bit typing and is hard to remember, so let's make a bash/zsh alias to make it a bit easier to remember. Add the following to your `~/.bashrc` or `~/.zshrc` file: + +```shell +alias mattermost-cli="cd /opt/gitlab/embedded/service/mattermost && sudo /opt/gitlab/embedded/bin/chpst -e /opt/gitlab/etc/mattermost/env -P -U mattermost:mattermost -u mattermost:mattermost /opt/gitlab/embedded/bin/mattermost --config=/var/opt/gitlab/mattermost/config.json $1" +``` + +Then source `~/.zshrc` or `~/.bashrc` with `source ~/.zshrc` or `source ~/.bashrc`. + +If successful, you can now run any Mattermost CLI command with your new shell alias `mattermost-cli`: + +```shell +$ mattermost-cli version + +[sudo] password for username: +{"level":"info","ts":1569614421.9058893,"caller":"utils/i18n.go:83","msg":"Loaded system translations for 'en' from '/opt/gitlab/embedded/service/mattermost/i18n/en.json'"} +{"level":"info","ts":1569614421.9062793,"caller":"app/server_app_adapters.go:58","msg":"Server is initializing..."} +{"level":"info","ts":1569614421.90976,"caller":"sqlstore/supplier.go:223","msg":"Pinging SQL master database"} +{"level":"info","ts":1569614422.0515099,"caller":"mlog/log.go:165","msg":"Starting up plugins"} +{"level":"info","ts":1569614422.0515954,"caller":"app/plugin.go:193","msg":"Syncing plugins from the file store"} +{"level":"info","ts":1569614422.086005,"caller":"app/plugin.go:228","msg":"Found no files in plugins file store"} +{"level":"info","ts":1569614423.9337213,"caller":"sqlstore/post_store.go:1301","msg":"Post.Message supports at most 16383 characters (65535 bytes)"} +{"level":"error","ts":1569614425.6317747,"caller":"go-plugin/stream.go:15","msg":" call to OnConfigurationChange failed, error: Must have a GitLab oauth client id","plugin_id":"com.github.manland.mattermost-plugin-gitlab","source":"plugin_stderr"} +{"level":"info","ts":1569614425.6875598,"caller":"mlog/sugar.go:19","msg":"Ensuring Surveybot exists","plugin_id":"com.mattermost.nps"} +{"level":"info","ts":1569614425.6953356,"caller":"app/server.go:216","msg":"Current version is 5.14.0 (5.14.2/Fri Aug 30 20:20:48 UTC 2019/817ee89711bf26d33f840ce7f59fba14da1ed168/none)"} +{"level":"info","ts":1569614425.6953766,"caller":"app/server.go:217","msg":"Enterprise Enabled: false"} +{"level":"info","ts":1569614425.6954057,"caller":"app/server.go:219","msg":"Current working directory is /opt/gitlab/embedded/service/mattermost/i18n"} +{"level":"info","ts":1569614425.6954265,"caller":"app/server.go:220","msg":"Loaded config","source":"file:///var/opt/gitlab/mattermost/config.json"} +Version: 5.14.0 +Build Number: 5.14.2 +Build Date: Fri Aug 30 20:20:48 UTC 2019 +Build Hash: 817ee89711bf26d33f840ce7f59fba14da1ed168 +Build Enterprise Ready: false +DB Version: 5.14.0 +``` + +For more details see [Mattermost Command Line Tools (CLI)](https://docs.mattermost.com/administration/command-line-tools.html) and the [Troubleshooting Mattermost CLI](#troubleshooting-the-mattermost-cli) below. + +## Configuring GitLab and Mattermost integrations + +As of 12.3, the Mattermost GitLab plugin is shipped with Omnibus GitLab: [Mattermost Plugin for GitLab documentation](https://github.com/mattermost/mattermost-plugin-gitlab). + +You can use the plugin to subscribe Mattermost to receive notifications about issues, merge requests, and pull requests as well as personal notifications regarding merge request reviews, unread messages, and task assignments. If you want to use slash commands to perform actions +such as creating and viewing issues, or to trigger deployments use GitLab [Mattermost slash commands](../../user/project/integrations/mattermost_slash_commands.md). + +The plugin and slash commands can be used together or individually. + +## Email Notifications + +### Setting up SMTP for GitLab Mattermost + +These settings are configured through the Mattermost System Console by the System Administrator. +On the **Environment > SMTP** tab of the **System Console**, you can enter the SMTP credentials given by your SMTP provider, or `127.0.0.1` and port `25` to use `sendmail`. More information on the specific settings +that are needed is available in the [Mattermost documentation](https://docs.mattermost.com/install/smtp-email-setup.html). + +These settings can also be configured in `/var/opt/gitlab/mattermost/config.json`. + +### Email batching + +Enabling this feature allows users to control how often they receive email notifications. + +Email batching can be enabled in the Mattermost **System Console** by going to the **Environment > SMTP** tab, and setting the **Enable Email Batching** setting to **True**. + +This setting can also be configured in `/var/opt/gitlab/mattermost/config.json`. + +## Upgrading GitLab Mattermost + +Below is a list of Mattermost versions for GitLab 11.10 and later: + +| GitLab Version | Mattermost Version | +| :------------ |:----------------| +| 11.11 | 5.10 | +| 12.0 | 5.11 | +| 12.1 | 5.12 | +| 12.2 | 5.13 | +| 12.3 | 5.14 | +| 12.4 | 5.15 | +| 12.5 | 5.16 | +| 12.6 | 5.17 | +| 12.7 | 5.17 | +| 12.8 | 5.19 | +| 12.9 | 5.20 | +| 12.10 | 5.21 | +| 13.0 | 5.22 | +| 13.1 | 5.23 | +| 13.2 | 5.24 | +| 13.3 | 5.25 | +| 13.4 | 5.26 | +| 13.5 | 5.27 | +| 13.6 | 5.28 | +| 13.7 | 5.29 | +| 13.8 | 5.30 | +| 13.9 | 5.31 | +| 13.10 | 5.32 | +| 13.11 | 5.33 | +| 13.12 | 5.34 | +| 14.0 | 5.35 | +| 14.1 | 5.36 | +| 14.2 | 5.37 | +| 14.3 | 5.38 | + +NOTE: +When upgrading the Mattermost version, it is essential to check the +[Important Upgrade Notes](https://docs.mattermost.com/administration/important-upgrade-notes.html) +for Mattermost to address any changes or migrations that need to be performed. + +Starting with GitLab 11.0, GitLab Mattermost can be upgraded through the regular Omnibus GitLab update process. When upgrading previous versions of +GitLab that process can only be used if Mattermost configuration settings have not been changed outside of GitLab (i.e., no changes to Mattermost's `config.json` +file have been made, either directly or via the Mattermost **System Console** which saves back changes to `config.json`.) + +If you are upgrading to at least GitLab 11.0 or have only configured Mattermost using `gitlab.rb`, you can upgrade GitLab using Omnibus and then run `gitlab-ctl reconfigure` to upgrade GitLab Mattermost to the latest version. + +If this is not the case, there are two options: + +1. Update [`gitlab.rb`](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/files/gitlab-config-template/gitlab.rb.template#L706) + with the changes done to `config.json`. This might require adding some parameters as not all + settings in `config.json` are available in `gitlab.rb`. Once complete, Omnibus GitLab should be + able to upgrade GitLab Mattermost from one version to the next. +1. Migrate Mattermost outside of the directory controlled by Omnibus GitLab so it can be administered + and upgraded independently. Follow the [Mattermost Migration Guide](https://docs.mattermost.com/administration/migrating.html) + to move your Mattermost configuration settings and data to another directory or server independent + from Omnibus GitLab. + +For a complete list of upgrade notices and special considerations for older versions, see the [Mattermost documentation](https://docs.mattermost.com/administration/important-upgrade-notes.html). + +## Upgrading GitLab Mattermost from versions prior to 11.0 + +With version 11.0, GitLab introduced breaking changes which affected Mattermost configuration. +In versions prior to GitLab 11.0 all +Mattermost-related settings were configurable from the `gitlab.rb` file, which +generated the Mattermost `config.json` file. However, Mattermost also +permitted configuration via its System Console. This configuration ended up in +the same `config.json` file, which resulted in changes made via the System Console being +overwritten when users ran `gitlab-ctl reconfigure`. + +To resolve this problem, `gitlab.rb` includes only the +configuration necessary for GitLab<=>Mattermost integration in 11.0. GitLab no longer +generates the `config.json` file, and instead passes limited configuration settings via environment variables. + +The settings that continue to be supported in `gitlab.rb` can be found in +[`gitlab.rb.template`](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/files/gitlab-config-template/gitlab.rb.template). + +From GitLab 11.0, other Mattermost settings can be configured through Mattermost's System Console, +by editing `/var/opt/gitlab/mattermost/config.json`, or by using `mattermost['env']` in `gitlab.rb`. + +If you would like to keep configuring Mattermost using `gitlab.rb`, you can take the following actions +in preparation for GitLab 11.0: + +1. Upgrade to version 10.x which supports the new `mattermost['env']` setting. +1. Configure any settings not listed above through the `mattermost['env']` setting. Mattermost requires + environment variables to be provided in `MM_<CATEGORY>SETTINGS_<ATTRIBUTE>` format. Below is an example + of how to convert the old settings syntax to the new one. + +The following settings in `gitlab.rb`: + +```ruby +mattermost['service_maximum_login_attempts'] = 10 +mattermost['team_teammate_name_display'] = "full_name" +mattermost['sql_max_idle_conns'] = 10 +mattermost['log_file_level'] = 'INFO' +mattermost['email_batching_interval'] = 30 +mattermost['file_enable_file_attachments'] = true +mattermost['ratelimit_memory_store_size'] = 10000 +mattermost['support_terms_of_service_link'] = "/static/help/terms.html" +mattermost['privacy_show_email_address'] = true +mattermost['localization_available_locales'] = "en,es,fr,ja,pt-BR" +mattermost['webrtc_enable'] = false +``` + +Would translate to: + +```ruby +mattermost['env'] = { + 'MM_SERVICESETTINGS_MAXIMUMLOGINATTEMPTS' => '10', + 'MM_TEAMSETTINGS_TEAMMATENAMEDISPLAY' => 'full_name', + 'MM_SQLSETTINGS_MAXIDLECONNS' => '10', + 'MM_LOGSETTINGS_FILELEVEL' => 'INFO', + 'MM_EMAILSETTINGS_BATCHINGINTERVAL' => '30', + 'MM_FILESETTINGS_ENABLEFILEATTACHMENTS' => 'true', + 'MM_RATELIMITSETTINGS_MEMORYSTORESIZE' => '10000', + 'MM_SUPPORTSETTINGS_TERMSOFSERVICELINK' => '/static/help/terms.html', + 'MM_PRIVACYSETTINGS_SHOWEMAILADDRESS' => 'true', + 'MM_LOCALIZATIONSETTINGS_AVAILABLELOCALES' => 'en,es,fr,ja,pt-BR', + 'MM_WEBRTCSETTINGS_ENABLE' => 'false' + } +``` + +Refer to the [Mattermost Configuration Settings +documentation](https://docs.mattermost.com/administration/config-settings.html) +for details about categories, configuration values, etc. + +There are a few exceptions to this rule: + +1. `ServiceSettings.ListenAddress` configuration of Mattermost is configured + by `mattermost['service_address']` and `mattermost['service_port']` settings. +1. Configuration settings named in an inconsistent way are given in the + following table. Use these mappings when converting them to environment + variables. + +|`gitlab.rb` configuration|Environment variable| +|---|---| +|`mattermost['service_lets_encrypt_cert_cache_file']`|`MM_SERVICESETTINGS_LETSENCRYPTCERTIFICATECACHEFILE`| +|`mattermost['service_user_access_tokens']`|`MM_SERVICESETTINGS_ENABLEUSERACCESSTOKENS`| +|`mattermost['log_console_enable']`|`MM_LOGSETTINGS_ENABLECONSOLE`| +|`mattermost['email_enable_batching']`|`MM_EMAILSETTINGS_ENABLEEMAILBATCHING`| +|`mattermost['email_batching_buffer_size']`|`MM_EMAILSETTINGS_EMAILBATCHINGBUFFERSIZE`| +|`mattermost['email_batching_interval']`|`MM_EMAILSETTINGS_EMAILBATCHINGINTERVAL`| +|`mattermost['email_smtp_auth']`|`MM_EMAILSETTINGS_ENABLESMTPAUTH`| +|`mattermost['email_notification_content_type']`|`MM_EMAILSETTINGS_NOTIFICATIONCONTENTTYPE`| +|`mattermost['ratelimit_enable_ratelimiter']`|`MM_RATELIMITSETTINGS_ENABLE`| +|`mattermost['support_email']`|`MM_SUPPORTSETTINGS_SUPPORTEMAIL`| +|`mattermost['localization_server_locale']`|`MM_LOCALIZATIONSETTINGS_DEFAULTSERVERLOCALE`| +|`mattermost['localization_client_locale']`|`MM_LOCALIZATIONSETTINGS_DEFAULTCLIENTLOCALE`| +|`mattermost['webrtc_gateway_stun_uri']`|`MM_WEBRTCSETTINGS_STUN_URI`| +|`mattermost['webrtc_gateway_turn_uri']`|`MM_WEBRTCSETTINGS_TURN_URI`| +|`mattermost['webrtc_gateway_turn_username']`|`MM_WEBRTCSETTINGS_TURN_USERNAME`| +|`mattermost['webrtc_gateway_turn_sharedkey']`|`MM_WEBRTCSETTINGS_TURN_SHAREDKEY`| + +NOTE: +GitLab 11.0 no longer generates `config.json` file from the configuration specified +in `gitlab.rb`. Users are responsible for managing this file which can be done via the +Mattermost System Console or manually. +If a configuration setting is specified via both the `gitlab.rb` (as an environment variable) +and `config.json` files, the environment variable gets precedence. + +If you encounter any issues [visit the GitLab Mattermost troubleshooting forum](https://forum.mattermost.org/t/upgrading-to-gitlab-mattermost-in-gitlab-8-9/1735) and share any relevant portions of `mattermost.log` along with the step at which you encountered issues. + +### Upgrading GitLab Mattermost outside of GitLab + +If you choose to upgrade Mattermost outside of the Omnibus GitLab automation, [follow this guide](https://docs.mattermost.com/administration/upgrade.html). + +## OAuth2 sequence diagram + +The following image is a sequence diagram for how GitLab works as an OAuth2 +provider for Mattermost. You can use this to troubleshoot errors +in getting the integration to work: + + + +## Troubleshooting the Mattermost CLI + +### Failed to ping DB retrying in 10 seconds err=dial tcp: lookup dockerhost: no such host + +As of version 11.0, majority of the Mattermost settings are now configured via environmental variables. The error is mainly due to the database connection string being commented out in `gitlab.rb` and the database connection settings being set in environmental variables. Additionally, the connection string in the `gitlab.rb` is for MySQL which is no longer supported as of 12.1. + +You can fix this by setting up a `mattermost-cli` [shell alias](#mattermost-command-line-tools-cli). + +## Community support resources + +For help and support around your GitLab Mattermost deployment please see: + +- [Troubleshooting Forum](https://forum.mattermost.org/t/how-to-use-the-troubleshooting-forum/150) for configuration questions and issues. +- [Troubleshooting FAQ](https://docs.mattermost.com/install/troubleshooting.html). +- [Mattermost GitLab Issues Support Handbook](https://docs.mattermost.com/process/support.html?highlight=omnibus#gitlab-issues). +- [GitLab Mattermost issue tracker](https://gitlab.com/gitlab-org/gitlab-mattermost/-/issues) for verified bugs with repro steps. diff --git a/doc/operations/error_tracking.md b/doc/operations/error_tracking.md index 7c258be13b4..e694105d794 100644 --- a/doc/operations/error_tracking.md +++ b/doc/operations/error_tracking.md @@ -130,12 +130,9 @@ You must use the GitLab API to enable it. #### How to enable -1. Enable the `integrated` error tracking setting for your project: +1. Select **GitLab** as the error tracking backend for your project: - ```shell - curl --request PATCH --header "PRIVATE-TOKEN: <your_access_token>" \ - "https://gitlab.example.com/api/v4/projects/PROJECT_ID/error_tracking/settings?active=true&integrated=true" - ``` +  1. Create a client key (DSN) to use with Sentry SDK in your application. Make sure to save the response, as it contains a DSN: diff --git a/doc/operations/img/error_tracking_setting_v14_3.png b/doc/operations/img/error_tracking_setting_v14_3.png Binary files differnew file mode 100644 index 00000000000..14306130c97 --- /dev/null +++ b/doc/operations/img/error_tracking_setting_v14_3.png diff --git a/doc/user/application_security/container_scanning/index.md b/doc/user/application_security/container_scanning/index.md index 1b1ec4da1d2..2b3d4dbfc0a 100644 --- a/doc/user/application_security/container_scanning/index.md +++ b/doc/user/application_security/container_scanning/index.md @@ -49,37 +49,9 @@ To enable container scanning in your pipeline, you need the following: ## Configuration -How you enable container scanning depends on your GitLab version: - -- GitLab 11.9 and later: [Include](../../../ci/yaml/index.md#includetemplate) the - [`Container-Scanning.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml) - that comes with your GitLab installation. -- GitLab versions earlier than 11.9: Copy and use the job from the - [`Container-Scanning.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml). - -Other changes: - -- GitLab 13.6 [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/263482) better support for - [FIPS](https://csrc.nist.gov/publications/detail/fips/140/2/final) by upgrading the - `CS_MAJOR_VERSION` from `2` to `3`. Version `3` of the `container_scanning` Docker image uses - [`centos:centos8`](https://hub.docker.com/_/centos) - as the new base. It also removes the use of the [start.sh](https://gitlab.com/gitlab-org/security-products/analyzers/klar/-/merge_requests/77) - script and instead executes the analyzer by default. Any customizations made to the - `container_scanning` job's [`before_script`](../../../ci/yaml/index.md#before_script) - and [`after_script`](../../../ci/yaml/index.md#after_script) - blocks may not work with the new version. To roll back to the previous [`alpine:3.11.3`](https://hub.docker.com/_/alpine)-based - Docker image, you can specify the major version through the [`CS_MAJOR_VERSION`](#available-cicd-variables) - variable. -- GitLab 13.9 [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/322656) integration with - [Trivy](https://github.com/aquasecurity/trivy) by upgrading `CS_MAJOR_VERSION` from `3` to `4`. -- GitLab 14.0 [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61850) - an integration with [Trivy](https://github.com/aquasecurity/trivy) - as the default for container scanning, and also [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/326279) - an integration with [Grype](https://github.com/anchore/grype) - as an alternative scanner. - -To include the `Container-Scanning.gitlab-ci.yml` template (GitLab 11.9 and later), add the -following to your `.gitlab-ci.yml` file: +To enable container scanning, add the +[`Container-Scanning.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml) +to your `.gitlab-ci.yml` file: ```yaml include: @@ -617,3 +589,29 @@ To prevent the error, ensure the Docker version that the runner is using is ### Getting warning message `gl-container-scanning-report.json: no matching files` For information on this, see the [general Application Security troubleshooting section](../../../ci/pipelines/job_artifacts.md#error-message-no-files-to-upload). + +## Changes + +- GitLab 13.6 [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/263482) better support for + [FIPS](https://csrc.nist.gov/publications/detail/fips/140/2/final) by upgrading the + `CS_MAJOR_VERSION` from `2` to `3`. Version `3` of the `container_scanning` Docker image uses + [`centos:centos8`](https://hub.docker.com/_/centos) + as the new base. It also removes the use of the [start.sh](https://gitlab.com/gitlab-org/security-products/analyzers/klar/-/merge_requests/77) + script and instead executes the analyzer by default. Any customizations made to the + `container_scanning` job's [`before_script`](../../../ci/yaml/index.md#before_script) + and [`after_script`](../../../ci/yaml/index.md#after_script) + blocks may not work with the new version. To roll back to the previous [`alpine:3.11.3`](https://hub.docker.com/_/alpine)-based + Docker image, you can specify the major version through the [`CS_MAJOR_VERSION`](#available-cicd-variables) + variable. +- GitLab 13.9 [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/322656) integration with + [Trivy](https://github.com/aquasecurity/trivy) by upgrading `CS_MAJOR_VERSION` from `3` to `4`. +- GitLab 13.9 [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/321451) the integration with + [Clair](https://github.com/quay/clair/). +- GitLab 14.0 [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61850) + an integration with [Trivy](https://github.com/aquasecurity/trivy) + as the default for container scanning, and also [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/326279) + an integration with [Grype](https://github.com/anchore/grype) + as an alternative scanner. + +Other changes to the container scanning analyzer can be found in the project's +[changelog](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning/-/blob/master/CHANGELOG.md). diff --git a/doc/user/project/integrations/mattermost_slash_commands.md b/doc/user/project/integrations/mattermost_slash_commands.md index 8824d0c549c..8027cc1c61e 100644 --- a/doc/user/project/integrations/mattermost_slash_commands.md +++ b/doc/user/project/integrations/mattermost_slash_commands.md @@ -22,7 +22,7 @@ on your configuration: - **Omnibus GitLab installations**: Mattermost is bundled with [Omnibus GitLab](https://docs.gitlab.com/omnibus/). To configure Mattermost for Omnibus GitLab, read the - [Omnibus GitLab Mattermost documentation](https://docs.gitlab.com/omnibus/gitlab-mattermost/). + [Omnibus GitLab Mattermost documentation](../../../integration/mattermost/index.md). - **If Mattermost is installed on the same server as GitLab**, use the [automated configuration](#automated-configuration). - **For all other installations**, use the [manual configuration](#manual-configuration). diff --git a/lib/gitlab/ci/variables/collection.rb b/lib/gitlab/ci/variables/collection.rb index 73d27399680..09c75a2b3f1 100644 --- a/lib/gitlab/ci/variables/collection.rb +++ b/lib/gitlab/ci/variables/collection.rb @@ -90,7 +90,7 @@ module Gitlab end def sort_and_expand_all(project, keep_undefined: false) - return self if Feature.disabled?(:variable_inside_variable, project) + return self if Feature.disabled?(:variable_inside_variable, project, default_enabled: :yaml) sorted = Sort.new(self) return self.class.new(self, sorted.errors) unless sorted.valid? diff --git a/lib/gitlab/database/partitioning.rb b/lib/gitlab/database/partitioning.rb new file mode 100644 index 00000000000..bbde2063c41 --- /dev/null +++ b/lib/gitlab/database/partitioning.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Partitioning + def self.register_models(models) + registered_models.merge(models) + end + + def self.registered_models + @registered_models ||= Set.new + end + + def self.sync_partitions(models_to_sync = registered_models) + MultiDatabasePartitionManager.new(models_to_sync).sync_partitions + end + end + end +end diff --git a/lib/gitlab/database/partitioning/multi_database_partition_manager.rb b/lib/gitlab/database/partitioning/multi_database_partition_manager.rb new file mode 100644 index 00000000000..5a93e3fb1fb --- /dev/null +++ b/lib/gitlab/database/partitioning/multi_database_partition_manager.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Partitioning + class MultiDatabasePartitionManager + def initialize(models) + @models = models + end + + def sync_partitions + Gitlab::AppLogger.info(message: "Syncing dynamic postgres partitions") + + models.each do |model| + Gitlab::Database::SharedModel.using_connection(model.connection) do + Gitlab::AppLogger.debug(message: "Switched database connection", + connection_name: connection_name, + table_name: model.table_name) + + PartitionManager.new(model).sync_partitions + end + end + + Gitlab::AppLogger.info(message: "Finished sync of dynamic postgres partitions") + end + + private + + attr_reader :models + + def connection_name + Gitlab::Database::SharedModel.connection.pool.db_config.name + end + end + end + end +end diff --git a/lib/gitlab/database/partitioning/partition_manager.rb b/lib/gitlab/database/partitioning/partition_manager.rb index 2ba5b35d6b9..8742c0ff166 100644 --- a/lib/gitlab/database/partitioning/partition_manager.rb +++ b/lib/gitlab/database/partitioning/partition_manager.rb @@ -6,60 +6,49 @@ module Gitlab class PartitionManager UnsafeToDetachPartitionError = Class.new(StandardError) - def self.register(model) - raise ArgumentError, "Only models with a #partitioning_strategy can be registered." unless model.respond_to?(:partitioning_strategy) - - models << model - end - - def self.models - @models ||= Set.new - end - LEASE_TIMEOUT = 1.minute MANAGEMENT_LEASE_KEY = 'database_partition_management_%s' RETAIN_DETACHED_PARTITIONS_FOR = 1.week - attr_reader :models - - def initialize(models = self.class.models) - @models = models + def initialize(model) + @model = model end def sync_partitions - Gitlab::AppLogger.info("Checking state of dynamic postgres partitions") + Gitlab::AppLogger.info(message: "Checking state of dynamic postgres partitions", table_name: model.table_name) - models.each do |model| - # Double-checking before getting the lease: - # The prevailing situation is no missing partitions and no extra partitions - next if missing_partitions(model).empty? && extra_partitions(model).empty? + # Double-checking before getting the lease: + # The prevailing situation is no missing partitions and no extra partitions + return if missing_partitions.empty? && extra_partitions.empty? - only_with_exclusive_lease(model, lease_key: MANAGEMENT_LEASE_KEY) do - partitions_to_create = missing_partitions(model) - create(partitions_to_create) unless partitions_to_create.empty? + only_with_exclusive_lease(model, lease_key: MANAGEMENT_LEASE_KEY) do + partitions_to_create = missing_partitions + create(partitions_to_create) unless partitions_to_create.empty? - if Feature.enabled?(:partition_pruning, default_enabled: :yaml) - partitions_to_detach = extra_partitions(model) - detach(partitions_to_detach) unless partitions_to_detach.empty? - end + if Feature.enabled?(:partition_pruning, default_enabled: :yaml) + partitions_to_detach = extra_partitions + detach(partitions_to_detach) unless partitions_to_detach.empty? end - rescue StandardError => e - Gitlab::AppLogger.error(message: "Failed to create / detach partition(s)", - table_name: model.table_name, - exception_class: e.class, - exception_message: e.message) end + rescue StandardError => e + Gitlab::AppLogger.error(message: "Failed to create / detach partition(s)", + table_name: model.table_name, + exception_class: e.class, + exception_message: e.message) end private - def missing_partitions(model) + attr_reader :model + delegate :connection, to: :model + + def missing_partitions return [] unless connection.table_exists?(model.table_name) model.partitioning_strategy.missing_partitions end - def extra_partitions(model) + def extra_partitions return [] unless connection.table_exists?(model.table_name) model.partitioning_strategy.extra_partitions @@ -121,13 +110,10 @@ module Gitlab def with_lock_retries(&block) Gitlab::Database::WithLockRetries.new( klass: self.class, - logger: Gitlab::AppLogger + logger: Gitlab::AppLogger, + connection: connection ).run(&block) end - - def connection - ActiveRecord::Base.connection - end end end end diff --git a/lib/gitlab/database/partitioning/partition_monitoring.rb b/lib/gitlab/database/partitioning/partition_monitoring.rb index 6963ecd2cc1..e5b561fc447 100644 --- a/lib/gitlab/database/partitioning/partition_monitoring.rb +++ b/lib/gitlab/database/partitioning/partition_monitoring.rb @@ -6,7 +6,7 @@ module Gitlab class PartitionMonitoring attr_reader :models - def initialize(models = PartitionManager.models) + def initialize(models = Gitlab::Database::Partitioning.registered_models) @models = models end diff --git a/lib/gitlab/database/postgres_foreign_key.rb b/lib/gitlab/database/postgres_foreign_key.rb index 94f74724295..72640f8785d 100644 --- a/lib/gitlab/database/postgres_foreign_key.rb +++ b/lib/gitlab/database/postgres_foreign_key.rb @@ -2,7 +2,7 @@ module Gitlab module Database - class PostgresForeignKey < ApplicationRecord + class PostgresForeignKey < SharedModel self.primary_key = :oid scope :by_referenced_table_identifier, ->(identifier) do diff --git a/lib/gitlab/database/postgres_partition.rb b/lib/gitlab/database/postgres_partition.rb index 7da60d8375d..eb080904f73 100644 --- a/lib/gitlab/database/postgres_partition.rb +++ b/lib/gitlab/database/postgres_partition.rb @@ -2,7 +2,7 @@ module Gitlab module Database - class PostgresPartition < ActiveRecord::Base + class PostgresPartition < SharedModel self.primary_key = :identifier belongs_to :postgres_partitioned_table, foreign_key: 'parent_identifier', primary_key: 'identifier' diff --git a/lib/gitlab/database/postgres_partitioned_table.rb b/lib/gitlab/database/postgres_partitioned_table.rb index 5d2eaa22ee4..3bd342f940f 100644 --- a/lib/gitlab/database/postgres_partitioned_table.rb +++ b/lib/gitlab/database/postgres_partitioned_table.rb @@ -2,7 +2,7 @@ module Gitlab module Database - class PostgresPartitionedTable < ActiveRecord::Base + class PostgresPartitionedTable < SharedModel DYNAMIC_PARTITION_STRATEGIES = %w[range list].freeze self.primary_key = :identifier diff --git a/lib/gitlab/database/shared_model.rb b/lib/gitlab/database/shared_model.rb new file mode 100644 index 00000000000..8f256758961 --- /dev/null +++ b/lib/gitlab/database/shared_model.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab + module Database + class SharedModel < ActiveRecord::Base + self.abstract_class = true + + class << self + def using_connection(connection) + raise 'cannot nest connection overrides for shared models' unless overriding_connection.nil? + + self.overriding_connection = connection + + yield + ensure + self.overriding_connection = nil + end + + def connection + if connection = self.overriding_connection + connection + else + super + end + end + + private + + def overriding_connection + Thread.current[:overriding_connection] + end + + def overriding_connection=(connection) + Thread.current[:overriding_connection] = connection + end + end + end + end +end diff --git a/lib/gitlab/devise_failure.rb b/lib/gitlab/devise_failure.rb index eb475307f27..111ea697ec2 100644 --- a/lib/gitlab/devise_failure.rb +++ b/lib/gitlab/devise_failure.rb @@ -2,18 +2,10 @@ module Gitlab class DeviseFailure < Devise::FailureApp - include ::SessionsHelper - # If the request format is not known, send a redirect instead of a 401 # response, since this is the outcome we're most likely to want def http_auth? request_format && super end - - def respond - limit_session_time - - super - end end end diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index 6e09031a1e1..a6738b01f18 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -118,7 +118,7 @@ namespace :gitlab do desc 'Create missing dynamic database partitions' task create_dynamic_partitions: :environment do - Gitlab::Database::Partitioning::PartitionManager.new.sync_partitions + Gitlab::Database::Partitioning.sync_partitions end # This is targeted towards deploys and upgrades of GitLab. diff --git a/package.json b/package.json index 8df8e9eef25..0b45f1c29e7 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "@gitlab/favicon-overlay": "2.0.0", "@gitlab/svgs": "1.212.0", "@gitlab/tributejs": "1.0.0", - "@gitlab/ui": "32.10.1", + "@gitlab/ui": "32.10.2", "@gitlab/visual-review-tools": "1.6.1", "@rails/actioncable": "6.1.3-2", "@rails/ujs": "6.1.3-2", diff --git a/spec/features/users/anonymous_sessions_spec.rb b/spec/features/users/anonymous_sessions_spec.rb index 273d3aa346f..6b21412ae3d 100644 --- a/spec/features/users/anonymous_sessions_spec.rb +++ b/spec/features/users/anonymous_sessions_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe 'Session TTLs', :clean_gitlab_redis_shared_state do + include SessionHelpers + it 'creates a session with a short TTL when login fails' do visit new_user_session_path # The session key only gets created after a post @@ -12,7 +14,7 @@ RSpec.describe 'Session TTLs', :clean_gitlab_redis_shared_state do expect(page).to have_content('Invalid login or password') - expect_single_session_with_expiration(Settings.gitlab['unauthenticated_session_expire_delay']) + expect_single_session_with_short_ttl end it 'increases the TTL when the login succeeds' do @@ -21,21 +23,17 @@ RSpec.describe 'Session TTLs', :clean_gitlab_redis_shared_state do expect(page).to have_content(user.name) - expect_single_session_with_expiration(Settings.gitlab['session_expire_delay'] * 60) + expect_single_session_with_authenticated_ttl end - def expect_single_session_with_expiration(expiration) - session_keys = get_session_keys - - expect(session_keys.size).to eq(1) - expect(get_ttl(session_keys.first)).to eq expiration - end + context 'with an unauthorized project' do + let_it_be(:project) { create(:project, :repository) } - def get_session_keys - Gitlab::Redis::SharedState.with { |redis| redis.scan_each(match: 'session:gitlab:*').to_a } - end + it 'creates a session with a short TTL' do + visit project_raw_path(project, 'master/README.md') - def get_ttl(key) - Gitlab::Redis::SharedState.with { |redis| redis.ttl(key) } + expect_single_session_with_short_ttl + expect(page).to have_current_path(new_user_session_path) + end end end diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 6c38d5d8b24..afd750d02eb 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -2,9 +2,10 @@ require 'spec_helper' -RSpec.describe 'Login' do +RSpec.describe 'Login', :clean_gitlab_redis_shared_state do include TermsHelper include UserLoginHelper + include SessionHelpers before do stub_authentication_activity_metrics(debug: true) @@ -59,6 +60,7 @@ RSpec.describe 'Login' do fill_in 'user_password', with: 'password' click_button 'Sign in' + expect_single_session_with_authenticated_ttl expect(current_path).to eq root_path end @@ -192,6 +194,7 @@ RSpec.describe 'Login' do enter_code(user.current_otp) expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated')) + expect_single_session_with_authenticated_ttl end it 'does not allow sign-in if the user password is updated before entering a one-time code' do @@ -210,6 +213,7 @@ RSpec.describe 'Login' do enter_code(user.current_otp) + expect_single_session_with_authenticated_ttl expect(current_path).to eq root_path end @@ -237,6 +241,8 @@ RSpec.describe 'Login' do expect(page).to have_content('Invalid two-factor code') enter_code(user.current_otp) + + expect_single_session_with_authenticated_ttl expect(current_path).to eq root_path end @@ -353,6 +359,7 @@ RSpec.describe 'Login' do sign_in_using_saml! + expect_single_session_with_authenticated_ttl expect(page).not_to have_content('Two-Factor Authentication') expect(current_path).to eq root_path end @@ -371,6 +378,7 @@ RSpec.describe 'Login' do enter_code(user.current_otp) + expect_single_session_with_authenticated_ttl expect(current_path).to eq root_path end end @@ -391,6 +399,7 @@ RSpec.describe 'Login' do gitlab_sign_in(user) + expect_single_session_with_authenticated_ttl expect(current_path).to eq root_path expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated')) end @@ -402,6 +411,7 @@ RSpec.describe 'Login' do gitlab_sign_in(user) visit new_user_session_path + expect_single_session_with_authenticated_ttl expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated')) end @@ -443,6 +453,7 @@ RSpec.describe 'Login' do gitlab_sign_in(user) + expect_single_session_with_short_ttl expect(page).to have_content('Invalid login or password.') end end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 112b6b678a1..72aeb54f7a7 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -426,139 +426,121 @@ RSpec.describe IssuesFinder do end end - shared_examples ':label_name parameter' do - context 'filtering by label' do - let(:params) { { label_name: label.title } } + context 'filtering by label' do + let(:params) { { label_name: label.title } } - it 'returns issues with that label' do - expect(issues).to contain_exactly(issue2) - end + it 'returns issues with that label' do + expect(issues).to contain_exactly(issue2) + end - context 'using NOT' do - let(:params) { { not: { label_name: label.title } } } + context 'using NOT' do + let(:params) { { not: { label_name: label.title } } } - it 'returns issues that do not have that label' do - expect(issues).to contain_exactly(issue1, issue3, issue4, issue5) - end + it 'returns issues that do not have that label' do + expect(issues).to contain_exactly(issue1, issue3, issue4, issue5) + end - # IssuableFinder first filters using the outer params (the ones not inside the `not` key.) - # Afterwards, it applies the `not` params to that resultset. This means that things inside the `not` param - # do not take precedence over the outer params with the same name. - context 'shadowing the same outside param' do - let(:params) { { label_name: label2.title, not: { label_name: label.title } } } + # IssuableFinder first filters using the outer params (the ones not inside the `not` key.) + # Afterwards, it applies the `not` params to that resultset. This means that things inside the `not` param + # do not take precedence over the outer params with the same name. + context 'shadowing the same outside param' do + let(:params) { { label_name: label2.title, not: { label_name: label.title } } } - it 'does not take precedence over labels outside NOT' do - expect(issues).to contain_exactly(issue3) - end + it 'does not take precedence over labels outside NOT' do + expect(issues).to contain_exactly(issue3) end + end - context 'further filtering outside params' do - let(:params) { { label_name: label2.title, not: { assignee_username: user2.username } } } + context 'further filtering outside params' do + let(:params) { { label_name: label2.title, not: { assignee_username: user2.username } } } - it 'further filters on the returned resultset' do - expect(issues).to be_empty - end + it 'further filters on the returned resultset' do + expect(issues).to be_empty end end end + end - context 'filtering by multiple labels' do - let(:params) { { label_name: [label.title, label2.title].join(',') } } - let(:label2) { create(:label, project: project2) } - - before do - create(:label_link, label: label2, target: issue2) - end - - it 'returns the unique issues with all those labels' do - expect(issues).to contain_exactly(issue2) - end + context 'filtering by multiple labels' do + let(:params) { { label_name: [label.title, label2.title].join(',') } } + let(:label2) { create(:label, project: project2) } - context 'using NOT' do - let(:params) { { not: { label_name: [label.title, label2.title].join(',') } } } + before do + create(:label_link, label: label2, target: issue2) + end - it 'returns issues that do not have any of the labels provided' do - expect(issues).to contain_exactly(issue1, issue4, issue5) - end - end + it 'returns the unique issues with all those labels' do + expect(issues).to contain_exactly(issue2) end - context 'filtering by a label that includes any or none in the title' do - let(:params) { { label_name: [label.title, label2.title].join(',') } } - let(:label) { create(:label, title: 'any foo', project: project2) } - let(:label2) { create(:label, title: 'bar none', project: project2) } + context 'using NOT' do + let(:params) { { not: { label_name: [label.title, label2.title].join(',') } } } - before do - create(:label_link, label: label2, target: issue2) + it 'returns issues that do not have any of the labels provided' do + expect(issues).to contain_exactly(issue1, issue4, issue5) end + end + end - it 'returns the unique issues with all those labels' do - expect(issues).to contain_exactly(issue2) - end + context 'filtering by a label that includes any or none in the title' do + let(:params) { { label_name: [label.title, label2.title].join(',') } } + let(:label) { create(:label, title: 'any foo', project: project2) } + let(:label2) { create(:label, title: 'bar none', project: project2) } - context 'using NOT' do - let(:params) { { not: { label_name: [label.title, label2.title].join(',') } } } + before do + create(:label_link, label: label2, target: issue2) + end - it 'returns issues that do not have ANY ONE of the labels provided' do - expect(issues).to contain_exactly(issue1, issue4, issue5) - end - end + it 'returns the unique issues with all those labels' do + expect(issues).to contain_exactly(issue2) end - context 'filtering by no label' do - let(:params) { { label_name: described_class::Params::FILTER_NONE } } + context 'using NOT' do + let(:params) { { not: { label_name: [label.title, label2.title].join(',') } } } - it 'returns issues with no labels' do + it 'returns issues that do not have ANY ONE of the labels provided' do expect(issues).to contain_exactly(issue1, issue4, issue5) end end + end - context 'filtering by any label' do - let(:params) { { label_name: described_class::Params::FILTER_ANY } } - - it 'returns issues that have one or more label' do - create_list(:label_link, 2, label: create(:label, project: project2), target: issue3) + context 'filtering by no label' do + let(:params) { { label_name: described_class::Params::FILTER_NONE } } - expect(issues).to contain_exactly(issue2, issue3) - end + it 'returns issues with no labels' do + expect(issues).to contain_exactly(issue1, issue4, issue5) end + end - context 'when the same label exists on project and group levels' do - let(:issue1) { create(:issue, project: project1) } - let(:issue2) { create(:issue, project: project1) } - - # Skipping validation to reproduce a "real-word" scenario. - # We still have legacy labels on PRD that have the same title on the group and project levels, example: `bug` - let(:project_label) { build(:label, title: 'somelabel', project: project1).tap { |r| r.save!(validate: false) } } - let(:group_label) { create(:group_label, title: 'somelabel', group: project1.group) } + context 'filtering by any label' do + let(:params) { { label_name: described_class::Params::FILTER_ANY } } - let(:params) { { label_name: 'somelabel' } } + it 'returns issues that have one or more label' do + create_list(:label_link, 2, label: create(:label, project: project2), target: issue3) - before do - create(:label_link, label: group_label, target: issue1) - create(:label_link, label: project_label, target: issue2) - end - - it 'finds both issue records' do - expect(issues).to contain_exactly(issue1, issue2) - end + expect(issues).to contain_exactly(issue2, issue3) end end - context 'when `optimized_issuable_label_filter` feature flag is off' do - before do - stub_feature_flags(optimized_issuable_label_filter: false) - end + context 'when the same label exists on project and group levels' do + let(:issue1) { create(:issue, project: project1) } + let(:issue2) { create(:issue, project: project1) } - it_behaves_like ':label_name parameter' - end + # Skipping validation to reproduce a "real-word" scenario. + # We still have legacy labels on PRD that have the same title on the group and project levels, example: `bug` + let(:project_label) { build(:label, title: 'somelabel', project: project1).tap { |r| r.save!(validate: false) } } + let(:group_label) { create(:group_label, title: 'somelabel', group: project1.group) } + + let(:params) { { label_name: 'somelabel' } } - context 'when `optimized_issuable_label_filter` feature flag is on' do before do - stub_feature_flags(optimized_issuable_label_filter: true) + create(:label_link, label: group_label, target: issue1) + create(:label_link, label: project_label, target: issue2) end - it_behaves_like ':label_name parameter' + it 'finds both issue records' do + expect(issues).to contain_exactly(issue1, issue2) + end end context 'filtering by issue term' do diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 01371ff8fcc..42197a6b103 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -227,56 +227,38 @@ RSpec.describe MergeRequestsFinder do end end - shared_examples ':label_name parameter' do - describe ':label_name parameter' do - let(:common_labels) { create_list(:label, 3) } - let(:distinct_labels) { create_list(:label, 3) } - let(:merge_requests) do - common_attrs = { - source_project: project1, target_project: project1, author: user - } - distinct_labels.map do |label| - labels = [label, *common_labels] - create(:labeled_merge_request, :closed, labels: labels, **common_attrs) - end - end - - def find(label_name) - described_class.new(user, label_name: label_name).execute - end - - it 'accepts a single label' do - found = find(distinct_labels.first.title) - common = find(common_labels.first.title) - - expect(found).to contain_exactly(merge_requests.first) - expect(common).to match_array(merge_requests) - end - - it 'accepts an array of labels, all of which must match' do - all_distinct = find(distinct_labels.pluck(:title)) - all_common = find(common_labels.pluck(:title)) - - expect(all_distinct).to be_empty - expect(all_common).to match_array(merge_requests) + describe ':label_name parameter' do + let(:common_labels) { create_list(:label, 3) } + let(:distinct_labels) { create_list(:label, 3) } + let(:merge_requests) do + common_attrs = { + source_project: project1, target_project: project1, author: user + } + distinct_labels.map do |label| + labels = [label, *common_labels] + create(:labeled_merge_request, :closed, labels: labels, **common_attrs) end end - end - context 'when `optimized_issuable_label_filter` feature flag is off' do - before do - stub_feature_flags(optimized_issuable_label_filter: false) + def find(label_name) + described_class.new(user, label_name: label_name).execute end - it_behaves_like ':label_name parameter' - end + it 'accepts a single label' do + found = find(distinct_labels.first.title) + common = find(common_labels.first.title) - context 'when `optimized_issuable_label_filter` feature flag is on' do - before do - stub_feature_flags(optimized_issuable_label_filter: true) + expect(found).to contain_exactly(merge_requests.first) + expect(common).to match_array(merge_requests) end - it_behaves_like ':label_name parameter' + it 'accepts an array of labels, all of which must match' do + all_distinct = find(distinct_labels.pluck(:title)) + all_common = find(common_labels.pluck(:title)) + + expect(all_distinct).to be_empty + expect(all_common).to match_array(merge_requests) + end end it 'filters by source project id' do diff --git a/spec/frontend/autosave_spec.js b/spec/frontend/autosave_spec.js index bbdf3c6f91d..c881e0f9794 100644 --- a/spec/frontend/autosave_spec.js +++ b/spec/frontend/autosave_spec.js @@ -15,28 +15,28 @@ describe('Autosave', () => { describe('class constructor', () => { beforeEach(() => { - jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').mockReturnValue(true); + jest.spyOn(AccessorUtilities, 'canUseLocalStorage').mockReturnValue(true); jest.spyOn(Autosave.prototype, 'restore').mockImplementation(() => {}); }); it('should set .isLocalStorageAvailable', () => { autosave = new Autosave(field, key); - expect(AccessorUtilities.isLocalStorageAccessSafe).toHaveBeenCalled(); + expect(AccessorUtilities.canUseLocalStorage).toHaveBeenCalled(); expect(autosave.isLocalStorageAvailable).toBe(true); }); it('should set .isLocalStorageAvailable if fallbackKey is passed', () => { autosave = new Autosave(field, key, fallbackKey); - expect(AccessorUtilities.isLocalStorageAccessSafe).toHaveBeenCalled(); + expect(AccessorUtilities.canUseLocalStorage).toHaveBeenCalled(); expect(autosave.isLocalStorageAvailable).toBe(true); }); it('should set .isLocalStorageAvailable if lockVersion is passed', () => { autosave = new Autosave(field, key, null, lockVersion); - expect(AccessorUtilities.isLocalStorageAccessSafe).toHaveBeenCalled(); + expect(AccessorUtilities.canUseLocalStorage).toHaveBeenCalled(); expect(autosave.isLocalStorageAvailable).toBe(true); }); }); diff --git a/spec/frontend/emoji/support/unicode_support_map_spec.js b/spec/frontend/emoji/support/unicode_support_map_spec.js index 945e804a9fa..37f74db30b5 100644 --- a/spec/frontend/emoji/support/unicode_support_map_spec.js +++ b/spec/frontend/emoji/support/unicode_support_map_spec.js @@ -8,14 +8,14 @@ describe('Unicode Support Map', () => { const stringSupportMap = 'stringSupportMap'; beforeEach(() => { - jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').mockImplementation(() => {}); + jest.spyOn(AccessorUtilities, 'canUseLocalStorage').mockImplementation(() => {}); jest.spyOn(JSON, 'parse').mockImplementation(() => {}); jest.spyOn(JSON, 'stringify').mockReturnValue(stringSupportMap); }); describe('if isLocalStorageAvailable is `true`', () => { beforeEach(() => { - jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').mockReturnValue(true); + jest.spyOn(AccessorUtilities, 'canUseLocalStorage').mockReturnValue(true); getUnicodeSupportMap(); }); @@ -38,7 +38,7 @@ describe('Unicode Support Map', () => { describe('if isLocalStorageAvailable is `false`', () => { beforeEach(() => { - jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').mockReturnValue(false); + jest.spyOn(AccessorUtilities, 'canUseLocalStorage').mockReturnValue(false); getUnicodeSupportMap(); }); diff --git a/spec/frontend/filtered_search/services/recent_searches_service_spec.js b/spec/frontend/filtered_search/services/recent_searches_service_spec.js index 6711ce03d40..dfa53652eb1 100644 --- a/spec/frontend/filtered_search/services/recent_searches_service_spec.js +++ b/spec/frontend/filtered_search/services/recent_searches_service_spec.js @@ -145,13 +145,13 @@ describe('RecentSearchesService', () => { let isAvailable; beforeEach(() => { - jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe'); + jest.spyOn(AccessorUtilities, 'canUseLocalStorage'); isAvailable = RecentSearchesService.isAvailable(); }); - it('should call .isLocalStorageAccessSafe', () => { - expect(AccessorUtilities.isLocalStorageAccessSafe).toHaveBeenCalled(); + it('should call .canUseLocalStorage', () => { + expect(AccessorUtilities.canUseLocalStorage).toHaveBeenCalled(); }); it('should return a boolean', () => { diff --git a/spec/frontend/frequent_items/store/actions_spec.js b/spec/frontend/frequent_items/store/actions_spec.js index dacfc7ce707..fb0321545c2 100644 --- a/spec/frontend/frequent_items/store/actions_spec.js +++ b/spec/frontend/frequent_items/store/actions_spec.js @@ -109,7 +109,7 @@ describe('Frequent Items Dropdown Store Actions', () => { }); it('should dispatch `receiveFrequentItemsError`', (done) => { - jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').mockReturnValue(false); + jest.spyOn(AccessorUtilities, 'canUseLocalStorage').mockReturnValue(false); mockedState.namespace = mockNamespace; mockedState.storageKey = mockStorageKey; diff --git a/spec/frontend/lib/utils/accessor_spec.js b/spec/frontend/lib/utils/accessor_spec.js index 752a88296e6..63497d795ce 100644 --- a/spec/frontend/lib/utils/accessor_spec.js +++ b/spec/frontend/lib/utils/accessor_spec.js @@ -6,60 +6,9 @@ describe('AccessorUtilities', () => { const testError = new Error('test error'); - describe('isPropertyAccessSafe', () => { - let base; - - it('should return `true` if access is safe', () => { - base = { - testProp: 'testProp', - }; - expect(AccessorUtilities.isPropertyAccessSafe(base, 'testProp')).toBe(true); - }); - - it('should return `false` if access throws an error', () => { - base = { - get testProp() { - throw testError; - }, - }; - - expect(AccessorUtilities.isPropertyAccessSafe(base, 'testProp')).toBe(false); - }); - - it('should return `false` if property is undefined', () => { - base = {}; - - expect(AccessorUtilities.isPropertyAccessSafe(base, 'testProp')).toBe(false); - }); - }); - - describe('isFunctionCallSafe', () => { - const base = {}; - - it('should return `true` if calling is safe', () => { - base.func = () => {}; - - expect(AccessorUtilities.isFunctionCallSafe(base, 'func')).toBe(true); - }); - - it('should return `false` if calling throws an error', () => { - base.func = () => { - throw new Error('test error'); - }; - - expect(AccessorUtilities.isFunctionCallSafe(base, 'func')).toBe(false); - }); - - it('should return `false` if function is undefined', () => { - base.func = undefined; - - expect(AccessorUtilities.isFunctionCallSafe(base, 'func')).toBe(false); - }); - }); - - describe('isLocalStorageAccessSafe', () => { + describe('canUseLocalStorage', () => { it('should return `true` if access is safe', () => { - expect(AccessorUtilities.isLocalStorageAccessSafe()).toBe(true); + expect(AccessorUtilities.canUseLocalStorage()).toBe(true); }); it('should return `false` if access to .setItem isnt safe', () => { @@ -67,19 +16,19 @@ describe('AccessorUtilities', () => { throw testError; }); - expect(AccessorUtilities.isLocalStorageAccessSafe()).toBe(false); + expect(AccessorUtilities.canUseLocalStorage()).toBe(false); }); it('should set a test item if access is safe', () => { - AccessorUtilities.isLocalStorageAccessSafe(); + AccessorUtilities.canUseLocalStorage(); - expect(window.localStorage.setItem).toHaveBeenCalledWith('isLocalStorageAccessSafe', 'true'); + expect(window.localStorage.setItem).toHaveBeenCalledWith('canUseLocalStorage', 'true'); }); it('should remove the test item if access is safe', () => { - AccessorUtilities.isLocalStorageAccessSafe(); + AccessorUtilities.canUseLocalStorage(); - expect(window.localStorage.removeItem).toHaveBeenCalledWith('isLocalStorageAccessSafe'); + expect(window.localStorage.removeItem).toHaveBeenCalledWith('canUseLocalStorage'); }); }); }); diff --git a/spec/frontend/pages/sessions/new/signin_tabs_memoizer_spec.js b/spec/frontend/pages/sessions/new/signin_tabs_memoizer_spec.js index 6aa725fbd7d..601fcfedbe0 100644 --- a/spec/frontend/pages/sessions/new/signin_tabs_memoizer_spec.js +++ b/spec/frontend/pages/sessions/new/signin_tabs_memoizer_spec.js @@ -21,7 +21,7 @@ describe('SigninTabsMemoizer', () => { beforeEach(() => { loadFixtures(fixtureTemplate); - jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').mockReturnValue(true); + jest.spyOn(AccessorUtilities, 'canUseLocalStorage').mockReturnValue(true); }); it('does nothing if no tab was previously selected', () => { @@ -90,7 +90,7 @@ describe('SigninTabsMemoizer', () => { }); it('should set .isLocalStorageAvailable', () => { - expect(AccessorUtilities.isLocalStorageAccessSafe).toHaveBeenCalled(); + expect(AccessorUtilities.canUseLocalStorage).toHaveBeenCalled(); expect(memo.isLocalStorageAvailable).toBe(true); }); }); diff --git a/spec/frontend/search/store/utils_spec.js b/spec/frontend/search/store/utils_spec.js index cd7f7dc3b5f..bcdad9f89dd 100644 --- a/spec/frontend/search/store/utils_spec.js +++ b/spec/frontend/search/store/utils_spec.js @@ -14,7 +14,7 @@ const CURRENT_TIME = new Date().getTime(); useLocalStorageSpy(); jest.mock('~/lib/utils/accessor', () => ({ - isLocalStorageAccessSafe: jest.fn().mockReturnValue(true), + canUseLocalStorage: jest.fn().mockReturnValue(true), })); describe('Global Search Store Utils', () => { diff --git a/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_utils_spec.js b/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_utils_spec.js index 47dabf0670a..d85b6e6d115 100644 --- a/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_utils_spec.js +++ b/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_utils_spec.js @@ -25,7 +25,7 @@ import { const mockStorageKey = 'recent-tokens'; function setLocalStorageAvailability(isAvailable) { - jest.spyOn(AccessorUtilities, 'isLocalStorageAccessSafe').mockReturnValue(isAvailable); + jest.spyOn(AccessorUtilities, 'canUseLocalStorage').mockReturnValue(isAvailable); } describe('Filtered Search Utils', () => { diff --git a/spec/graphql/resolvers/merge_requests_resolver_spec.rb b/spec/graphql/resolvers/merge_requests_resolver_spec.rb index 64ee0d4f9cc..a897acf7eba 100644 --- a/spec/graphql/resolvers/merge_requests_resolver_spec.rb +++ b/spec/graphql/resolvers/merge_requests_resolver_spec.rb @@ -294,16 +294,6 @@ RSpec.describe Resolvers::MergeRequestsResolver do nils_last(mr.metrics.merged_at) end - context 'when label filter is given and the optimized_issuable_label_filter feature flag is off' do - before do - stub_feature_flags(optimized_issuable_label_filter: false) - end - - it 'does not raise PG::GroupingError' do - expect { resolve_mr(project, sort: :merged_at_desc, labels: %w[a b]) }.not_to raise_error - end - end - context 'when sorting by closed at' do before do merge_request_1.metrics.update!(latest_closed_at: 10.days.ago) diff --git a/spec/lib/gitlab/database/partitioning/multi_database_partition_manager_spec.rb b/spec/lib/gitlab/database/partitioning/multi_database_partition_manager_spec.rb new file mode 100644 index 00000000000..3c94c1bf4ea --- /dev/null +++ b/spec/lib/gitlab/database/partitioning/multi_database_partition_manager_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Partitioning::MultiDatabasePartitionManager, '#sync_partitions' do + subject(:sync_partitions) { manager.sync_partitions } + + let(:manager) { described_class.new(models) } + let(:models) { [model1, model2] } + + let(:model1) { double('model1', connection: connection1, table_name: 'table1') } + let(:model2) { double('model2', connection: connection1, table_name: 'table2') } + + let(:connection1) { double('connection1') } + let(:connection2) { double('connection2') } + + let(:target_manager_class) { Gitlab::Database::Partitioning::PartitionManager } + let(:target_manager1) { double('partition manager') } + let(:target_manager2) { double('partition manager') } + + before do + allow(manager).to receive(:connection_name).and_return('name') + end + + it 'syncs model partitions, setting up the appropriate connection for each', :aggregate_failures do + expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(model1.connection).and_yield.ordered + expect(target_manager_class).to receive(:new).with(model1).and_return(target_manager1).ordered + expect(target_manager1).to receive(:sync_partitions) + + expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(model2.connection).and_yield.ordered + expect(target_manager_class).to receive(:new).with(model2).and_return(target_manager2).ordered + expect(target_manager2).to receive(:sync_partitions) + + sync_partitions + end +end diff --git a/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb b/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb index 24ca0357b6e..8f1f5b5ba1b 100644 --- a/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb +++ b/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb @@ -12,31 +12,18 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do end end - describe '.register' do - let(:model) { double(partitioning_strategy: nil) } - - it 'remembers registered models' do - expect { described_class.register(model) }.to change { described_class.models }.to include(model) - end - - after do - # Do not leak the double to other specs - described_class.models.delete(model) - end - end - context 'creating partitions (mocked)' do - subject(:sync_partitions) { described_class.new(models).sync_partitions } + subject(:sync_partitions) { described_class.new(model).sync_partitions } - let(:models) { [model] } - let(:model) { double(partitioning_strategy: partitioning_strategy, table_name: table) } + let(:model) { double(partitioning_strategy: partitioning_strategy, table_name: table, connection: connection) } let(:partitioning_strategy) { double(missing_partitions: partitions, extra_partitions: []) } + let(:connection) { ActiveRecord::Base.connection } let(:table) { "some_table" } before do - allow(ActiveRecord::Base.connection).to receive(:table_exists?).and_call_original - allow(ActiveRecord::Base.connection).to receive(:table_exists?).with(table).and_return(true) - allow(ActiveRecord::Base.connection).to receive(:execute).and_call_original + allow(connection).to receive(:table_exists?).and_call_original + allow(connection).to receive(:table_exists?).with(table).and_return(true) + allow(connection).to receive(:execute).and_call_original stub_exclusive_lease(described_class::MANAGEMENT_LEASE_KEY % table, timeout: described_class::LEASE_TIMEOUT) end @@ -49,35 +36,23 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do end it 'creates the partition' do - expect(ActiveRecord::Base.connection).to receive(:execute).with(partitions.first.to_sql) - expect(ActiveRecord::Base.connection).to receive(:execute).with(partitions.second.to_sql) + expect(connection).to receive(:execute).with(partitions.first.to_sql) + expect(connection).to receive(:execute).with(partitions.second.to_sql) sync_partitions end - context 'error handling with 2 models' do - let(:models) do - [ - double(partitioning_strategy: strategy1, table_name: table), - double(partitioning_strategy: strategy2, table_name: table) - ] - end - - let(:strategy1) { double('strategy1', missing_partitions: nil, extra_partitions: []) } - let(:strategy2) { double('strategy2', missing_partitions: partitions, extra_partitions: []) } - - it 'still creates partitions for the second table' do - expect(strategy1).to receive(:missing_partitions).and_raise('this should never happen (tm)') - expect(ActiveRecord::Base.connection).to receive(:execute).with(partitions.first.to_sql) - expect(ActiveRecord::Base.connection).to receive(:execute).with(partitions.second.to_sql) + context 'when an error occurs during partition management' do + it 'does not raise an error' do + expect(partitioning_strategy).to receive(:missing_partitions).and_raise('this should never happen (tm)') - sync_partitions + expect { sync_partitions }.not_to raise_error end end end context 'creating partitions' do - subject(:sync_partitions) { described_class.new([my_model]).sync_partitions } + subject(:sync_partitions) { described_class.new(my_model).sync_partitions } let(:connection) { ActiveRecord::Base.connection } let(:my_model) do @@ -106,15 +81,15 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do context 'detaching partitions (mocked)' do subject(:sync_partitions) { manager.sync_partitions } - let(:manager) { described_class.new(models) } - let(:models) { [model] } - let(:model) { double(partitioning_strategy: partitioning_strategy, table_name: table)} + let(:manager) { described_class.new(model) } + let(:model) { double(partitioning_strategy: partitioning_strategy, table_name: table, connection: connection) } let(:partitioning_strategy) { double(extra_partitions: extra_partitions, missing_partitions: []) } + let(:connection) { ActiveRecord::Base.connection } let(:table) { "foo" } before do - allow(ActiveRecord::Base.connection).to receive(:table_exists?).and_call_original - allow(ActiveRecord::Base.connection).to receive(:table_exists?).with(table).and_return(true) + allow(connection).to receive(:table_exists?).and_call_original + allow(connection).to receive(:table_exists?).with(table).and_return(true) stub_exclusive_lease(described_class::MANAGEMENT_LEASE_KEY % table, timeout: described_class::LEASE_TIMEOUT) end @@ -136,24 +111,6 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do sync_partitions end - - context 'error handling' do - let(:models) do - [ - double(partitioning_strategy: error_strategy, table_name: table), - model - ] - end - - let(:error_strategy) { double(extra_partitions: nil, missing_partitions: []) } - - it 'still drops partitions for the other model' do - expect(error_strategy).to receive(:extra_partitions).and_raise('injected error!') - extra_partitions.each { |p| expect(manager).to receive(:detach_one_partition).with(p) } - - sync_partitions - end - end end context 'with the partition_pruning feature flag disabled' do @@ -176,7 +133,7 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do end end - subject { described_class.new([my_model]).sync_partitions } + subject { described_class.new(my_model).sync_partitions } let(:connection) { ActiveRecord::Base.connection } let(:my_model) do @@ -285,11 +242,11 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do it 'creates partitions for the future then drops the oldest one after a month' do # 1 month for the current month, 1 month for the old month that we're retaining data for, headroom expected_num_partitions = (Gitlab::Database::Partitioning::MonthlyStrategy::HEADROOM + 2.months) / 1.month - expect { described_class.new([my_model]).sync_partitions }.to change { num_partitions(my_model) }.from(0).to(expected_num_partitions) + expect { described_class.new(my_model).sync_partitions }.to change { num_partitions(my_model) }.from(0).to(expected_num_partitions) travel 1.month - expect { described_class.new([my_model]).sync_partitions }.to change { has_partition(my_model, 2.months.ago.beginning_of_month) }.from(true).to(false).and(change { num_partitions(my_model) }.by(0)) + expect { described_class.new(my_model).sync_partitions }.to change { has_partition(my_model, 2.months.ago.beginning_of_month) }.from(true).to(false).and(change { num_partitions(my_model) }.by(0)) end end end diff --git a/spec/lib/gitlab/database/partitioning_spec.rb b/spec/lib/gitlab/database/partitioning_spec.rb new file mode 100644 index 00000000000..f163b45e01e --- /dev/null +++ b/spec/lib/gitlab/database/partitioning_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Partitioning do + describe '.sync_partitions' do + let(:partition_manager_class) { described_class::MultiDatabasePartitionManager } + let(:partition_manager) { double('partition manager') } + + context 'when no partitioned models are given' do + it 'calls the partition manager with the registered models' do + expect(partition_manager_class).to receive(:new) + .with(described_class.registered_models) + .and_return(partition_manager) + + expect(partition_manager).to receive(:sync_partitions) + + described_class.sync_partitions + end + end + + context 'when partitioned models are given' do + it 'calls the partition manager with the given models' do + models = ['my special model'] + + expect(partition_manager_class).to receive(:new) + .with(models) + .and_return(partition_manager) + + expect(partition_manager).to receive(:sync_partitions) + + described_class.sync_partitions(models) + end + end + end +end diff --git a/spec/lib/gitlab/database/shared_model_spec.rb b/spec/lib/gitlab/database/shared_model_spec.rb new file mode 100644 index 00000000000..5d616aeb05f --- /dev/null +++ b/spec/lib/gitlab/database/shared_model_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::SharedModel do + describe 'using an external connection' do + let!(:original_connection) { described_class.connection } + let(:new_connection) { double('connection') } + + it 'overrides the connection for the duration of the block', :aggregate_failures do + expect_original_connection_around do + described_class.using_connection(new_connection) do + expect(described_class.connection).to be(new_connection) + end + end + end + + it 'does not affect connections in other threads', :aggregate_failures do + expect_original_connection_around do + described_class.using_connection(new_connection) do + expect(described_class.connection).to be(new_connection) + + Thread.new do + expect(described_class.connection).not_to be(new_connection) + end.join + end + end + end + + context 'when the block raises an error', :aggregate_failures do + it 're-raises the error, removing the overridden connection' do + expect_original_connection_around do + expect do + described_class.using_connection(new_connection) do + expect(described_class.connection).to be(new_connection) + + raise 'here comes an error!' + end + end.to raise_error(RuntimeError, 'here comes an error!') + end + end + end + + def expect_original_connection_around + # For safety, ensure our original connection is distinct from our double + # This should be the case, but in case of something leaking we should verify + expect(original_connection).not_to be(new_connection) + expect(described_class.connection).to be(original_connection) + + yield + + expect(described_class.connection).to be(original_connection) + end + end +end diff --git a/spec/lib/gitlab/devise_failure_spec.rb b/spec/lib/gitlab/devise_failure_spec.rb deleted file mode 100644 index a452de59795..00000000000 --- a/spec/lib/gitlab/devise_failure_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::DeviseFailure do - let(:env) do - { - 'REQUEST_URI' => 'http://test.host/', - 'HTTP_HOST' => 'test.host', - 'REQUEST_METHOD' => 'GET', - 'warden.options' => { scope: :user }, - 'rack.session' => {}, - 'rack.session.options' => {}, - 'rack.input' => "", - 'warden' => OpenStruct.new(message: nil) - } - end - - let(:response) { described_class.call(env).to_a } - let(:request) { ActionDispatch::Request.new(env) } - - context 'When redirecting' do - it 'sets the expire_after key' do - response - - expect(env['rack.session.options']).to have_key(:expire_after) - end - - it 'returns to the default redirect location' do - expect(response.first).to eq(302) - expect(request.flash[:alert]).to eq('You need to sign in or sign up before continuing.') - expect(response.second['Location']).to eq('http://test.host/users/sign_in') - end - end -end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 75168a3ad5e..3e264867703 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -91,6 +91,9 @@ RSpec.describe ApplicationSetting do .is_less_than(::Gitlab::Pages::MAX_SIZE / 1.megabyte) end + it { is_expected.to validate_presence_of(:jobs_per_stage_page_size) } + it { is_expected.to validate_numericality_of(:jobs_per_stage_page_size).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(7).for(:minimum_password_length) } it { is_expected.not_to allow_value(129).for(:minimum_password_length) } it { is_expected.not_to allow_value(nil).for(:minimum_password_length) } diff --git a/spec/models/concerns/partitioned_table_spec.rb b/spec/models/concerns/partitioned_table_spec.rb index c37fb81a1cf..714db4e21bd 100644 --- a/spec/models/concerns/partitioned_table_spec.rb +++ b/spec/models/concerns/partitioned_table_spec.rb @@ -35,11 +35,5 @@ RSpec.describe PartitionedTable do expect(my_class.partitioning_strategy.partitioning_key).to eq(key) end - - it 'registers itself with the PartitionCreator' do - expect(Gitlab::Database::Partitioning::PartitionManager).to receive(:register).with(my_class) - - subject - end end end diff --git a/spec/requests/api/graphql/ci/stages_spec.rb b/spec/requests/api/graphql/ci/stages_spec.rb index cd48a24b9c8..50d2cf75097 100644 --- a/spec/requests/api/graphql/ci/stages_spec.rb +++ b/spec/requests/api/graphql/ci/stages_spec.rb @@ -4,11 +4,13 @@ require 'spec_helper' RSpec.describe 'Query.project.pipeline.stages' do include GraphqlHelpers - let(:project) { create(:project, :repository, :public) } - let(:user) { create(:user) } - let(:pipeline) { create(:ci_pipeline, project: project, user: user) } - let(:stage_graphql_data) { graphql_data['project']['pipeline']['stages'] } + subject(:post_query) { post_graphql(query, current_user: user) } + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) } + + let(:stage_nodes) { graphql_data_at(:project, :pipeline, :stages, :nodes) } let(:params) { {} } let(:fields) do @@ -33,14 +35,42 @@ RSpec.describe 'Query.project.pipeline.stages' do ) end - before do + before_all do create(:ci_stage_entity, pipeline: pipeline, name: 'deploy') - post_graphql(query, current_user: user) + create_list(:ci_build, 2, pipeline: pipeline, stage: 'deploy') end - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query' do + before do + post_query + end + end it 'returns the stage of a pipeline' do - expect(stage_graphql_data['nodes'].first['name']).to eq('deploy') + post_query + + expect(stage_nodes.first['name']).to eq('deploy') + end + + describe 'job pagination' do + let(:job_nodes) { graphql_dig_at(stage_nodes, :jobs, :nodes) } + + it 'returns up to default limit jobs per stage' do + post_query + + expect(job_nodes.count).to eq(2) + end + + context 'when the limit is manually set' do + before do + stub_application_setting(jobs_per_stage_page_size: 1) + end + + it 'returns up to custom limit jobs per stage' do + post_query + + expect(job_nodes.count).to eq(1) + end + end end end diff --git a/spec/requests/api/issues/get_group_issues_spec.rb b/spec/requests/api/issues/get_group_issues_spec.rb index cebde747210..3663a82891c 100644 --- a/spec/requests/api/issues/get_group_issues_spec.rb +++ b/spec/requests/api/issues/get_group_issues_spec.rb @@ -402,14 +402,7 @@ RSpec.describe API::Issues do expect_paginated_array_response([group_closed_issue.id, group_issue.id]) end - shared_examples 'labels parameter' do - it 'returns an array of labeled group issues' do - get api(base_url, user), params: { labels: group_label.title } - - expect_paginated_array_response(group_issue.id) - expect(json_response.first['labels']).to eq([group_label.title]) - end - + context 'labels parameter' do it 'returns an array of labeled group issues' do get api(base_url, user), params: { labels: group_label.title } @@ -458,22 +451,6 @@ RSpec.describe API::Issues do end end - context 'when `optimized_issuable_label_filter` feature flag is off' do - before do - stub_feature_flags(optimized_issuable_label_filter: false) - end - - it_behaves_like 'labels parameter' - end - - context 'when `optimized_issuable_label_filter` feature flag is on' do - before do - stub_feature_flags(optimized_issuable_label_filter: true) - end - - it_behaves_like 'labels parameter' - end - it 'returns issues matching given search string for title' do get api(base_url, user), params: { search: group_issue.title } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index ac86c922813..527e548ad19 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -9,6 +9,7 @@ RSpec.describe API::Users do let_it_be(:gpg_key) { create(:gpg_key, user: user) } let_it_be(:email) { create(:email, user: user) } + let(:blocked_user) { create(:user, :blocked) } let(:omniauth_user) { create(:omniauth_user) } let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } let(:private_user) { create(:user, private_profile: true) } @@ -2626,16 +2627,14 @@ RSpec.describe API::Users do end context 'for a blocked user' do - before do - user.block - end + let(:user_id) { blocked_user.id } it 'returns 403' do activate expect(response).to have_gitlab_http_status(:forbidden) expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated') - expect(user.reload.state).to eq('blocked') + expect(blocked_user.reload.state).to eq('blocked') end end @@ -2723,16 +2722,14 @@ RSpec.describe API::Users do end context 'for a blocked user' do - before do - user.block - end + let(:user_id) { blocked_user.id } it 'returns 403' do deactivate expect(response).to have_gitlab_http_status(:forbidden) expect(json_response['message']).to eq('403 Forbidden - A blocked user cannot be deactivated by the API') - expect(user.reload.state).to eq('blocked') + expect(blocked_user.reload.state).to eq('blocked') end end @@ -2787,8 +2784,6 @@ RSpec.describe API::Users do describe 'POST /users/:id/approve' do subject(:approve) { post api("/users/#{user_id}/approve", api_user) } - let_it_be(:blocked_user) { create(:user, :blocked) } - context 'performed by a non-admin user' do let(:api_user) { user } let(:user_id) { pending_user.id } @@ -2923,7 +2918,6 @@ RSpec.describe API::Users do end context 'for a blocked user' do - let(:blocked_user) { create(:user, :blocked) } let(:user_id) { blocked_user.id } it 'does not reject a blocked user' do @@ -3006,7 +3000,6 @@ RSpec.describe API::Users do end context 'with a blocked user' do - let(:blocked_user) { create(:user, state: 'blocked') } let(:user_id) { blocked_user.id } it 'returns a 201 if user is already blocked' do @@ -3042,7 +3035,6 @@ RSpec.describe API::Users do end context 'with a blocked user' do - let(:blocked_user) { create(:user, state: 'blocked') } let(:user_id) { blocked_user.id } it 'unblocks a blocked user' do diff --git a/spec/support/helpers/migrations_helpers.rb b/spec/support/helpers/migrations_helpers.rb index ef212938af5..7799e49d4c1 100644 --- a/spec/support/helpers/migrations_helpers.rb +++ b/spec/support/helpers/migrations_helpers.rb @@ -30,7 +30,7 @@ module MigrationsHelpers end end - klass.tap { Gitlab::Database::Partitioning::PartitionManager.new.sync_partitions } + klass.tap { Gitlab::Database::Partitioning.sync_partitions([klass]) } end def migrations_paths diff --git a/spec/support/helpers/session_helpers.rb b/spec/support/helpers/session_helpers.rb new file mode 100644 index 00000000000..f58df2f438b --- /dev/null +++ b/spec/support/helpers/session_helpers.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module SessionHelpers + def expect_single_session_with_authenticated_ttl + expect_single_session_with_expiration(Settings.gitlab['session_expire_delay'] * 60) + end + + def expect_single_session_with_short_ttl + expect_single_session_with_expiration(Settings.gitlab['unauthenticated_session_expire_delay']) + end + + def expect_single_session_with_expiration(expiration) + session_keys = get_session_keys + + expect(session_keys.size).to eq(1) + expect(get_ttl(session_keys.first)).to eq expiration + end + + def get_session_keys + Gitlab::Redis::SharedState.with { |redis| redis.scan_each(match: 'session:gitlab:*').to_a } + end + + def get_ttl(key) + Gitlab::Redis::SharedState.with { |redis| redis.ttl(key) } + end +end diff --git a/spec/workers/database/partition_management_worker_spec.rb b/spec/workers/database/partition_management_worker_spec.rb index 01b7f209b2d..9ded36743a8 100644 --- a/spec/workers/database/partition_management_worker_spec.rb +++ b/spec/workers/database/partition_management_worker_spec.rb @@ -6,16 +6,14 @@ RSpec.describe Database::PartitionManagementWorker do describe '#perform' do subject { described_class.new.perform } - let(:manager) { instance_double('PartitionManager', sync_partitions: nil) } let(:monitoring) { instance_double('PartitionMonitoring', report_metrics: nil) } before do - allow(Gitlab::Database::Partitioning::PartitionManager).to receive(:new).and_return(manager) allow(Gitlab::Database::Partitioning::PartitionMonitoring).to receive(:new).and_return(monitoring) end - it 'delegates to PartitionManager' do - expect(manager).to receive(:sync_partitions) + it 'delegates to Partitioning' do + expect(Gitlab::Database::Partitioning).to receive(:sync_partitions) subject end diff --git a/yarn.lock b/yarn.lock index 5c3a022ca35..17e64b8cef7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -974,10 +974,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/tributejs/-/tributejs-1.0.0.tgz#672befa222aeffc83e7d799b0500a7a4418e59b8" integrity sha512-nmKw1+hB6MHvlmPz63yPwVs1qQkycHwsKgxpEbzmky16Y6mL4EJMk3w1b8QlOAF/AIAzjCERPhe/R4MJiohbZw== -"@gitlab/ui@32.10.1": - version "32.10.1" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-32.10.1.tgz#36c1e6688c5db85ddea7dcc1fc7e83ea8e3f091f" - integrity sha512-Z0qrhbrE1Y4XR9pUPZVLyjVUYOWwZHHwqw81mD3yP3sbSv0ZIxFRq7Ij0JOM1WPW74SM2nHPvSfwe3217xQkyg== +"@gitlab/ui@32.10.2": + version "32.10.2" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-32.10.2.tgz#d663faaddd5aecf52e7e97b598dfb71a14da60c5" + integrity sha512-WJEVktJjwUoTrR9z8xa3gx73dWwLTq+wsXYH0mfRWzkXHnvjdDp3h8cYj3s1NZ0eAPQ6yVrkvYrTTgLVHZgSbA== dependencies: "@babel/standalone" "^7.0.0" bootstrap-vue "2.18.1" |
