diff options
49 files changed, 1040 insertions, 377 deletions
diff --git a/.rubocop_todo/gitlab/feature_available_usage.yml b/.rubocop_todo/gitlab/feature_available_usage.yml index 05372071d6c..1cdc8040ff9 100644 --- a/.rubocop_todo/gitlab/feature_available_usage.yml +++ b/.rubocop_todo/gitlab/feature_available_usage.yml @@ -133,6 +133,5 @@ Gitlab/FeatureAvailableUsage: - 'ee/lib/incident_management/incident_sla.rb' - 'ee/spec/models/ee/project_spec.rb' - 'ee/spec/models/instance_security_dashboard_spec.rb' - - 'ee/spec/models/license_spec.rb' - 'lib/api/helpers/related_resources_helpers.rb' - 'spec/models/concerns/featurable_spec.rb' diff --git a/.rubocop_todo/rspec/context_wording.yml b/.rubocop_todo/rspec/context_wording.yml index ec3ac9ff220..ef2e1e1d45e 100644 --- a/.rubocop_todo/rspec/context_wording.yml +++ b/.rubocop_todo/rspec/context_wording.yml @@ -466,7 +466,6 @@ RSpec/ContextWording: - 'ee/spec/models/geo_node_spec.rb' - 'ee/spec/models/geo_node_status_spec.rb' - 'ee/spec/models/gitlab_subscription_spec.rb' - - 'ee/spec/models/gitlab_subscriptions/features_spec.rb' - 'ee/spec/models/group_member_spec.rb' - 'ee/spec/models/group_wiki_repository_spec.rb' - 'ee/spec/models/incident_management/escalation_rule_spec.rb' diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 04aa3c7aba6..e989e1f0910 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -11,7 +11,6 @@ RSpec/MissingFeatureCategory: - 'ee/spec/controllers/projects/merge_requests_controller_spec.rb' - 'ee/spec/controllers/projects/pipelines_controller_spec.rb' - 'ee/spec/controllers/projects/security/configuration_controller_spec.rb' - - 'ee/spec/controllers/projects_controller_spec.rb' - 'ee/spec/controllers/users_controller_spec.rb' - 'ee/spec/db/production/license_spec.rb' - 'ee/spec/elastic/migrate/20201105181100_apply_max_analyzed_offset_spec.rb' @@ -661,7 +660,6 @@ RSpec/MissingFeatureCategory: - 'ee/spec/lib/ee/gitlab/hook_data/group_member_builder_spec.rb' - 'ee/spec/lib/ee/gitlab/hook_data/issue_builder_spec.rb' - 'ee/spec/lib/ee/gitlab/hook_data/user_builder_spec.rb' - - 'ee/spec/lib/ee/gitlab/import_export/after_export_strategies/custom_template_export_import_strategy_spec.rb' - 'ee/spec/lib/ee/gitlab/import_export/group/tree_restorer_spec.rb' - 'ee/spec/lib/ee/gitlab/import_export/group/tree_saver_spec.rb' - 'ee/spec/lib/ee/gitlab/import_export/project/tree_saver_spec.rb' @@ -852,7 +850,6 @@ RSpec/MissingFeatureCategory: - 'ee/spec/lib/gitlab/import_export/group/relation_factory_spec.rb' - 'ee/spec/lib/gitlab/import_export/project/object_builder_spec.rb' - 'ee/spec/lib/gitlab/import_export/project/relation_factory_spec.rb' - - 'ee/spec/lib/gitlab/import_sources_spec.rb' - 'ee/spec/lib/gitlab/incident_management_spec.rb' - 'ee/spec/lib/gitlab/ingestion/bulk_insertable_task_spec.rb' - 'ee/spec/lib/gitlab/insights/executors/issuable_executor_spec.rb' @@ -1455,8 +1452,6 @@ RSpec/MissingFeatureCategory: - 'ee/spec/services/projects/after_rename_service_spec.rb' - 'ee/spec/services/projects/alerting/notify_service_spec.rb' - 'ee/spec/services/projects/cleanup_service_spec.rb' - - 'ee/spec/services/projects/create_from_template_service_spec.rb' - - 'ee/spec/services/projects/create_service_spec.rb' - 'ee/spec/services/projects/disable_deploy_key_service_spec.rb' - 'ee/spec/services/projects/disable_legacy_inactive_projects_service_spec.rb' - 'ee/spec/services/projects/enable_deploy_key_service_spec.rb' @@ -5502,7 +5497,6 @@ RSpec/MissingFeatureCategory: - 'spec/serializers/user_serializer_spec.rb' - 'spec/serializers/web_ide_terminal_entity_spec.rb' - 'spec/serializers/web_ide_terminal_serializer_spec.rb' - - 'spec/services/application_settings/update_service_spec.rb' - 'spec/services/applications/create_service_spec.rb' - 'spec/services/gpg_keys/destroy_service_spec.rb' - 'spec/services/metrics/dashboard/grafana_metric_embed_service_spec.rb' diff --git a/app/assets/javascripts/boards/components/board_form.vue b/app/assets/javascripts/boards/components/board_form.vue index 4986c3780e5..d12478b42d8 100644 --- a/app/assets/javascripts/boards/components/board_form.vue +++ b/app/assets/javascripts/boards/components/board_form.vue @@ -178,6 +178,9 @@ export default { }, methods: { ...mapActions(['setError', 'unsetError', 'setBoard']), + isFocusMode() { + return Boolean(document.querySelector('.content-wrapper > .js-focus-mode-board.is-focused')); + }, cancel() { this.$emit('cancel'); }, @@ -281,6 +284,7 @@ export default { modal-class="board-config-modal" content-class="gl-absolute gl-top-7" visible + :static="isFocusMode()" :hide-footer="readonly" :title="title" :action-primary="primaryProps" diff --git a/app/assets/javascripts/boards/components/board_settings_sidebar.vue b/app/assets/javascripts/boards/components/board_settings_sidebar.vue index 58db2c9ac2a..89e13625210 100644 --- a/app/assets/javascripts/boards/components/board_settings_sidebar.vue +++ b/app/assets/javascripts/boards/components/board_settings_sidebar.vue @@ -166,7 +166,7 @@ export default { <mounting-portal mount-to="#js-right-sidebar-portal" name="board-settings-sidebar" append> <gl-drawer v-bind="$attrs" - class="js-board-settings-sidebar gl-absolute boards-sidebar" + class="js-board-settings-sidebar boards-sidebar" :open="showSidebar" variant="sidebar" @close="unsetActiveListId" diff --git a/app/assets/javascripts/ci/pipeline_schedules/components/pipeline_schedules_form.vue b/app/assets/javascripts/ci/pipeline_schedules/components/pipeline_schedules_form.vue index ea019a95cd2..0c3ede47015 100644 --- a/app/assets/javascripts/ci/pipeline_schedules/components/pipeline_schedules_form.vue +++ b/app/assets/javascripts/ci/pipeline_schedules/components/pipeline_schedules_form.vue @@ -1,8 +1,7 @@ <script> import { GlButton, - GlDropdown, - GlDropdownItem, + GlCollapsibleListbox, GlFormCheckbox, GlForm, GlFormGroup, @@ -27,8 +26,7 @@ const scheduleId = queryToObject(window.location.search).id; export default { components: { GlButton, - GlDropdown, - GlDropdownItem, + GlCollapsibleListbox, GlForm, GlFormCheckbox, GlFormGroup, @@ -144,10 +142,6 @@ export default { revealText: __('Reveal values'), hideText: __('Hide values'), }, - typeOptions: { - [VARIABLE_TYPE]: __('Variable'), - [FILE_TYPE]: __('File'), - }, formElementClasses: 'gl-md-mr-3 gl-mb-3 gl-flex-basis-quarter gl-flex-shrink-0 gl-flex-grow-0', computed: { dropdownTranslations() { @@ -155,7 +149,7 @@ export default { dropdownHeader: this.$options.i18n.targetBranchTag, }; }, - typeOptionsListbox() { + typeOptions() { return [ { text: __('Variable'), @@ -232,9 +226,9 @@ export default { empty: true, }); }, - setVariableAttribute(key, attribute, value) { + setVariableType(typeValue, key) { const variable = this.variables.find((v) => v.key === key); - variable[attribute] = value; + variable.variableType = typeValue; }, removeVariable(index) { this.variables[index].destroy = true; @@ -387,19 +381,15 @@ export default { class="gl-display-flex gl-align-items-stretch gl-flex-direction-column gl-md-flex-direction-row gl-mb-3 gl-pb-2" data-testid="ci-variable-row" > - <gl-dropdown - :text="$options.typeOptions[variable.variableType]" + <gl-collapsible-listbox + :items="typeOptions" + :selected="variable.variableType" :class="$options.formElementClasses" + block data-testid="pipeline-form-ci-variable-type" - > - <gl-dropdown-item - v-for="type in Object.keys($options.typeOptions)" - :key="type" - @click="setVariableAttribute(variable.key, 'variableType', type)" - > - {{ $options.typeOptions[type] }} - </gl-dropdown-item> - </gl-dropdown> + @select="setVariableType($event, variable.key)" + /> + <gl-form-input v-model="variable.key" :placeholder="s__('CiVariables|Input variable key')" @@ -414,7 +404,6 @@ export default { value="*****************" disabled class="gl-mb-3 gl-h-7!" - :style="$options.textAreaStyle" :no-resize="false" data-testid="pipeline-form-ci-variable-hidden-value" /> @@ -424,7 +413,6 @@ export default { v-model="variable.value" :placeholder="s__('CiVariables|Input variable value')" class="gl-mb-3 gl-h-7!" - :style="$options.textAreaStyle" :no-resize="false" data-testid="pipeline-form-ci-variable-value" data-qa-selector="ci_variable_value_field" diff --git a/app/assets/javascripts/locale/ensure_single_line.cjs b/app/assets/javascripts/locale/ensure_single_line.cjs index f7790cadc48..abdd56c3589 100644 --- a/app/assets/javascripts/locale/ensure_single_line.cjs +++ b/app/assets/javascripts/locale/ensure_single_line.cjs @@ -1,15 +1,11 @@ const SPLIT_REGEX = /\s*[\r\n]+\s*/; /** - * - * strips newlines from strings and replaces them with a single space - * + * Strips newlines from strings and replaces them with a single space. * @example - * * ensureSingleLine('foo \n bar') === 'foo bar' - * - * @param {String} str - * @returns {String} + * @param {string} - str + * @returns {string} */ module.exports = function ensureSingleLine(str) { // This guard makes the function significantly faster diff --git a/app/assets/javascripts/locale/index.js b/app/assets/javascripts/locale/index.js index 600654794a5..48cf06e0793 100644 --- a/app/assets/javascripts/locale/index.js +++ b/app/assets/javascripts/locale/index.js @@ -19,22 +19,22 @@ if (hasTranslations) { } /** - Translates `text` - @param text The text to be translated - @returns {String} The translated text -*/ + * Translates `text`. + * @param {string} text - The text to be translated + * @returns {string} The translated text + */ const gettext = (text) => locale.gettext(ensureSingleLine(text)); /** - Translate the text with a number - if the number is more than 1 it will use the `pluralText` translation. - This method allows for contexts, see below re. contexts - - @param text Singular text to translate (eg. '%d day') - @param pluralText Plural text to translate (eg. '%d days') - @param count Number to decide which translation to use (eg. 2) - @returns {String} Translated text with the number replaced (eg. '2 days') -*/ + * Translate the text with a number. + * + * If the number is more than 1 it will use the `pluralText` translation. + * This method allows for contexts, see below re. contexts + * @param {string} text - Singular text to translate (e.g. '%d day') + * @param {string} pluralText - Plural text to translate (e.g. '%d days') + * @param {number} count - Number to decide which translation to use (e.g. 2) + * @returns {string} Translated text with the number replaced (e.g. '2 days') + */ const ngettext = (text, pluralText, count) => { const translated = locale .ngettext(ensureSingleLine(text), ensureSingleLine(pluralText), count) @@ -45,16 +45,16 @@ const ngettext = (text, pluralText, count) => { }; /** - Translate context based text - Either pass in the context translation like `Context|Text to translate` - or allow for dynamic text by doing passing in the context first & then the text to translate - - @param keyOrContext Can be either the key to translate including the context - (eg. 'Context|Text') or just the context for the translation - (eg. 'Context') - @param key Is the dynamic variable you want to be translated - @returns {String} Translated context based text -*/ + * Translate context based text. + * @example + * s__('Context|Text to translate'); + * @example + * s__('Context', 'Text to translate'); + * @param {string} keyOrContext - Context and a key to translation (e.g. 'Context|Text') + * or just a context (e.g. 'Context') + * @param {string} [key] - if `keyOrContext` is just a context, this is the key to translation + * @returns {string} Translated context based text + */ const pgettext = (keyOrContext, key) => { const normalizedKey = ensureSingleLine(key ? `${keyOrContext}|${key}` : keyOrContext); const translated = gettext(normalizedKey).split('|'); @@ -102,21 +102,19 @@ export const getPreferredLocales = () => { }; /** - Creates an instance of Intl.DateTimeFormat for the current locale. - - @param formatOptions for available options, please see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat - @returns {Intl.DateTimeFormat} -*/ + * Creates an instance of Intl.DateTimeFormat for the current locale. + * @param {Intl.DateTimeFormatOptions} [formatOptions] - for available options, please see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat + * @returns {Intl.DateTimeFormat} + */ const createDateTimeFormat = (formatOptions) => Intl.DateTimeFormat(getPreferredLocales(), formatOptions); /** * Formats a number as a string using `toLocaleString`. - * - * @param {Number} value - number to be converted - * @param {options?} options - options to be passed to - * `toLocaleString` such as `unit` and `style`. - * @param {langCode?} langCode - If set, forces a different + * @param {number} value - number to be converted + * @param {Intl.NumberFormatOptions} [options] - options to be passed to + * `toLocaleString`. + * @param {string|string[]} [langCode] - If set, forces a different * language code from the one currently in the document. * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat * diff --git a/app/assets/javascripts/locale/sprintf.js b/app/assets/javascripts/locale/sprintf.js index 12df67670f9..7faf9390684 100644 --- a/app/assets/javascripts/locale/sprintf.js +++ b/app/assets/javascripts/locale/sprintf.js @@ -1,17 +1,15 @@ import { escape } from 'lodash'; /** - Very limited implementation of sprintf supporting only named parameters. - - @param input (translated) text with parameters (e.g. '%{num_users} users use us') - @param {Object} parameters object mapping parameter names to values (e.g. { num_users: 5 }) - @param {Boolean} escapeParameters whether parameter values should be escaped (see https://lodash.com/docs/4.17.15#escape) - @returns {String} the text with parameters replaces (e.g. '5 users use us') - - @see https://ruby-doc.org/core-2.3.3/Kernel.html#method-i-sprintf - @see https://gitlab.com/gitlab-org/gitlab-foss/issues/37992 -*/ -export default (input, parameters, escapeParameters = true) => { + * Very limited implementation of sprintf supporting only named parameters. + * @param {string} input - (translated) text with parameters (e.g. '%{num_users} users use us') + * @param {Object.<string, string|number>} [parameters] - object mapping parameter names to values (e.g. { num_users: 5 }) + * @param {boolean} [escapeParameters=true] - whether parameter values should be escaped (see https://lodash.com/docs/4.17.15#escape) + * @returns {string} the text with parameters replaces (e.g. '5 users use us') + * @see https://ruby-doc.org/core-2.3.3/Kernel.html#method-i-sprintf + * @see https://gitlab.com/gitlab-org/gitlab-foss/issues/37992 + */ +export default function sprintf(input, parameters, escapeParameters = true) { let output = input; output = output.replace(/%+/g, '%'); @@ -29,4 +27,4 @@ export default (input, parameters, escapeParameters = true) => { } return output; -}; +} diff --git a/app/assets/javascripts/pages/projects/show/index.js b/app/assets/javascripts/pages/projects/show/index.js index c43a0eb597c..bee0731d711 100644 --- a/app/assets/javascripts/pages/projects/show/index.js +++ b/app/assets/javascripts/pages/projects/show/index.js @@ -1,13 +1,11 @@ import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation'; - import initClustersDeprecationAlert from '~/projects/clusters_deprecation_alert'; import leaveByUrl from '~/namespaces/leave_by_url'; import initVueNotificationsDropdown from '~/notifications'; -import Star from '~/projects/star'; +import { initStarButton } from '~/projects/project_star_button'; import initTerraformNotification from '~/projects/terraform_notification'; import { initUploadFileTrigger } from '~/projects/upload_file'; import initReadMore from '~/read_more'; - import initForksButton from '~/forks/init_forks_button'; // Project show page loads different overview content based on user preferences @@ -46,7 +44,7 @@ initClustersDeprecationAlert(); initTerraformNotification(); initReadMore(); -new Star(); // eslint-disable-line no-new +initStarButton(); if (document.querySelector('.js-autodevops-banner')) { import(/* webpackChunkName: 'userCallOut' */ '~/user_callout') diff --git a/app/assets/javascripts/projects/project_star_button.js b/app/assets/javascripts/projects/project_star_button.js new file mode 100644 index 00000000000..06f982b500d --- /dev/null +++ b/app/assets/javascripts/projects/project_star_button.js @@ -0,0 +1,46 @@ +import { createAlert } from '~/alert'; +import axios from '~/lib/utils/axios_utils'; +import { spriteIcon } from '~/lib/utils/common_utils'; +import { __, s__ } from '~/locale'; + +export function initStarButton(containerSelector = '.project-home-panel') { + const container = document.querySelector(containerSelector); + const starToggle = container?.querySelector('.toggle-star'); + + if (!starToggle) { + return; + } + + starToggle.addEventListener('click', function toggleStarClickCallback() { + const starSpan = starToggle.querySelector('span'); + const starIcon = starToggle.querySelector('svg'); + const iconClasses = Array.from(starIcon.classList.values()); + + axios + .post(starToggle.dataset.endpoint) + .then(({ data }) => { + const isStarred = starSpan.classList.contains('starred'); + starToggle.parentNode.querySelector('.count').textContent = data.star_count; + + if (isStarred) { + starSpan.classList.remove('starred'); + starSpan.textContent = s__('StarProject|Star'); + starIcon.remove(); + // eslint-disable-next-line no-unsanitized/method + starSpan.insertAdjacentHTML('beforebegin', spriteIcon('star-o', iconClasses)); + } else { + starSpan.classList.add('starred'); + starSpan.textContent = __('Unstar'); + starIcon.remove(); + + // eslint-disable-next-line no-unsanitized/method + starSpan.insertAdjacentHTML('beforebegin', spriteIcon('star', iconClasses)); + } + }) + .catch(() => + createAlert({ + message: __('Star toggle failed. Try again later.'), + }), + ); + }); +} diff --git a/app/assets/javascripts/projects/star.js b/app/assets/javascripts/projects/star.js deleted file mode 100644 index f294811dfff..00000000000 --- a/app/assets/javascripts/projects/star.js +++ /dev/null @@ -1,43 +0,0 @@ -import { createAlert } from '~/alert'; -import axios from '~/lib/utils/axios_utils'; -import { spriteIcon } from '~/lib/utils/common_utils'; -import { __, s__ } from '~/locale'; - -export default class Star { - constructor(containerSelector = '.project-home-panel') { - const container = document.querySelector(containerSelector); - const starToggle = container.querySelector('.toggle-star'); - starToggle?.addEventListener('click', function toggleStarClickCallback() { - const starSpan = starToggle.querySelector('span'); - const starIcon = starToggle.querySelector('svg'); - const iconClasses = Array.from(starIcon.classList.values()); - - axios - .post(starToggle.dataset.endpoint) - .then(({ data }) => { - const isStarred = starSpan.classList.contains('starred'); - starToggle.parentNode.querySelector('.count').textContent = data.star_count; - - if (isStarred) { - starSpan.classList.remove('starred'); - starSpan.textContent = s__('StarProject|Star'); - starIcon.remove(); - // eslint-disable-next-line no-unsanitized/method - starSpan.insertAdjacentHTML('beforebegin', spriteIcon('star-o', iconClasses)); - } else { - starSpan.classList.add('starred'); - starSpan.textContent = __('Unstar'); - starIcon.remove(); - - // eslint-disable-next-line no-unsanitized/method - starSpan.insertAdjacentHTML('beforebegin', spriteIcon('star', iconClasses)); - } - }) - .catch(() => - createAlert({ - message: __('Star toggle failed. Try again later.'), - }), - ); - }); - } -} diff --git a/app/assets/javascripts/work_items/components/shared/work_item_links_menu.vue b/app/assets/javascripts/work_items/components/shared/work_item_links_menu.vue index 53e8eedf060..12b7bade31d 100644 --- a/app/assets/javascripts/work_items/components/shared/work_item_links_menu.vue +++ b/app/assets/javascripts/work_items/components/shared/work_item_links_menu.vue @@ -1,29 +1,28 @@ <script> -import { GlIcon, GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import { GlDisclosureDropdown, GlDisclosureDropdownItem } from '@gitlab/ui'; export default { components: { - GlDropdownItem, - GlDropdown, - GlIcon, + GlDisclosureDropdownItem, + GlDisclosureDropdown, }, }; </script> <template> <div class="gl-ml-5"> - <gl-dropdown + <gl-disclosure-dropdown category="tertiary" toggle-class="btn-icon btn-sm" - :right="true" + icon="ellipsis_v" data-testid="work_items_links_menu" + :aria-label="__(`More actions`)" + text-sr-only + no-caret > - <template #button-content> - <gl-icon name="ellipsis_v" :size="14" /> - </template> - <gl-dropdown-item @click="$emit('removeChild')"> - {{ s__('WorkItem|Remove') }} - </gl-dropdown-item> - </gl-dropdown> + <gl-disclosure-dropdown-item @action="$emit('removeChild')"> + <template #list-item>{{ s__('WorkItem|Remove') }}</template> + </gl-disclosure-dropdown-item> + </gl-disclosure-dropdown> </div> </template> diff --git a/app/models/clusters/agent_token.rb b/app/models/clusters/agent_token.rb index d9a0f8d27d7..f4c497a42cc 100644 --- a/app/models/clusters/agent_token.rb +++ b/app/models/clusters/agent_token.rb @@ -2,10 +2,15 @@ module Clusters class AgentToken < ApplicationRecord + TOKEN_PREFIX = "glagent-" + include RedisCacheable include TokenAuthenticatable - add_authentication_token_field :token, encrypted: :required, token_generator: -> { Devise.friendly_token(50) } + add_authentication_token_field :token, + encrypted: :required, + token_generator: -> { Devise.friendly_token(50) }, + format_with_prefix: :glagent_prefix cached_attr_reader :last_used_at self.table_name = 'cluster_agent_tokens' @@ -31,5 +36,9 @@ module Clusters def to_ability_name :cluster end + + def glagent_prefix + TOKEN_PREFIX + end end end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 630f5ee868f..6be7f247ece 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -2613,6 +2613,15 @@ :weight: 1 :idempotent: false :tags: [] +- :name: database_lock_tables + :worker_name: Database::LockTablesWorker + :feature_category: :cell + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: default :worker_name: :feature_category: :not_owned diff --git a/app/workers/database/lock_tables_worker.rb b/app/workers/database/lock_tables_worker.rb new file mode 100644 index 00000000000..12c8e508ad5 --- /dev/null +++ b/app/workers/database/lock_tables_worker.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Database + class LockTablesWorker + include ApplicationWorker + + TableShouldNotBeLocked = Class.new(StandardError) + + sidekiq_options retry: false + feature_category :cell + data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency + idempotent! + + version 1 + + def perform(database_name, tables) + check_if_should_lock_database(database_name) + + connection = ::Gitlab::Database.database_base_models_with_gitlab_shared[database_name].connection + check_if_should_lock_tables(tables, database_name, connection) + + performed_actions = tables.map do |table_name| + lock_writes_manager(table_name, connection, database_name).lock_writes + end + + log_extra_metadata_on_done(:performed_actions, performed_actions) + end + + private + + def check_if_should_lock_database(database_name) + raise TableShouldNotBeLocked, 'GitLab is not running in multiple database mode' unless + Gitlab::Database.database_mode == Gitlab::Database::MODE_MULTIPLE_DATABASES + + raise TableShouldNotBeLocked, "database '#{database_name}' does not support locking writes on tables" unless + ::Gitlab::Database.database_base_models_with_gitlab_shared.include?(database_name) + end + + def check_if_should_lock_tables(tables, database_name, connection) + tables.each do |table_name| + unless should_lock_writes_on_table?(connection, database_name, table_name) + raise TableShouldNotBeLocked, "table '#{table_name}' should not be locked on the database '#{database_name}'" + end + end + end + + def should_lock_writes_on_table?(connection, database_name, table_name) + db_info = Gitlab::Database.all_database_connections.fetch(database_name) + table_schema = Gitlab::Database::GitlabSchema.table_schema!(table_name.to_s) + + Gitlab::Database.gitlab_schemas_for_connection(connection).exclude?(table_schema) && + db_info.lock_gitlab_schemas.include?(table_schema) + end + + def lock_writes_manager(table_name, connection, database_name) + Gitlab::Database::LockWritesManager.new( + table_name: table_name, + connection: connection, + database_name: database_name, + with_retries: !connection.transaction_open?, + logger: nil, + dry_run: false + ) + end + end +end diff --git a/app/workers/database/monitor_locked_tables_worker.rb b/app/workers/database/monitor_locked_tables_worker.rb index 66296ea1c0d..4a23d25edf4 100644 --- a/app/workers/database/monitor_locked_tables_worker.rb +++ b/app/workers/database/monitor_locked_tables_worker.rb @@ -33,6 +33,13 @@ module Database handle_lock_writes_result(tables_lock_info_per_db, result) end + tables_lock_info_per_db.each do |database_name, database_results| + next if database_results[:tables_need_lock].empty? + break if Feature.disabled?(:lock_tables_in_monitoring, type: :ops) + + LockTablesWorker.perform_async(database_name, database_results[:tables_need_lock]) + end + log_extra_metadata_on_done(:results, tables_lock_info_per_db) end diff --git a/config/feature_flags/development/product_analytics_snowplow_support.yml b/config/feature_flags/development/feature_available_check_with_usage_ping.yml index f3684a3f26b..649ad9d9e1b 100644 --- a/config/feature_flags/development/product_analytics_snowplow_support.yml +++ b/config/feature_flags/development/feature_available_check_with_usage_ping.yml @@ -1,8 +1,8 @@ --- -name: product_analytics_snowplow_support -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116317 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/403418 -milestone: '15.11' +name: feature_available_check_with_usage_ping +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129723 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/422614 +milestone: '16.4' type: development -group: group::product analytics +group: group::pipeline execution default_enabled: false diff --git a/config/feature_flags/ops/lock_tables_in_monitoring.yml b/config/feature_flags/ops/lock_tables_in_monitoring.yml new file mode 100644 index 00000000000..5f6ffdff77b --- /dev/null +++ b/config/feature_flags/ops/lock_tables_in_monitoring.yml @@ -0,0 +1,8 @@ +--- +name: lock_tables_in_monitoring +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129748 +rollout_issue_url: +milestone: '16.4' +type: ops +group: group::tenant scale +default_enabled: true diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 16c9a367a1a..8600b3114c0 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -177,6 +177,8 @@ - 1 - - cronjob - 1 +- - database_lock_tables + - 1 - - default - 1 - - delete_diff_files diff --git a/db/post_migrate/20230804122825_add_unique_index_on_uuid_convert_string_to_uuid.rb b/db/post_migrate/20230804122825_add_unique_index_on_uuid_convert_string_to_uuid.rb new file mode 100644 index 00000000000..ac94bca4de4 --- /dev/null +++ b/db/post_migrate/20230804122825_add_unique_index_on_uuid_convert_string_to_uuid.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddUniqueIndexOnUuidConvertStringToUuid < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = "index_vulnerability_occurrences_on_uuid_1" + + def up + add_concurrent_index( + :vulnerability_occurrences, + :uuid_convert_string_to_uuid, + unique: true, + name: INDEX_NAME + ) + end + + def down + remove_concurrent_index_by_name( + :vulnerability_occurrences, + INDEX_NAME + ) + end +end diff --git a/db/post_migrate/20230804123252_add_unique_index_on_uuid_convert_string_to_uuid_including_vulnerability_id.rb b/db/post_migrate/20230804123252_add_unique_index_on_uuid_convert_string_to_uuid_including_vulnerability_id.rb new file mode 100644 index 00000000000..7906ed48db6 --- /dev/null +++ b/db/post_migrate/20230804123252_add_unique_index_on_uuid_convert_string_to_uuid_including_vulnerability_id.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddUniqueIndexOnUuidConvertStringToUuidIncludingVulnerabilityId < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = "index_vuln_findings_on_uuid_including_vuln_id_1" + + def up + execute <<~SQL + CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS #{INDEX_NAME} + ON vulnerability_occurrences(uuid_convert_string_to_uuid) + INCLUDE(vulnerability_id) + SQL + end + + def down + remove_concurrent_index_by_name( + :vulnerability_occurrences, + INDEX_NAME + ) + end +end diff --git a/db/schema_migrations/20230804122825 b/db/schema_migrations/20230804122825 new file mode 100644 index 00000000000..ec59e1dbb30 --- /dev/null +++ b/db/schema_migrations/20230804122825 @@ -0,0 +1 @@ +9db599bb939f27e81035c8aa22ae7fbb4d1376f5a9075b684b109d15da312faf
\ No newline at end of file diff --git a/db/schema_migrations/20230804123252 b/db/schema_migrations/20230804123252 new file mode 100644 index 00000000000..5e1e5cf0c9d --- /dev/null +++ b/db/schema_migrations/20230804123252 @@ -0,0 +1 @@ +a0c3883429cee1f9a3f170d3a563f9390937dba5cab2b661dc583aa7ffe1479f
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 4ba01a8903e..c453e9bb25f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -33846,6 +33846,8 @@ COMMENT ON INDEX index_verification_codes_on_phone_and_visitor_id_code IS 'JiHu- CREATE UNIQUE INDEX index_vuln_findings_on_uuid_including_vuln_id ON vulnerability_occurrences USING btree (uuid) INCLUDE (vulnerability_id); +CREATE UNIQUE INDEX index_vuln_findings_on_uuid_including_vuln_id_1 ON vulnerability_occurrences USING btree (uuid_convert_string_to_uuid) INCLUDE (vulnerability_id); + CREATE UNIQUE INDEX index_vuln_historical_statistics_on_project_id_and_date ON vulnerability_historical_statistics USING btree (project_id, date); CREATE INDEX index_vuln_reads_common_query_on_resolved_on_default_branch ON vulnerability_reads USING btree (project_id, state, report_type, vulnerability_id DESC) WHERE (resolved_on_default_branch IS TRUE); @@ -33964,6 +33966,8 @@ CREATE INDEX index_vulnerability_occurrences_on_scanner_id ON vulnerability_occu CREATE UNIQUE INDEX index_vulnerability_occurrences_on_uuid ON vulnerability_occurrences USING btree (uuid); +CREATE UNIQUE INDEX index_vulnerability_occurrences_on_uuid_1 ON vulnerability_occurrences USING btree (uuid_convert_string_to_uuid); + CREATE INDEX index_vulnerability_occurrences_on_vulnerability_id ON vulnerability_occurrences USING btree (vulnerability_id); CREATE INDEX index_vulnerability_reads_common_finder_query_2 ON vulnerability_reads USING btree (project_id, state, report_type, severity, vulnerability_id DESC, dismissal_reason); diff --git a/doc/architecture/blueprints/cells/glossary.md b/doc/architecture/blueprints/cells/glossary.md index 11a1fc5acc9..69824663867 100644 --- a/doc/architecture/blueprints/cells/glossary.md +++ b/doc/architecture/blueprints/cells/glossary.md @@ -1,106 +1,6 @@ --- -stage: enablement -group: Tenant Scale -description: 'Cells: Glossary' +redirect_to: 'goals.md#glossary' +remove_date: '2023-11-24' --- -# Cells: Glossary - -We use the following terms to describe components and properties of the Cells architecture. - -## Cell - -> Pod was renamed to Cell in <https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/121163> - -A Cell is a set of infrastructure components that contains multiple top-level groups that belong to different organizations. The components include both datastores (PostgreSQL, Redis etc.) and stateless services (web etc.). The infrastructure components provided within a Cell are shared among organizations and their top-level groups but not shared with other Cells. This isolation of infrastructure components means that Cells are independent from each other. - -<img src="diagrams/term-cell.drawio.png" height="200"> - -### Cell properties - -- Each cell is independent from the others -- Infrastructure components are shared by organizations and their top-level groups within a Cell -- More Cells can be provisioned to provide horizontal scalability -- A failing Cell does not lead to failure of other Cells -- Noisy neighbor effects are limited to within a Cell -- Cells are not visible to organizations; it is an implementation detail -- Cells may be located in different geographical regions (for example, EU, US, JP, UK) - -Discouraged synonyms: GitLab instance, cluster, shard - -## Cluster - -A cluster is a collection of Cells. - -<img src="diagrams/term-cluster.drawio.png" height="300"> - -### Cluster properties - -- A cluster holds cluster-wide metadata, for example Users, Routes, Settings. - -Discouraged synonyms: whale - -## Organizations - -GitLab references [Organizations in the initial set up](../../../topics/set_up_organization.md) and users can add a (free text) organization to their profile. There is no Organization entity established in the GitLab codebase. - -As part of delivering Cells, we propose the introduction of an `organization` entity. Organizations would represent billable entities or customers. - -Organizations are a known concept, present for example in [AWS](https://docs.aws.amazon.com/whitepapers/latest/organizing-your-aws-environment/core-concepts.html) and [GCP](https://cloud.google.com/resource-manager/docs/cloud-platform-resource-hierarchy#organizations). - -Organizations work under the following assumptions: - -1. Users care about what happens within their organizations. -1. Features need to work within an organization. -1. Only few features need to work across organizations. -1. Users understand that the majority of pages they view are only scoped to a single organization at a time. -1. Organizations are located on a single cell. - - - -### Organization properties - -- Top-level groups belong to organizations -- Organizations are isolated from each other by default meaning that cross-group features will only work for group that exist within a single organization -- User namespaces must not belong to an organization - -Discouraged synonyms: Billable entities, customers - -## Top-Level group - -Top-level group is the name given to the top most group of all other groups. Groups and projects are nested underneath the top-level group. - -Example: - -`https://gitlab.com/gitlab-org/gitlab/`: - -- `gitlab-org` is a `top-level group`; the root for all groups and projects of an organization -- `gitlab` is a `project`; a project of the organization. - -The top-level group has served as the defacto Organization entity. With the creation of Organization, top-level groups will be [nested underneath Organizations](https://gitlab.com/gitlab-org/gitlab/-/issues/394796). - -Over time there won't be a distinction between a top-level group and a group. All features that make Top-level groups different from groups will move to Organization. - -Discouraged synonyms: Root-level namespace - - - -### Top-level group properties - -- Top-level groups belonging to an organization are located on the same Cell -- Top-level groups can interact with other top-level groups that belong to the same organization - -## Users - -Users are available globally and not restricted to a single Cell. Users belong to a single organization, but can participate in many organizations through group and project membership with varying permissions. Inside organizations, users can create multiple top-level groups. User activity is not limited to a single organization but their contributions (for example TODOs) are only aggregated within an organization. This avoids the need for aggregating across cells. - -### User properties - -- Users are shared globally across all Cells -- Users can create multiple top-level groups -- Users can be a member of multiple top-level groups -- Users belong to one organization. See [!395736](https://gitlab.com/gitlab-org/gitlab/-/issues/395736) -- Users can be members of groups and projects in different organizations -- Users can administer organizations -- User activity is aggregated in an organization -- Every user has one personal namespace +This document was moved to [another location](goals.md#glossary). diff --git a/doc/architecture/blueprints/cells/goals.md b/doc/architecture/blueprints/cells/goals.md index 3f3923aa255..f1a255aa879 100644 --- a/doc/architecture/blueprints/cells/goals.md +++ b/doc/architecture/blueprints/cells/goals.md @@ -6,7 +6,9 @@ description: 'Cells: Goals' # Cells: Goals -## Scalability +## Goals + +### Scalability The main goal of this new shared-infrastructure architecture is to provide additional scalability for our SaaS Platform. GitLab.com is largely monolithic and we have estimated (internally) that the current architecture has scalability limitations, @@ -16,34 +18,41 @@ even when database partitioning and decomposition are taken into account. Cells provide a horizontally scalable solution because additional Cells can be created based on demand. Cells can be provisioned and tuned as needed for optimal scalability. -## Increased availability +### Increased availability -A major challenge for shared-infrastructure architectures is a lack of isolation between top-level groups. This can lead to noisy neighbor effects. A organization's behavior inside a top-level group can impact all other organizations. This is highly undesirable. Cells provide isolation at the cell level. A group of organizations is fully isolated from other organizations located on a different Cell. This minimizes noisy neighbor effects while still benefiting from the cost-efficiency of shared infrastructure. +A major challenge for shared-infrastructure architectures is a lack of isolation between top-level Groups. +This can lead to noisy neighbor effects. +An organization's behavior inside a top-level Group can impact all other organizations. +This is highly undesirable. +Cells provide isolation at the Cell level. +A group of Organizations is fully isolated from other Organizations located on a different Cell. +This minimizes noisy neighbor effects while still benefiting from the cost-efficiency of a shared infrastructure. -Additionally, Cells provide a way to implement disaster recovery capabilities. Entire Cells may be replicated to read-only standbys with automatic failover capabilities. +Additionally, Cells provide a way to implement disaster recovery capabilities. +Entire Cells may be replicated to read-only standbys with automatic failover capabilities. -## A consistent experience +### A consistent experience Organizations should have the same user experience on our SaaS platform as they do on a self-managed GitLab instance. -## Regions - -GitLab.com is only hosted within the United States of America. Organizations located in other regions have voiced demand for local SaaS offerings. Cells provide a path towards [GitLab Regions](https://gitlab.com/groups/gitlab-org/-/epics/6037) because Cells may be deployed within different geographies. Depending on which of the organization's data is located outside a Cell, this may solve data residency and compliance problems. +### Regions -## Market segment +GitLab.com is only hosted within the United States of America. +Organizations located in other regions have voiced demand for local SaaS offerings. +Cells provide a path towards [GitLab Regions](https://gitlab.com/groups/gitlab-org/-/epics/6037) because Cells may be deployed within different geographies. +Depending on which of an organization's data is located outside a Cell, this may solve data residency and compliance problems. -Cells would provide a solution for organizations in the small to medium business (up to 100 users) and the mid-market segment (up to 2000 users). -(See [segmentation definitions](https://about.gitlab.com/handbook/sales/field-operations/gtm-resources/#segmentation).) -Larger organizations may benefit substantially from [GitLab Dedicated](../../../subscriptions/gitlab_dedicated/index.md). +### Market segment -At this moment, GitLab.com has "social-network"-like capabilities that may not fit well into a more isolated organization model. Removing those features, however, possesses some challenges: +At this moment, GitLab.com has "social network"-like capabilities that may not fit well into a more isolated Organization model. +Removing those features, however, poses some challenges: -1. How will existing `gitlab-org` contributors contribute to the namespace?? -1. How do we move existing top-level groups into the new model (effectively breaking their social features)? +1. How will existing `gitlab-org` contributors contribute to the namespace? +1. How do we move existing top-level Groups into the new model (effectively breaking their social features)? -We should evaluate if the SMB and mid market segment is interested in these features, or if not having them is acceptable in most cases. +We should evaluate if the small to medium business and mid-market segment is interested in these features, or if not having them is acceptable in most cases. -## Self-managed +### Self-managed For reasons of consistency, it is expected that self-managed instances will adopt the cells architecture as well. To expand, self-managed instances can @@ -51,13 +60,341 @@ continue with just a single Cell while supporting the option of adding additiona Cells. Organizations, and possible User decomposition will also be adopted for self-managed instances. -## High-level architecture problems to solve +## Requirements + +| Type | Requirement | Severity | +| ----------- | ------------------------------------------------- | -------- | +| Product | Aggregation of cluster-wide data | Medium | +| Product | All Cells are under a single GitLab.com domain | High | +| Product | User can interact with many Cells | Medium | +| Product | Minimal impact on contributor workflows | High | +| Product | On-premise like experience | Medium | +| Product | Minimize breaking changes | High | +| Product | Enables support for Regions | Low | +| Product | Limit impact on self-managed customers | Low | +| Operational | Provides 10x headroom | High | +| Operational | Provides 100x headroom | Medium | +| Operational | Improve Service Availability | High | +| Operational | Cost per user similar or lower to GitLab.com | Medium | +| Operational | Unified way of deploying Cells | Medium | +| Operational | Cells running in mixed deployments | High | +| Operational | High resilience to a single Cell failure | High | +| Operational | Small Cells | Medium | +| Migration | Robust rollback and disaster recovery scenarios | High | +| Migration | Scale existing GitLab.com database | High | +| Migration | Re-balancing of Cells | Low | +| Migration | Customer can migrate from GitLab.com to Dedicated | Low | +| Development | Easy usage in development environment | Medium | + +### Aggregation of cluster-wide data + +The architecture should provide a way to present the cluster of Cells in a single view. +This might mean to: + +- Aggregate user To-Dos from different Organizations +- Perform cluster-wide searches of public Projects, or published source code + +### All Cells are under a single GitLab.com domain + +General users should be unaware of the presence of Cells. +Cells would only be visible to instance administrators. +Users using Organizations on another Cell or us migrating Organizations +between Cells should be an operational detail that does not require +any user intervention or is not visible to the user. + +### User can interact with many Cells + +Usage of many accounts to interact with different Organizations +that might be on different Cells is strongly discouraged. +The need to use many accounts will reduce adoption and put a barrier +on users wanting to contribute to open-source Projects. + +In the future, users could have Organization specific settings, +depending on the specific customer expectations. +For instance, users might be shown with different display names in different Organizations. + +Users should not be required to use different SSH keys to access different +Organizations due to the complexity of managing many SSH keys by the user. + +### Minimal impact on contributor workflows + +GitLab.com has a number of open-source and open-core projects (including [`gitlab-org/gitlab`](https://gitlab.com/gitlab-org/gitlab)). +Cells should not make it harder to contribute to public Projects. +Learning new ways to contribute is likely to hinder the adoption of Cells. +The introduced architecture should focus on changing the existing workflows in the most minimal way. + +### On-premise like experience + +Currently on-premise has numerous advantages compared to our SaaS offering. +On-premise allows to control all aspects of a GitLab installation, including but not limited to: managing users, access controls, or instance-wide settings. + +The difference between SaaS and on-premise is problematic for our customers, +and us as it results in a different user experience. + +### Minimize breaking changes + +The introduction of Cells will imply changes in the supported GitLab.com workflows. +Cells might affect how people use GitLab, as long as it provides a compelling user experience story. +It is desired that each change introduced provides either operational value or user value, ideally both. + +Any Cells architecture should focus on not introducing breaking changes, +or if introducing them provide a long window to support the change. +The introduction of breaking changes will reduce the adoption rate +and effectiveness of Cells and make rollout of the architecture more risky. + +It is important to distinguish between different severity of breaking changes: + +- Required: migration to a new architecture by design introduces a breaking change. + In such a case a customer has to use a new workflow, new API, or new endpoints right away + and is required to adapt to all changes at once. The customer has limited options to roll back. + +- Optional: migration to a new architecture does not enforce introducing breaking changes. + In such a case a customer can slowly adapt to introduced changes: a new workflow, new API, + or new endpoints, but retaining major aspects of the system to work as before. + The customer has a way to roll back to a previous known working state. + +### Enables support for Regions + +Support for Regions should enable us to run GitLab in different availability zones, +or completely different data centers. + +This includes, but is not limited to allowing users to create Organizations in Europe, the US West or US East, +and GitLab Inc. to use different cloud infrastructure providers (Google Cloud, AWS) to serve customers. + +Cells enable existing customers running Organizations to move their Organization between different deployment regions/data centers that are supported by GitLab on GitLab.com. + +### Limit impact on self-managed customers (Omnibus/CNG) + +The introduction of Cells should not impact small installations. +Cells should not require running additional components by self-managed customers +unless they are interested in Cells that will increase resource requirements. + +### Provides 10x headroom + +The Cells architecture must provide at least 10x headroom. As such our architecture must be suitable to run 10 Cells. We do not need to initially solve for the complexities that might come with running more than 10 Cells. + +### Provides 100x headroom + +The Cells architecture should be able to run more than 10 Cells. + +### Improve Service Availability + +The Cells architecture should allow us to provide better SLA for some customers +by putting them on specific Cells of the cluster: + +- Cells have to reduce the impact of noisy neighbors, such as legitimate usage caused by a spiky workload. +- Cells have to reduce the impact caused by abuse, for instance CI being used for cryptocurrency mining. + +### Cost per user similar or lower to GitLab.com + +The current GitLab.com architecture is rather cost-effective. +The introduction of Cells will result in more infrastructure components +due to data distribution: + +- The proposed Cells architecture should evaluate the cost of running additional Cells with respect + to the achieved infrastructure component isolation. In some cases it might be beneficial to reuse + infrastructure components to reduce the running cost of Cells. +- The proposed Cells architecture should ensure a high degree of multi-tenancy on Cells. +- The proposed Cells architecture could enable a way to balance the Cells load long term. + +### Unified way of deploying Cells + +The proposed Cells architecture anticipates a need to run 100 Cells and more in a cluster. +This becomes operational overhead. It is strongly desired for the Cells architecture +to reuse existing infrastructure tooling as much as possible for: deployment, monitoring, +logging, disaster recovery, and customer support. + +### Cells running in mixed deployments + +It is strongly required for the Cells architecture to run different versions of applications across the cluster. The purpose is to provide staged deployments allowing to test changes on part of the cluster, and to update part of the cluster less frequently. + +This also indicates that Cells should be able to work with different versions of the underlying +infrastructure components: PostgreSQL version, Redis version, Gitaly version, Elasticsearch version. + +Usage of different versions within a Cell has to have no effect on other Cells. + +### High resilience to a single Cell failure + +Unavailability of a single Cell should not result in other Cells being unable to function properly: + +- Intra-cluster communication between Cells should be resilient to single Cell failure. +- Intra-cluster communication should limit its dependence on data stored in other Cells. +- Cluster-shared data should provide an anti-corruption layer to prevent a single Cell from causing a cluster-wide outage. +- Cells should be monitored and their availability status should be accessible to all other Cells. +- Cluster-wide data aggregation should take the availability status of each Cell into account. +- The loss of cell-local data (groups and projects) does only affect ability to access data located on this cell. + +### Small Cells + +The Cells architecture should provide a cost effective way to run small Cells. +Running small Cells allows to achieve superior quality in terms of latency or performance +due to processing a significantly smaller data set and allowing significant single-node vertical scaling. + +The ability to run small Cells could result in reduced complexity of individual Cell deployment: + +- Prefer single-node vertical scaling as a first technique for increasing capacity (increase CPU count, available memory). +- Reduce amount of data stored within a Cell to reduce time-to-recovery in case of outages. +- Less data results in faster database migrations, improved latency, and superior user-facing performance. + +### Robust rollback and disaster recovery scenarios + +Rollout of the Cells architecture is a fundamental change to how GitLab.com will operate. +This will involve introducing a number of new components, and distributing data horizontally across +different Cells. + +We anticipate that something will go wrong at some point. Each deployment phase of the Cells architecture needs to provide a way to roll back to a prior working state. This includes any user-facing changes, as well as any infrastructure deployed components. + +### Scale existing GitLab.com database + +The existing GitLab.com database will continue to grow while we deploy Cells. +The Cells architecture should provide a way to increase Cell 1 (existing GitLab.com) +capacity as soon as possible. + +### Re-balancing of Cells + +It is expected that data distribution on Cells will become uneven at some point. + +The problem of re-balancing Cells is rather complex, but at some scale it might +provide a positive ROI compared to the running cost of imbalanced Cells. + +This might involve understanding how to selectively migrate customer data between +Cells onto already existing Cells. + +### Customer can migrate from GitLab.com to Dedicated + +Cells will enforce an architecture with more isolation. However, it is expected +that some customers would like to move from SaaS GitLab.com to GitLab Dedicated. + +This implies being able to selectively migrate data from the GitLab.com cluster to Dedicated. +This could be achieved by: + +- Migrating a customer to a Dedicated Cell on the GitLab.com cluster. +- Disconnecting a Cell from the GitLab.com cluster into its own standalone Dedicated Cell. + +### Easy usage in development environment + +The Cells architecture should be easy to run locally by developers as needed to be able to: + +- Implement cluster-wide features. +- Model Cells not being available. + +## Non-Goals + +The following objectives are not part of this document. At some point they +were considered, but dismissed as creating distraction from the goals and requirements documented above. + +### Federation between distinct GitLab instances + +The Cells architecture is meant to provide trusted intra-cluster communication +with some set of data being shared. Federation is meant to solve the problem +of data flow between two completely distinct and external instances. + +### Usage of cloud managed services + +Cells should not interfere with other efforts to move towards using more managed services in the cloud. + +### Cells should replace canary deployments + +When we have Cells we won't need to run canary deployments anymore as we can roll out changes to single Cells at a time. +Our main benefit of canary deployments today is that we can roll out code changes to internal users first. +With Cells, we will have internal users on a specific Cell, which can be deployed first. + +Additionally Cells may solve some issues with canaries today, for example not having a way to upgrade Sidekiq code for a subset of users. + +## Glossary + +| Term | Description | Discouraged Terms | +| ----------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------ | +| [Cell](#cell) | A Cell is a set of infrastructure components that contains multiple top-level Groups that belong to different Organizations. | GitLab instance, cluster, shard, Pod | +| [Cluster](#cluster) | A cluster is a collection of Cells. | Whale, GitLab Dedicated instance, instance | +| [Organizations](#organizations) | An Organization is the umbrella for one or multiple top-level Groups. | Billable entities, customers | +| [Top-Level Group](#top-level-group) | Top-level Group is the name given to the top-most Group of all other Groups. Groups and Projects are nested underneath the top-level Group. | Root-level namespace | +| [Users](#users) | An account containing its own personal namespace associated with an email. | Customer | + +### Cell + +> Pod was renamed to Cell in <https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/121163> + +A Cell is a set of infrastructure components that contains multiple top-level Groups that belong to different Organizations. The components include both data stores (PostgreSQL, Redis, etc.) and stateless services (web, etc.). The infrastructure components provided within a Cell are shared among Organizations and their top-level Groups, but are not shared with other Cells. This isolation of infrastructure components means that Cells are independent from each other. + +<img src="diagrams/term-cell.drawio.png" height="200"> + +- Each Cell is independent from other Cells +- Infrastructure components are shared by Organizations and their top-level Groups within a Cell +- More Cells can be provisioned to provide horizontal scalability +- A failing Cell does not lead to failure of other Cells +- Noisy neighbor effects are limited to within a Cell +- Cells are not visible to Organizations - they are an implementation detail +- Cells may be located in different geographical regions (for example: EU, US, JP, UK) +- A GitLab Dedicated instance can join the cluster, and become a Cell + +Discouraged synonyms: GitLab instance, cluster, shard, Pod + +### Cluster + +A cluster is a collection of Cells. + +<img src="diagrams/term-cluster.drawio.png" height="300"> + +- A cluster holds cluster-wide metadata, for example: Users, Routes, Settings. + +Discouraged synonyms: Whale, GitLab Dedicated instance, instance + +### Organizations + +An Organization is the umbrella for one or multiple top-level Groups. Organizations are isolated from each other by default meaning that cross-namespace features will only work for namespaces that exist in a single Organization. + + + +See the [Organization blueprint](../../blueprints/organization/index.md). + +Organizations are a known concept, present for example in [AWS](https://docs.aws.amazon.com/whitepapers/latest/organizing-your-aws-environment/core-concepts.html) and [GCP](https://cloud.google.com/resource-manager/docs/cloud-platform-resource-hierarchy#organizations). + +Organizations work under the following assumptions: + +1. Users care about what happens within their Organization. +1. Features need to work within an Organization. +1. Only few features need to work across Organizations. +1. Users understand that the majority of pages they view are only scoped to a single Organization at a time. +1. Organizations are located on a single Cell. + +Properties: + +- Top-level Groups belong to Organizations. +- Organizations are isolated from each other by default, meaning that cross-Group features will only work for Groups that exist within a single Organization. +- Personal namespaces must not belong to an Organization. + +Discouraged synonyms: Billable entities, customers + +### Top-level Group + +Example: + +`https://gitlab.com/gitlab-org/gitlab/`: + +- `gitlab-org` is a `top-level group`; the root for all Groups and Projects of an Organization +- `gitlab` is a `project`; a Project of the Organization. + +The top-level Group has served as the de facto Organization entity. With the creation of Organizations, top-level Groups will be [nested underneath Organizations](https://gitlab.com/gitlab-org/gitlab/-/issues/394796). + +Over time, there won't be a distinction between a top-level Group and a Group. All features that make top-level Groups different from Groups will move to the Organization. + + + +- Top-level Groups belonging to the same Organization are located on the same Cell. +- Top-level Groups can interact with other top-level Groups that belong to the same Organization. + +Discouraged synonyms: Root-level namespace + +### Users -A number of technical issues need to be resolved to implement Cells (in no particular order). This section will be expanded. +Users are available globally and not restricted to a single Cell. Users belong to a single Organization, but can participate in many Organizations through Group and Project membership with varying permissions. Inside Organizations, users can create multiple top-level Groups. User activity is not limited to a single Organization but their contributions (for example To-Dos) are only aggregated within an Organization. This avoids the need for aggregating across Cells. -1. How are Cells provisioned? - [Design discussion](https://gitlab.com/gitlab-org/gitlab/-/issues/396641) -1. What is a Cells topology? - [Design discussion](https://gitlab.com/gitlab-org/gitlab/-/issues/396641) -1. How are users of an organization routed to the correct Cell? - -1. How do users authenticate with Cells and Organizations? - [Design discussion](https://gitlab.com/gitlab-org/gitlab/-/issues/395736) -1. How are Cells rebalanced? -1. How can Cells implement disaster recovery capabilities? +- Users are shared globally across all Cells. +- Users can create multiple top-level Groups. +- Users can be members of multiple top-level Groups. +- Users belong to one Organization. See [!395736](https://gitlab.com/gitlab-org/gitlab/-/issues/395736). +- Users can be members of Groups and Projects in different Organizations. +- Users can administer Organizations. +- User activity is aggregated in an Organization. +- Every user has one personal namespace. diff --git a/doc/architecture/blueprints/cells/index.md b/doc/architecture/blueprints/cells/index.md index 3c2ac378fe5..24d9b6b705b 100644 --- a/doc/architecture/blueprints/cells/index.md +++ b/doc/architecture/blueprints/cells/index.md @@ -18,8 +18,7 @@ Cells is a new architecture for our software as a service platform. This archite For more information about Cells, see also: -- [Glossary](glossary.md) -- [Goals](goals.md) +- [Goals, Glossary and Requirements](goals.md) ## Work streams diff --git a/doc/architecture/blueprints/cells/proposal-stateless-router-with-buffering-requests.md b/doc/architecture/blueprints/cells/proposal-stateless-router-with-buffering-requests.md index c1ca0c60dcd..847532a36dc 100644 --- a/doc/architecture/blueprints/cells/proposal-stateless-router-with-buffering-requests.md +++ b/doc/architecture/blueprints/cells/proposal-stateless-router-with-buffering-requests.md @@ -27,10 +27,10 @@ monolith. This architecture also supports regions by allowing for low traffic databases to be replicated across regions. Users are not directly exposed to the concept of Cells but instead they see -different data dependent on their chosen "organization". -[Organizations](glossary.md#organizations) will be a new model introduced to enforce isolation in the -application and allow us to decide which request route to which cell, since an -organization can only be on a single cell. +different data dependent on their chosen Organization. +[Organizations](goals.md#organizations) will be a new entity introduced to enforce isolation in the +application and allow us to decide which request routes to which Cell, since an +Organization can only be on a single Cell. ## Differences diff --git a/doc/architecture/blueprints/cells/proposal-stateless-router-with-routes-learning.md b/doc/architecture/blueprints/cells/proposal-stateless-router-with-routes-learning.md index 3b3d481914f..962f71673df 100644 --- a/doc/architecture/blueprints/cells/proposal-stateless-router-with-routes-learning.md +++ b/doc/architecture/blueprints/cells/proposal-stateless-router-with-routes-learning.md @@ -27,10 +27,10 @@ monolith. This architecture also supports regions by allowing for low traffic databases to be replicated across regions. Users are not directly exposed to the concept of Cells but instead they see -different data dependent on their chosen "organization". -[Organizations](glossary.md#organizations) will be a new model introduced to enforce isolation in the -application and allow us to decide which request route to which cell, since an -organization can only be on a single cell. +different data dependent on their chosen Organization. +[Organizations](goals.md#organizations) will be a new entity introduced to enforce isolation in the +application and allow us to decide which request routes to which Cell, since an +Organization can only be on a single Cell. ## Differences diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index fb38de0f7de..ef7491cb609 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -719,9 +719,67 @@ default: alias: docker ``` +### `Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?` + +You might get the following error when trying to run a `docker` command +to access a `dind` service: + +```shell +$ docker ps +Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? +``` + +Make sure your job has defined these environment variables: + +- `DOCKER_HOST` +- `DOCKER_TLS_CERTDIR` (optional) +- `DOCKER_TLS_VERIFY` (optional) + +You may also want to update the image that provides the Docker +client. For example, the [`docker/compose` images are obsolete](https://hub.docker.com/r/docker/compose) and should be +replaced with [`docker`](https://hub.docker.com/_/docker). + +As described in [runner issue 30944](https://gitlab.com/gitlab-org/gitlab-runner/-/issues/30944#note_1514250909), +this error can happen if your job previously relied on environment variables derived from the deprecated +[Docker `--link` parameter](https://docs.docker.com/network/links/#environment-variables), +such as `DOCKER_PORT_2375_TCP`. Your job fails with this error if: + +- Your CI/CD image relies on a legacy variable, such as `DOCKER_PORT_2375_TCP`. +- The [runner feature flag `FF_NETWORK_PER_BUILD`](https://docs.gitlab.com/runner/configuration/feature-flags.html) is set to `true`. +- `DOCKER_HOST` is not explicitly set. + ### Error: `Error response from daemon: Get "https://registry-1.docker.io/v2/": unauthorized: incorrect username or password` This error appears when you use the deprecated variable, `CI_BUILD_TOKEN`. To prevent users from receiving this error, you should: - Use [CI_JOB_TOKEN](../jobs/ci_job_token.md) instead. - Change from `gitlab-ci-token/CI_BUILD_TOKEN` to `$CI_REGISTRY_USER/$CI_REGISTRY_PASSWORD`. + +### Error: `error during connect: Post "https://docker:2376/v1.24/auth": dial tcp: lookup docker on 127.0.0.11:53: no such host` + +This error appears when the `dind` service has failed to start. Check +the job log to see if `mount: permission denied (are you root?)` +appears. For example: + +```plaintext +Service container logs: +2023-08-01T16:04:09.541703572Z Certificate request self-signature ok +2023-08-01T16:04:09.541770852Z subject=CN = docker:dind server +2023-08-01T16:04:09.556183222Z /certs/server/cert.pem: OK +2023-08-01T16:04:10.641128729Z Certificate request self-signature ok +2023-08-01T16:04:10.641173149Z subject=CN = docker:dind client +2023-08-01T16:04:10.656089908Z /certs/client/cert.pem: OK +2023-08-01T16:04:10.659571093Z ip: can't find device 'ip_tables' +2023-08-01T16:04:10.660872131Z modprobe: can't change directory to '/lib/modules': No such file or directory +2023-08-01T16:04:10.664620455Z mount: permission denied (are you root?) +2023-08-01T16:04:10.664692175Z Could not mount /sys/kernel/security. +2023-08-01T16:04:10.664703615Z AppArmor detection and --privileged mode might break. +2023-08-01T16:04:10.665952353Z mount: permission denied (are you root?) +``` + +This indicates the GitLab Runner does not have permission to start the +`dind` service: + +1. Check that `privileged = true` is set in the `config.toml`. +1. Make sure the CI job has the right Runner tags to use these +privileged runners. diff --git a/doc/development/pipelines/internals.md b/doc/development/pipelines/internals.md index 8804cb36de2..195c85d422f 100644 --- a/doc/development/pipelines/internals.md +++ b/doc/development/pipelines/internals.md @@ -164,7 +164,7 @@ that are scoped to a single [configuration keyword](../../ci/yaml/index.md#job-k | `.use-pg15-ee` | Same as `.use-pg15` but also use an `elasticsearch` service (see [`.gitlab/ci/global.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/ci/global.gitlab-ci.yml) for the specific version of the service). | | `.use-kaniko` | Allows a job to use the `kaniko` tool to build Docker images. | | `.as-if-foss` | Simulate the FOSS project by setting the `FOSS_ONLY='1'` CI/CD variable. | -| `.use-docker-in-docker` | Allows a job to use Docker in Docker. | +| `.use-docker-in-docker` | Allows a job to use Docker in Docker. For more details, see the [handbook about CI/CD configuration](https://about.gitlab.com/handbook/engineering/gitlab-repositories/#cicd-configuration). | ## `rules`, `if:` conditions and `changes:` patterns @@ -417,3 +417,25 @@ For this scenario, you have to: - `spec/simplecov_env.rb` Additionally, `scripts/utils.sh` is always downloaded from the API when this pattern is used (this file contains the code for `.fast-no-clone-job`). + +#### Runner tags + +On GitLab.com, both unprivileged and privileged runners are +available. For projects in the `gitlab-org` group and forks of those +projects, only one of the following tags should be added to a job: + +- `gitlab-org`: Jobs randomly use privileged and unprivileged runners. +- `gitlab-org-docker`: Jobs must use a privileged runner. If you need [Docker-in-Docker support](../../ci/docker/using_docker_build.md#use-docker-in-docker), +use `gitlab-org-docker` instead of `gitlab-org`. + +The `gitlab-org-docker` tag is added by the `.use-docker-in-docker` job +definition above. + +To ensure compatibility with forks, avoid using both `gitlab-org` and +`gitlab-org-docker` simultaneously. No instance runners +have both `gitlab-org` and `gitlab-org-docker` tags. For forks of +`gitlab-org` projects, jobs will get stuck if both tags are supplied because +no matching runners are available. + +See [the GitLab Repositories handbook page](https://about.gitlab.com/handbook/engineering/gitlab-repositories/#cicd-configuration) +for more information. diff --git a/doc/user/application_security/policies/scan-execution-policies.md b/doc/user/application_security/policies/scan-execution-policies.md index 945d35c89da..dc59d615b65 100644 --- a/doc/user/application_security/policies/scan-execution-policies.md +++ b/doc/user/application_security/policies/scan-execution-policies.md @@ -119,7 +119,7 @@ FLAG: On self-managed GitLab, security policy bot users are available. To hide the feature, an administrator can [disable the feature flags](../../../administration/feature_flags.md) named `scan_execution_group_bot_users` and `scan_execution_bot_users`. On GitLab.com, this feature is available. -This rule enforces the defined actions and schedules a scan on the provided date/time. +This rule schedules a scan pipeline, enforcing the defined actions on the schedule defined in the `cadence` field. A scheduled pipeline does not run other jobs defined in the project's `.gitlab-ci.yml` file. When a project is linked to a security policy project, a security policy bot is created in the project and will become the author of any scheduled pipelines. | Field | Type | Required | Possible values | Description | |------------|------|----------|-----------------|-------------| diff --git a/doc/user/application_security/policies/scan-result-policies.md b/doc/user/application_security/policies/scan-result-policies.md index c5cfd63641b..f7b0f68e10c 100644 --- a/doc/user/application_security/policies/scan-result-policies.md +++ b/doc/user/application_security/policies/scan-result-policies.md @@ -84,6 +84,12 @@ the following sections and tables provide an alternative. ## Scan result policy schema +> The `approval_settings` field was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/418752) in GitLab 16.4 [with a flag](../../../administration/feature_flags.md) named `scan_result_policy_settings`. Disabled by default. + +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `scan_result_policy_settings`. +On GitLab.com, this feature is not available. + | Field | Type | Required |Possible values | Description | |-------|------|----------|----------------|-------------| | `name` | `string` | true | | Name of the policy. Maximum of 255 characters.| @@ -91,6 +97,7 @@ the following sections and tables provide an alternative. | `enabled` | `boolean` | true | `true`, `false` | Flag to enable (`true`) or disable (`false`) the policy. | | `rules` | `array` of rules | true | | List of rules that the policy applies. | | `actions` | `array` of actions | true | | List of actions that the policy enforces. | +| `approval_settings` | `object` | false | `{prevent_approval_by_author: boolean, prevent_approval_by_commit_author: boolean, remove_approvals_with_new_commit: boolean, require_password_to_approve: boolean}` | Project settings that the policy overrides. | ## `scan_finding` rule type @@ -131,6 +138,19 @@ This rule enforces the defined actions based on license findings. | `license_types` | `array` of `string` | true | license types | [SPDX license names](https://spdx.org/licenses) to match on, for example `Affero General Public License v1.0` or `MIT License`. | | `license_states` | `array` of `string` | true | `newly_detected`, `detected` | Whether to match newly detected and/or previously detected licenses. The `newly_detected` state triggers approval when either a new package is introduced or when a new license for an existing package is detected. | +## `any_merge_request` rule type + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/418752) in GitLab 16.4. + +This rule enforces the defined actions for any merge request based on the commits signature. + +| Field | Type | Required | Possible values | Description | +|---------------|---------------------|--------------------------------------------|---------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `type` | `string` | true | `any_merge_request` | The rule's type. | +| `branches` | `array` of `string` | true if `branch_type` field does not exist | `[]` or the branch's name | Applicable only to protected target branches. An empty array, `[]`, applies the rule to all protected target branches. Cannot be used with the `branch_type` field. | +| `branch_type` | `string` | true if `branches` field does not exist | `default` or `protected` | The types of branches the given policy applies to. Cannot be used with the `branches` field. | +| `commits` | `string` | true | `any`, `unsigned` | Whether the rule matches for any commits, or only if unsigned commits are detected in the merge request. | + ## `require_approval` action type This action sets an approval rule to be required when conditions are met for at least one rule in diff --git a/doc/user/group/index.md b/doc/user/group/index.md index e088bc48366..484fd8c533b 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -170,7 +170,9 @@ As a user, you can request to be a member of a group, if an administrator allows 1. On the left sidebar, select **Search or go to**. 1. Select **View all my groups**. 1. At the top right side, select **Explore groups**. -1. Under the group name, select **Request Access**. +1. Search for the group by name. +1. In the search results, select the name of the group. +1. On the group page, under the group name, select **Request Access**. As many as ten of the most-recently-active group owners receive an email with your request. Any group owner can approve or decline the request. diff --git a/doc/user/search/index.md b/doc/user/search/index.md index 8fc65899341..74b89b079d9 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -17,23 +17,23 @@ Both types of search are the same, except when you are searching through code. > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68640) in GitLab 14.3. -To improve the performance of your instance's global search, a GitLab administrator -can limit the search scope by disabling the following [`ops` feature flags](../../development/feature_flags/index.md#ops-type). - -| Scope | Feature flag | Description | -|--|--|--| -| Code | `global_search_code_tab` | When enabled, global search includes code. | -| Commits | `global_search_commits_tab` | When enabled, global search includes commits. | -| Issues | `global_search_issues_tab` | When enabled, global search includes issues. | -| Merge requests | `global_search_merge_requests_tab` | When enabled, global search includes merge requests. | -| Users | `global_search_users_tab` | When enabled, global search includes users. | -| Wiki | `global_search_wiki_tab` | When enabled, global search includes project and [group wikis](../project/wiki/group.md). | +To improve the performance of your instance's global search, an administrator can limit the search scope +by disabling one or more [`ops` feature flags](../../development/feature_flags/index.md#ops-type). + +| Scope | Feature flag | Description | +|----------------|------------------------------------|-------------------------------------------------------------------------------------------| +| Code | `global_search_code_tab` | When enabled, global search includes code. | +| Commits | `global_search_commits_tab` | When enabled, global search includes commits. | +| Issues | `global_search_issues_tab` | When enabled, global search includes issues. | +| Merge requests | `global_search_merge_requests_tab` | When enabled, global search includes merge requests. | +| Users | `global_search_users_tab` | When enabled, global search includes users. | +| Wiki | `global_search_wiki_tab` | When enabled, global search includes project and [group wikis](../project/wiki/group.md). | All global search scopes are enabled by default on self-managed instances. ## Global search validation -Global search ignores and logs as abusive any search with: +Global search ignores and logs as abusive any search that includes: - Fewer than two characters - A term longer than 100 characters (URL search terms must not exceed 200 characters) @@ -47,6 +47,19 @@ Global search only flags with an error any search that includes more than: - 4096 characters - 64 terms +## Autocomplete suggestions + +As you type in the search box, autocomplete suggestions are displayed for: + +- [Projects](#search-for-a-project-by-full-path) and groups +- Users +- Help pages +- Project features (for example, milestones) +- Settings (for example, user settings) +- Recently viewed merge requests +- Recently viewed issues and epics +- [GitLab Flavored Markdown references](../markdown.md#gitlab-specific-references) for issues in a project + ## Search in all GitLab To search in all GitLab: @@ -177,28 +190,3 @@ in your browser. To run a search from history: - For merge requests, on the left sidebar, select **Code > Merge requests**. Above the list, to the left of the search box, select **Recent searches**. 1. From the dropdown list, select a search. - -## Remove search filters - -Individual filters can be removed by selecting the filter's (x) button or backspacing. The entire search filter can be cleared by selecting the search box's (x) button or via <kbd>⌘</kbd> (Mac) + <kbd>⌫</kbd>. - -To delete filter tokens one at a time, the <kbd>⌥</kbd> (Mac) / <kbd>Control</kbd> + <kbd>⌫</kbd> keyboard combination can be used. - -## Autocomplete suggestions - -As you type in the search box, autocomplete suggestions are displayed for: - -- [Projects](#search-for-a-project-by-full-path) and groups -- Users -- Help pages -- Project features (for example, milestones) -- Settings (for example, user settings) -- Recently viewed merge requests -- Recently viewed issues and epics -- [GitLab Flavored Markdown references](../markdown.md#gitlab-specific-references) for issues in a project - -## Search settings - -You can search inside a Project, Group, Administrator, or User's settings by entering -a search term in the search box located at the top of the page. The search results -appear highlighted in the sections that match the search term. diff --git a/lib/gitlab/import_export/decompressed_archive_size_validator.rb b/lib/gitlab/import_export/decompressed_archive_size_validator.rb index 23a0c9a1f71..13510cb43ca 100644 --- a/lib/gitlab/import_export/decompressed_archive_size_validator.rb +++ b/lib/gitlab/import_export/decompressed_archive_size_validator.rb @@ -7,10 +7,8 @@ module Gitlab ServiceError = Class.new(StandardError) - def initialize(archive_path:, max_bytes: self.class.max_bytes, timeout: self.class.timeout) + def initialize(archive_path:) @archive_path = archive_path - @max_bytes = max_bytes - @timeout = timeout end def valid? @@ -19,14 +17,6 @@ module Gitlab end end - def self.max_bytes - Gitlab::CurrentSettings.current_application_settings.max_decompressed_archive_size.megabytes - end - - def self.timeout - Gitlab::CurrentSettings.current_application_settings.decompress_archive_file_timeout - end - private def validate @@ -35,7 +25,7 @@ module Gitlab validate_archive_path - Timeout.timeout(@timeout) do + Timeout.timeout(timeout) do stderr_r, stderr_w = IO.pipe stdout, wait_threads = Open3.pipeline_r(*command, pgroup: true, err: stderr_w) @@ -54,7 +44,7 @@ module Gitlab if status.success? result = stdout.readline - if @max_bytes > 0 && result.to_i > @max_bytes + if max_bytes > 0 && result.to_i > max_bytes valid_archive = false log_error('Decompressed archive size limit reached') @@ -73,7 +63,7 @@ module Gitlab valid_archive rescue Timeout::Error - log_error("Timeout of #{@timeout} seconds reached during archive decompression") + log_error("Timeout of #{timeout} seconds reached during archive decompression") pgrps.each { |pgrp| Process.kill(-1, pgrp) } if pgrps @@ -110,6 +100,14 @@ module Gitlab import_upload_archive_size: archive_size ) end + + def timeout + Gitlab::CurrentSettings.current_application_settings.decompress_archive_file_timeout + end + + def max_bytes + Gitlab::CurrentSettings.current_application_settings.max_decompressed_archive_size.megabytes + end end end end diff --git a/spec/frontend/ci/pipeline_schedules/components/pipeline_schedules_form_spec.js b/spec/frontend/ci/pipeline_schedules/components/pipeline_schedules_form_spec.js index 79a0cfa0dc9..33cf24c9ed1 100644 --- a/spec/frontend/ci/pipeline_schedules/components/pipeline_schedules_form_spec.js +++ b/spec/frontend/ci/pipeline_schedules/components/pipeline_schedules_form_spec.js @@ -97,6 +97,7 @@ describe('Pipeline schedules form', () => { const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); // Variables const findVariableRows = () => wrapper.findAllByTestId('ci-variable-row'); + const findVariableTypes = () => wrapper.findAllByTestId('pipeline-form-ci-variable-type'); const findKeyInputs = () => wrapper.findAllByTestId('pipeline-form-ci-variable-key'); const findValueInputs = () => wrapper.findAllByTestId('pipeline-form-ci-variable-value'); const findHiddenValueInputs = () => @@ -182,6 +183,16 @@ describe('Pipeline schedules form', () => { mock.restore(); }); + it('changes variable type', async () => { + expect(findVariableTypes().at(0).props('selected')).toBe('ENV_VAR'); + + findVariableTypes().at(0).vm.$emit('select', 'FILE'); + + await nextTick(); + + expect(findVariableTypes().at(0).props('selected')).toBe('FILE'); + }); + it('creates blank variable on input change event', async () => { expect(findVariableRows()).toHaveLength(1); diff --git a/spec/frontend/work_items/components/shared/work_item_links_menu_spec.js b/spec/frontend/work_items/components/shared/work_item_links_menu_spec.js index 721db6c3315..338a70feae4 100644 --- a/spec/frontend/work_items/components/shared/work_item_links_menu_spec.js +++ b/spec/frontend/work_items/components/shared/work_item_links_menu_spec.js @@ -1,4 +1,4 @@ -import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import { GlDisclosureDropdown, GlDisclosureDropdownItem } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import WorkItemLinksMenu from '~/work_items/components/shared/work_item_links_menu.vue'; @@ -10,8 +10,8 @@ describe('WorkItemLinksMenu', () => { wrapper = shallowMountExtended(WorkItemLinksMenu); }; - const findDropdown = () => wrapper.findComponent(GlDropdown); - const findRemoveDropdownItem = () => wrapper.findComponent(GlDropdownItem); + const findDropdown = () => wrapper.findComponent(GlDisclosureDropdown); + const findRemoveDropdownItem = () => wrapper.findComponent(GlDisclosureDropdownItem); beforeEach(() => { createComponent(); @@ -23,7 +23,7 @@ describe('WorkItemLinksMenu', () => { }); it('emits removeChild event on click Remove', () => { - findRemoveDropdownItem().vm.$emit('click'); + findRemoveDropdownItem().vm.$emit('action'); expect(wrapper.emitted('removeChild')).toHaveLength(1); }); diff --git a/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb b/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb index 9f76e4131b2..06b66b599ab 100644 --- a/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb @@ -250,7 +250,7 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat end context 'when user name is invalid' do - let(:user_name) { '.' } + let(:user_name) { ',' } let!(:snippet) { snippets.create!(id: 4, type: 'PersonalSnippet', author_id: user.id, file_name: file_name, content: content) } let(:ids) { [4, 4] } @@ -262,7 +262,7 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migrat end context 'when both user name and snippet file_name are invalid' do - let(:user_name) { '.' } + let(:user_name) { ',' } let!(:other_user) do users.create!( id: 2, diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index 0e93a85764f..df1b12e479f 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -2,15 +2,14 @@ require 'spec_helper' -RSpec.describe Gitlab::CurrentSettings do +RSpec.describe Gitlab::CurrentSettings, feature_category: :shared do before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') end shared_context 'with settings in cache' do before do - create(:application_setting) - described_class.current_application_settings # warm the cache + 2.times { described_class.current_application_settings } # warm the cache end end @@ -29,7 +28,7 @@ RSpec.describe Gitlab::CurrentSettings do context 'when there are allowed domains' do before do - create(:application_setting, domain_allowlist: ['www.gitlab.com']) + stub_application_setting(domain_allowlist: ['www.gitlab.com']) end it { is_expected.to be_truthy } @@ -37,7 +36,7 @@ RSpec.describe Gitlab::CurrentSettings do context 'when there are email restrictions' do before do - create(:application_setting, email_restrictions_enabled: true) + stub_application_setting(email_restrictions_enabled: true) end it { is_expected.to be_truthy } @@ -45,7 +44,7 @@ RSpec.describe Gitlab::CurrentSettings do context 'when the admin has to approve signups' do before do - create(:application_setting, require_admin_approval_after_user_signup: true) + stub_application_setting(require_admin_approval_after_user_signup: true) end it { is_expected.to be_truthy } @@ -53,7 +52,7 @@ RSpec.describe Gitlab::CurrentSettings do context 'when new users are set to external' do before do - create(:application_setting, user_default_external: true) + stub_application_setting(user_default_external: true) end it { is_expected.to be_truthy } @@ -61,7 +60,7 @@ RSpec.describe Gitlab::CurrentSettings do context 'when there are no restrictions' do before do - create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false, user_default_external: false) + stub_application_setting(domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false, user_default_external: false) end it { is_expected.to be_falsey } @@ -73,7 +72,7 @@ RSpec.describe Gitlab::CurrentSettings do context 'when signup is enabled' do before do - create(:application_setting, signup_enabled: true) + stub_application_setting(signup_enabled: true) end it { is_expected.to be_falsey } @@ -81,7 +80,7 @@ RSpec.describe Gitlab::CurrentSettings do context 'when signup is disabled' do before do - create(:application_setting, signup_enabled: false) + stub_application_setting(signup_enabled: false) end it { is_expected.to be_truthy } @@ -90,11 +89,9 @@ RSpec.describe Gitlab::CurrentSettings do describe '#current_application_settings', :use_clean_rails_memory_store_caching do it 'allows keys to be called directly' do - db_settings = create(:application_setting, - home_page_url: 'http://mydomain.com', - signup_enabled: false) + described_class.update!(home_page_url: 'http://mydomain.com', signup_enabled: false) - expect(described_class.home_page_url).to eq(db_settings.home_page_url) + expect(described_class.home_page_url).to eq('http://mydomain.com') expect(described_class.signup_enabled?).to be_falsey expect(described_class.signup_enabled).to be_falsey expect(described_class.metrics_sample_interval).to be(15) @@ -253,12 +250,14 @@ RSpec.describe Gitlab::CurrentSettings do end context 'with an existing ApplicationSetting DB record' do - let!(:db_settings) { ApplicationSetting.build_from_defaults(home_page_url: 'http://mydomain.com').save! && ApplicationSetting.last } + before do + described_class.update!(home_page_url: 'http://mydomain.com') + end it_behaves_like 'a non-persisted ApplicationSetting object' it 'uses the value from the DB attribute if present and not overridden by an accessor' do - expect(current_settings.home_page_url).to eq(db_settings.home_page_url) + expect(current_settings.home_page_url).to eq('http://mydomain.com') end end end @@ -277,10 +276,11 @@ RSpec.describe Gitlab::CurrentSettings do describe '#current_application_settings?', :use_clean_rails_memory_store_caching do before do allow(described_class).to receive(:current_application_settings?).and_call_original + ApplicationSetting.delete_all # ensure no settings exist end it 'returns true when settings exist' do - create(:application_setting, + described_class.update!( home_page_url: 'http://mydomain.com', signup_enabled: false) diff --git a/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb b/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb index a73f5d44b50..04e25dee905 100644 --- a/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb +++ b/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb @@ -13,13 +13,13 @@ RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator, feature_c FileUtils.rm(filepath) end - subject { described_class.new(archive_path: filepath, max_bytes: max_bytes) } + subject { described_class.new(archive_path: filepath) } describe '#valid?' do - let(:max_bytes) { 1 } - context 'when file does not exceed allowed decompressed size' do - let(:max_bytes) { 20 } + before do + stub_application_setting(max_decompressed_archive_size: 20) + end it 'returns true' do expect(subject.valid?).to eq(true) @@ -35,6 +35,10 @@ RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator, feature_c end context 'when file exceeds allowed decompressed size' do + before do + stub_application_setting(max_decompressed_archive_size: 0.000001) + end + it 'logs error message returns false' do expect(Gitlab::Import::Logger) .to receive(:info) diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb index d449446d7be..ef118d2987c 100644 --- a/spec/lib/gitlab/import_export/file_importer_spec.rb +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -198,8 +198,7 @@ RSpec.describe Gitlab::ImportExport::FileImporter, feature_category: :importers context 'when validate_import_decompressed_archive_size feature flag is enabled' do before do stub_feature_flags(validate_import_decompressed_archive_size: true) - - allow(Gitlab::ImportExport::DecompressedArchiveSizeValidator).to receive(:max_bytes).and_return(1) + stub_application_setting(max_decompressed_archive_size: 0.000001) end it 'returns false and sets an error on shared' do diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb index 932a1f213fd..bc158fc9117 100644 --- a/spec/models/clusters/agent_token_spec.rb +++ b/spec/models/clusters/agent_token_spec.rb @@ -88,6 +88,13 @@ RSpec.describe Clusters::AgentToken, feature_category: :deployment_management do agent_token = create(:cluster_agent_token) expect(agent_token.token.length).to be >= 50 end + + it 'has a prefix' do + agent_token = build(:cluster_agent_token, token_encrypted: nil) + agent_token.save! + + expect(agent_token.token).to start_with described_class::TOKEN_PREFIX + end end describe '#to_ability_name' do diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index 9d73a4a6cee..caf503ad6bc 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe ApplicationSettings::UpdateService do +RSpec.describe ApplicationSettings::UpdateService, feature_category: :shared do include ExternalAuthorizationServiceHelpers - let(:application_settings) { create(:application_setting) } + let(:application_settings) { ::Gitlab::CurrentSettings.current_application_settings } let(:admin) { create(:user, :admin) } let(:params) { {} } diff --git a/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb index d952fca25a5..8d612174a0b 100644 --- a/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb +++ b/spec/services/ci/runners/set_runner_associated_projects_service_spec.rb @@ -97,7 +97,7 @@ RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute', fe end expect(execute).to be_error - expect(runner.reload.projects).to eq(original_projects) + expect(runner.reload.projects.order(:id)).to eq(original_projects) end end @@ -117,7 +117,7 @@ RSpec.describe ::Ci::Runners::SetRunnerAssociatedProjectsService, '#execute', fe it 'returns error response and rolls back transaction' do expect(execute).to be_error expect(execute.errors).to contain_exactly('user is not authorized to add runners to project') - expect(runner.reload.projects).to eq(original_projects) + expect(runner.reload.projects.order(:id)).to eq(original_projects) end end end diff --git a/spec/workers/database/lock_tables_worker_spec.rb b/spec/workers/database/lock_tables_worker_spec.rb new file mode 100644 index 00000000000..cb720959db0 --- /dev/null +++ b/spec/workers/database/lock_tables_worker_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Database::LockTablesWorker, feature_category: :cell do + using RSpec::Parameterized::TableSyntax + + let(:worker) { described_class.new } + let(:exception_class) { described_class::TableShouldNotBeLocked } + + describe '#perform' do + context 'when running with single database' do # this covers both single-db and single-db-ci-connection cases + before do + skip_if_database_exists(:ci) + end + + it 'skips executing the job' do + expect do + worker.perform('ci', %w[ci_pipelines]) + end.to raise_error(exception_class, 'GitLab is not running in multiple database mode') + end + end + + context 'when running in decomposed database' do + before do + skip_if_shared_database(:ci) + end + + context 'when the table is wrong' do + context 'when trying to lock tables on an unknown database' do + it 'raises an exception' do + expect do + worker.perform('foobar', %w[ci_pipelines]) + end.to raise_error(exception_class, /does not support locking writes on tables/) + end + end + + context 'when trying to lock tables on the database that does not support locking' do + it 'raises an exception' do + expect do + worker.perform('geo', %w[ci_pipelines]) # ci tables should be locked only on main + end.to raise_error(exception_class, /does not support locking writes on tables/) + end + end + + context 'when trying to lock tables on the wrong database' do + it 'raises an exception' do + expect do + worker.perform('ci', %w[ci_pipelines]) # ci tables should be locked only on main + end.to raise_error(exception_class, "table 'ci_pipelines' should not be locked on the database 'ci'") + end + end + + context 'when trying to lock shared tables on the database' do + it 'raises an exception' do + expect do + worker.perform('main', %w[loose_foreign_keys_deleted_records]) + end.to raise_error(exception_class, /should not be locked on the database 'main'/) + end + end + end + + context 'when the table is correct' do + context 'when the table is not locked for writes' do + where(:database_name, :tables) do + :ci | %w[users namespaces] + :main | %w[ci_pipelines ci_builds] + end + + with_them do + it 'locks the tables on the corresponding database' do + tables.each do |table_name| + unlock_table(database_name, table_name) + expect(lock_writes_manager(database_name, table_name).table_locked_for_writes?).to eq(false) + end + + expected_log_results = tables.map do |table_name| + { action: "locked", database: database_name, dry_run: false, table: table_name } + end + expect(worker).to receive(:log_extra_metadata_on_done).with(:performed_actions, expected_log_results) + + worker.perform(database_name, tables) + tables.each do |table_name| + expect(lock_writes_manager(database_name, table_name).table_locked_for_writes?).to eq(true) + end + end + end + + context 'when the table is already locked for writes' do + where(:database_name, :tables) do + :ci | %w[users namespaces] + :main | %w[ci_pipelines ci_builds] + end + + with_them do + it 'skips locking the tables on the corresponding database' do + tables.each do |table_name| + lock_table(database_name, table_name) + end + + expected_log_results = tables.map do |table_name| + { action: 'skipped', database: database_name, dry_run: false, table: table_name } + end + expect(worker).to receive(:log_extra_metadata_on_done).with(:performed_actions, expected_log_results) + + worker.perform(database_name, tables) + tables.each do |table_name| + expect(lock_writes_manager(database_name, table_name).table_locked_for_writes?).to eq(true) + end + end + end + end + end + end + end + end + + def lock_table(database_name, table_name) + lock_writes_manager(database_name, table_name).lock_writes + end + + def unlock_table(database_name, table_name) + lock_writes_manager(database_name, table_name).unlock_writes + end + + def lock_writes_manager(database_name, table_name) + connection = Gitlab::Database.database_base_models_with_gitlab_shared[database_name].connection + Gitlab::Database::LockWritesManager.new( + table_name: table_name, + connection: connection, + database_name: database_name, + with_retries: false, + dry_run: false + ) + end +end diff --git a/spec/workers/database/monitor_locked_tables_worker_spec.rb b/spec/workers/database/monitor_locked_tables_worker_spec.rb index 47475a0ad4a..7e900259265 100644 --- a/spec/workers/database/monitor_locked_tables_worker_spec.rb +++ b/spec/workers/database/monitor_locked_tables_worker_spec.rb @@ -19,6 +19,10 @@ RSpec.describe Database::MonitorLockedTablesWorker, feature_category: :cell do end context 'when running in decomposed database' do + before do + skip_if_shared_database(:ci) + end + context 'when the feature flag is disabled' do before do stub_feature_flags(monitor_database_locked_tables: false) @@ -32,7 +36,6 @@ RSpec.describe Database::MonitorLockedTablesWorker, feature_category: :cell do context 'when the feature flag is enabled' do before do - skip_if_shared_database(:ci) stub_feature_flags(monitor_database_locked_tables: true) allow(Gitlab::Database::TablesLocker).to receive(:new).and_return(tables_locker) end @@ -73,6 +76,56 @@ RSpec.describe Database::MonitorLockedTablesWorker, feature_category: :cell do worker.perform end + + context 'with automatically locking the unlocked tables' do + context 'when there are no tables to be locked' do + before do + stub_feature_flags(lock_tables_in_monitoring: true) + allow(tables_locker).to receive(:lock_writes).and_return([]) + end + + it 'does not call the Database::LockTablesWorker' do + expect(Database::LockTablesWorker).not_to receive(:perform_async) + end + end + + context 'when there are tables to be locked' do + before do + lock_writes_results = [ + { table: 'users', database: 'ci', action: 'needs_lock' }, + { table: 'projects', database: 'ci', action: 'needs_lock' }, + { table: 'ci_builds', database: 'main', action: 'needs_lock' }, + { table: 'ci_pipelines', database: 'main', action: 'skipped' } + ] + allow(tables_locker).to receive(:lock_writes).and_return(lock_writes_results) + end + + context 'when feature flag lock_tables_in_monitoring is enabled' do + before do + stub_feature_flags(lock_tables_in_monitoring: true) + end + + it 'locks the tables that need to be locked' do + expect(Database::LockTablesWorker).to receive(:perform_async).once.with('ci', %w[users projects]) + expect(Database::LockTablesWorker).to receive(:perform_async).once.with('main', %w[ci_builds]) + + worker.perform + end + end + + context 'when feature flag lock_tables_in_monitoring is disabled' do + before do + stub_feature_flags(lock_tables_in_monitoring: false) + end + + it 'does not lock the tables that need to be locked' do + expect(Database::LockTablesWorker).not_to receive(:perform_async) + + worker.perform + end + end + end + end end end end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 3d39095e214..88f460c62ad 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -195,6 +195,7 @@ RSpec.describe 'Every Sidekiq worker', feature_category: :shared do 'CreateGithubWebhookWorker' => 3, 'CreateNoteDiffFileWorker' => 3, 'CreatePipelineWorker' => 3, + 'Database::LockTablesWorker' => false, 'Database::BatchedBackgroundMigration::CiExecutionWorker' => 0, 'Database::BatchedBackgroundMigration::MainExecutionWorker' => 0, 'DeleteContainerRepositoryWorker' => 3, |